[HybridApp] Allow classic experience users to use NewDot travel page#49602
Conversation
|
@AndrewGable should I disable explanation modal in this flow? You can see how it looks right now in iOS native section. |
|
FYI: Something from main has broken the PR and navigation. I need to investigate the root cause. |
| UNSTABLE_usePreventRemove(shouldReturnToOldDot, () => { | ||
| Navigation.setParams({singleNewDotEntry: undefined}); | ||
| NativeModules.HybridAppModule.closeReactNativeApp(false, false); | ||
| }); | ||
|
|
There was a problem hiding this comment.
To clarify for reviewers, it is stable 😄 The same code is present in the react-navigation v7 just without UNSTABLE prefix.
However, this is an argument for considering upgrading to v7. We see other use cases where features from v7 would also be useful.
|
Reassure Performance has failed. I'm not sure if the error is related to my changes |
situchan
left a comment
There was a problem hiding this comment.
Please merge main to see if it fixes perf issue.
As it doesn't happen on main, there should be no check failures before merge.
|
@situchan it looks like we still experience performance issue. I'm not sure what change might have caused it.. I need to investigate it 🤔 |
|
Conflicts! |
|
All conflicts are resolved and tests has passed cc: @situchan @trjExpensify |
|
Dope! Over to @AndrewGable for a secondary review then, right? |
|
I was hoping @situchan could give it a quick code review! |
|
But seems like there are still conflicts (or new ones) |
|
Code review is done |
|
Awesome thanks! |
| RECENTLY_USED_CURRENCIES: 'nvp_recentlyUsedCurrencies', | ||
|
|
||
| /** States whether we transitioned from OldDot to show only certain group of screens. It should be undefined on pure NewDot. */ | ||
| IS_SINGLE_NEW_DOT_ENTRY: 'isSingleNewDotEntry', |
|
I hope we won't encounter more conflicts here 😅 |
|
✋ 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/AndrewGable in version: 9.0.51-1 🚀
|
|
Looks like this caused 2 blockers, so I'm looking into reverting in #51155 Discussing in slack here - https://expensify.slack.com/archives/C04878MDF34/p1729497719145869 |
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.51-4 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.51-4 🚀
|
Details
OldDot part of this feature: https://github.com/Expensify/Mobile-Expensify/pull/13146
Remaining parts of this PR before marking it as "ready for a review":
Fixed Issues
$ #47734
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests
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
back.button.android.mov
open.url.android.mov
Android: mWeb Chrome
iOS: Native
iOS.demo.mov
Back.button.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop