Skip to content
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

[$250][Back nav] mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page #57513

Open
2 of 8 tasks
mitarachim opened this issue Feb 27, 2025 · 18 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@mitarachim
Copy link

mitarachim commented Feb 27, 2025

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.0.7-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): ibellicotest+320@gmail.com
Issue reported by: Applause Internal Team
Device used: Motorola MotoG60 - Android 12 - Chrome
App Component: Workspace Settings

Action Performed:

  1. Open the staging.new.expensify.com website.
  2. Log in with a new account or with any account without workspaces.
  3. Tap on FAB and select "New Workspace"
  4. Tap on "Default Currency"
  5. Use device´s back button to return to Workspace confirmation page.
  6. Check where is the user redirected with this action.
  7. Tap on workspace switcher and select "Get Started"
  8. Tap on "Default Currency"
  9. Use device´s back button to return to workspace confirmation page.
  10. Check where is the user redirected with this action.

Expected Result:

User should land on confirmation page when using device´s back button on currency page while creating the first workspace.

Actual Result:

User lands on LHN instead of workspace confirmation page, when opening defualt currency and using device´s back button.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6755722_1740619731795.Currency.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021895485272246968923
  • Upwork Job ID: 1895485272246968923
  • Last Price Increase: 2025-03-07
  • Automatic offers:
    • Krishna2323 | Contributor | 106456651
Issue OwnerCurrent Issue Owner: @shubham1206agra
@mitarachim mitarachim added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Feb 27, 2025
Copy link

melvin-bot bot commented Feb 27, 2025

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Feb 27, 2025

🚨 Edited by proposal-police: This proposal was edited at 2025-02-27 02:03:04 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page

What is the root cause of that problem?

  • shouldHandleNavigationBack isn't passed to Modal in CurrencyPicker component.
    <Modal
    type={CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED}
    isVisible={isPickerVisible}
    onClose={hidePickerModal}
    onModalHide={hidePickerModal}
    hideModalContentWhileAnimating
    shouldEnableNewFocusManagement
    useNativeDriver
    onBackdropPress={Navigation.dismissModal}
    >

What changes do you think we should make in order to solve the problem?

  • Pass shouldHandleNavigationBack to the Modal component.
  • We should also check other components which are using Modal component and fix those too.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A -- Navigation issue.

What alternative solutions did you explore? (Optional)

Result

back_button_currency_modal.mp4

@twilight2294
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

User lands on LHN instead of WS confirmation when returning from currency page

What is the root cause of that problem?

For the currency modal:

<Modal
type={CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED}

We do not pass shouldHandleNavigationBack, this causes the navigation history to not have the confirmation screen in the navigation history as we do not set shouldGoBack to true:
if (shouldHandleNavigationBack) {
window.history.pushState({shouldGoBack: true}, '', null);
window.addEventListener('popstate', handlePopStateRef.current);

What changes do you think we should make in order to solve the problem?

  • We need to pass shouldHandleNavigationBack to the modal.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We have navigation goback tests here in GoBackTests here, so we will update these tests to add a new test for this use case. we will first define the screen component of the workspace switcher empty state like we do here , Then we will navigate to the screen of confirmation screen page, then open the currency picker modal, then we will act on the current stack and navigate like we do here, then we will navigate back again and expect that the current navigation stack has the confirmation screen page like we check for other screen here

What alternative solutions did you explore? (Optional)

N/A

@isabelastisser isabelastisser added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Feb 28, 2025
Copy link

melvin-bot bot commented Feb 28, 2025

Job added to Upwork: https://www.upwork.com/jobs/~021895485272246968923

@melvin-bot melvin-bot bot changed the title mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page [$250] mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page Feb 28, 2025
Copy link

melvin-bot bot commented Feb 28, 2025

Triggered auto assignment to Contributor-plus team member for initial proposal review - @shubham1206agra (External)

@mallenexpensify mallenexpensify changed the title [$250] mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page [$250][Back nav] mWeb-Workspace- User lands on LHN instead of WS confirmation when returning from currency page Mar 2, 2025
@melvin-bot melvin-bot bot added the Overdue label Mar 2, 2025
@isabelastisser
Copy link
Contributor

@shubham1206agra, can you please review the proposal above? Thanks!

Copy link

melvin-bot bot commented Mar 4, 2025

@shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick!

@isabelastisser
Copy link
Contributor

Bump @shubham1206agra. I will DM you!

@shubham1206agra
Copy link
Contributor

@twilight2294 Can you please first try to add a test and show it to me in a test branch?

Thanks

@melvin-bot melvin-bot bot removed the Overdue label Mar 5, 2025
@Krishna2323
Copy link
Contributor

@shubham1206agra, I'm not sure why we need a test here 😕. This issue is similar, and we didn't need a test there because the solution was only to prevent navigation using the existing approach.

@twilight2294
Copy link
Contributor

@shubham1206agra here's the branch for you to test:

main...twilight2294:App:patch-29

@twilight2294
Copy link
Contributor

@shubham1206agra let me know if you need any more info, thanks!

Copy link

melvin-bot bot commented Mar 7, 2025

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Mar 7, 2025
@shubham1206agra
Copy link
Contributor

Actually I will go with @Krishna2323's proposal since proposed test are not really helpful in other proposal.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Mar 7, 2025

Triggered auto assignment to @mjasikowski, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@mjasikowski
Copy link
Contributor

Let's go @Krishna2323

@melvin-bot melvin-bot bot added the Overdue label Mar 10, 2025
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 10, 2025
Copy link

melvin-bot bot commented Mar 10, 2025

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@Krishna2323
Copy link
Contributor

@shubham1206agra, PR is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: LOW
Development

No branches or pull requests

6 participants