Unify hybrid & standalone deeplink navigation#64056
Conversation
dangrous
left a comment
There was a problem hiding this comment.
Generally this makes sense to me! I have one question, and then I think we can go ahead and grab a C+ for testing!
|
@allgandalf 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2025-07-01.at.5.59.01.PM.moviOS: HybridAppScreen.Recording.2025-07-01.at.5.26.05.PM.moviOS: mWeb SafariScreen.Recording.2025-07-01.at.5.50.35.PM.movMacOS: Chrome / SafariScreen.Recording.2025-07-01.at.4.05.01.PM.movMacOS: DesktopScreen.Recording.2025-07-01.at.4.38.18.PM.mov |
|
@war-in I think we need to test on adhoc, I'll ask someone to trigger the built |
|
Also can you please update platform recordings ? |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@war-in can you merge latest main? |
|
@allgandalf main merged! I'll try to post the videos today |
|
@allgandalf videos updated! |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
@allgandalf AdHoc builds generated and ready for your testing here |
Julesssss
left a comment
There was a problem hiding this comment.
This is working nicely for app actions. But I noticed that the second action doesn't work if you remain on the expense/distance/scan page after the first link, and then you attempt a second action. This is blocking bigger HybridApp changes with tight deadlines, so I'm happy to create an issue for that separately though.
@war-in as we've seen before I don't believe we can verify the deep links until we're on the prod release environment -- as we did with previous testing. Let me know if we managed to get around that issue though
Testing the adhocs ! |
@Julesssss I believe we can verify android deeplins by enabling them in the phone settings |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Created follow-up issue for the '2nd use fails while remaining on original flow' bug here. |
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.74-2 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.74-10 🚀
|
| } | ||
| Navigation.navigate(parsedUrl); | ||
| const parsedUrl = Navigation.parseHybridAppUrl(state.url as Route); | ||
| openReportFromDeepLink(parsedUrl); |
There was a problem hiding this comment.
This opens the URL after onboarding. The logic itself wasn't wrong, but it conflicted with the test drive modal, which also opens after onboarding. (See issue #65508)
App/src/libs/actions/Report.ts
Line 3453 in 139b41c
Fixed in PRs https://github.com/Expensify/Mobile-Expensify/pull/13613 and #65615.
There was a problem hiding this comment.
@QichenZhu thanks for an update! I agree it wasn't perfect and that's why we created another PR to improve it. It was merged yesterday
Explanation of Change
When implementing
HybridAppHandlerwe extracted HybridApp deeplink navigation to a separate function insetup/hybridApp. It turns out that we already have a deeplink handling function in the standalone app (openReportFromDeepLink) and can use it for hybrid app too.Fixed Issues
$ #62857
PROPOSAL:
Tests
HybridApp:
Check if clicking on a ND shortcut or deeplink redirects to the correct place:
Manually createor any other)https://new.expensify.com/settingsor any other)All platforms:
while app is open
Not found pageis opened onceOffline tests
QA Steps
Check if clicking on a ND shortcut or deeplink redirects to the correct place:
Manually createor any other)https://new.expensify.com/settingsor any other)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.-.web.mp4
iOS: Native
ios.-.native.mov
iOS: mWeb Safari
ios.-.web.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov