Fix: On tapping device back button to close troubleshoot box from LHN, navigated out of site#62345
Conversation
|
@hungvu193 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
will review this weekend |
|
on Android, when I change options for categories and open test tool modal, it's navigated back to Category main page. Screen.Recording.2025-06-02.at.09.28.21.mov |
Will check on this today |
|
PR updated @hungvu193 |
Thanks @nyomanjyotisa I'll do more testing today. |
Ah yeah, in the previous PR, we called |
|
Still under review. The progress is a bit slower since I've been slammed by some other deploy blockers. |
|
All the cases I was concerned about were passed 🎉 I'll handle the checklist |
|
Should we build the ad-hoc and then ask the QA team to test this PR before you approve it? I think it would be better if the QA team could help. What do you think about this, @hungvu193? |
|
yeah I would love if we could do it. What do you think? @blimpich ? |
| if (!canListenPopState) { | ||
| return; | ||
| } | ||
|
|
||
| const handlePopState = (event: PopStateEvent) => { |
There was a problem hiding this comment.
it should be helpful if you can leave a comment here to explain how it works.
| TestToolsNavigatorOuterView: (shouldUseNarrowLayout: boolean) => ({ | ||
| flex: 1, | ||
| justifyContent: shouldUseNarrowLayout ? 'flex-end' : 'center', | ||
| alignItems: 'center', | ||
| }), | ||
|
|
||
| TestToolsNavigatorInnerView: (shouldUseNarrowLayout: boolean, isAuthenticated: boolean) => { |
There was a problem hiding this comment.
These two style name are different with our current convention (camel case), let's change their name to match the current convention then?
I think it can be: getTestToolNavigatorOuterView or so.
There was a problem hiding this comment.
Sure thing, PR updated!
|
Hmm, strange, not sure why the android build failed... 🙁 |
|
Hmm yeah, that was pretty weird. Could you try again? |
|
🚧 @dylanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
Hi I just came across multiple workflow failures from this PR. They stem from my PR #62058. I knew the root cause and am creating a fix now. If testing Android is not urgent, can you please hold on several hours before @roryabraham gets online and review the fix? Thanks! |
|
Yeah. Sure |
|
@hungvu193 @nyomanjyotisa Can you try to merge |
|
I've merged with the latest main |
|
🚧 @dylanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
Re-running ad-hoc build here: https://github.com/Expensify/App/actions/runs/15530663255 |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Asked for QA to test this here |
|
Issue was not reproducible on the Ad-Hoc builds 🎉 Videos below Evidenceaz_recorder_20250611_222756.mp41000127113.mp462345.mp4 |
|
Nice 🚀. Thanks for checking. I'll handle checklist today! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-06-12.at.15.55.20.movAndroid: mWeb ChromeScreen.Recording.2025-06-12.at.15.43.13.movmAndroid.moviOS: HybridAppfade.movpush.from.bottom.movScreen.Recording.2025-05-08.at.16.26.52.moviOS: mWeb SafariSafari.movScreen.Recording.2025-06-12.at.15.00.19.movMacOS: Chrome / SafariChrome.movScreen.Recording.2025-06-12.at.14.52.23.movMacOS: DesktopDesktop.mov |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.1.66-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|
Explanation of Change
Fixed Issues
$ #58383
PROPOSAL: #58383 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android-Native.mp4
Android: mWeb Chrome
Android-mWeb.Chrome.mp4
iOS: Native
iOS-Native.mp4
iOS: mWeb Safari
iOS-mWeb.Safari.mp4
MacOS: Chrome / Safari
MacOS-Chrome.mp4
MacOS: Desktop
MacOS-Desktop.mp4