diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d0200049f..07ee9eae8 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -42,7 +42,7 @@ Meshtastic-Android is a Kotlin Multiplatform (KMP) application for off-grid, dec | `core:network` | KMP networking layer using Ktor, MQTT abstractions, and shared transport (`StreamFrameCodec`, `TcpTransport`, `SerialTransport`). | | `core:di` | Common DI qualifiers and dispatchers. | | `core:navigation` | Shared navigation keys/routes for Navigation 3. | -| `core:ui` | Shared Compose UI components (`AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | +| `core:ui` | Shared Compose UI components (`MeshtasticAppShell`, `MeshtasticNavDisplay`, `MeshtasticNavigationSuite`, `AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | | `core:service` | KMP service layer; Android bindings stay in `androidMain`. | | `core:api` | Public AIDL/API integration module for external clients. | | `core:prefs` | KMP preferences layer built on DataStore abstractions. | @@ -79,7 +79,8 @@ Meshtastic-Android is a Kotlin Multiplatform (KMP) application for off-grid, dec - **KMP file naming:** In KMP modules, `commonMain` and platform source sets (`androidMain`, `jvmMain`) share the same package namespace. If both contain a file with the same name (e.g., `LogExporter.kt`), the Kotlin/JVM compiler will produce a duplicate class error. Use distinct filenames: keep the `expect` declaration in `LogExporter.kt` and put shared helpers in a separate file like `LogFormatter.kt`. - **Concurrency:** Use Kotlin Coroutines and Flow. - **Dependency Injection:** Use **Koin Annotations** with the K2 compiler plugin (`koin-plugin` in version catalog). The `koin-annotations` library version is unified with `koin-core` (both use `version.ref = "koin"`). The `KoinConventionPlugin` uses the typed `KoinGradleExtension` to configure the K2 plugin (e.g., `compileSafety.set(false)`). Keep root graph assembly in `app`. -- **ViewModels:** Follow the MVI/UDF pattern. Use the multiplatform `androidx.lifecycle.ViewModel` in `commonMain`. Both `app` and `desktop` pass `ViewModelStoreNavEntryDecorator` to `NavDisplay`, so ViewModels obtained via `koinViewModel()` inside `entry` blocks are scoped to the entry's backstack lifetime and cleared on pop. +- **ViewModels:** Follow the MVI/UDF pattern. Use the multiplatform `androidx.lifecycle.ViewModel` in `commonMain`. Both `app` and `desktop` use `MeshtasticNavDisplay` from `core:ui/commonMain`, which configures `ViewModelStoreNavEntryDecorator` + `SaveableStateHolderNavEntryDecorator`. ViewModels obtained via `koinViewModel()` inside `entry` blocks are scoped to the entry's backstack lifetime and cleared on pop. +- **Navigation host:** Use `MeshtasticNavDisplay` from `core:ui/commonMain` instead of calling `NavDisplay` directly. It provides entry-scoped ViewModel decoration, `DialogSceneStrategy` for dialog entries, and a shared 350 ms crossfade transition. Host modules (`app`, `desktop`) should NOT configure `entryDecorators`, `sceneStrategies`, or `transitionSpec` themselves. - **BLE:** All Bluetooth communication must route through `core:ble` using Kable. - **Networking:** Pure **Ktor** — no OkHttp anywhere. Engines: `ktor-client-android` for Android, `ktor-client-java` for desktop/JVM. Use Ktor `Logging` plugin for HTTP debug logging (not OkHttp interceptors). `HttpClient` is provided via Koin in `app/di/NetworkModule` and `core:network/di/CoreNetworkAndroidModule`. - **Image Loading (Coil):** Use `coil-network-ktor3` with `KtorNetworkFetcherFactory` on **all** platforms. `ImageLoader` is configured in host modules only (`app` via Koin `@Single`, `desktop` via `setSingletonImageLoaderFactory`). Feature modules depend only on `libs.coil` (coil-compose) for `AsyncImage` — never add `coil-network-*` or `coil-svg` to feature modules. diff --git a/AGENTS.md b/AGENTS.md index 24815bac4..c4375c016 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -42,7 +42,7 @@ Meshtastic-Android is a Kotlin Multiplatform (KMP) application for off-grid, dec | `core:network` | KMP networking layer using Ktor, MQTT abstractions, and shared transport (`StreamFrameCodec`, `TcpTransport`, `SerialTransport`, `BleRadioInterface`). | | `core:di` | Common DI qualifiers and dispatchers. | | `core:navigation` | Shared navigation keys/routes for Navigation 3, `DeepLinkRouter` for typed backstack synthesis, and `MeshtasticNavSavedStateConfig` for backstack persistence. | -| `core:ui` | Shared Compose UI components (`MeshtasticAppShell`, `MeshtasticNavigationSuite`, `AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | +| `core:ui` | Shared Compose UI components (`MeshtasticAppShell`, `MeshtasticNavDisplay`, `MeshtasticNavigationSuite`, `AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | | `core:service` | KMP service layer; Android bindings stay in `androidMain`. | | `core:api` | Public AIDL/API integration module for external clients. | | `core:prefs` | KMP preferences layer built on DataStore abstractions. | diff --git a/GEMINI.md b/GEMINI.md index ef91358c2..0db75b419 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -42,7 +42,7 @@ Meshtastic-Android is a Kotlin Multiplatform (KMP) application for off-grid, dec | `core:network` | KMP networking layer using Ktor, MQTT abstractions, and shared transport (`StreamFrameCodec`, `TcpTransport`, `SerialTransport`, `BleRadioInterface`). | | `core:di` | Common DI qualifiers and dispatchers. | | `core:navigation` | Shared navigation keys/routes for Navigation 3. | -| `core:ui` | Shared Compose UI components (`AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | +| `core:ui` | Shared Compose UI components (`MeshtasticAppShell`, `MeshtasticNavDisplay`, `MeshtasticNavigationSuite`, `AlertHost`, `SharedDialogs`, `PlaceholderScreen`, `MainAppBar`, dialogs, preferences) and platform abstractions. | | `core:service` | KMP service layer; Android bindings stay in `androidMain`. | | `core:api` | Public AIDL/API integration module for external clients. | | `core:prefs` | KMP preferences layer built on DataStore abstractions. | diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 0aa13896b..752b2be0b 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -250,7 +250,6 @@ dependencies { implementation(libs.androidx.lifecycle.process) implementation(libs.jetbrains.lifecycle.viewmodel.compose) implementation(libs.jetbrains.lifecycle.runtime.compose) - implementation(libs.jetbrains.lifecycle.viewmodel.navigation3) implementation(libs.jetbrains.navigation3.ui) implementation(libs.androidx.paging.compose) implementation(libs.ktor.client.android) diff --git a/app/src/main/kotlin/org/meshtastic/app/ui/Main.kt b/app/src/main/kotlin/org/meshtastic/app/ui/Main.kt index 5c46dbde2..940de1741 100644 --- a/app/src/main/kotlin/org/meshtastic/app/ui/Main.kt +++ b/app/src/main/kotlin/org/meshtastic/app/ui/Main.kt @@ -26,19 +26,12 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember -import androidx.compose.runtime.saveable.rememberSaveableStateHolder import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle -import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner -import androidx.lifecycle.viewmodel.navigation3.ViewModelStoreNavEntryDecorator -import androidx.lifecycle.viewmodel.navigation3.ViewModelStoreNavEntryDecoratorDefaults import androidx.navigation3.runtime.NavKey -import androidx.navigation3.runtime.SaveableStateHolderNavEntryDecorator import androidx.navigation3.runtime.entryProvider import androidx.navigation3.runtime.rememberNavBackStack -import androidx.navigation3.ui.NavDisplay import co.touchlab.kermit.Logger import org.koin.compose.viewmodel.koinViewModel import org.meshtastic.app.BuildConfig @@ -49,6 +42,7 @@ import org.meshtastic.core.resources.Res import org.meshtastic.core.resources.app_too_old import org.meshtastic.core.resources.must_update import org.meshtastic.core.ui.component.MeshtasticAppShell +import org.meshtastic.core.ui.component.MeshtasticNavDisplay import org.meshtastic.core.ui.viewmodel.UIViewModel import org.meshtastic.feature.connections.navigation.connectionsGraph import org.meshtastic.feature.firmware.navigation.firmwareGraph @@ -90,24 +84,9 @@ fun MainScreen(uIViewModel: UIViewModel = koinViewModel()) { settingsGraph(backStack) firmwareGraph(backStack) } - val saveableStateHolder = rememberSaveableStateHolder() - val viewModelStoreOwner = checkNotNull(LocalViewModelStoreOwner.current) - val removeOnPop = ViewModelStoreNavEntryDecoratorDefaults.removeViewModelStoreOnPop() - NavDisplay( + MeshtasticNavDisplay( backStack = backStack, entryProvider = provider, - entryDecorators = - listOf( - remember(saveableStateHolder) { - SaveableStateHolderNavEntryDecorator(saveableStateHolder) - }, - remember(viewModelStoreOwner) { - ViewModelStoreNavEntryDecorator( - viewModelStore = viewModelStoreOwner.viewModelStore, - removeViewModelStoreOnPop = removeOnPop, - ) - }, - ), modifier = Modifier.fillMaxSize().recalculateWindowInsets().safeDrawingPadding(), ) } diff --git a/core/ui/build.gradle.kts b/core/ui/build.gradle.kts index 5cca049c9..81ed465c6 100644 --- a/core/ui/build.gradle.kts +++ b/core/ui/build.gradle.kts @@ -57,6 +57,8 @@ kotlin { implementation(libs.jetbrains.compose.material3.adaptive.navigation.suite) implementation(libs.jetbrains.navigationevent.compose) implementation(libs.jetbrains.navigation3.ui) + implementation(libs.jetbrains.lifecycle.viewmodel.navigation3) + implementation(libs.jetbrains.lifecycle.viewmodel.compose) } val jvmAndroidMain by getting { dependencies { implementation(libs.compose.multiplatform.ui.tooling) } } diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/MeshtasticNavDisplay.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/MeshtasticNavDisplay.kt new file mode 100644 index 000000000..c7aab787f --- /dev/null +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/MeshtasticNavDisplay.kt @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2025-2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.ui.component + +import androidx.compose.animation.AnimatedContentTransitionScope +import androidx.compose.animation.ContentTransform +import androidx.compose.animation.core.tween +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator +import androidx.navigation3.scene.DialogSceneStrategy +import androidx.navigation3.scene.Scene +import androidx.navigation3.scene.SinglePaneSceneStrategy +import androidx.navigation3.ui.NavDisplay + +/** + * Duration in milliseconds for the shared crossfade transition between navigation scenes. + * + * This is faster than the library's Android default (700 ms) and matches Material 3 motion guidance for medium-emphasis + * container transforms (~300-400 ms). + */ +private const val TRANSITION_DURATION_MS = 350 + +/** + * Shared [NavDisplay] wrapper that configures the standard Meshtastic entry decorators, scene strategies, and + * transition animations for all platform hosts. + * + * **Entry decorators** (applied to every backstack entry): + * - [rememberSaveableStateHolderNavEntryDecorator] — saveable state per entry. + * - [rememberViewModelStoreNavEntryDecorator] — entry-scoped `ViewModelStoreOwner` so that ViewModels obtained via + * `koinViewModel()` are automatically cleared when the entry is popped. + * + * **Scene strategies** (evaluated in order): + * - [DialogSceneStrategy] — entries annotated with `metadata = DialogSceneStrategy.dialog()` render as overlay + * [Dialog][androidx.compose.ui.window.Dialog] windows with proper backstack lifecycle. + * - [SinglePaneSceneStrategy] — default single-pane fallback. + * + * **Transitions**: A uniform 350 ms crossfade for both forward and pop navigation. + * + * @param backStack the navigation backstack, typically from [rememberNavBackStack]. + * @param entryProvider the entry provider built from feature navigation graphs. + * @param modifier modifier applied to the underlying [NavDisplay]. + */ +@Composable +fun MeshtasticNavDisplay( + backStack: List, + entryProvider: (key: NavKey) -> NavEntry, + modifier: Modifier = Modifier, +) { + NavDisplay( + backStack = backStack, + entryProvider = entryProvider, + entryDecorators = + listOf(rememberSaveableStateHolderNavEntryDecorator(), rememberViewModelStoreNavEntryDecorator()), + sceneStrategies = listOf(DialogSceneStrategy(), SinglePaneSceneStrategy()), + transitionSpec = meshtasticTransitionSpec(), + popTransitionSpec = meshtasticTransitionSpec(), + modifier = modifier, + ) +} + +/** + * Shared crossfade [ContentTransform] used for both forward and pop navigation. Returns a lambda compatible with + * [NavDisplay]'s `transitionSpec` / `popTransitionSpec` parameters. + */ +private fun meshtasticTransitionSpec(): AnimatedContentTransitionScope>.() -> ContentTransform = { + ContentTransform( + fadeIn(animationSpec = tween(TRANSITION_DURATION_MS)), + fadeOut(animationSpec = tween(TRANSITION_DURATION_MS)), + ) +} diff --git a/desktop/build.gradle.kts b/desktop/build.gradle.kts index c535b4696..ad2d1e949 100644 --- a/desktop/build.gradle.kts +++ b/desktop/build.gradle.kts @@ -193,7 +193,6 @@ dependencies { // Navigation 3 (JetBrains fork — multiplatform) implementation(libs.jetbrains.navigation3.ui) implementation(libs.jetbrains.lifecycle.viewmodel.compose) - implementation(libs.jetbrains.lifecycle.viewmodel.navigation3) implementation(libs.jetbrains.lifecycle.runtime.compose) // Koin DI diff --git a/desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt b/desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt index 6dcf90e8a..f35c5fc58 100644 --- a/desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt +++ b/desktop/src/main/kotlin/org/meshtastic/desktop/ui/DesktopMainScreen.kt @@ -24,22 +24,20 @@ import androidx.compose.material3.Surface import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp -import androidx.lifecycle.viewmodel.navigation3.rememberViewModelStoreNavEntryDecorator import androidx.navigation3.runtime.NavBackStack import androidx.navigation3.runtime.NavKey import androidx.navigation3.runtime.entryProvider -import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator -import androidx.navigation3.ui.NavDisplay import org.koin.compose.viewmodel.koinViewModel import org.meshtastic.core.ui.component.MeshtasticAppShell +import org.meshtastic.core.ui.component.MeshtasticNavDisplay import org.meshtastic.core.ui.viewmodel.UIViewModel import org.meshtastic.desktop.navigation.desktopNavGraph /** - * Desktop main screen — Navigation 3 shell with a persistent [NavigationRail] and [NavDisplay]. + * Desktop main screen — Navigation 3 shell with a persistent [NavigationRail] and shared [MeshtasticNavDisplay]. * - * Uses the same shared routes from `core:navigation` and the same `NavDisplay` + `entryProvider` pattern as the Android - * app, proving the shared backstack architecture works across targets. + * Uses the same shared routes from `core:navigation` and the same `MeshtasticNavDisplay` + `entryProvider` pattern as + * the Android app, proving the shared backstack architecture works across targets. */ @Composable @Suppress("LongMethod") @@ -56,17 +54,7 @@ fun DesktopMainScreen(backStack: NavBackStack, uiViewModel: UIViewModel ) { val provider = entryProvider { desktopNavGraph(backStack, uiViewModel) } - NavDisplay( - backStack = backStack, - onBack = { backStack.removeLastOrNull() }, - entryDecorators = - listOf( - rememberSaveableStateHolderNavEntryDecorator(), - rememberViewModelStoreNavEntryDecorator(), - ), - entryProvider = provider, - modifier = Modifier.fillMaxSize(), - ) + MeshtasticNavDisplay(backStack = backStack, entryProvider = provider, modifier = Modifier.fillMaxSize()) } } } diff --git a/docs/decisions/navigation3-api-alignment-2026-03.md b/docs/decisions/navigation3-api-alignment-2026-03.md index 4c71be572..a7f6452d8 100644 --- a/docs/decisions/navigation3-api-alignment-2026-03.md +++ b/docs/decisions/navigation3-api-alignment-2026-03.md @@ -79,31 +79,36 @@ This means the project **cannot** currently use the official M3 Adaptive Scene b ### P0: Add `ViewModelStoreNavEntryDecorator` to NavDisplay (high-value, low-risk) -Gives each backstack entry its own ViewModel scope. ViewModels are automatically cleared when their entry is popped. This is the primary intended usage of `lifecycle-viewmodel-navigation3`. +**Status:** ✅ Adopted (2026-03-26). Each backstack entry now gets its own `ViewModelStoreOwner` via `rememberViewModelStoreNavEntryDecorator()`. ViewModels obtained via `koinViewModel()` are automatically cleared when their entry is popped. Encapsulated in `MeshtasticNavDisplay` in `core:ui/commonMain`. **Impact:** Fixes subtle ViewModel leaks where popped entries retain their ViewModel in the Activity/Window store. Eliminates the need for manual `key = "metrics-$destNum"` ViewModel keying patterns over time. -**Files:** `app/Main.kt`, `desktop/DesktopMainScreen.kt`, `core/ui/build.gradle.kts` (dependency). - ### P1: Add default NavDisplay transitions (medium-value, low-risk) -Currently there are zero animations when navigating between entries. The `NavDisplay` supports `transitionSpec`, `popTransitionSpec`, and `predictivePopTransitionSpec` parameters for default transitions (slide, fade, etc.). +**Status:** ✅ Adopted (2026-03-26). A shared 350 ms crossfade (`fadeIn` + `fadeOut`) is applied for both forward and pop navigation via `MeshtasticNavDisplay`. This replaces the library's platform defaults (Android: 700 ms fade; Desktop: no animation) with a faster, consistent transition. -**Impact:** Immediate UX improvement on both Android and Desktop with ~10 lines of code. - -**Files:** `app/Main.kt`, `desktop/DesktopMainScreen.kt` (or a shared composable wrapper in `core:ui`). +**Impact:** Immediate UX improvement on both Android and Desktop. Desktop now has visible navigation transitions. ### P2: Adopt `DialogSceneStrategy` for navigation-driven dialogs (medium-value, medium-risk) -Routes that render as dialogs (e.g., confirmation dialogs, share sheet) can use `entry(metadata = DialogSceneStrategy.dialog()) { ... }` instead of manual `Dialog` composable wrapping. This keeps them on the backstack with proper predictive-back support. +**Status:** ✅ Adopted (2026-03-26). `MeshtasticNavDisplay` includes `DialogSceneStrategy` in `sceneStrategies` before `SinglePaneSceneStrategy`. Feature modules can now use `entry(metadata = DialogSceneStrategy.dialog()) { ... }` to render entries as overlay Dialogs with proper backstack lifecycle and predictive-back support. -**Impact:** Cleaner dialog lifecycle management. Currently low urgency since most dialogs use `AlertHost`. +**Impact:** Cleaner dialog lifecycle management available for future dialog routes. Existing dialogs via `AlertHost` are unaffected. + +### Consolidation: `MeshtasticNavDisplay` shared wrapper + +**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` +- 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`. ### P3: Per-entry transition metadata (low-value until Scene adoption) Individual entries can declare custom transitions via `entry(metadata = NavDisplay.transitionSpec { ... })`. This is most useful when different route types should animate differently (e.g., detail screens slide in, settings screens fade). -**Impact:** Polish improvement. Low priority until default transitions (P1) are established. +**Impact:** Polish improvement. Low priority until default transitions (P1) are established. Now unblocked by P1 adoption. ### Deferred: Scene-based multi-pane layout @@ -111,7 +116,9 @@ The `MaterialListDetailSceneStrategy` is not available in the JetBrains adaptive ## Decision -Adopt **P0** (ViewModel scoping) and **P1** (default transitions) now. Defer P2/P3 and Scene-based multi-pane until the JetBrains adaptive fork adds `MaterialListDetailSceneStrategy`. +~~Adopt **P0** (ViewModel scoping) and **P1** (default transitions) now. Defer P2/P3 and Scene-based multi-pane until the JetBrains adaptive fork adds `MaterialListDetailSceneStrategy`.~~ + +**Updated 2026-03-26:** P0, P1, and P2 adopted and consolidated into `MeshtasticNavDisplay` in `core:ui/commonMain`. P3 (per-entry transitions) is available for incremental adoption by feature modules. Scene-based multi-pane remains deferred. ## References