Skip to content

refactor(navigation): hierarchical, route-dispatching CodeNavigator#1073

Merged
bmc08gt merged 20 commits into
code/cashfrom
refactor/simplify-navigators
Jul 13, 2026
Merged

refactor(navigation): hierarchical, route-dispatching CodeNavigator#1073
bmc08gt merged 20 commits into
code/cashfrom
refactor/simplify-navigators

Conversation

@bmc08gt

@bmc08gt bmc08gt commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Simplifies the "too many navigators inside a flow" problem. Inside a FlowHost a screen previously had to reason about several navigator handles (inner flow navigator, outer/app navigator, sheet navigator, FlowNavigator), and LocalCodeNavigator.current silently resolved to different instances depending on context. This makes CodeNavigator hierarchical and route-type-dispatching so a screen reaches every scope through the one LocalCodeNavigator it holds — a FlowStep stays on the flow stack, anything else bubbles up the parent chain 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)

  • CodeNavigator gains parent, isFlowNavigator, and a FlowScope; navigate() dispatches by route type.
  • New navigateBackWithResult() and dismiss() intents (delegating to FlowScope).
  • FlowHost wires the inner navigator with parent + FlowScope; sheet-dismiss guarded on isSheetRoot.

Phase 2 — FlowNavigator thin adapter

  • InnerFlowNavigator reimplemented as a thin adapter delegating back/navigate/exitWithResult to the engine; redundant outerNavigator field removed. Behavior-preserving.

Phase 3 — call-site migration (kill the context-ambiguity)

  • Chat, deposit, withdrawal, contact-list, and swap flows migrated to a single dispatching LocalCodeNavigator, removing silent flowAnnotatedEntry / CompositionLocalProvider re-shadows and collapsing redundant handles.

Phase 4 — eliminate LocalOuterCodeNavigator

  • Two engine primitives: dispatchTarget(route) (the navigator a route lands on) and rootNavigator (the sheet-hosting root).
  • navigateForResult registers its callback on dispatchTarget(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-flow FlowStep results are unchanged (dispatchTarget returns this).
  • openAsSheet operates on rootNavigator — so it works from a flow's inner navigator. Backward-compatible: existing root callers get rootNavigator === this.
  • Swap and Login migrated to LocalCodeNavigator; dead flowAnnotatedEntry and LocalOuterCodeNavigator deleted. LocalSheetNavigator intentionally 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. ChatInputScaffold now uses SubcomposeLayout (correct list padding on frame 1), refreshSettled is rememberSaveable, and OnChatOpened skips 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:testDebugUnitTest green; full assembleDebug green.
  • In-app verified: chat send/back flow + reflow fix.
  • In-app checks still to run on this build: (1) swap → verification resumes after verifying (cross-boundary result delivery); (2) login → Lab opens as a bottom sheet (openAsSheet via rootNavigator).

@github-actions github-actions Bot added type: refactor Code restructuring, no behavior change area: ui Compose UI, theme, components, resources and removed type: refactor Code restructuring, no behavior change labels Jul 12, 2026
bmc08gt added 12 commits July 13, 2026 11:39
…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.
@bmc08gt bmc08gt force-pushed the refactor/simplify-navigators branch from 51ba2f9 to 2a5cefd Compare July 13, 2026 16:46
@github-actions github-actions Bot added type: refactor Code restructuring, no behavior change area: payments Payments, transfers, intents, billing area: onramp Deposit, purchase, Coinbase, fiat on-ramp area: tokens Token accounts, balances, token info labels Jul 13, 2026
@bmc08gt bmc08gt changed the title refactor(navigation): hierarchical, route-dispatching CodeNavigator (Phase 1) refactor(navigation): hierarchical, route-dispatching CodeNavigator + call-site migration (Phases 1-3) Jul 13, 2026
@bmc08gt bmc08gt changed the title refactor(navigation): hierarchical, route-dispatching CodeNavigator + call-site migration (Phases 1-3) refactor(navigation): hierarchical, route-dispatching CodeNavigator + call-site migration Jul 13, 2026
@bmc08gt bmc08gt changed the title refactor(navigation): hierarchical, route-dispatching CodeNavigator + call-site migration refactor(navigation): hierarchical, route-dispatching CodeNavigator (Phases 1-4) Jul 13, 2026
@bmc08gt bmc08gt changed the title refactor(navigation): hierarchical, route-dispatching CodeNavigator (Phases 1-4) refactor(navigation): hierarchical, route-dispatching CodeNavigator Jul 13, 2026
@bmc08gt bmc08gt self-assigned this Jul 13, 2026
@bmc08gt bmc08gt merged commit 800aa76 into code/cash Jul 13, 2026
3 checks passed
@bmc08gt bmc08gt deleted the refactor/simplify-navigators branch July 13, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding area: onramp Deposit, purchase, Coinbase, fiat on-ramp area: payments Payments, transfers, intents, billing area: tokens Token accounts, balances, token info area: ui Compose UI, theme, components, resources type: refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant