-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for payment 2021-11-22] Keyboard is not dismissed if the user navigates to LHN when editing a comment - Reported by: @parasharrajat #6158
Comments
Triggered auto assignment to @tylerkaraszewski ( |
There are two causes for this issue.
Proposal
We previously disabled this behavior which comes by default with React navigation for a reason.
I think we should not revert that change instead I have a generic approach to close all the keyboards in this case. When users navigate back to LHN, we call App/src/libs/Navigation/Navigation.js Line 41 in 6299f48
so we can add |
Seems like an obvious external issue. |
Triggered auto assignment to @kadiealexander ( |
Upwork job is live: https://www.upwork.com/jobs/~01cc79eda5fa4843d7 |
Hey everyone ! I am new here. I just saw this on Upwork and as per the requirement first I looked into the issue. So here are my findings, Seems like this UI is designed with Tablets in mind as well, so on the mobile it is implemented as the separate screens but those are actually not. HeaderView.js file has the following code,
This file shows back button only for the small screens. On pressing the back button it opens the LHN (I think it is left hand navigation) but the Edit Comment text field still remains in focus. If you type something on keyboard even after coming to LHN and then open the same thread again, you will see what you typed there. So the text field never looses the focus even on pressing the back button, therefore the keyboard remains visible. A quick solution just to fix this issue is call Keyboard.dismiss() in ReportScreen.js. Like this,
This will forcefully close the keyboard, but note that the text field still remains in focus even if we do this. We can work more on this together. :) |
Triggered auto assignment to @mountiny ( |
@mountiny do you have any thoughts on @thecodingeek's proposal above? :) |
Hello @thecodingeek! Great to have you onboard. Thank you very much for your proposal. I must admit I like @parasharrajat more general solution a little bit more 😬 Essentially, as you proposed to add the We would want to close the keyboard anytime we go back to the Drawer so it make sense to put it into that place. Therefore, I am sorry, but I will pick @parasharrajatfor this job as he has reported this issue and proposed solid solution before you. @thecodingeek Please, have a look around existing issues with @parasharrajat Could you please apply for the job on Upwork and let @kadiealexander once you do so 😊 |
Hello @mountiny, hmm if the keyboard is supposed to be closed anytime user moves to Drawer, then it is better to put the same in I will check some other issue too :) |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.14-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2021-11-22. 🎊 |
@parasharrajat I've hired you in Upwork, could you please let me know once you've accepted so I can get payment out quickly for you? Thanks! |
@kadiealexander Done |
Paid $500 for reporting and fixing, thanks @parasharrajat!! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Keyboard should be dismissed when navigating to LHN.
Actual Result:
Keyboard is not dismissed when navigating to LHN.
Workaround:
User can dismiss the keyboard manually.
Platform:
Where is this issue occurring?
Version Number: 1.1.12-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
keyboard-edit.mp4
Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1635751023286900
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: