Migrate E/App to use `PlatformStackNavigation` #49937

chrispader · 2024-09-30T14:32:08Z

@mountiny @ishpaul777 @chiragsalian

Details

This PR migrates the App to use PlatformStackNavigation types and navigator components implemented in #37891

Fixed Issues

$ #29948
PROPOSAL: #37891 (comment)

Tests

Open app
press on search filed
be sure that:
- screen transition is from left to right (Android/iOS/mWeb)
- drawer tranistion is from left to right content behind drawer is still visible (web)
- screen search can be loaded (i. e. it's not infinite loading) -> [HOLD for payment 2024-03-07] [Android] infinite loading search screen #37156
close screen
- on iOS test that it can be dismissed via gesture from right to left [HOLD for payment 2024-03-14] Gesture to hide the search page should be right to left #37354 and back button
- on mWeb iOS it can be dismissed via gesture from left to right and back button
- on Android can be closed via system gestures and back button
- on web can be closed via back button
open any chat
tap on composer
go to previous screen
- on iOS/mWeb iOS do it via gesture from left to right and back button
- on Android/web do it via back button
be sure there is no keyboard jumps on previous screen -> [HOLD for payment 2024-10-17] [HOLD for payment 2024-06-06] [HOLD #37891] Keyboard jumps around when swiping back from chat to LHN with gesture #37923
open chat again
tap on task
tap on Assignee
be sure there is no infinite loading for the screen -> [HOLD for payment 2024-03-07] Android - Task-Tapping assignee shows infinite loading page #37252
go back to task
tap on Title
be sure keyboard is opened automatically -> [HOLD for payment 2024-03-07] [$500] Android - Task - Keyboard does not open after selecting task title #37273
go back to main page
tap on user avatar (to open Account settings)
tap on wallet, tap on "Add bank account"
tap on "Debit card"
be sure keyboard gets opened automatically -> [HOLD for payment 2024-03-07] [$500] Android - Wallet - The keyboard does not open automatically when the debit card is opened #37325
go back to main page
press " " in TabBar
tap Request Money
enter any amount
press "Next"
be sure that on participant screen keyboard appears from the bottom -> [HOLD for payment 2024-03-07] [$500] IOU - Keyboard comes out from right to left on participant screen #37257
Verify that no errors appear in the JS console

Offline tests

Same as tests

QA Steps

Same as tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.Nov.26.2024.Expensify.Dev.mp4

Android: mWeb Chrome

android.web.webm

iOS: Native

ScreenRecording_11-26-2024.15-56-14_1.MP4

iOS: mWeb Safari

Running mWeb in iOS simulator is currently not possible due to this issue:

MacOS: Chrome / Safari

Screen.Recording.2024-11-26.at.15.32.29.mov

MacOS: Desktop

Screen.Recording.2024-11-26.at.15.43.23.mov

melvin-bot · 2024-09-30T14:32:42Z

@srikarparsi 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]

srikarparsi · 2024-09-30T16:50:25Z

Re-assigning to @chiragsalian and @mountiny for review since it looks like they have more context.

chrispader · 2024-10-01T13:08:10Z

@mountiny or @chiragsalian please from now on create ad-hoc builds from here. I fixed all remaining reported errors yesterday, so we can probably trigger a new build already.

chrispader · 2024-10-01T16:56:46Z

The ad-hoc iOS build fail is also going to be related to the patch from #49936

ishpaul777 · 2024-10-03T11:05:15Z

@mountiny Please run adhoc builds here when you have a moment, i plan to work on testing this fully today

mountiny · 2024-10-03T23:10:53Z

Running

github-actions · 2024-10-03T23:44:31Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
❌ FAILED ❌	❌ FAILED ❌
The QR code can't be generated, because the android build failed	The QR code can't be generated, because the iOS build failed
Desktop 💻	Web 🕸️
❌ FAILED ❌	https://49937.pr-testing.expensify.com
The QR code can't be generated, because the Desktop build failed

👀 View the workflow run that generated this build 👀

chrispader · 2024-10-04T08:55:47Z

@mountiny Please run adhoc builds here when you have a moment, i plan to work on testing this fully today

The PR didn't have the changes to the react-native-vision-camera patches up until now. Please trigger another set of ad-hoc builds :)

mountiny · 2024-10-04T11:46:12Z

https://github.com/Expensify/App/actions/runs/11179336829

github-actions · 2024-10-04T12:29:22Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/49937/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/49937/index.html

Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/49937/NewExpensify.dmg	https://49937.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

mountiny · 2024-10-04T12:50:24Z

Bug on the workspace page

mountiny · 2024-10-04T12:52:06Z

@chrispader Looking great! I think there are two things I have noticed:

There is a noticeable delay of idk 0.5s after clicking on the chat row for the animation to start
The chat finder modal is opening from the bottom, we want to keep the same animation we have now in main - animating from left

ishpaul777 · 2024-10-04T17:46:26Z

package-lock.json

commenting so this shows on my k2 for easy access

chrispader · 2024-10-06T18:10:57Z

Bug on the workspace page

@chrispader Looking great! I think there are two things I have noticed:

There is a noticeable delay of idk 0.5s after clicking on the chat row for the animation to start

The chat finder modal is opening from the bottom, we want to keep the same animation we have now in main - animating from left

@mountiny Both bugs are fixed!

I couldn't really see a difference between Stack navigator and Native-Stack while testing in this PR: (you meant when clicking on an item in the Sidebar right?)

Stack

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-06.at.17.38.30.mp4

Native Stack

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-06.at.20.09.51.mp4

chrispader · 2024-10-06T18:12:27Z

I will be OOO the next week, but i will hand this over to one of our other Margelo engineers. If there are any urgent issues i can also help with that, but i'm not gonna be on my Macbook unfortunately

ishpaul777 · 2024-10-06T18:19:38Z

I am now available for around ~2 hours i'll pull and start testing this locally

mountiny

Changes look good to me, its a massive pr and effort that spanned many months.

The latest changes been thoroughly tested by @ishpaul777 and @c3024 and also by @staszekscp in the Hybrid app and no issues been found so I dont think we should prolong this further and we should pull the trigger.

mountiny · 2024-11-30T00:06:35Z

The ELint is expected to fail with no migration to useOnyx

mountiny · 2024-11-30T00:22:57Z

@kirillzyusko @hannojg @chrispader can you please fix the prettier and lint (other than the useOnyx)

melvin-bot · 2024-11-30T13:31:40Z

@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

mountiny · 2024-11-30T13:31:49Z

Eslint failing because of the useOnyx migration

github-actions · 2024-11-30T14:23:29Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.0.69-1 🚀

platform	result
🤖 android 🤖	false ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	success ✅

github-actions · 2024-12-02T21:34:10Z

🚀 Deployed to production by https://github.com/puneetlath in version: 9.0.69-4 🚀

platform	result
🤖 android 🤖	skipped 🚫
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

shubham1206agra · 2024-12-13T16:01:14Z

src/libs/Navigation/AppNavigator/useRootNavigatorOptions.ts

+        },
+        onboardingModalNavigator: {
+            web: {
+                presentation: Presentation.TRANSPARENT_MODAL,


This line caused #53342 as the setting should be applied on all platform not just web.

suneox · 2024-12-15T16:15:03Z

src/pages/workspace/tags/TagSettingsPage.tsx

@@ -190,7  190,7 @@ function TagSettingsPage({route, navigation}: TagSettingsPageProps) {
                            />
                            {approverDisabled && (
                                <Text style={[styles.flexRow, styles.alignItemsCenter, styles.mv2, styles.mh5]}>
-                                    <Text style={[styles.textLabel, styles.colorMuted]}>{translate('workspace.rules.categoryRules.goTo')}</Text>{' '}
+                                    <Text style={[styles.textLabel, styles.colorMuted]}>{translate('workspace.rules.categoryRules.goTo')}</Text>


Coming from the checklist in #53358, we need to add spacing in this case

ntdiary · 2024-12-19T14:08:29Z

src/libs/Navigation/AppNavigator/useNavigationResetOnLayoutChange.ts

-        // We need to separately reset state of this navigator to trigger getRehydratedState.
-        navigation.reset(navigation.getState());
-        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
+        navigationRef.resetRoot(navigationRef.getRootState());


Coming from #53395, after this change, the FullScreenNavigator state could not be reset because this navigator is nested in rootState, so we revert this change and introduce a new useNavigationResetRootOnLayoutChange

eh2077 · 2025-01-02T15:18:58Z

src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx

+        // Whether the active workspace or the "Everything" page is loaded
+        const isWorkspaceOrEverythingLoaded = !!activeWorkspace || activeWorkspaceID === undefined;
+
+        // If we are currently switching workspaces, we don't want to do anything until the target workspace is loaded
+        if (isSwitchingWorkspace.current) {
+            if (isWorkspaceOrEverythingLoaded) {
+                isSwitchingWorkspace.current = false;
+            }
+            return;
+        }
+
+        // Otherwise, if the workspace is already loaded, we don't need to do anything
+        if (isWorkspaceOrEverythingLoaded) {
            return;
        }

+        isSwitchingWorkspace.current = true;


This change caused issue App is closed when switching workspace and using device back button.. This changed was reverted and the fix #54030 runs dispatching the navigation action in InteractionManager.runAfterInteractions.

chrispader requested a review from a team as a code owner September 30, 2024 14:32

melvin-bot bot requested review from srikarparsi and removed request for a team September 30, 2024 14:32

chrispader changed the title ~~[WIP] Use PlatformStackNavigation in E/App (introduce native-stack navigation on native)~~ [WIP] Use PlatformStackNavigation in E/App (introduce native-stack navigation on mobile) Sep 30, 2024

chrispader changed the title ~~[WIP] Use PlatformStackNavigation in E/App (introduce native-stack navigation on mobile)~~ [WIP] Migrate E/App to use PlatformStackNavigation Sep 30, 2024

kirillzyusko mentioned this pull request Sep 30, 2024

[NoQA] Perf: PlatormStackNavigation - Use @react-navigation/native-stack on mobile platforms #37891

Merged

48 tasks

chrispader changed the title ~~[WIP] Migrate E/App to use PlatformStackNavigation~~ [HOLD on #37891] Migrate E/App to use PlatformStackNavigation Sep 30, 2024

chrispader force-pushed the @chrispader/use-platform-stack-navigation-in-app branch from 3b921d3 to 5701c46 Compare September 30, 2024 16:30

srikarparsi requested review from chiragsalian and mountiny and removed request for srikarparsi September 30, 2024 16:50

mountiny assigned chrispader Oct 1, 2024

mountiny added the Ready To Build label Oct 1, 2024

This comment has been minimized.

Sign in to view

ishpaul777 reviewed Oct 4, 2024

View reviewed changes

package-lock.json Outdated

Copy link

Contributor

ishpaul777 Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commenting so this shows on my k2 for easy access

mountiny previously approved these changes Nov 30, 2024

View reviewed changes

fix: eslint/prettier

85c0dd7

kirillzyusko dismissed mountiny’s stale review via 85c0dd7 November 30, 2024 11:04

mountiny approved these changes Nov 30, 2024

View reviewed changes

mountiny merged commit a1c94f3 into Expensify:main Nov 30, 2024
18 of 19 checks passed

melvin-bot bot added the Emergency label Nov 30, 2024

mountiny removed the Emergency label Nov 30, 2024

github-actions bot mentioned this pull request Nov 30, 2024

Deploy Checklist: New Expensify 2024-11-30 #53334

Closed

41 tasks

shubham1206agra mentioned this pull request Dec 2, 2024

[HOLD for payment 2024-12-25] [$250] iOS/Android WS features -Feature is not enabled after enabling it and upgrading workspace to Control #53360

Open

8 tasks

CyberAndrii mentioned this pull request Dec 3, 2024

[HOLD for payment 2024-12-20] [$250] Web - Workspace - Central pane gets blank after resizing and clicking back icon #53395

Closed

8 tasks

ntdiary mentioned this pull request Dec 3, 2024

The central page page is not rendered when resized to narrow and then to wide view again #53466

Merged

50 tasks

AndrewGable mentioned this pull request Dec 3, 2024

[HOLD for payment 2024-12-17] [$250] iOS/Android - Contact method - Contact method not displayed until re-login or cleaning cache #53481

Closed

8 tasks

huult mentioned this pull request Dec 5, 2024

[HOLD for payment 2024-12-25] [Hold for #53360] [$250] Company cards toggle is disabled after upgrading workspace on mobile #53505

Closed

8 tasks

linhvovan29546 mentioned this pull request Dec 9, 2024

[HOLD for payment 2025-01-02] [$250] Hybrid - Android - WS Switcher - App is closed when switching workspace and using device back button. #53533

Open

8 tasks

mountiny mentioned this pull request Dec 9, 2024

[$250] LOW: Investigate using native-stack on native platforms instead of stack #29948

Closed

This was referenced Dec 13, 2024

fix: WS switcher blank screen, keep workspace history #54030

Merged

[NoQA] fix: bottom tab transitions duration #53754

Merged

shubham1206agra reviewed Dec 13, 2024

View reviewed changes

suneox reviewed Dec 15, 2024

View reviewed changes

This was referenced Dec 18, 2024

[HOLD for payment 2025-01-02] [HOLD for payment 2024-12-30] [$500] Onboarding flow is not showing upon first sign in #54322

Closed

Revert "The central page page is not rendered when resized to narrow and then to wide view again" #54328

Closed

c3024 mentioned this pull request Dec 19, 2024

chore: migrate AuthScreens from withOnyx to useOnyx #52417

Open

49 tasks

ntdiary reviewed Dec 19, 2024

View reviewed changes

eh2077 reviewed Jan 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate E/App to use `PlatformStackNavigation` #49937

Migrate E/App to use `PlatformStackNavigation` #49937

chrispader commented Sep 30, 2024 •

edited by mountiny

Loading

melvin-bot bot commented Sep 30, 2024

srikarparsi commented Sep 30, 2024

chrispader commented Oct 1, 2024

This comment has been minimized.

chrispader commented Oct 1, 2024

ishpaul777 commented Oct 3, 2024

mountiny commented Oct 3, 2024

github-actions bot commented Oct 3, 2024

chrispader commented Oct 4, 2024

mountiny commented Oct 4, 2024

github-actions bot commented Oct 4, 2024

mountiny commented Oct 4, 2024

mountiny commented Oct 4, 2024 •

edited

Loading

ishpaul777 Oct 4, 2024

chrispader commented Oct 6, 2024 •

edited

Loading

chrispader commented Oct 6, 2024

ishpaul777 commented Oct 6, 2024 •

edited

Loading

mountiny left a comment

mountiny commented Nov 30, 2024

mountiny commented Nov 30, 2024

melvin-bot bot commented Nov 30, 2024

mountiny commented Nov 30, 2024

github-actions bot commented Nov 30, 2024

github-actions bot commented Dec 2, 2024

shubham1206agra Dec 13, 2024

suneox Dec 15, 2024

ntdiary Dec 19, 2024

eh2077 Jan 2, 2025 •

edited

Loading

Migrate E/App to use PlatformStackNavigation #49937

Migrate E/App to use PlatformStackNavigation #49937

Conversation

chrispader commented Sep 30, 2024 • edited by mountiny Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Sep 30, 2024

srikarparsi commented Sep 30, 2024

chrispader commented Oct 1, 2024

This comment has been minimized.

chrispader commented Oct 1, 2024

ishpaul777 commented Oct 3, 2024

mountiny commented Oct 3, 2024

github-actions bot commented Oct 3, 2024

chrispader commented Oct 4, 2024

mountiny commented Oct 4, 2024

github-actions bot commented Oct 4, 2024

mountiny commented Oct 4, 2024

mountiny commented Oct 4, 2024 • edited Loading

ishpaul777 Oct 4, 2024

Choose a reason for hiding this comment

chrispader commented Oct 6, 2024 • edited Loading

Stack

Native Stack

chrispader commented Oct 6, 2024

ishpaul777 commented Oct 6, 2024 • edited Loading

mountiny left a comment

Choose a reason for hiding this comment

mountiny commented Nov 30, 2024

mountiny commented Nov 30, 2024

melvin-bot bot commented Nov 30, 2024

mountiny commented Nov 30, 2024

github-actions bot commented Nov 30, 2024

github-actions bot commented Dec 2, 2024

shubham1206agra Dec 13, 2024

Choose a reason for hiding this comment

suneox Dec 15, 2024

Choose a reason for hiding this comment

ntdiary Dec 19, 2024

Choose a reason for hiding this comment

eh2077 Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Migrate E/App to use `PlatformStackNavigation` #49937

Migrate E/App to use `PlatformStackNavigation` #49937

chrispader commented Sep 30, 2024 •

edited by mountiny

Loading

mountiny commented Oct 4, 2024 •

edited

Loading

chrispader commented Oct 6, 2024 •

edited

Loading

ishpaul777 commented Oct 6, 2024 •

edited

Loading

eh2077 Jan 2, 2025 •

edited

Loading