-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[HOLD #22850] [$500] App does not focus back on merchant on back from search / send message #29009
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01c81606735a82e17e |
ProposalPlease re-state the problem that we are trying to solve in this issue.App does not focus back on merchant on back from search / send message What is the root cause of that problem?We are not focusing he input element when the screen is focused to MoneyRequestMerchantPage component again, this happens if we switch from other component or even if we reload. What changes do you think we should make in order to solve the problem?we need to follow same code as in MoneyRequestDescriptionPage.js App/src/pages/iou/MoneyRequestDescriptionPage.js Lines 63 to 77 in 3e4731d
We need to create on ref i.e. focusTimeoutRef and then paste the above code for useFocusEffect. What alternative solutions did you explore? (Optional)N/A |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.App does not focus back on merchant on back from search / send message What is the root cause of that problem?The main problem is that we only focus once App/src/pages/iou/WaypointEditor.js Line 202 in 5dc3c25
What changes do you think we should make in order to solve the problem?To avoid the problem that we only focus once
I like this option better as opposed to using What alternative solutions did you explore? (Optional)NA |
ProposalPlease re-state the problem that we are trying to solve in this issueApp does not focus back on merchant on back from search / send message What is the root cause of that problem?TextInput on
What changes do you think we should make in order to solve the problem?We should use this code to focus the TextInput everytime when screen mount
What alternative solutions did you explore? (Optional)We should try to bind this functionality Internally in the Custom TextInput component and activate it through a prop name |
ProposalPlease re-state the problem that we are trying to solve in this issue.App does not focus back on merchant field on back from search What is the root cause of that problem?In here, we only focus on the input What changes do you think we should make in order to solve the problem?It's difficult to handle this focus correctly, all the proposals that use the above timeout are flaky since sometimes the inputRef is not initialized at the time the timeout ends, so the input is still not focused. Also those will duplicate the timeout code all over the places. A best practice is to use a common So we should:
What alternative solutions did you explore? (Optional)NA |
I would agree with @tienifr that we should hold this issue until the hook is created and then use that. |
Triggered auto assignment to @puneetlath ( |
Bug0 Triage Checklist (Main S/O)
|
Makes sense to me. Putting it on hold. |
@allroundexperts looks like the issue we were holding on is done. Shall we un-hold this? |
@puneetlath Yep, Let's go ahead with @tienifr's proposal. 🎀 👀 🎀 C+ reviewed |
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($500) |
📣 @tienifr 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
PR ready for review: #30675. |
This issue has not been updated in over 15 days. @puneetlath, @allroundexperts, @tienifr eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
I'm confused. Is this still on hold or is this done? |
@puneetlath This is all done. PR deployed to production 19 days ago but Melvin did not notify us. We can wrap up payment now. Thanks! |
Payment summary:
@allroundexperts please go ahead and make the request on NewDot. Thanks everyone! |
$500 payment approved for @allroundexperts based on comment above. |
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:
App should focus back on merchant field on back from search
Actual Result:
App does not focus back on merchant field on back from search
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.79-3
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.7.mp4
mac.chrome.desktop.merchant.no.refocus.mov
windows.chrome.merchant.does.not.focus.on.back.from.search.mp4
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696574111976749
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: