From 4013068439bb63efbefeeef0a89581d51941f76f Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 31 Aug 2026 12:31:42 -0400 Subject: [PATCH 1/2] feat(navigation): keep tabs alive across a tab switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tab press cleared the whole backstack, so every tab was rebuilt from nothing on every press. Nav3 gives each entry its own ViewModelStore and saveable state, and both die with the entry, so returning to the wallet meant a cold ViewModel behind its loading spinner — which is what made the scanner-to-wallet switch after a claim read as a reload rather than a tab change. Tab homes now stay on the stack and the target moves to the top. Its entry is the one already there, so the tab comes back with its ViewModels, rememberSaveable state and list scroll position intact. Everything that is not a tab home is still dropped: the screens pushed on the outgoing tab and any sheet over them, which clearing always discarded and which would otherwise sit under the tab being opened. Back walks the visited tabs before it leaves the app, rather than leaving from whichever tab is showing. That is the cost of the retention — an entry has to be on the stack to survive — and it is not avoidable by refusing to pop: NavDisplay enables predictive back on stack depth alone, so a swallowed pop would show a seek preview and snap back. Two readers assumed the old shape. The nav bar's selection walked the stack from the bottom, which is now the tab visited first rather than the tab showing, and the scanner ran its camera only when it was the only entry on the stack, which never held again. Both now ask what is on top. MainRoot compares its launch graph from the active tab up, so the retained tabs beneath it don't read as a mismatch and reset the user to the launch route. Deeplinks are unaffected — they still arrive through navigateAll, which clears. A link is an entry point into the app rather than a move between tabs. --- .../app/internal/ui/AppNavigationBar.kt | 21 ++--- .../app/internal/ui/navigation/MainRoot.kt | 16 +++- .../flipcash/app/core/navigation/TabSwitch.kt | 57 +++++++++++++ .../app/core/navigation/TabSwitchTest.kt | 82 +++++++++++++++++++ .../flipcash/app/scanner/internal/Scanner.kt | 9 +- 5 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt create mode 100644 apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt index a7d69bd70..2136226d0 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt @@ -24,7 +24,7 @@ import com.flipcash.app.core.LocalUserManager import dev.chrisbanes.haze.HazeState import com.flipcash.app.core.navigation.NavBarButton import com.flipcash.app.core.navigation.asNavBarTab -import com.flipcash.app.core.navigation.destinationRoute +import com.flipcash.app.core.navigation.switchTab import com.flipcash.app.core.ui.NavigationBar import com.flipcash.app.core.ui.rememberNavigationBarState import com.flipcash.app.session.LocalSessionController @@ -39,8 +39,9 @@ import kotlinx.coroutines.flow.map /** * The hoisted navigation bar — root chrome, not owned by any screen. It renders over whichever - * top-level route is a tab home and switches tabs by **swapping the current screen** (single - * backstack, like a tab bar — hence [CodeNavigator.replaceAll], not a sheet). + * top-level route is a tab home and switches tabs by moving the target's home to the top of the + * backstack (see [switchTab]), keeping the tabs already visited alive rather than rebuilding each + * one on every press. * * Only visible when the current route maps to a tab. * @@ -60,9 +61,12 @@ internal fun AppNavigationBar( // the expansion doesn't recompose the bar. cardExpansion: CardExpansionController? = null, ) { - // Selection follows the base of the backstack (the tab "home"), so it stays correct while a - // sheet/modal sits on top and is right on launch. The top route only gates visibility. - val selectedTab = navigator.backStack.firstNotNullOfOrNull { (it as? AppRoute)?.asNavBarTab() } + // Selection follows the topmost tab home, so it stays correct while a sheet/modal or a pushed + // detail sits over it and is right on launch. Read from the top down rather than the bottom up + // because the tabs below the active one are the retained ones (see [switchTab]) — the base of + // the stack is the tab visited first, not the tab showing. The top route only gates visibility. + val selectedTab = navigator.backStack.asReversed() + .firstNotNullOfOrNull { (it as? AppRoute)?.asNavBarTab() } val topTab = (navigator.currentRouteKey as? AppRoute)?.asNavBarTab() // A BottomBar modal (e.g. Add Money) renders in the nav content, above this bar; hide the bar so @@ -126,10 +130,7 @@ internal fun AppNavigationBar( .padding(horizontal = CodeTheme.dimens.grid.x8) .padding(bottom = CodeTheme.dimens.grid.x3), state = state, - onButtonClick = { button -> - // Tab bar semantics: swap the current screen (single backstack). - navigator.replaceAll(button.destinationRoute()) - }, + onButtonClick = { button -> navigator.switchTab(button) }, hazeState = hazeState, avatar = avatar, ) diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt index 72712c060..493a6a6d4 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt @@ -27,6 +27,7 @@ import com.flipcash.app.core.AppRoute import com.flipcash.app.core.DisplayNameSource import com.flipcash.app.core.userprofile.UpdateProfileStep import com.flipcash.app.core.navigation.DeeplinkAction +import com.flipcash.app.core.navigation.asNavBarTab import com.flipcash.app.core.navigation.homeRoute import com.flipcash.app.core.extensions.navigateAll import com.flipcash.app.core.extensions.resolveBackStack @@ -123,7 +124,7 @@ internal fun MainRoot( } if (launch != null) { - val current = navigator.backStack.toList() + val current = navigator.backStack.toList().fromActiveTab() val target = launch.resolvedBackStack() // Skip if the current stack already matches or extends the target. @@ -169,6 +170,19 @@ internal data class LaunchNavGraph( resolveBackStack(baseRoutes, deeplinkRoutes) } +/** + * The backstack from the active tab up, dropping the tab homes retained beneath it. + * + * A tab switch keeps the tabs already visited on the stack (see `switchTab`), so the base of the + * stack is the tab visited first, not the one showing. The launch graph only ever describes the + * active tab and what sits over it, so comparing against the whole stack would read the retained + * tabs as a mismatch and reset the user to the launch route on any auth/flags re-emission. + */ +private fun List.fromActiveTab(): List { + val active = indexOfLast { (it as? AppRoute)?.asNavBarTab() != null } + return if (active > 0) drop(active) else this +} + /** * Returns true if [this] list starts with [prefix] (element-wise structural equality). * An exact match also returns true (prefix == full list). diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt new file mode 100644 index 000000000..d14524845 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt @@ -0,0 +1,57 @@ +package com.flipcash.app.core.navigation + +import androidx.compose.runtime.snapshots.Snapshot +import androidx.navigation3.runtime.NavKey +import com.flipcash.app.core.AppRoute +import com.getcode.navigation.core.CodeNavigator + +/** + * Switch to [tab]'s home, keeping the tab homes already visited. + * + * A tab press used to clear the whole backstack, so every tab was rebuilt from nothing on every + * press: Nav3 gives each entry its own `ViewModelStore` and saveable state, and both die with the + * entry. Returning to the wallet meant a cold ViewModel behind its loading spinner, which is what + * made the scanner-to-wallet switch after a claim read as a reload rather than a tab change. + * + * So the tab homes stay on the stack and the target moves to the top. Its entry — and with it its + * ViewModels, `rememberSaveable` state and list scroll position — is the one already there, so the + * tab comes back as the user left it. + * + * Back now walks the visited tabs before it leaves the app, rather than leaving from whichever tab + * is showing. That is the cost of the retention: an entry has to be on the stack to survive, and a + * stack the user can see is a stack they can go back through. + * + * Deeplinks are unaffected — they still arrive through `navigateAll`, which clears. A link is an + * entry point into the app rather than a move between tabs, and it should not inherit whatever the + * previous session left behind it. + */ +fun CodeNavigator.switchTab(tab: NavBarButton) { + val current = backStack.toList() + val next = current.afterSwitchingTo(tab) + if (next == current) return + + Snapshot.withMutableSnapshot { + backStack.clear() + backStack.addAll(next) + } +} + +/** + * The stack [switchTab] leaves behind, in the order back will walk it. + * + * Everything that is not a tab home is dropped: the screens pushed on the outgoing tab, and any + * sheet over them. Clearing the stack always discarded those, and carrying them across would leave + * another tab's detail screen sitting under the one being opened. + * + * A pure function over the keys, because that is the part worth asserting on — [switchTab] applies + * it to a live [androidx.navigation3.runtime.NavBackStack] inside a snapshot, so the whole swap + * lands in one frame and no entry is seen to leave and come back. + */ +fun List.afterSwitchingTo(tab: NavBarButton): List { + val destination = tab.destinationRoute() + val retained = filter { key -> + val home = (key as? AppRoute)?.asNavBarTab() + home != null && home != tab + } + return retained + destination +} diff --git a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt new file mode 100644 index 000000000..3fa810154 --- /dev/null +++ b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt @@ -0,0 +1,82 @@ +package com.flipcash.app.core.navigation + +import androidx.navigation3.runtime.NavKey +import com.flipcash.app.core.AppRoute +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertSame + +/** + * A tab press used to clear the backstack, which took each tab's ViewModels and saved state with it. + * These assert the shape of the stack it leaves instead — the retained entries are the whole point, + * since an entry that stays on the stack is an entry Nav3 does not tear down. + */ +class TabSwitchTest { + + private val scanner = AppRoute.Main.Scanner + private val wallet = AppRoute.Sheets.Wallet + private val menu = AppRoute.Sheets.Menu + + @Test + fun `the tab already visited stays on the stack, under the one being opened`() { + val stack = listOf(wallet) + + assertEquals(listOf(wallet, scanner), stack.afterSwitchingTo(NavBarButton.Scanner)) + } + + @Test + fun `switching back reuses the entry that is already there`() { + val stack = listOf(wallet).afterSwitchingTo(NavBarButton.Scanner) + + val back = stack.afterSwitchingTo(NavBarButton.Wallet) + + assertEquals(listOf(scanner, wallet), back) + // Same key instance, so it keeps the same contentKey and Nav3 hands the entry — its + // ViewModelStore, its saveable state — straight back rather than building a new one. + assertSame(wallet, back.last()) + } + + @Test + fun `a tab is never on the stack twice, however often it is pressed`() { + var stack = listOf(wallet) + repeat(3) { + stack = stack.afterSwitchingTo(NavBarButton.Scanner).afterSwitchingTo(NavBarButton.Wallet) + } + + assertEquals(listOf(scanner, wallet), stack) + } + + @Test + fun `screens pushed on the outgoing tab are dropped, not carried across`() { + val stack = listOf(wallet, AppRoute.Main.Sheet(AppRoute.Sheets.ActivityHistory)) + + // The wallet's home is kept — that is the entry being retained — but its open sheet is not, + // or it would sit underneath the scanner and be what back returned to. + assertEquals(listOf(wallet, scanner), stack.afterSwitchingTo(NavBarButton.Scanner)) + } + + @Test + fun `pressing the tab you are already on pops back to its home`() { + val stack = listOf(wallet, AppRoute.Main.Sheet(AppRoute.Sheets.ActivityHistory)) + + assertEquals(listOf(wallet), stack.afterSwitchingTo(NavBarButton.Wallet)) + } + + @Test + fun `pressing the tab you are on with nothing over it changes nothing`() { + val stack = listOf(scanner, wallet) + + assertEquals(stack, stack.afterSwitchingTo(NavBarButton.Wallet)) + } + + @Test + fun `back walks the tabs in the order they were visited`() { + var stack = listOf(wallet) + stack = stack.afterSwitchingTo(NavBarButton.TipCard) + stack = stack.afterSwitchingTo(NavBarButton.Scanner) + + // Back from the scanner returns to the You tab, then the wallet, then leaves the app — + // the deliberate cost of keeping the entries alive. + assertEquals(listOf(wallet, menu, scanner), stack) + } +} diff --git a/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt b/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt index 75c3ee410..12018a0f4 100644 --- a/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt +++ b/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt @@ -157,8 +157,13 @@ internal fun Scanner() { } } - LaunchedEffect(navigator.backStack.size) { - previewing = navigator.backStack.size <= 1 + // The camera runs while nothing covers the scanner. This used to ask whether the scanner was the + // only entry on the stack, which stopped meaning the same thing once a tab switch began keeping + // the tabs already visited underneath it (see `switchTab`): the scanner tab is reached with those + // beneath it, so the stack is never depth-1 and the preview never came up. Depth was only ever a + // proxy for "nothing is on top"; ask that instead. + LaunchedEffect(navigator.currentRouteKey) { + previewing = navigator.currentRouteKey == AppRoute.Main.Scanner } LaunchedEffect(billState.bill) { From 012ca2cb1c91643c9b62b14d6a28638cfd77a978 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 31 Aug 2026 13:12:42 -0400 Subject: [PATCH 2/2] feat(navigation): hold tab state outside the back stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keeping the tab homes on the stack, as the previous commit did, made back walk the tabs the user had visited before it left the app. That was the price of that retention, and it is not worth paying: NavDisplay enables back on `scene.previousEntries.isNotEmpty()`, so an entry left on the stack for state reasons is also an entry back stops at, and swallowing the pop only buys a predictive-back preview that snaps back. The state does not have to be on the stack. `rememberViewModelStoreOwner` documents its store as outliving the composition that used it, destroyed only by an explicit `clearKey`, and a `NavEntryDecorator`'s `onPop` is where Nav3's own decorators make that call. RetainedEntryState is that pair of decorators with `clearKey` and `SaveableStateHolder.removeState` skipped for the content keys the host names — here the four routes a tab press produces. A tab press still clears the stack, so back leaves the app from any tab exactly as before, and the tab still comes back to its ViewModels, rememberSaveable state and scroll position. Held state is dropped once the back stack holds no tab route, so a signed-out account's ViewModels are not waiting for whoever signs in next. A pop is reported only after the entry's content leaves composition, which lands it a transition later than the back stack change that caused it — meaning the release routinely runs before the pops it means to discard. RetentionLedger marks itself released rather than only emptying, so those late pops clear too. The stack shape is unchanged, so the tab-switch helper and the three readers the previous commit had to adjust are gone with it. --- .../app/internal/ui/AppNavigationBar.kt | 21 ++- .../app/internal/ui/navigation/AppContent.kt | 24 +++ .../app/internal/ui/navigation/MainRoot.kt | 16 +- .../flipcash/app/core/navigation/TabSwitch.kt | 57 ------- .../app/core/navigation/TabSwitchTest.kt | 82 ---------- .../flipcash/app/scanner/internal/Scanner.kt | 9 +- ui/navigation/build.gradle.kts | 1 + .../com/getcode/navigation/AppNavHost.kt | 12 +- .../decorators/RetainedEntryState.kt | 150 ++++++++++++++++++ .../decorators/RetentionLedgerTest.kt | 91 +++++++++++ 10 files changed, 286 insertions(+), 177 deletions(-) delete mode 100644 apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt delete mode 100644 apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt create mode 100644 ui/navigation/src/main/kotlin/com/getcode/navigation/decorators/RetainedEntryState.kt create mode 100644 ui/navigation/src/test/kotlin/com/getcode/navigation/decorators/RetentionLedgerTest.kt diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt index 2136226d0..a7d69bd70 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/AppNavigationBar.kt @@ -24,7 +24,7 @@ import com.flipcash.app.core.LocalUserManager import dev.chrisbanes.haze.HazeState import com.flipcash.app.core.navigation.NavBarButton import com.flipcash.app.core.navigation.asNavBarTab -import com.flipcash.app.core.navigation.switchTab +import com.flipcash.app.core.navigation.destinationRoute import com.flipcash.app.core.ui.NavigationBar import com.flipcash.app.core.ui.rememberNavigationBarState import com.flipcash.app.session.LocalSessionController @@ -39,9 +39,8 @@ import kotlinx.coroutines.flow.map /** * The hoisted navigation bar — root chrome, not owned by any screen. It renders over whichever - * top-level route is a tab home and switches tabs by moving the target's home to the top of the - * backstack (see [switchTab]), keeping the tabs already visited alive rather than rebuilding each - * one on every press. + * top-level route is a tab home and switches tabs by **swapping the current screen** (single + * backstack, like a tab bar — hence [CodeNavigator.replaceAll], not a sheet). * * Only visible when the current route maps to a tab. * @@ -61,12 +60,9 @@ internal fun AppNavigationBar( // the expansion doesn't recompose the bar. cardExpansion: CardExpansionController? = null, ) { - // Selection follows the topmost tab home, so it stays correct while a sheet/modal or a pushed - // detail sits over it and is right on launch. Read from the top down rather than the bottom up - // because the tabs below the active one are the retained ones (see [switchTab]) — the base of - // the stack is the tab visited first, not the tab showing. The top route only gates visibility. - val selectedTab = navigator.backStack.asReversed() - .firstNotNullOfOrNull { (it as? AppRoute)?.asNavBarTab() } + // Selection follows the base of the backstack (the tab "home"), so it stays correct while a + // sheet/modal sits on top and is right on launch. The top route only gates visibility. + val selectedTab = navigator.backStack.firstNotNullOfOrNull { (it as? AppRoute)?.asNavBarTab() } val topTab = (navigator.currentRouteKey as? AppRoute)?.asNavBarTab() // A BottomBar modal (e.g. Add Money) renders in the nav content, above this bar; hide the bar so @@ -130,7 +126,10 @@ internal fun AppNavigationBar( .padding(horizontal = CodeTheme.dimens.grid.x8) .padding(bottom = CodeTheme.dimens.grid.x3), state = state, - onButtonClick = { button -> navigator.switchTab(button) }, + onButtonClick = { button -> + // Tab bar semantics: swap the current screen (single backstack). + navigator.replaceAll(button.destinationRoute()) + }, hazeState = hazeState, avatar = avatar, ) diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppContent.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppContent.kt index fbc204bb8..760ab37a9 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppContent.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppContent.kt @@ -12,8 +12,10 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp @@ -28,8 +30,10 @@ import com.flipcash.app.cardexpand.LocalCardExpansion import com.flipcash.app.core.AppRoute import com.flipcash.app.core.navigation.DeeplinkAction import com.flipcash.app.core.navigation.LocalTabBarVisibility +import com.flipcash.app.core.navigation.NavBarButton import com.flipcash.app.core.navigation.TabBarVisibilityController import com.flipcash.app.core.navigation.asNavBarTab +import com.flipcash.app.core.navigation.destinationRoute import com.flipcash.app.core.ui.transitions.CardExpandTransition import com.flipcash.app.internal.ui.AppNavigationBar import com.flipcash.app.internal.ui.navigation.decorators.rememberNavBillOverlayEntryDecorator @@ -38,6 +42,7 @@ import com.flipcash.app.internal.ui.navigation.decorators.rememberNavMessagingEn import com.flipcash.app.internal.ui.navigation.decorators.rememberNavTabBarInsetEntryDecorator import com.getcode.navigation.AppNavHost import com.getcode.navigation.core.CodeNavigator +import com.getcode.navigation.decorators.rememberRetainedEntryState import com.getcode.navigation.results.NavResultStateRegistry import com.getcode.navigation.scenes.ModalBottomSheetSceneStrategy import com.getcode.ui.components.bars.BarManager @@ -86,6 +91,24 @@ internal fun AppContent( // full screen in place, so there's no route change for the visibility rule below to notice. val tabBarVisibility = remember { TabBarVisibilityController() } + // A tab press replaces the whole back stack (tab-bar semantics — see AppNavigationBar), so every + // tab home was destroyed and rebuilt on each switch: the wallet re-fetched its balances and the + // chat list scrolled back to the top. Hold each tab home's ViewModels and scroll state outside the + // back stack so a switch shows what the tab last had. Only the four routes a tab press produces + // are held, so variants of a tab route (a resumed Tips, say) still get a fresh screen. + val tabHomeKeys = remember { + NavBarButton.entries.map { it.destinationRoute().toString() }.toSet() + } + val entryState = rememberRetainedEntryState { it in tabHomeKeys } + + // Leaving the tabs altogether — signing out — is where held state stops being the user's own. A + // push keeps its tab home on the stack, so this fires on a replaceAll away from the tabs. + LaunchedEffect(entryState, codeNavigator.backStack) { + snapshotFlow { + codeNavigator.backStack.any { (it as? AppRoute)?.asNavBarTab() != null } + }.collect { onTabs -> if (!onTabs) entryState.releaseAll() } + } + // Card-expand (iOS #587): the wallet requests an expansion (via LocalCardExpansion); the detail is // drawn by CardExpandHost inside the wallet entry, driven by one progress scalar, so the deck stays // composed and reorganises behind it. See CardExpansionController / CurrencyInfoExpansion. @@ -101,6 +124,7 @@ internal fun AppContent( AppNavHost( navigator = codeNavigator, resultStateRegistry = resultStateRegistry, + entryState = entryState, decorators = listOf( // First = outermost, and outermost draws last: a bottom bar message is a prompt // that has to be answered, so it sits above everything the entry draws — the diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt index 493a6a6d4..72712c060 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/MainRoot.kt @@ -27,7 +27,6 @@ import com.flipcash.app.core.AppRoute import com.flipcash.app.core.DisplayNameSource import com.flipcash.app.core.userprofile.UpdateProfileStep import com.flipcash.app.core.navigation.DeeplinkAction -import com.flipcash.app.core.navigation.asNavBarTab import com.flipcash.app.core.navigation.homeRoute import com.flipcash.app.core.extensions.navigateAll import com.flipcash.app.core.extensions.resolveBackStack @@ -124,7 +123,7 @@ internal fun MainRoot( } if (launch != null) { - val current = navigator.backStack.toList().fromActiveTab() + val current = navigator.backStack.toList() val target = launch.resolvedBackStack() // Skip if the current stack already matches or extends the target. @@ -170,19 +169,6 @@ internal data class LaunchNavGraph( resolveBackStack(baseRoutes, deeplinkRoutes) } -/** - * The backstack from the active tab up, dropping the tab homes retained beneath it. - * - * A tab switch keeps the tabs already visited on the stack (see `switchTab`), so the base of the - * stack is the tab visited first, not the one showing. The launch graph only ever describes the - * active tab and what sits over it, so comparing against the whole stack would read the retained - * tabs as a mismatch and reset the user to the launch route on any auth/flags re-emission. - */ -private fun List.fromActiveTab(): List { - val active = indexOfLast { (it as? AppRoute)?.asNavBarTab() != null } - return if (active > 0) drop(active) else this -} - /** * Returns true if [this] list starts with [prefix] (element-wise structural equality). * An exact match also returns true (prefix == full list). diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt deleted file mode 100644 index d14524845..000000000 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/navigation/TabSwitch.kt +++ /dev/null @@ -1,57 +0,0 @@ -package com.flipcash.app.core.navigation - -import androidx.compose.runtime.snapshots.Snapshot -import androidx.navigation3.runtime.NavKey -import com.flipcash.app.core.AppRoute -import com.getcode.navigation.core.CodeNavigator - -/** - * Switch to [tab]'s home, keeping the tab homes already visited. - * - * A tab press used to clear the whole backstack, so every tab was rebuilt from nothing on every - * press: Nav3 gives each entry its own `ViewModelStore` and saveable state, and both die with the - * entry. Returning to the wallet meant a cold ViewModel behind its loading spinner, which is what - * made the scanner-to-wallet switch after a claim read as a reload rather than a tab change. - * - * So the tab homes stay on the stack and the target moves to the top. Its entry — and with it its - * ViewModels, `rememberSaveable` state and list scroll position — is the one already there, so the - * tab comes back as the user left it. - * - * Back now walks the visited tabs before it leaves the app, rather than leaving from whichever tab - * is showing. That is the cost of the retention: an entry has to be on the stack to survive, and a - * stack the user can see is a stack they can go back through. - * - * Deeplinks are unaffected — they still arrive through `navigateAll`, which clears. A link is an - * entry point into the app rather than a move between tabs, and it should not inherit whatever the - * previous session left behind it. - */ -fun CodeNavigator.switchTab(tab: NavBarButton) { - val current = backStack.toList() - val next = current.afterSwitchingTo(tab) - if (next == current) return - - Snapshot.withMutableSnapshot { - backStack.clear() - backStack.addAll(next) - } -} - -/** - * The stack [switchTab] leaves behind, in the order back will walk it. - * - * Everything that is not a tab home is dropped: the screens pushed on the outgoing tab, and any - * sheet over them. Clearing the stack always discarded those, and carrying them across would leave - * another tab's detail screen sitting under the one being opened. - * - * A pure function over the keys, because that is the part worth asserting on — [switchTab] applies - * it to a live [androidx.navigation3.runtime.NavBackStack] inside a snapshot, so the whole swap - * lands in one frame and no entry is seen to leave and come back. - */ -fun List.afterSwitchingTo(tab: NavBarButton): List { - val destination = tab.destinationRoute() - val retained = filter { key -> - val home = (key as? AppRoute)?.asNavBarTab() - home != null && home != tab - } - return retained + destination -} diff --git a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt b/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt deleted file mode 100644 index 3fa810154..000000000 --- a/apps/flipcash/core/src/test/kotlin/com/flipcash/app/core/navigation/TabSwitchTest.kt +++ /dev/null @@ -1,82 +0,0 @@ -package com.flipcash.app.core.navigation - -import androidx.navigation3.runtime.NavKey -import com.flipcash.app.core.AppRoute -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertSame - -/** - * A tab press used to clear the backstack, which took each tab's ViewModels and saved state with it. - * These assert the shape of the stack it leaves instead — the retained entries are the whole point, - * since an entry that stays on the stack is an entry Nav3 does not tear down. - */ -class TabSwitchTest { - - private val scanner = AppRoute.Main.Scanner - private val wallet = AppRoute.Sheets.Wallet - private val menu = AppRoute.Sheets.Menu - - @Test - fun `the tab already visited stays on the stack, under the one being opened`() { - val stack = listOf(wallet) - - assertEquals(listOf(wallet, scanner), stack.afterSwitchingTo(NavBarButton.Scanner)) - } - - @Test - fun `switching back reuses the entry that is already there`() { - val stack = listOf(wallet).afterSwitchingTo(NavBarButton.Scanner) - - val back = stack.afterSwitchingTo(NavBarButton.Wallet) - - assertEquals(listOf(scanner, wallet), back) - // Same key instance, so it keeps the same contentKey and Nav3 hands the entry — its - // ViewModelStore, its saveable state — straight back rather than building a new one. - assertSame(wallet, back.last()) - } - - @Test - fun `a tab is never on the stack twice, however often it is pressed`() { - var stack = listOf(wallet) - repeat(3) { - stack = stack.afterSwitchingTo(NavBarButton.Scanner).afterSwitchingTo(NavBarButton.Wallet) - } - - assertEquals(listOf(scanner, wallet), stack) - } - - @Test - fun `screens pushed on the outgoing tab are dropped, not carried across`() { - val stack = listOf(wallet, AppRoute.Main.Sheet(AppRoute.Sheets.ActivityHistory)) - - // The wallet's home is kept — that is the entry being retained — but its open sheet is not, - // or it would sit underneath the scanner and be what back returned to. - assertEquals(listOf(wallet, scanner), stack.afterSwitchingTo(NavBarButton.Scanner)) - } - - @Test - fun `pressing the tab you are already on pops back to its home`() { - val stack = listOf(wallet, AppRoute.Main.Sheet(AppRoute.Sheets.ActivityHistory)) - - assertEquals(listOf(wallet), stack.afterSwitchingTo(NavBarButton.Wallet)) - } - - @Test - fun `pressing the tab you are on with nothing over it changes nothing`() { - val stack = listOf(scanner, wallet) - - assertEquals(stack, stack.afterSwitchingTo(NavBarButton.Wallet)) - } - - @Test - fun `back walks the tabs in the order they were visited`() { - var stack = listOf(wallet) - stack = stack.afterSwitchingTo(NavBarButton.TipCard) - stack = stack.afterSwitchingTo(NavBarButton.Scanner) - - // Back from the scanner returns to the You tab, then the wallet, then leaves the app — - // the deliberate cost of keeping the entries alive. - assertEquals(listOf(wallet, menu, scanner), stack) - } -} diff --git a/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt b/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt index 12018a0f4..75c3ee410 100644 --- a/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt +++ b/apps/flipcash/features/scanner/src/main/kotlin/com/flipcash/app/scanner/internal/Scanner.kt @@ -157,13 +157,8 @@ internal fun Scanner() { } } - // The camera runs while nothing covers the scanner. This used to ask whether the scanner was the - // only entry on the stack, which stopped meaning the same thing once a tab switch began keeping - // the tabs already visited underneath it (see `switchTab`): the scanner tab is reached with those - // beneath it, so the stack is never depth-1 and the preview never came up. Depth was only ever a - // proxy for "nothing is on top"; ask that instead. - LaunchedEffect(navigator.currentRouteKey) { - previewing = navigator.currentRouteKey == AppRoute.Main.Scanner + LaunchedEffect(navigator.backStack.size) { + previewing = navigator.backStack.size <= 1 } LaunchedEffect(billState.bill) { diff --git a/ui/navigation/build.gradle.kts b/ui/navigation/build.gradle.kts index f06c3bd5a..6e8e974f1 100644 --- a/ui/navigation/build.gradle.kts +++ b/ui/navigation/build.gradle.kts @@ -27,6 +27,7 @@ dependencies { implementation(libs.bundles.hilt) ksp(libs.bundles.hilt.compiler) + api(libs.compose.view.models) api(libs.navigation3.runtime) api(libs.navigation3.ui) api(libs.lifecycle.viewmodel.navigation3) diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/AppNavHost.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/AppNavHost.kt index dc77b04ea..e2b8a2ce4 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/AppNavHost.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/AppNavHost.kt @@ -22,18 +22,18 @@ import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.luminance import androidx.compose.ui.graphics.toArgb -import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator import androidx.navigation3.runtime.NavEntry import androidx.navigation3.runtime.NavEntryDecorator import androidx.navigation3.runtime.NavKey -import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator import androidx.navigation3.scene.Scene import androidx.navigation3.scene.SceneStrategy import androidx.navigation3.scene.SinglePaneSceneStrategy import androidx.navigation3.ui.NavDisplay import com.getcode.animation.LocalSharedTransitionScope import com.getcode.navigation.core.CodeNavigator +import com.getcode.navigation.decorators.RetainedEntryState import com.getcode.navigation.decorators.rememberNavResultScopeEntryDecorator +import com.getcode.navigation.decorators.rememberRetainedEntryState import com.getcode.navigation.results.NavResultStateRegistry import com.getcode.navigation.results.rememberNavResultStateRegistry import com.getcode.theme.CodeTheme @@ -62,6 +62,10 @@ fun AppNavHost( popTransitionSpec() }, onBack: (() -> Unit)? = null, + // Owns each entry's ViewModel store and rememberSaveable state. Defaults to retaining nothing, + // which is Nav3's own behaviour; a host passes one that retains keys whose state should survive + // their entry (the tab homes). + entryState: RetainedEntryState = rememberRetainedEntryState(), entryProvider: (key: NavKey) -> NavEntry, decorators: List> = emptyList(), ) { @@ -102,9 +106,7 @@ fun AppNavHost( transitionSpec = transitionSpec, popTransitionSpec = popTransitionSpec, predictivePopTransitionSpec = predictivePopTransitionSpec, - entryDecorators = listOf( - rememberSaveableStateHolderNavEntryDecorator(), - rememberViewModelStoreNavEntryDecorator(), + entryDecorators = entryState.decorators + listOf( rememberNavResultScopeEntryDecorator( backStack = navigator.backStack, navResultStore = navigator.resultStore, diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/decorators/RetainedEntryState.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/decorators/RetainedEntryState.kt new file mode 100644 index 000000000..57d2d1550 --- /dev/null +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/decorators/RetainedEntryState.kt @@ -0,0 +1,150 @@ +package com.getcode.navigation.decorators + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.Stable +import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.SaveableStateHolder +import androidx.compose.runtime.saveable.rememberSaveableStateHolder +import androidx.lifecycle.viewmodel.ViewModelStoreProvider +import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner +import androidx.lifecycle.viewmodel.compose.rememberViewModelStoreOwner +import androidx.lifecycle.viewmodel.compose.rememberViewModelStoreProvider +import androidx.navigation3.runtime.NavEntryDecorator +import androidx.navigation3.runtime.NavKey +import androidx.savedstate.compose.LocalSavedStateRegistryOwner + +/** + * Book-keeping for [RetainedEntryState]: which content keys keep their state after their entry is + * gone, and when that stops being true. + * + * Split out from the decorators because the ordering here is the whole difficulty, and none of it + * needs Compose to be exercised. A pop is reported only once the entry's content has left + * composition, so it lands a transition later than the back stack change that caused it — which + * means [release] routinely runs *before* the pops it is meant to discard. Marking the ledger + * released rather than only emptying it is what makes those late arrivals clear instead of being + * held onto for the next account. + * + * @param retains whether a content key is one whose state should outlive its entry. + */ +internal class RetentionLedger(private val retains: (contentKey: Any) -> Boolean) { + private val held = linkedSetOf() + private var released = false + + /** A retainable entry is on screen again, so retention resumes. */ + fun onRendered(contentKey: Any) { + if (retains(contentKey)) released = false + } + + /** True when [contentKey]'s state should survive the pop that just happened. */ + fun onPopped(contentKey: Any): Boolean { + val keep = !released && retains(contentKey) + if (keep) held += contentKey + return keep + } + + /** + * Everything currently held, dropped. Pops still in flight are dropped too, until the next + * retainable entry renders. + */ + fun release(): List { + released = true + val dropped = held.toList() + held.clear() + return dropped + } +} + +/** + * Per-entry ViewModel and `rememberSaveable` state for a [androidx.navigation3.ui.NavDisplay], with + * an opt-in for the entries whose state should survive them. + * + * Nav3 scopes both to the entry and destroys both with it, which is right for a screen that was + * pushed and popped. It is wrong for the tab homes: a tab press replaces the whole back stack, so + * every tab came back cold — a fresh ViewModel with its default state, and a list scrolled back to + * the top. Retaining the entries on the back stack instead would fix that, but it also puts them in + * front of back: [androidx.navigation3.ui.NavDisplay] enables its back handler on + * `scene.previousEntries.isNotEmpty()`, so anything left underneath for state reasons is also + * something back walks through. Holding the state here keeps the back stack — and back — as they + * were. + * + * The mechanism is the one AndroidX documents on + * [androidx.lifecycle.viewmodel.compose.rememberViewModelStoreOwner]: a store outlives the + * composition that used it and is destroyed only by an explicit + * [ViewModelStoreProvider.clearKey]. These decorators are the stock pair with that call, and + * [SaveableStateHolder.removeState], skipped for the retained keys. + * + * State is held against the entry's `contentKey`, so a tab comes back to what it had only if it + * comes back under the same key. + */ +@Stable +class RetainedEntryState internal constructor( + private val saveableStateHolder: SaveableStateHolder, + private val viewModelStoreProvider: ViewModelStoreProvider, + private val ledger: RetentionLedger, +) { + /** + * Outermost first. Saveable state wraps the ViewModel store because a store's + * [androidx.lifecycle.SavedStateHandle] is restored through the saved state registry the + * saveable decorator provides. + */ + val decorators: List> = listOf( + NavEntryDecorator( + onPop = { contentKey -> + if (!ledger.onPopped(contentKey)) saveableStateHolder.removeState(contentKey) + }, + decorate = { entry -> + ledger.onRendered(entry.contentKey) + saveableStateHolder.SaveableStateProvider(entry.contentKey) { entry.Content() } + }, + ), + NavEntryDecorator( + onPop = { contentKey -> + if (!ledger.onPopped(contentKey)) viewModelStoreProvider.clearKey(contentKey) + }, + decorate = { entry -> + val owner = rememberViewModelStoreOwner( + key = entry.contentKey, + provider = viewModelStoreProvider, + savedStateRegistryOwner = LocalSavedStateRegistryOwner.current, + ) + CompositionLocalProvider(LocalViewModelStoreOwner provides owner) { entry.Content() } + }, + ), + ) + + /** + * Drop everything being held, and stop holding until a retainable entry renders again. + * + * The caller decides when retention has stopped meaning anything — for the tab homes, when the + * user is no longer on the tabs at all. Without this the ViewModels of a signed-out account + * would be waiting for whoever signs in next. + */ + fun releaseAll() { + ledger.release().forEach { contentKey -> + saveableStateHolder.removeState(contentKey) + viewModelStoreProvider.clearKey(contentKey) + } + } +} + +/** + * A [RetainedEntryState] remembered across recompositions. + * + * @param retains whether an entry's `contentKey` names state that should outlive the entry. + * Retains nothing by default, which is Nav3's own behaviour. + */ +@Composable +fun rememberRetainedEntryState( + retains: (contentKey: Any) -> Boolean = { false }, +): RetainedEntryState { + val saveableStateHolder = rememberSaveableStateHolder() + val viewModelStoreProvider = rememberViewModelStoreProvider() + return remember(saveableStateHolder, viewModelStoreProvider) { + RetainedEntryState( + saveableStateHolder = saveableStateHolder, + viewModelStoreProvider = viewModelStoreProvider, + ledger = RetentionLedger(retains), + ) + } +} diff --git a/ui/navigation/src/test/kotlin/com/getcode/navigation/decorators/RetentionLedgerTest.kt b/ui/navigation/src/test/kotlin/com/getcode/navigation/decorators/RetentionLedgerTest.kt new file mode 100644 index 000000000..67387f392 --- /dev/null +++ b/ui/navigation/src/test/kotlin/com/getcode/navigation/decorators/RetentionLedgerTest.kt @@ -0,0 +1,91 @@ +package com.getcode.navigation.decorators + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class RetentionLedgerTest { + + private val tabs = setOf("Wallet", "Chats", "TipCard", "Scanner") + + private fun ledger() = RetentionLedger { it in tabs } + + @Test + fun `a retained key survives its pop`() { + assertTrue(ledger().onPopped("Wallet")) + } + + @Test + fun `an unretained key does not`() { + assertFalse(ledger().onPopped("CurrencyInfo(usdc)")) + } + + @Test + fun `release drops everything held`() { + val ledger = ledger() + ledger.onPopped("Wallet") + ledger.onPopped("Scanner") + ledger.onPopped("CurrencyInfo(usdc)") + + assertEquals(listOf("Wallet", "Scanner"), ledger.release()) + } + + @Test + fun `release drops each key once`() { + val ledger = ledger() + ledger.onPopped("Wallet") + ledger.release() + + assertEquals(emptyList(), ledger.release()) + } + + @Test + fun `switching between the same two tabs holds one entry each`() { + val ledger = ledger() + repeat(3) { + ledger.onPopped("Wallet") + ledger.onPopped("Scanner") + } + + assertEquals(listOf("Wallet", "Scanner"), ledger.release()) + } + + // A pop is reported only once the entry's content has left composition, so on logout the pops of + // the tab homes land after the back stack has already stopped containing any. Retaining them then + // would leave the signed-out account's ViewModels waiting for whoever signs in next. + @Test + fun `a pop arriving after release is not retained`() { + val ledger = ledger() + ledger.release() + + assertFalse(ledger.onPopped("Wallet")) + } + + @Test + fun `a pop arriving after release is not held for a later release`() { + val ledger = ledger() + ledger.release() + ledger.onPopped("Wallet") + + assertEquals(emptyList(), ledger.release()) + } + + @Test + fun `rendering a retained key resumes retention`() { + val ledger = ledger() + ledger.release() + ledger.onRendered("Wallet") + + assertTrue(ledger.onPopped("Wallet")) + } + + @Test + fun `rendering an unretained key does not resume retention`() { + val ledger = ledger() + ledger.release() + ledger.onRendered("CurrencyInfo(usdc)") + + assertFalse(ledger.onPopped("Wallet")) + } +}