Migrate from Navigation 2 to Navigation 3#1315
Merged
Merged
Conversation
|
👋 Hi @Elforama, thanks for the contribution! Before this PR can be reviewed and merged, please add:
Just drag-and-drop the file(s) into the PR description and GitHub will upload them. This check re-runs automatically when you edit the description. If this change has no user-visible effect, tick the "no user-visible / UI effect" box in the description (or ask a maintainer to add the |
… 3 deps Prerequisite for the Navigation 2 -> Navigation 3 migration: navigation3-ui 1.1.1 requires Compose >= 1.10.0 and compileSdk 36. material-icons-extended stays at 1.7.8 (last published version). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace androidx.navigation:navigation-compose with androidx.navigation3: - Routes implement NavKey; back stack owned by the app via rememberNavBackStack - New Navigator + NavOptions DSL (popUpTo<T>, popUpToRoot, launchSingleTop) replacing NavHostController and NavigationUtils' throttled helpers - NavHost -> NavDisplay in Host.kt and ImageCaptureActivity, with saveable-state and ViewModel-store entry decorators and the same 500ms fade transitions - trackedComposable -> custom screen-tracking NavEntryDecorator (Crashlytics breadcrumbs); HandleUIEvents gains an origin-is-top guard replacing the Nav2 RESUMED check - Org deep link parsed manually from the launch intent (cold start only, matching previous behavior) - NavigatorTest + FakeNavigator; previews/screenshot harness on LocalNavigator Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NavDisplay's back handling crashed on launch: AppCompatActivity's setContentView (appcompat 1.7.x) plants only the pre-navigationevent view-tree owners, so nav3 could not find a NavigationEventDispatcher. - Bump androidx.activity to 1.12.0, where ComponentActivity implements NavigationEventDispatcherOwner - Plant the view-tree owner on the decor view in TreeTrackerActivity Verified on a Pixel 7 API 35 emulator: cold start, splash auto-nav, signup back handling, and org deep link all work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ution - OrgDeepLinkTest (10 cases): parseStartRoute over valid/invalid scheme, host, path, query params, and URL-encoding (Robolectric for android.net.Uri) - RouteRegistryTest (8 cases): no-arg resolution to NavKey, arg-route null, alias normalization, isValidRoute - SettingsViewModelTest: two cases now execute the NavigationEvent lambda against a FakeNavigator and assert the resulting back stack, covering the event -> Navigator wiring and the new popUpToRoot() logout path Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 300 ms navigation throttle lived on a single lastMutationTime field on the one Navigator per NavDisplay, making it a global lock across every screen and button: navigating on one screen dropped any navigation from another for 300 ms, so a user could not tap "next" through a flow quickly. Bind each screen its own origin-scoped Navigator (via the per-entry decorator) and gate throttled calls on "is my screen still the top of the stack?" rather than a wall-clock timer: - Navigator gains an `origin` binding and `scopedTo(originContentKey)`; isThrottled() drops a throttled call only when the caller's origin is no longer on top. Unscoped navigators keep the time-based debounce as a fallback for tests and non-entry hosts. - rememberScreenTrackingNavEntryDecorator re-provides LocalNavigator as a view scoped to the entry's contentKey, so every screen gets per-screen double-tap protection with no call-site changes. Result: rapid "next" across different screens all succeed (no time gap), while a double-tap on one screen drops the second navigation (origin no longer top). Also removes the side effect where a non-throttled navigate armed the window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reduce the NavDisplay transition duration from 500ms to 300ms. On a pop, NavDisplay deliberately keeps the outgoing screen on top for the full fade (targetZIndex = initialZIndex - 1f), so during that window it intercepts taps meant for the revealed screen behind it. Halving the window makes tapping the revealed screen right after Back land reliably, and makes transitions snappier overall. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What & why
Migrates the app from Jetpack Navigation 2 (
androidx.navigation:navigation-compose2.8.5) to Navigation 3 (androidx.navigation31.1.1). Navigation 3 is Google's Compose-native successor: the app owns the back stack (NavBackStack) instead of aNavController, destinations areNavKeys rendered byNavDisplay, and ViewModel/saved-state scoping is done with entry decorators.Every existing navigation use case is reimplemented — nothing is dropped.
navigation-composeis fully removed (zeroandroidx.navigation.imports remain).How it's structured (4 commits)
material-icons-extendedpinned to 1.7.8 (its last release);androidx.activity→ 1.12.0. Green build on Nav2 before any nav-code change.NavKey; newNavigator+NavOptionsDSL (popUpTo<T>,popUpToRoot()replacingpopUpTo(graph.id),launchSingleTop,popBackStackTo<T>) replacingNavHostController;NavHost→NavDisplayin bothHost.ktandImageCaptureActivity;trackedComposable→ a custom screen-trackingNavEntryDecorator(keeps Crashlytics breadcrumbs); deep link parsed manually from the launch intent (Nav3 has no deep-link support — cold-start only, matching prior behavior).NavDisplay's back handling needs aNavigationEventDispatcherOwner, whichAppCompatActivity.setContentView(appcompat 1.7.x) doesn't plant — caught on the emulator, fixed by planting the view-tree owner inTreeTrackerActivity.NavigatorTest(16 cases),OrgDeepLinkTest(10),RouteRegistryTest(8), and twoSettingsViewModelTestcases that execute theNavigationEventlambda against aFakeNavigatorto cover the event →Navigatorwiring and the newpopUpToRoot()logout path.Behavior notes for reviewers
launchSingleTopis implemented as replace-top (recreates the entry if args differ) — unobservable for all current call sites, but documented inNavigator.HandleUIEvents(a nav event only fires when its hosting entry is the top of the stack). Net effect on double-taps is equal or stronger; a nav issued 300–500 ms after another (mid-fade) now succeeds where the RESUMED check dropped it.singleTask, noonNewIntent), same as before.Task write-up:
docs/tasks/task_24/.Verification
./gradlew :app:assembleDebug :app:testDebugUnitTest :app:verifyRoborazziDebug :app:ktlintCheck :app:detekt :app:lintDebugall pass.Manually driven end-to-end on a Pixel 7 API 35 emulator (no crashes throughout):
ImageCaptureActivity(the second, standaloneNavDisplay; retake + approve) → dashboard.adb ... -d "app://mobile.treetracker.org/org?id=...&name=..."→ logcatOrgLink: Deeplink received).📸 Proof of change (REQUIRED)
🎥 Screen recording / video (always required)
🖼️ Screenshots (UI change — transitions/back behavior)
This is primarily a refactor: screens render identically and the 500 ms fade transitions are preserved. The visible surface exercised during verification (dashboard after capturing 3 trees, capture screen, selfie flow) is unchanged from
master. Before/after screenshots to be attached alongside the recording.Fixes #<issue_number_goes_here> 🦕