-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
" user is typing ... " doesn't appear after opening and closing the RHP #25486
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue." user is typing ... " doesn't appear after changing the priority mode. What is the root cause of that problem?The root cause of this issue is that we unsubscribe the typing status event in the cleanup function of this useEffect here
And we don't re-subscribe the typing status event in this useEffect and in the other useEffect App/src/pages/home/report/ReportActionsView.js Lines 268 to 278 in 0f83752
also doesn't re-subscribe the event because the dependency array
doesn't trigger the effect to re-run. So, the typing status event is unsubscribed and won't get re-subscribed as long as this useEffect is triggered to re-run. Note we explicitly call the cleanup function here apart from being calling every re-render as the official doc mentions. What changes do you think we should make in order to solve the problem?To fix this issue, we should re-subscribe the typing status event if it's unsubscribed by the cleanup function. To achieve it,
didSubscribeToReportTypingEvents.current = false; after the line to unsubscribe the event.
What alternative solutions did you explore? (Optional)We can also merge the two useEffects, useEffect and useEffect, into one useEffect. |
ProposalPlease re-state the problem that we are trying to solve in this issue."User is typing ... " doesn't appear after changing the priority mode. What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
App/src/pages/home/report/ReportActionsView.js Lines 180 to 185 in 4ac83ed
What alternative solutions did you explore? (Optional) |
This comment was marked as outdated.
This comment was marked as outdated.
Triggered auto assignment to @adelekennedy ( |
Bug0 Triage Checklist (Main S/O)
|
Yes @kavimuru , some follow up change we can do which I have mentioned here. not reproducible in dev env. Cause for this issue which won't be reproducible nowProposalPlease re-state the problem that we are trying to solve in this issue.'User is typing' isn't shown once we lose focus from the report(not specifically required to change priority mode) What is the root cause of that problem?Currently, we are unsubscribing the Typing Pusher event at the wrong place. Cleanup function will be called whenever the dep gets changed but Typing event needs to be unsubscribed when the component unmounts. So the current issue is whenever the report focus gets lost it will trigger this cleanup and unsubscribe the typing event. Seems it persists after this PR App/src/pages/home/report/ReportActionsView.js Lines 180 to 192 in 77ac9d1
What changes do you think we should make in order to solve the problem?We need to move App/src/pages/home/report/ReportActionsView.js Lines 135 to 138 in 77ac9d1
|
It seems like this was fixed by #18637 - let's close this for now |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
" user is typing... " message should appear for User A while User B is typing.
Actual Result:
After opening and closing the RHP, the " user is typing ... " message doesn't appear for User A while User B is typing.
Note: Refreshing the page or leaving and returning to the chat resolves the issue.
Notes:
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.55-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Recording.5911.mp4
Screencast.from.10-08-23.03_22_46.1.webm
Expensify/Expensify Issue URL:
Issue reported by: @Ahmed-Abdella
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691544088203549
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: