-
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
[HOLD for payment 2024-04-05] [$500] [Wave Collect] [Ideal Nav] Chat list scroll position is reset on refocus #35602
Comments
Triggered auto assignment to @muttmuure ( |
@muttmuure, @hayata-suenaga Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Job added to Upwork: https://www.upwork.com/jobs/~01af2fb1053774c16b |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.We are not able to remember the state of the chats in the home route, when navigating to the all-settings page. What is the root cause of that problem?This is because BOTTOM_TAB_NAVIGATOR is responsible for both routes and can only display one at a time. When navigating to all-settings it will unmount the chats home route and it's chat components. What changes do you think we should make in order to solve the problem?We could move the all-settings route, and place it in our existing RIGHT_MODAL_NAVIGATOR. This Navigator can render alongside the BOTTOM_TAB_NAVIGATOR. this navigator is a modal that would overlay the home chats. So that when you close it would show the previous underlying state. This implementation requires correct verification of back buttons, as there will be some deeplinking in all the nested navigation routes of all-settings. Overall not a small bug fix but rather a new feature. What alternative solutions did you explore? (Optional) |
still accepting proposals |
Hey @hayata-suenaga this task may be something I should take a look at |
📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
@muttmuure, @adamgrzybowski, @hayata-suenaga, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@adamgrzybowski will take a look at this |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.57-5 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 2024-04-05. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@situchan could you do the checklist when you have time? 😄 |
Will get to this tomorrow |
@situchan paid $500 for C+ |
@situchan waiting for you to complete the checklist |
@muttmuure, I think situation is OOO for whole April. Let's wait for them to come back. I changed the priority of this issue to weekly 🙇 |
Not overdue |
Looks like this was already paid out |
Action Performed:
Expected Result:
The scroll position of the chat list should be preserved.
Actual Result:
The scroll position is reset.
Additional Note
As part of a potential solution, we can make sure that we don't reload the chat/report list when switching between Chat and Settings on the Bottom Tab.
Workaround:
N/A
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
video -> #33280 (comment)
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: