feat(navigation): hold tab state outside the back stack - #1379
Merged
Conversation
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.
github-actions
Bot
added
type: feature
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.
bmc08gt
changed the title
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
1 participant
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.
A tab press clears the whole backstack, so every tab was rebuilt from nothing on every press. Nav3 gives each entry its own
ViewModelStoreand saveable state, and both die with the entry, so returning to the wallet meant a cold ViewModel behind its loading spinner. That is what made the scanner-to-wallet switch after a claim read as a reload rather than a tab change.The obvious fix is to keep the tab homes on the stack, and it costs more than it looks.
NavDisplayenables back onscene.previousEntries.isNotEmpty()(NavDisplay.kt:557), so an entry left on the stack for state reasons is also an entry back stops at — back would walk the tabs the user had visited before leaving the app. Swallowing the pop doesn't help either; the predictive-back preview still runs and snaps back.So the state is held off the stack instead.
rememberViewModelStoreOwnerdocuments its store as outliving the composition that used it and being destroyed only by an explicitclearKey, and aNavEntryDecorator'sonPopis where Nav3's own decorators make that call.RetainedEntryStateis that pair of decorators withclearKeyandSaveableStateHolder.removeStateskipped for the content keys the host names.The host names the four routes a tab press produces, so a variant of a tab route — a resumed
Tips, say — still gets a fresh screen rather than being held forever. Everything else behaves as it did: a tab press still clears the stack, back still leaves the app from any tab, pushed screens and sheets are still dropped with the tab they were on, and deeplinks still arrive throughnavigateAll.Releasing
Held state is dropped once the back stack holds no tab route at all — signing out — so one account's ViewModels aren't waiting for whoever signs in next. A push keeps its tab home on the stack, so this only fires on a
replaceAllaway from the tabs.The ordering is the difficult part. 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, so the release routinely runs before the pops it is meant to discard.
RetentionLedgermarks itself released rather than only emptying, and those late pops clear instead of being held for the next account.Tests
RetentionLedgerTestcovers the ledger as a pure function: retained keys survive their pop and unretained ones don't, release drops what it holds exactly once, repeated switching between two tabs holds one entry each, a pop arriving after a release is neither retained nor held for a later release, and a retained key rendering again resumes retention.