refactor(navigation): hierarchical, route-dispatching CodeNavigator#1073
Merged
Conversation
…ator
- Convert CodeNavigator from data class to class (copy()/componentN() unused)
- Add optional parent, isFlowNavigator, flowScope fields (all defaulted)
- Add FlowScope interface with exitWithResult + dismiss contract
- Thread new params through rememberCodeNavigator (existing callers unaffected)
- Stand up JVM unit-test harness: TestNav fixtures + CodeNavigatorHarnessTest
- Add kotlin("test") + bundles.unit.testing to ui:navigation test deps
…t chain FlowSteps land on the nearest flow navigator; all other routes bubble up to the app-root stack via the parent chain, so feature screens need only one navigator handle regardless of context.
…gator InnerFlowNavigator.back(), exitWithResult(), and navigate() now delegate to the hierarchical CodeNavigator methods (navigateBack, navigateBackWithResult, navigate) rather than manipulating state directly or holding a separate outerNavigator reference. The class is promoted to internal so JVM tests can construct it directly. Characterization tests added covering all six delegation paths.
The conversation screen was rebuilt from zero-initialized layout state on every open and every pop-back from the amount-entry step, causing a visible reflow of the message list: - ChatInputScaffold measured its top/bottom bars via onSizeChanged into mutableStateOf(0.dp), so the message LazyColumn received (0,0) contentPadding on the first frame and snapped to the real bar heights on the next — a ~70dp vertical jump. Replaced with a SubcomposeLayout that measures the bars before the content in the same pass, so padding is correct on frame 1. - refreshSettled was a plain remember(false) that re-armed the content gate on pop-back; made it rememberSaveable so it survives the round-trip. - OnChatOpened re-dispatched on re-entry with no idempotency guard, re-running loadMessages() and invalidating Paging. Skip the redundant reload when re-entering the same already-open chat; live delivery via the app-scoped event stream keeps Room current independently, so no messages are missed.
51ba2f9 to
2a5cefd
Compare
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.
Summary
Simplifies the "too many navigators inside a flow" problem. Inside a
FlowHosta screen previously had to reason about several navigator handles (inner flow navigator, outer/app navigator, sheet navigator,FlowNavigator), andLocalCodeNavigator.currentsilently resolved to different instances depending on context. This makesCodeNavigatorhierarchical and route-type-dispatching so a screen reaches every scope through the oneLocalCodeNavigatorit holds — aFlowStepstays on the flow stack, anything else bubbles up theparentchain to the app-root navigator.Rebased onto current
code/cash.Phases (staged, each behind the previous)
Phase 1 — engine (behind existing API, no behavior change)
CodeNavigatorgainsparent,isFlowNavigator, and aFlowScope;navigate()dispatches by route type.navigateBackWithResult()anddismiss()intents (delegating toFlowScope).FlowHostwires the inner navigator withparent+FlowScope; sheet-dismiss guarded onisSheetRoot.Phase 2 —
FlowNavigatorthin adapterInnerFlowNavigatorreimplemented as a thin adapter delegatingback/navigate/exitWithResultto the engine; redundantouterNavigatorfield removed. Behavior-preserving.Phase 3 — call-site migration (kill the context-ambiguity)
LocalCodeNavigator, removing silentflowAnnotatedEntry/CompositionLocalProviderre-shadows and collapsing redundant handles.Phase 4 — eliminate
LocalOuterCodeNavigatordispatchTarget(route)(the navigator a route lands on) androotNavigator(the sheet-hosting root).navigateForResultregisters its callback ondispatchTarget(route)— so a cross-flow-boundary result nav works from the inner navigator (the callback lands on the store the returning screen delivers to). Intra-flowFlowStepresults are unchanged (dispatchTargetreturnsthis).openAsSheetoperates onrootNavigator— so it works from a flow's inner navigator. Backward-compatible: existing root callers getrootNavigator === this.LocalCodeNavigator; deadflowAnnotatedEntryandLocalOuterCodeNavigatordeleted.LocalSheetNavigatorintentionally retained (chat's legitimate sheet-host handle).Also included
fix(chat): eliminate message-list reflow on open and back-navigation. The conversation screen rebuilt from zero-initialized layout state on every open/pop.ChatInputScaffoldnow usesSubcomposeLayout(correct list padding on frame 1),refreshSettledisrememberSaveable, andOnChatOpenedskips a redundant reload on re-entry (live delivery via the app-scoped event stream keeps Room current, so no messages are missed).Testing
:ui:navigation:testDebugUnitTest,:apps:flipcash:shared:router:testDebugUnitTestgreen; fullassembleDebuggreen.