Split the navigation graph per feature - #72
Merged
Conversation
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.
Why
AslNavHost.ktwas 296 lines with a single 254-line composable holding 24 destinations for every feature in the app — onboarding, hub, projects, terminal, editor, git, settings, crash report, all interleaved.Finding how one screen was wired meant scrolling the whole file, and adding a destination for any feature meant editing the one file every feature shares.
What changed
Destinations moved into per-feature
NavGraphBuilderextension functions in a newnavigation/NavGraphs.kt:editorGraphfurther delegates to one function per destination (editorDestination,gitDiffDestination,gitHistoryDestination,gitBlameDestination,gitRefsDestination,gitConflictsDestination), since those carry the parameterised routes.AslNavHostdrops from 296 lines to 44 and now reads as a table of contents. Every extension takes a singlenavControllerparameter.Behavior
No behavior change — and since navigation here is stringly-typed, the compiler verifies almost none of that, so it was checked by diffing against
develop:navArgumentdefinitions: identical names and configuration. A typo here is a runtime crash on navigation, not a compile error.Routes.kt: untouched.savedStateHandlehandoffs: unchanged — folder picker writespicked_folderto the previous entry for Hub and Create Project to consume; conflicts writesgit_conflict_pathfor the editor.rememberUpdatedState(onOpenProjectConsumed)stays in the@Composable AslNavHost.NavGraphBuilderextensions are not composable and cannot call it, and it exists specifically to stop a stale callback being invoked — the same bug class fixed in #70. The resolved value is passed down as a plain parameter.Nav transitions (
aslEnter/aslExit/aslPopEnter/aslPopExit) remain NavHost-level defaults rather than moving onto individual graphs.Scope
:appnavigation wiring only. No feature module, no route constant, and no detekt configuration was touched.Tests
No new tests. A structural move with no new logic; the full suite passes.
Verification
./gradlew test— all modules pass../gradlew :app:compileDebugKotlin— passes../gradlew detekt— passes.navArgumentconfiguration each diffed againstdevelopand confirmed identical.rememberUpdatedStateconfirmed present inAslNavHostand absent from every graph extension.Baseline
app/detekt-baseline.xmlregenerated — 1 removed, 0 added:LongMethod:AslNavHost. Project total 243 → 242.ParameterNaming:AslNavHost$onOpenProjectConsumedcorrectly remains: it names a completed event, where past tense is right and the compose-rules present-tense convention does not apply.