-
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
[PAYMENT DUE 2024-03-06] [$500] RHP - User redirected from "Room" tab to "Chat" tab when tapped on the back button #35143
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01a7c80f42a2fdfb07 |
Triggered auto assignment to @peterdbarkerUK ( |
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.User redirected from "Room" tab to "Chat" tab when tapped on the back button What is the root cause of that problem?there is no on App/src/pages/NewChatSelectorPage.js Line 37 in b3ada0d
What changes do you think we should make in order to solve the problem?need to add modal dismiss as the action of the <HeaderWithBackButton
onBackButtonPress={Navigation.dismissModal}
title={props.translate('sidebarScreen.fabNewChat')}
/> alternativelywe can use navigation replace instead of the navigate when clicking on a tab so that it replaces the current navigation state with the target tab route
|
ProposalPlease re-state the problem that we are trying to solve in this issue.On back for new chat selector the modal is not dismissed What is the root cause of that problem?It seems to be intentional from this PR where we have removed dismissing the modal for issue #27669. Issue was - when the modal is opened for settings and if we use keyboard shortcut CTRL+SHIFT+K to open start chat and click back then it is not going back to the settings modal. What changes do you think we should make in order to solve the problem?If the requirement is to dismiss the modal then we can re-add the |
Proposal Please re-state the problem that we are trying to solve in this issue. What is the root cause of that problem? HeaderWithBackButton: The HeaderWithBackButton is responsible for rendering the back button on the header. The specific behavior when the back button is pressed (such as closing the panel or switching tabs) is not detailed here, which suggests that the logic is defined elsewhere, possibly in a parent component or within the HeaderWithBackButton's implementation itself.
OnyxTabNavigator: The OnyxTabNavigator manages the tab navigation. It includes a custom tabBar prop that uses TabSelector. If the logic associated with the back button is tied into the tab navigator’s state, it might be causing the incorrect navigation.
What changes do you think we should make in order to solve the problem? Review and Update HeaderWithBackButton Logic: Adjust Navigation State Management in OnyxTabNavigator: |
ProposalPlease re-state the problem that we are trying to solve in this issue.Pressing back in start chat page goes to the previous tab. What is the root cause of that problem?The back button will call
App/src/libs/Navigation/NavigationRoot.tsx Lines 96 to 100 in 83a7535
and when we call What changes do you think we should make in order to solve the problem?Use NewChatSelectorPage.js
|
ProposalPlease re-state the problem that we are trying to solve in this issue.After selecting 'Room' Tab in Right Hand Panel, and then clicking on Back Button, the user is redirected to 'Chat' Tab. The ideal behaviour should be closing the Right Hand Panel and going back to LHN. What is the root cause of that problem?The root cause is not passing an updated prop value for the function 'onBackButtonPress()', in the component HeaderWithBackButton. This leads to its default behaviour. Integration of Component: App/src/pages/NewChatSelectorPage.js Line 36 in 83a7535
The default behaviour of 'onBackButtonPress()' function defined in the component is to call 'Navigation.goBack(ROUTES.Home)', as can be seen in the following line. The goBack() function is implemented in 'libs/Navigation' in such a way that it takes multiple arguments, in which one argument is "shouldPopToTop" which is "false" by default. Hence, when we are calling the goBack function it is not going to LHN, as we are not passing the "shouldPopToTop" param as true in the default function above.
What changes do you think we should make in order to solve the problem?We can update the 'onBackButtonPress()' in the call as below, after importing Navigation from libs in 'NewChatSelectorPage'. Or passing True for 'shouldPopToTop' which will take the user directly to LHN.
|
📣 @adarsh0raj! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Search page is moved to LHP so New chat page can no longer be compared to Search page for consistency check. |
Personally I am looking for solution to skip tab navigation default behavior on navigation back. So as if there were not tab navigator. |
My proposal did this. |
@bernhardoj's proposal looks good to me. 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
PR is ready cc: @situchan |
Triggered auto assignment to @zanyrenney ( |
@Beamanator, @bernhardoj, @zanyrenney, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick! |
PR still in progress |
what's the ETA on the PR @Beamanator ? |
@Beamanator, @bernhardoj, @zanyrenney, @situchan Eep! 4 days overdue now. Issues have feelings too... |
PR was merged |
nice stuff re the merge. thanks! |
PR was merged and deployed to prod5 days ago, waiting the 7 day period and then will do payments. |
Payment summary |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.31-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
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
Action Performed:
Expected Result:
RHP should be closed and user should be navigated to LHN when user tapped on back button
Actual Result:
User redirected from "Room" tab to "Chat" tab when tapped on the back button
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?
Screenshots/Videos
Add any screenshot/video evidence
Bug6354481_1706178877493.screen-20240125-020533.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: