App: [HOLD for payment 2023-10-02] [$1000] Add a mandatory route param to Navigation.goBack to prevent bugs
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Problem
Calling Navigation.goBack() in App without a route can cause problems since it will navigate to the wrong page if the desired page to go back to is not the last page in the navigation history.
Solution
Add a mandatory route param to Navigation.goBack so that the screen to navigate back to must be specified. Discussed in Slack here.
Platforms:
Which of our officially supported platforms is this issue occurring on? All
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
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 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL: Issue reported by: @neil-marcellini Slack conversation:
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01ee5e103f65c1a8fa
- Upwork Job ID: 1688984859314130944
- Last Price Increase: 2023-08-08
- Automatic offers:
- abdulrahuman5196 | Contributor | 26164451
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 55 (26 by maintainers)
Sure. Will wait for @abdulrahuman5196 to post a proposal.
@mananjadhav FYI I want to give first pick on this to @abdulrahuman5196 since we planned it together in Slack here. Abdul please post a proposal soon, and Manan please review his first.
We can treat this as a feature request, and not directly related to any specific feature here. We don’t need a regression suite here as these can be covered in other features. We only changed by forcing an arg in the method call.
To be clear, @Prince-Mendiratta and @abdulrahuman5196 will be splitting the payment for this issue.
I think we need @neil-marcellini to give the final sign off - thank you abdul for the prompt!
@mananjadhav @Prince-Mendiratta already expressed dibs if I didn’t get time. So I think he can do the PR, if it’s ok with you all?
Anyhow I don’t think I might be able to create PR today given things on my plate.
Apologies for the delay. I will have the PR ready before Friday. If there is more delay from my end I am fine to get an contributor to implement my proposal.
@abdulrahuman5196 - one more bump for you here else we’ll need to start considering another implementor!
@neil-marcellini Bump on this.
@neil-marcellini I think there is a subtle difference on what is being targetted there and what we are targetting here.
Navigation.goBack()to have fallback routes.Navigation.goBack(). So if we make the fallback param mandatory,Navigation.goBack(fallback)would be only way to use the function. So if they don’t want any specific screen, they can give home as fallback or they have to give some proper screen as fallback.