chore: standardize resources and update documentation for Navigation 3 (#4961)

This commit is contained in:
James Rich 2026-03-31 16:25:37 -05:00 committed by GitHub
parent 1faa802fe6
commit 464a12b9f7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
42 changed files with 216 additions and 149 deletions

View file

@ -9,12 +9,12 @@ Use `AGENTS.md` as the source of truth for architecture boundaries and required
When checking upstream docs/examples, match these repository-pinned versions from `gradle/libs.versions.toml`:
- Kotlin: `2.3.20`
- Koin: `4.2.0` (`koin-annotations` `2.1.0`, compiler plugin `0.4.1`)
- JetBrains Navigation 3: `1.1.0-alpha04` (`org.jetbrains.androidx.navigation3`)
- JetBrains Lifecycle (multiplatform): `2.10.0-beta01` (`org.jetbrains.androidx.lifecycle`)
- Koin: `4.2.0` (`koin-annotations` `4.2.0`, compiler plugin `0.4.1`)
- JetBrains Navigation 3: `1.1.0-beta01` (`org.jetbrains.androidx.navigation3`)
- JetBrains Lifecycle (multiplatform): `2.11.0-alpha02` (`org.jetbrains.androidx.lifecycle`)
- AndroidX Lifecycle (Android-only): `2.10.0` (`androidx.lifecycle`)
- Kotlin Coroutines: `1.10.2`
- Compose Multiplatform: `1.11.0-alpha04`
- Compose Multiplatform: `1.11.0-beta01`
- JetBrains Material 3 Adaptive: `1.3.0-alpha06` (`org.jetbrains.compose.material3.adaptive`)
Prefer versioned docs pages that match those versions (for example, Koin `4.2` docs rather than older `4.0/4.1` pages).

View file

@ -19,8 +19,9 @@ This document captures discoverable patterns that are already used in the reposi
## 3) Navigation conventions (Navigation 3)
- Use Navigation 3 types (`NavKey`, `NavBackStack`, entry providers) instead of legacy controller-first patterns.
- Example graph using `EntryProviderScope<NavKey>` and `backStack.add/removeLastOrNull`: `feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`.
- Example feature flow using `rememberNavBackStack` and `NavDisplay<NavKey>`: `feature/intro/src/androidMain/kotlin/org/meshtastic/feature/intro/AppIntroductionScreen.kt`.
- Example graph using `EntryProviderScope<NavKey>` and `backStack.add/removeLastOrNull`: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`.
- Hosts should render navigation via `MeshtasticNavDisplay` from `core:ui/commonMain` (not raw `NavDisplay`) so entry decorators, scene strategies, and transitions stay consistent.
- Host examples: `app/src/main/kotlin/org/meshtastic/app/ui/Main.kt`, `desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt`.
## 4) UI and resources

View file

@ -44,7 +44,7 @@ Version note: align guidance with repository-pinned versions in `gradle/libs.ver
- Typed routes: `core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/Routes.kt`
- Shared saved-state config: `core/navigation/src/commonMain/kotlin/org/meshtastic/core/navigation/NavigationConfig.kt`
- App root backstack + `NavDisplay`: `app/src/main/kotlin/org/meshtastic/app/ui/Main.kt`
- App root backstack + `MeshtasticNavDisplay`: `app/src/main/kotlin/org/meshtastic/app/ui/Main.kt`
- Shared graph entry provider pattern: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`
- Desktop Navigation 3 shell: `desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt`
- Desktop nav graph assembly: `desktop/src/main/kotlin/org/meshtastic/desktop/navigation/DesktopNavigation.kt`

View file

@ -19,12 +19,12 @@ Reference examples:
1. Implement or extend base ViewModel logic in `feature/<name>/src/commonMain/...`.
2. Keep shared class free of Android framework dependencies.
3. Keep Android framework dependencies out of shared logic; if the module already uses Koin annotations in `commonMain`, keep patterns consistent and ensure app root inclusion.
4. Update navigation entry points in `feature/*/src/androidMain/kotlin/org/meshtastic/feature/*/navigation/...` to resolve ViewModels with `koinViewModel()`.
4. Update shared navigation entry points in `feature/*/src/commonMain/kotlin/org/meshtastic/feature/*/navigation/...` to resolve ViewModels with `koinViewModel()`.
Reference examples:
- Shared base: `feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/MessageViewModel.kt`
- Shared base UI ViewModel: `core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/BaseUIViewModel.kt`
- Navigation usage: `feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`
- Navigation usage: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`
- Desktop navigation usage: `desktop/src/main/kotlin/org/meshtastic/desktop/navigation/DesktopSettingsNavigation.kt`
## Playbook C: Add a new dependency or service binding
@ -50,7 +50,8 @@ Reference examples:
Reference examples:
- Shared graph wiring: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`
- Android specific content: `feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsMainScreen.kt`
- Shared graph content: `feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt`
- Android-specific content actual: `feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsMainScreen.kt`
- Desktop specific content: `feature/settings/src/jvmMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsMainScreen.kt`
- Feature intro graph pattern: `feature/intro/src/androidMain/kotlin/org/meshtastic/feature/intro/IntroNavGraph.kt`
- Desktop nav shell: `desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt`
@ -77,7 +78,7 @@ Reference examples:
4. Add `kotlinx-coroutines-swing` (JVM/Desktop) or the equivalent platform coroutines dispatcher module. Without it, `Dispatchers.Main` is unavailable and any code using `lifecycle.coroutineScope` will crash at runtime.
5. Progressively replace stubs with real implementations (e.g., serial transport for desktop, CoreBluetooth for iOS).
6. Add `<platform>()` target to feature modules as needed (all `core:*` modules already declare `jvm()`).
7. Update CI JVM smoke compile step in `.github/workflows/reusable-check.yml` to include new modules.
7. Ensure the new module applies the expected KMP convention plugin so root `kmpSmokeCompile` auto-discovers and validates it in CI.
8. If `commonMain` code fails to compile for the new target, it's a KMP migration debt — fix the shared code, not the target.
Reference examples:

View file

@ -30,8 +30,8 @@ Notes:
- `feature/commonMain logic` changes:
- `spotlessCheck`, `detekt`, `test`, `assembleDebug`.
- `navigation/DI wiring` changes (app graph, Koin module/wrapper changes):
- `spotlessCheck`, `detekt`, `assembleDebug`, `test`, plus `testDebugUnitTest` if available locally.
- If touching any KMP module, also run the relevant `:compileKotlinJvm` task. CI validates all 22 KMP modules + `desktop:test`.
- `spotlessCheck`, `detekt`, `assembleDebug`, `test`, plus `testFdroidDebugUnitTest` and `testGoogleDebugUnitTest` when available locally.
- If touching any KMP module, also run `kmpSmokeCompile`.
- `worker/service/background` changes:
- `spotlessCheck`, `detekt`, `assembleDebug`, `test`, and targeted tests around WorkManager/service behavior.
- `BLE/networking/core repository` changes:
@ -57,8 +57,8 @@ Current reusable check workflow includes:
`app:lintFdroidDebug app:lintGoogleDebug core:barcode:lintFdroidDebug core:barcode:lintGoogleDebug core:api:lintDebug mesh_service_example:lintDebug`
- Host tests plus coverage aggregation:
`test koverXmlReport app:koverXmlReportFdroidDebug app:koverXmlReportGoogleDebug core:api:koverXmlReportDebug core:barcode:koverXmlReportFdroidDebug core:barcode:koverXmlReportGoogleDebug mesh_service_example:koverXmlReportDebug desktop:koverXmlReport`
- JVM smoke compile for all KMP JVM targets (all compile-only modules remain explicit):
`:core:proto:compileKotlinJvm :core:common:compileKotlinJvm :core:model:compileKotlinJvm :core:repository:compileKotlinJvm :core:di:compileKotlinJvm :core:navigation:compileKotlinJvm :core:resources:compileKotlinJvm :core:datastore:compileKotlinJvm :core:database:compileKotlinJvm :core:domain:compileKotlinJvm :core:prefs:compileKotlinJvm :core:network:compileKotlinJvm :core:data:compileKotlinJvm :core:ble:compileKotlinJvm :core:nfc:compileKotlinJvm :core:service:compileKotlinJvm :core:testing:compileKotlinJvm :core:ui:compileKotlinJvm :feature:intro:compileKotlinJvm :feature:messaging:compileKotlinJvm :feature:connections:compileKotlinJvm :feature:map:compileKotlinJvm :feature:node:compileKotlinJvm :feature:settings:compileKotlinJvm :feature:firmware:compileKotlinJvm`
- KMP smoke compile lifecycle task (auto-discovers KMP modules and runs JVM + iOS simulator compile checks):
`kmpSmokeCompile`
- Android build tasks:
`app:assembleFdroidDebug app:assembleGoogleDebug mesh_service_example:assembleDebug`
- Instrumented tests (when emulator tests are enabled):

View file

@ -30,46 +30,36 @@
### 1. NavDisplay — Scene Architecture (available since `1.1.0-alpha04`, stable in `beta01`)
**Available APIs we're NOT using:**
**Remaining APIs we're NOT using broadly yet:**
| API | Purpose | Status in project |
|---|---|---|
| `sceneStrategies: List<SceneStrategy<T>>` | Allows NavDisplay to render multi-pane Scenes | ❌ Not used — defaulting to `SinglePaneSceneStrategy` |
| `sceneStrategies: List<SceneStrategy<T>>` | Allows NavDisplay to render multi-pane Scenes | ⚠️ Partially used — `MeshtasticNavDisplay` applies dialog/list-detail/supporting/single strategies |
| `SceneStrategy<T>` interface | Custom scene calculation from backstack entries | ❌ Not used |
| `DialogSceneStrategy` | Renders `entry<T>(metadata = dialog())` entries as overlay Dialogs | ❌ Not used — dialogs handled manually |
| `DialogSceneStrategy` | Renders `entry<T>(metadata = dialog())` entries as overlay Dialogs | ✅ Enabled in shared host wrapper |
| `SceneDecoratorStrategy<T>` | Wraps/decorates scenes with additional UI | ❌ Not used |
| `NavEntry.metadata` | Attaches typed metadata to entries (transitions, dialog hints, Scene classification) | ❌ Not used |
| `NavDisplay.TransitionKey` / `PopTransitionKey` / `PredictivePopTransitionKey` | Per-entry custom transitions via metadata | ❌ Not used |
| `transitionSpec` / `popTransitionSpec` / `predictivePopTransitionSpec` params | Default transition animations for NavDisplay | ❌ Not used — no transitions at all |
| `transitionSpec` / `popTransitionSpec` / `predictivePopTransitionSpec` params | Default transition animations for NavDisplay | ⚠️ Partially used — shared forward/pop crossfade adopted; predictive-pop custom spec not yet used |
| `sharedTransitionScope: SharedTransitionScope?` | Shared element transitions between scenes | ❌ Not used |
| `entryDecorators: List<NavEntryDecorator<T>>` | Wraps entry content with additional behavior | ❌ Not used (defaulting to `SaveableStateHolderNavEntryDecorator`) |
| `entryDecorators: List<NavEntryDecorator<T>>` | Wraps entry content with additional behavior | ✅ Used via `MeshtasticNavDisplay` (`SaveableStateHolder` + `ViewModelStore`) |
**APIs we ARE using correctly:**
| API | Usage |
|---|---|
| `NavDisplay(backStack, entryProvider, modifier)` | Both `app/Main.kt` and `desktop/DesktopMainScreen.kt` |
| `MeshtasticNavDisplay(...)` wrapper around `NavDisplay` | Both `app/Main.kt` and `desktop/DesktopMainScreen.kt` |
| `rememberNavBackStack(SavedStateConfiguration, startKey)` | Backstack persistence |
| `entryProvider<NavKey> { entry<T> { ... } }` | All feature graph registrations |
| `NavigationBackHandler` from `navigationevent-compose` | Used in `AdaptiveListDetailScaffold` |
### 2. ViewModel Scoping (`lifecycle-viewmodel-navigation3` `2.11.0-alpha02`)
**Key finding:** The `ViewModelStoreNavEntryDecorator` is available and provides automatic per-entry ViewModel scoping tied to backstack lifetime. The project declares this dependency in `desktop/build.gradle.kts` but does **not** pass it as an `entryDecorator` to `NavDisplay`.
Currently, `koinViewModel()` calls inside `entry<T>` blocks use the nearest `ViewModelStoreOwner` from the composition — which is the Activity/Window level. This means:
- ViewModels are **not** automatically cleared when their entry is popped from the backstack.
- The project works around this with manual `key = "metrics-$destNum"` parameter keying.
**Opportunity:** Adding `rememberViewModelStoreNavEntryDecorator()` to `NavDisplay.entryDecorators` would give each backstack entry its own `ViewModelStoreOwner`, so `koinViewModel()` calls would be automatically scoped to the entry's lifetime.
**Current status:** Adopted. `MeshtasticNavDisplay` applies `rememberViewModelStoreNavEntryDecorator()` with `rememberSaveableStateHolderNavEntryDecorator()`, so `koinViewModel()` instances are entry-scoped and clear on pop.
### 3. Material 3 Adaptive — Nav3 Scene Integration
**Key finding:** The JetBrains `adaptive-navigation` artifact at `1.3.0-alpha06` does **NOT** include `MaterialListDetailSceneStrategy`. That API only exists in the Google AndroidX version (`androidx.compose.material3.adaptive:adaptive-navigation:1.3.0-alpha09+`).
This means the project **cannot** currently use the official M3 Adaptive Scene bridge through `NavDisplay(sceneStrategies = ...)`. The current approach of hosting `ListDetailPaneScaffold` inside `entry<T>` blocks (via `AdaptiveListDetailScaffold`) is the correct pattern for the JetBrains fork at this version.
**When to revisit:** Monitor the JetBrains adaptive fork for `MaterialListDetailSceneStrategy` inclusion. It will likely arrive when the JetBrains fork catches up to the AndroidX `1.3.0-alpha09+` feature set.
**Current status:** Adopted for shared host-level strategies. `MeshtasticNavDisplay` uses adaptive Navigation 3 scene strategies (`rememberListDetailSceneStrategy`, `rememberSupportingPaneSceneStrategy`) with draggable pane expansion handles, while feature-level scaffold composition remains valid for route-specific layouts.
### 4. NavigationSuiteScaffold (`1.11.0-alpha05`)
@ -99,7 +89,7 @@ This means the project **cannot** currently use the official M3 Adaptive Scene b
**Status:** ✅ Adopted (2026-03-26). A new `MeshtasticNavDisplay` composable in `core:ui/commonMain` encapsulates the standard `NavDisplay` configuration:
- Entry decorators: `rememberSaveableStateHolderNavEntryDecorator` + `rememberViewModelStoreNavEntryDecorator`
- Scene strategies: `DialogSceneStrategy` + `SinglePaneSceneStrategy`
- Scene strategies: `DialogSceneStrategy` + adaptive list-detail/supporting pane strategies + `SinglePaneSceneStrategy`
- Transition specs: 350 ms crossfade (forward + pop)
Both `app/Main.kt` and `desktop/DesktopMainScreen.kt` now call `MeshtasticNavDisplay` instead of configuring `NavDisplay` directly. The `lifecycle-viewmodel-navigation3` dependency was moved from host modules to `core:ui`.
@ -112,7 +102,7 @@ Individual entries can declare custom transitions via `entry<T>(metadata = NavDi
### Deferred: Scene-based multi-pane layout
The `MaterialListDetailSceneStrategy` is not available in the JetBrains adaptive fork at `1.3.0-alpha06`. The project's `AdaptiveListDetailScaffold` wrapper is the correct approach for now. Revisit when the JetBrains fork includes the Scene bridge, or consider writing a custom `SceneStrategy` that integrates with the existing `ListDetailPaneScaffold`.
Additional route-level Scene metadata adoption is deferred. The project now applies shared adaptive scene strategies in `MeshtasticNavDisplay`, and feature-level `AdaptiveListDetailScaffold` remains valid for route-specific layouts. Revisit custom per-route `SceneStrategy` policies when multi-pane route classification needs expand.
## Decision

View file

@ -48,15 +48,15 @@ Source reviewed: Navigation 3 `1.1.0-beta01` (JetBrains fork), CMP `1.11.0-beta0
- New `sharedTransitionScope: SharedTransitionScope?` parameter for shared element transitions.
- Existing shell patterns in `app` and `desktop` remain valid using the default `SinglePaneSceneStrategy`.
2. **Entry-scoped ViewModel lifecycle adopted.**
- Both `app` and `desktop` now pass `ViewModelStoreNavEntryDecorator` + `SaveableStateHolderNavEntryDecorator` as explicit `entryDecorators` to `NavDisplay`.
- Both `app` and `desktop` now use `MeshtasticNavDisplay` (`core:ui/commonMain`), which applies `ViewModelStoreNavEntryDecorator` + `SaveableStateHolderNavEntryDecorator` per active backstack.
- ViewModels obtained via `koinViewModel()` inside `entry<T>` blocks are now scoped to the entry's backstack lifetime.
3. **No direct Navigation 3 API breakage.**
- Release is beta (API stabilized). No migration from alpha04 was required for existing usage patterns.
4. **Primary risk is dependency wiring drift, not runtime behavior.**
- JetBrains Navigation 3 currently publishes `navigation3-ui` coordinates (no separate `navigation3-runtime` artifact in Maven Central). The `jetbrains-navigation3-runtime` alias intentionally points to `navigation3-ui` and is documented in the version catalog.
- Note: The `remember*` composable factory functions from `navigation3-runtime` are not visible in non-KMP Android modules due to Kotlin metadata resolution. Use direct class constructors instead (as done in `app/Main.kt`).
5. **Saved-state and typed-route parity risk remains unchanged.**
- Desktop still uses manual serializer registration; this is an existing risk and not introduced by beta01.
5. **Saved-state and typed-route parity improved.**
- Both hosts share `MeshtasticNavSavedStateConfig` from `core:navigation/commonMain` via `MultiBackstack`, reducing platform drift risk in serializer registration.
6. **Updated active docs to reflect the current dependency baseline (`1.11.0-beta01`, `1.1.0-beta01`, `1.3.0-alpha06`, `2.11.0-alpha02`).**
### Actions Taken
@ -66,7 +66,7 @@ Source reviewed: Navigation 3 `1.1.0-beta01` (JetBrains fork), CMP `1.11.0-beta0
- `jetbrains-navigation3-runtime`, `jetbrains-navigation3-ui`
- Documented in the version catalog that `jetbrains-navigation3-runtime` intentionally maps to `navigation3-ui` until a separate runtime artifact is published.
- Migrated `core:data` `commonMain` from `androidx.lifecycle:lifecycle-runtime` (Google) to `org.jetbrains.androidx.lifecycle:lifecycle-runtime` (JetBrains fork) for full consistency.
- Updated active docs to reflect the current dependency baseline (`1.11.0-alpha04`, `1.1.0-alpha04`, `1.3.0-alpha06`, `2.10.0-beta01`).
- Updated active docs to reflect the current dependency baseline (`1.11.0-beta01`, `1.1.0-beta01`, `1.3.0-alpha06`, `2.11.0-alpha02`).
- Consolidated `app` adaptive dependencies to JetBrains Material 3 Adaptive coordinates (`org.jetbrains.compose.material3.adaptive:*`) so Android and Desktop consume the same adaptive artifact family. The Android-only navigation suite remains on `androidx.compose.material3:material3-adaptive-navigation-suite`.
### Deferred Follow-ups

View file

@ -107,7 +107,7 @@ Based on the latest codebase investigation, the following steps are proposed to
| Navigation 3 parity model (shared `TopLevelDestination` + platform adapters) | ✅ Done | Both shells use shared enum + parity tests. See [`decisions/navigation3-parity-2026-03.md`](./decisions/navigation3-parity-2026-03.md) |
| Hilt → Koin | ✅ Done | See [`decisions/koin-migration.md`](./decisions/koin-migration.md) |
| BLE abstraction (Kable) | ✅ Done | See [`decisions/ble-strategy.md`](./decisions/ble-strategy.md) |
| Material 3 Adaptive (JetBrains) | ✅ Done | Version `1.3.0-alpha06` aligned with CMP `1.11.0-alpha04`; supports Large (1200dp) and Extra-large (1600dp) breakpoints |
| Material 3 Adaptive (JetBrains) | ✅ Done | Version `1.3.0-alpha06` aligned with CMP `1.11.0-beta01`; supports Large (1200dp) and Extra-large (1600dp) breakpoints |
| JetBrains lifecycle/nav3 alias alignment | ✅ Done | All forked deps use `jetbrains-*` prefix in version catalog; `core:data` commonMain uses JetBrains lifecycle runtime |
| Expect/actual consolidation | ✅ Done | 7 pairs eliminated; 15+ genuinely platform-specific retained |
| Transport deduplication | ✅ Done | `StreamFrameCodec`, `TcpTransport`, and `SerialTransport` shared in `core:network` |
@ -131,7 +131,7 @@ Based on the latest codebase investigation, the following steps are proposed to
All major ViewModels have now been extracted to `commonMain` and no longer rely on Android-specific subclasses. Platform-specific dependencies (like `android.net.Uri` or Location permissions) have been successfully isolated behind injected `core:repository` interfaces (e.g., `FileService`, `LocationService`).
**The extraction of all feature-specific navigation graphs, background services, and widgets out of `:app` is complete.** The `:app` module now only serves as the root DI assembler and NavHost container.
**The extraction of all feature-specific navigation graphs, background services, and widgets out of `:app` is complete.** The `:app` module now only serves as the root DI assembler and shared Navigation 3 host shell (`MeshtasticNavDisplay`) container.
Extracted to shared `commonMain` (no longer app-only):
- `SettingsViewModel``feature:settings/commonMain`