-
Notifications
You must be signed in to change notification settings - Fork 3k
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-03-19] [$500] [MEDIUM] mweb/Chrome - Receipt - Closing receipt view using device navigation redirects user to expense report #33549
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01532bdbfc376a4913 |
Triggered auto assignment to @bfitzexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Browser back button closes the money request and also the receipt preview What is the root cause of that problem?The money request and also the receipt preview currently shares the same URL, basically the receipt preview is a "state" of the money request page, where the user wants to view the receipt in full size. So when pressing device native back button, it will dismiss both since they share the same URL. What changes do you think we should make in order to solve the problem?We can use same approach as in #28999 where we can add a new route to view the receipt preview, or we can use the existing route for attachment (the Or an alternative that's worth considering is: Since the receipt preview is essentially a "state" of the money request page, we should indicate as such in the URL. Normally a page state will be represented by a param or a hash. If the money request page URL is at In the money request page, we'll receive the Since the preview state of money request page is on a different URL, clicking go back will work normally. What alternative solutions did you explore? (Optional)NA |
ProposalPlease re-state the problem that we are trying to solve in this issue.mweb/Chrome - Receipt - Closing receipt view using device navigation redirects user to expense report What is the root cause of that problem?The same rootcause with #33419 When opening the receipt view by Modal we don't update any history state, so when the user clicks on the browser back button it will navigate back to the previous page and the modal on the current page will be closed What changes do you think we should make in order to solve the problem?Currently, the root cause of this issue also happens on a lot of pages using modal on web, and I also found another page same root cause at Select Year on Date of Birth also happens. So we need to fix at Modal for web, when showModal we will pushState to history and add My proposal is not affected when we separate the modal to another page, but now it can reduce a lot of issues the same with this, and new features in the future use the Modal View more implementation at this commit What alternative solutions did you explore? (Optional) |
@bfitzexpensify Screen.Recording.2023-12-27.at.07.51.11.movThe go-back behavior for the menu modal between native device and web mobile, on native device when press go-back button just only closes the menu but on the web when pressing go back button will navigate to the previous page Screen.Recording.2023-12-27.at.07.50.19.movMy proposal can resolve all issues related to Modal |
@bfitzexpensify I don't think so since this is not in that PR's scope. |
@bfitzexpensify I am also C+ on that linked issue and it's really out of scope. Let's keep this open and hold rather than close. |
Sorry @tienifr your comment wasn't showing for me before. Fair if this is out of scope - @aimane-chnaif just confirming you're suggesting we hold this on #27856? |
yes, correct |
@bfitzexpensify, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too... |
Held on #27856 |
This issue has not been updated in over 15 days. @rlinoz, @bfitzexpensify, @aimane-chnaif, @tienifr eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
PR is ongoing |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.50-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-03-19. 🎊 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:
|
Payments complete - please complete the BZ checklist when you get a chance @aimane-chnaif, thank you |
This ended up being a new feature. Regression Test Proposal
|
Sweet. Regression test proposed here - https://github.com/Expensify/Expensify/issues/381504. Closing this out, thanks everyone! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.16-4
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:
User returns to request details page
Actual Result:
User returns to expense report
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6324979_1703347266794.1000005051.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: