Don't navigate to public screen deep link after sign in#42130
Conversation
|
@rayane-djouah 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] |
|
Additional video showing we can still deep link to auth screen after sign in Screen.Recording.2024-05-14.at.15.12.34.mov |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-05-21.at.11.02.51.AM.movAndroid: mWeb Chromescreen-20240521-113628.mp4iOS: NativeSimulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-05-21.at.11.13.14.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-05-21.at.10.32.33.mp4MacOS: Chrome / SafariScreen.Recording.2024-05-21.at.10.25.59.AM.movMacOS: DesktopScreen.Recording.2024-05-21.at.10.57.26.AM.mov |
There was a problem hiding this comment.
Please:
- Rename
isPublicScreentoisPublicScreenRoutefor clarity. - Refactor https://github.com/Expensify/App/blob/main/src/ROUTES.ts in this way:
const PUBLIC_SCREENS_ROUTES = {
// routes
}
const ROUTES = {
...PUBLIC_SCREENS_ROUTES,
// other routes
}and use PUBLIC_SCREENS_ROUTES in isPublicScreenRoute function
- Use routes regex from
CONST.REGEX.ROUTESinisPublicScreenRoutefunction
Lines 1954 to 1958 in e8ae3c5
This will ensure that the code is DRY.
|
1 & 2 done
I don't understand this one. The regex is only for VALIDATE_LOGIN and UNLINK_LOGIN and it won't match /v/1/2 for example (it only matches the /v/ part). |
rayane-d
left a comment
There was a problem hiding this comment.
Thank you!
Changes look good to me and test well. The reviewer checklist is also complete.
🎀👀🎀 C+ reviewed
|
@bernhardoj Can you please resolve conflicts |
|
Conflicts solved. |
|
🚀 Deployed to staging by https://github.com/johnmlee101 in version: 1.4.77-11 🚀
|
|
🚀 Deployed to staging by https://github.com/johnmlee101 in version: 1.4.77-11 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.78-5 🚀
|
Details
Currently, we navigate to any deep link after signing in, including the public screen. This PR change it so we won't navigate after sign in for public screen.
Fixed Issues
$ #41333
PROPOSAL: #41333 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.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
Screen.Recording.2024-05-14.at.14.41.11.mov
Android: mWeb Chrome
Screen.Recording.2024-05-14.at.14.48.35.mov
iOS: Native
Screen.Recording.2024-05-14.at.14.32.08.mov
iOS: mWeb Safari
Screen.Recording.2024-05-14.at.14.46.06.mov
MacOS: Chrome / Safari
Screen.Recording.2024-05-14.at.14.43.30.mov
MacOS: Desktop
Screen.Recording.2024-05-14.at.14.51.21.mov