refactor(nav): split AppRoute by how a route arrives - #1418
Open
bmc08gt wants to merge 2 commits into
Open
Conversation
AppRoute.Sheets held three unrelated populations: the four nav-bar tab homes, screens that are only ever pushed (ActivityHistory, TransactionDetails, Give), and actual modals. resolveRoutes wrapped every member in Main.Sheet, so the tab homes needed an asNavBarTab() carve-out to escape and the pushed screens had no escape at all — a deeplink naming one landed in a modal instead of on the screen the rest of the app pushes. Three groupings now say how a route arrives: - Tabs — the four nav-bar homes. navBarButton() is exhaustive over the type, so a new tab home fails to compile until it is given a button. - Sheets — modals that host an inner backstack, which is the only thing the Main.Sheet wrapping buys. A modal needing no nested navigation implements com.getcode.navigation.Sheet directly; that marker, not this grouping, is what ModalBottomSheetSceneStrategy reads. - Main — ordinary pushes. resolveRoutes loses the carve-out and wraps on membership alone. In-app behaviour is unchanged: every entry to the three moved screens already went through navigator.push, and AppRouter only feeds tab homes into navigateAll. A deeplink to one of them now lands on the pushed screen. Route class names change, so a back stack saved by an older build will not restore across the update. Maestro tags derive from simpleName, all of which are preserved.
Three scanner destinations the new AppRoute.Tabs type makes visible as errors: The wallet's new-user tutorial dispatched "scan a tip card" as OpenScreen, which WalletScreen handles with navigator.push — so the scanner arrived stacked on the wallet, and Back returned to the wallet instead of behaving like the nav bar. It now dispatches SwitchTab, handled with replaceAll, the same call AppNavigationBar makes. PurchaseAccountScreen released to the scanner on account creation rather than to homeRoute, which every other onboarding exit uses. NotificationPermissionScreen and NotificationPermissionRationaleScreen were a second copy of the permission screens left behind by the FlowHost migration: nothing calls them, and AppRoute.Onboarding.NotificationPermission has no annotatedEntry to reach them by. The live path is OnboardingFlowScreen's NotificationPermissionStepContent, which releases to homeRoute. Deleted rather than repointed — the file's only effect was to make the wrong destination look supported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1417 — review that first.
AppRoute.Sheetsheld three unrelated populations: the four nav-bar tab homes, screens that are only ever pushed (ActivityHistory,TransactionDetails,Give), and actual modals.resolveRouteswrapped every member inMain.Sheet, so the tab homes needed anasNavBarTab()carve-out to escape it and the pushed screens had no escape at all — a deeplink naming one landed in a modal instead of on the screen the rest of the app pushes.Three groupings now say how a route arrives:
Tabs— the four nav-bar homes (Scanner,Wallet,Tips,Menu).navBarButton()is exhaustive over the type, so a new tab home fails to compile until it is given a button rather than silently behaving as an ordinary push.Sheets— modals that host an inner backstack (TokenSelection,TipAmountEntry,ShareApp). ThatMain.Sheetpacking is the only thing membership buys, so a modal needing no nested navigation implementscom.getcode.navigation.Sheetdirectly instead — that marker, not this grouping, is whatModalBottomSheetSceneStrategyreads to present a route as a sheet.Main— ordinary pushes onto whatever stack the user is on.resolveRoutesdrops the carve-out and wraps on membership alone;leadsWithTabbecomesroutes.firstOrNull() is AppRoute.Tabs.Behaviour
Unchanged in-app. Every entry to the three moved screens already goes through
navigator.push(WalletScreen.kt:53,TokenInfoScreen.kt:195,CurrencyInfoExpansion.kt:123,ActivityHistoryScreen.kt:69), andAppRouteronly ever feeds tab homes intonavigateAll. The one change is the fix: a deeplink naming one of the three now lands on the pushed screen.Route class names change, so a back stack saved by an older build will not restore across the update — inherent to any route rename. Maestro tags derive from
simpleName, all of which are preserved, so the flows are untouched.Three scanner destinations the new type made visible
Second commit. Once the tab homes are their own type, code that treats one as an ordinary push reads as a mistake rather than as a convention.
OpenScreen, whichWalletScreenhandles withnavigator.push— the scanner arrived stacked on the wallet, and Back returned to the wallet instead of behaving like the nav bar. It now dispatches aSwitchTabevent handled withreplaceAll, the same callAppNavigationBarmakes.PurchaseAccountScreenreleased to the scanner on account creation rather than tohomeRoute, which every other onboarding exit uses.NotificationPermissionScreen.ktwas a second copy of the permission screens left behind by the FlowHost migration, navigating to the scanner from both of its composables. Nothing calls them andAppRoute.Onboarding.NotificationPermissionhas noannotatedEntryto reach them by; the live path isOnboardingFlowScreen'sNotificationPermissionStepContent, which releases tohomeRoute. Deleted rather than repointed.