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

[HOLD for Payment [2024-05-15][$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. #41579

Closed
6 tasks
luacmartins opened this issue May 3, 2024 · 13 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

@luacmartins
Copy link
Contributor

luacmartins commented May 3, 2024

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:
Reproducible in staging?:
Reproducible in production?:
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:
Slack conversation:

Action Performed

cc @adamgrzybowski @Kicu @WojtekBoman

  1. On web only
  2. Open the search page, Profile > Troubleshoot > New Search Page
  3. Click on a transaction
  4. Verify that it opens in the RHP
  5. Resize the window to the narrow view
  6. Click the back button
  7. Verify that it navigates to a not found page and the URL is /Search_Bottom_Tab

Expected Result:

Navigation should work

Actual Result:

Navigation is broken

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?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Screen.Recording.2024-05-03.at.8.26.47.AM.mov

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017875690bb8f76af2
  • Upwork Job ID: 1786402424713957376
  • Last Price Increase: 2024-05-03
  • Automatic offers:
    • DylanDylann | Reviewer | 0
Issue OwnerCurrent Issue Owner: @DylanDylann
@luacmartins luacmartins added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 3, 2024
@luacmartins luacmartins self-assigned this May 3, 2024
@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label May 3, 2024
@melvin-bot melvin-bot bot changed the title [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. [$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. May 3, 2024
Copy link

melvin-bot bot commented May 3, 2024

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

Copy link

melvin-bot bot commented May 3, 2024

Triggered auto assignment to @Christinadobrzyn (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.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 3, 2024
Copy link

melvin-bot bot commented May 3, 2024

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

@cretadn22
Copy link
Contributor

Proposal

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

The not found page appear when clicking the back button

What is the root cause of that problem?

When you select a transaction detail on the search page, you're taken to the ReportScreen. On both the ReportScreen and the MoneyRequestHeader, which is responsible for displaying the header, clicking the back button triggers goBack function

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

onBackButtonPress={() => Navigation.goBack(undefined, false, true)}

If a transaction is opened on the search page (RHP), we should use the dismissModal function instead of the goBack function.

                  onBackButtonPress={() => {
                        if(shouldUseNarrowLayout) {
                            Navigation.dismissModal()
                            return;
                        } 
                        Navigation.goBack(undefined, false, true)
                    }}

Optional: Additionally, for consistency, it's advisable to apply the same fix to ReportScreen Component, ensuring that the dismissModal function is used instead of the goBack function if this component is opened as RHP

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@luacmartins
Copy link
Contributor Author

@cretadn22 your solution looks hood. Maybe we can use a ternary though for simplicity and also apply the solution for the other headers in the RHP.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 3, 2024
Copy link

melvin-bot bot commented May 3, 2024

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

Offer link
Upwork job

Copy link

melvin-bot bot commented May 3, 2024

📣 @cretadn22 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 3, 2024
@Christinadobrzyn
Copy link
Contributor

looks like PR is in the works! #41611

@luacmartins
Copy link
Contributor Author

That PR has been merged. Just waiting on deploy.

@Christinadobrzyn
Copy link
Contributor

PR in production on May 9th payment due May 16th.

@Christinadobrzyn Christinadobrzyn changed the title [$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. [Payment due 16/5][$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. May 14, 2024
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented May 14, 2024

Payouts due:

Upwork job is here.

Do we need a regression test for this?

@luacmartins
Copy link
Contributor Author

No test needed, we'll add when we wrap up the project.

@Christinadobrzyn Christinadobrzyn changed the title [Payment due 16/5][$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. [HOLD for Payment [2024-05-15][$250] [Search v1] Resizing the screen to switch from the large screen to small screen breaks the back navigation on RHP. May 14, 2024
@Christinadobrzyn
Copy link
Contributor

Awesome, thanks @luacmartins. Paid this out based on this payment summary. Closing!

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
No open projects
Archived in project
Development

No branches or pull requests

4 participants