From 6fc2f560b783173bb28e67862931e813e8d6f208 Mon Sep 17 00:00:00 2001 From: QIU Date: Thu, 13 Aug 2026 16:10:41 +0000 Subject: [PATCH] fix(nav): resolve the menu layout in core:domain so page order actually applies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 用户报告"把 AMSAT 从更多菜单移到主菜单没生效"。排查后发现这是两个互相 掩盖的 bug, 其中一个会让用户永久无法进入设置页。 ## Bug 1 (致命): 移任意页进主菜单 -> 设置入口从所有菜单消失 对"主菜单上限 5"的定义两处不一致: - SettingsScreen.kt:892 判断 mainItems.filter { it != "Settings" }.size >= 5, 上限是【不含 Settings 的 5 个】, 于是认为 [Satellites,Passes,Radar,Map] 还有空位, 直接追加 -> 共 6 项 - MainScreen.kt:165 的 .take(5) 上限是【含 Settings 的前 5 个】, 而 Settings 排在最后 -> 被截断丢弃 结果 Settings 既不在底栏也不在更多菜单(它不在 subMenuOrder 里), 用户再也 进不去设置页, 无法自行改回, 只能清数据或重装。 ## Bug 2 (用户报告): AMSAT / WavelogLog 移到主菜单被静默撤销 MainScreen.kt:161-163 给老用户补新页面的迁移逻辑【无条件执行】: .let { list -> if ("AMSAT" in list) list else list + "AMSAT" } 用户把 AMSAT 移出子菜单后 subMenuOrder 里没有它, 这段又加回去, 第 165 行 filter { it.screenId !in subOrder } 于是永远过滤掉 AMSAT。 副作用: 这个 bug 恰好把主菜单拉回 5 项内, 反而掩盖了 Bug 1 —— 所以用户只 看到"AMSAT 移不过去", 没触发"设置锁死"。 ## Bug 3: 溢出页面凭空消失而非落入更多菜单 .take(5) 直接丢弃超出的页面, 它们既不在底栏也不在更多菜单。 ## Bug 4: 设置页展示顺序与底栏实际顺序不一致 (WYSIWYG 失效) 两处各自实现排序: 设置页不做 take(5)、用 sortedWith 把 Settings 钉最后; MainScreen 做 take(5)、Settings 位置由 screenOrder 决定。 ## Bug 5: 更多菜单里 AMSAT / Roaming 当前页不高亮 MoreMenuPopup.kt:69-79 的 when (currentKey) 缺 AmSat 与 Roaming 分支, MainScreen.kt:188-198 缺 Roaming 分支, 靠 else -> false 兜底, 新增页面时 不会有编译错误提醒。Screen 子类都是 data object, 直接用 currentKey == screen 即可, 新增页面自动生效。 ## Bug 6: 设置页主菜单区不过滤 hiddenScreens MainScreen 会过滤隐藏页, 设置页不过滤, 于是隐藏的页面仍占据设置页的位置且 "移出主菜单"按钮可点, 与底栏实际情况不符。 ## 改动 新增 core/domain/navigation/MenuLayout.kt (纯 Kotlin, 为 KMP 就绪) 作为菜单 布局的唯一入口: - resolve(): 持久化偏好 -> 底栏 + 更多菜单。先给 Settings 预留名额再截断, 溢出页面落入更多菜单而非丢弃; 两个列表都没提到的页面按默认归位(升级不丢页) - moveToMain() / moveToMore(): 移动语义集中一处, 拒绝把 Settings 移出 MainScreen 与 SettingsScreen 改为共用它, 删除双份实现与无条件迁移逻辑。 ## 死代码清理 (每项已 grep 全仓库确认零引用) - SettingsAction.ResetScreenOrder: 无任何派发点, 仅定义与 when 分支 - UiSettingsCard 的 onReorder 参数: 函数体内两个 DragOrderList 的 onReorder 实参都调 onUpdateMenu, 从未调用该参数 - SettingsAction.ReorderScreens: 唯一引用是上面那个死参数 - MainScreen 的 navigateToRadar 参数: 函数体内唯一出现是注释掉的一行 - Navigation.kt 的 defaultScreenOrder / defaultSubMenuOrder: 迁移后零引用, 默认值已在 MenuLayout 内 保留 entry 分支不动 —— RadarDestination NavKey 被 PassDetailsMatcher deeplink 使用。 ## 验证 - MenuLayoutTest 13 个新测试全绿, 每个对应上面一个 bug 场景 - :core:domain:test 全量 100 个测试 0 失败 0 错误 (DataParser 19 / Doppler 17 / Qth 8 / Transponder 7 / CwCtc 7 / CwDeepBuffer 13 / CwGolden 3 / CwSpectrogram 8 / MenuLayout 13 / WaveLogApi 5) - :core:domain:compileKotlin + :core:presentation + :app + :feature:settings compileDebugKotlin => BUILD SUCCESSFUL - 用 Python 复刻新规则重跑当初失败的全部场景: 5 个页面逐一移入主菜单, 设置入口全部保住、页面无丢失; 隐藏页 + 移动组合无页面丢失; 幂等性通过 --- .../java/com/rtbishop/look4sat/MainScreen.kt | 61 +++---- .../com/rtbishop/look4sat/MoreMenuPopup.kt | 15 +- .../core/domain/navigation/MenuLayout.kt | 139 ++++++++++++++++ .../core/domain/navigation/MenuLayoutTest.kt | 154 ++++++++++++++++++ .../look4sat/core/presentation/Navigation.kt | 6 - .../feature/settings/SettingsScreen.kt | 59 +++---- .../feature/settings/SettingsState.kt | 4 - .../feature/settings/SettingsViewModel.kt | 2 - 8 files changed, 344 insertions(+), 96 deletions(-) create mode 100644 core/domain/src/main/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayout.kt create mode 100644 core/domain/src/test/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayoutTest.kt diff --git a/app/src/main/java/com/rtbishop/look4sat/MainScreen.kt b/app/src/main/java/com/rtbishop/look4sat/MainScreen.kt index 7e3e7cdc..ba8261ff 100644 --- a/app/src/main/java/com/rtbishop/look4sat/MainScreen.kt +++ b/app/src/main/java/com/rtbishop/look4sat/MainScreen.kt @@ -80,6 +80,7 @@ import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator import androidx.navigation3.ui.NavDisplay import com.rtbishop.look4sat.core.domain.repository.IContainerProvider import com.rtbishop.look4sat.core.domain.repository.MutualPassData +import com.rtbishop.look4sat.core.domain.navigation.MenuLayout import com.rtbishop.look4sat.core.presentation.DeeplinkResolver import com.rtbishop.look4sat.core.presentation.ElevationThresholds import com.rtbishop.look4sat.core.presentation.LocalElevationThresholds @@ -124,7 +125,7 @@ fun NavRoot(deeplink: String? = null) { rememberViewModelStoreNavEntryDecorator() // Required for ViewModel scoping per entry ), entryProvider = entryProvider { - entry { MainScreen(navigateToRadar = { rootBackStack.add(RadarDestination) }) } + entry { MainScreen() } entry { Scaffold { innerPadding -> RadarDestination(navigateUp = navigateBack) @@ -136,7 +137,7 @@ fun NavRoot(deeplink: String? = null) { } @Composable -fun MainScreen(navigateToRadar: () -> Unit = {}) { +fun MainScreen() { val backStack = rememberNavBackStack(Screen.Passes) val currentKey = backStack.lastOrNull() val navigateBack: () -> Unit = { backStack.removeLastOrNull() } @@ -145,27 +146,27 @@ fun MainScreen(navigateToRadar: () -> Unit = {}) { val container = (context.applicationContext as IContainerProvider).getMainContainer() val trackingState by container.radioTrackingService.state.collectAsStateWithLifecycle() val otherSettings by container.settingsRepo.otherSettings.collectAsStateWithLifecycle() - // UI settings: sort by screenOrder (empty = default order), then filter by hiddenScreens (Settings always kept) - val allNavItems = listOf(Screen.Satellites, Screen.Passes, Screen.Radar, Screen.Mutual, Screen.Roaming, Screen.CwDecode, Screen.WavelogLog, Screen.AmSat, Screen.Map, Screen.Settings) - .sortedBy { screen -> - // Unknown pages (e.g. CwDecode not in old persisted order): use default-order position (Roaming<->Map), then fall back to last - val idx = otherSettings.screenOrder.indexOf(screen.screenId) - if (idx != -1) idx - else com.rtbishop.look4sat.core.presentation.defaultScreenOrder.indexOf(screen.screenId).let { - if (it != -1) it else Int.MAX_VALUE - } - } - .filter { it.screenId !in otherSettings.hiddenScreens || it is Screen.Settings } - // 4.5.1 foldable menu: main menu (5 bottom-bar slots) + More menu (overflow page) - // Legacy migration: persisted subMenuOrder lacks new pages (WavelogLog) -> append to the sub-menu tail - val subOrder = (otherSettings.subMenuOrder.ifEmpty { com.rtbishop.look4sat.core.presentation.defaultSubMenuOrder }) - .let { list -> if ("WavelogLog" in list) list else list + "WavelogLog" } - .let { list -> if ("AMSAT" in list) list else list + "AMSAT" } - val mainNavItems = remember(allNavItems, subOrder) { - allNavItems.filter { it.screenId !in subOrder }.take(5) + // Menu layout is resolved in core:domain so the bar and the settings editor + // cannot disagree, and so Settings can never be pushed out of both menus. + val allNavItems = listOf( + Screen.Satellites, Screen.Passes, Screen.Radar, Screen.Mutual, Screen.Roaming, + Screen.CwDecode, Screen.WavelogLog, Screen.AmSat, Screen.Map, Screen.Settings + ) + val menuLayout = remember( + otherSettings.screenOrder, otherSettings.subMenuOrder, otherSettings.hiddenScreens + ) { + MenuLayout.resolve( + allScreenIds = allNavItems.map { it.screenId }, + screenOrder = otherSettings.screenOrder, + subMenuOrder = otherSettings.subMenuOrder, + hiddenScreenIds = otherSettings.hiddenScreens + ) } - val moreNavItems = remember(allNavItems, subOrder) { - subOrder.mapNotNull { id -> allNavItems.find { it.screenId == id } } + val mainNavItems = remember(menuLayout) { + menuLayout.mainIds.mapNotNull { id -> allNavItems.find { it.screenId == id } } + } + val moreNavItems = remember(menuLayout) { + menuLayout.moreIds.mapNotNull { id -> allNavItems.find { it.screenId == id } } } var moreExpanded by remember { mutableStateOf(false) } // Intercept Back while the More menu is open: close the menu first @@ -185,18 +186,9 @@ fun MainScreen(navigateToRadar: () -> Unit = {}) { NavigationSuiteScaffold( navigationSuiteItems = { mainNavItems.forEach { screen -> - val isSelected = when (currentKey) { - is Screen.Satellites -> screen is Screen.Satellites - is Screen.Passes -> screen is Screen.Passes - is Screen.Radar -> screen is Screen.Radar - is Screen.Mutual -> screen is Screen.Mutual - is Screen.CwDecode -> screen is Screen.CwDecode - is Screen.WavelogLog -> screen is Screen.WavelogLog - is Screen.AmSat -> screen is Screen.AmSat - is Screen.Map -> screen is Screen.Map - is Screen.Settings -> screen is Screen.Settings - else -> false - } + // Screen subclasses are data objects, so identity is enough and + // newly added pages highlight without touching this call site. + val isSelected = currentKey == screen item( icon = { Icon(painterResource(screen.iconResId), stringResource(screen.titleResId)) }, label = { Text(stringResource(screen.titleResId)) }, @@ -257,7 +249,6 @@ fun MainScreen(navigateToRadar: () -> Unit = {}) { container.setMutualPassData(MutualPassData()) container.satelliteRepo.selectPass(catNum, aosTime) backStack.add(Screen.Radar) - // navigateToRadar() } } entry { diff --git a/app/src/main/java/com/rtbishop/look4sat/MoreMenuPopup.kt b/app/src/main/java/com/rtbishop/look4sat/MoreMenuPopup.kt index cb7d556b..3d40900f 100644 --- a/app/src/main/java/com/rtbishop/look4sat/MoreMenuPopup.kt +++ b/app/src/main/java/com/rtbishop/look4sat/MoreMenuPopup.kt @@ -66,17 +66,10 @@ fun MoreMenuPopup( ) { Column(modifier = Modifier.padding(vertical = 4.dp)) { items.forEach { screen -> - val isSelected = when (currentKey) { - is Screen.Satellites -> screen is Screen.Satellites - is Screen.Passes -> screen is Screen.Passes - is Screen.Radar -> screen is Screen.Radar - is Screen.Mutual -> screen is Screen.Mutual - is Screen.CwDecode -> screen is Screen.CwDecode - is Screen.WavelogLog -> screen is Screen.WavelogLog - is Screen.Map -> screen is Screen.Map - is Screen.Settings -> screen is Screen.Settings - else -> false - } + // Screen subclasses are data objects, so identity is enough. The + // old per-type when was missing AmSat and Roaming, leaving those + // pages unhighlighted while open. + val isSelected = currentKey == screen Row( verticalAlignment = Alignment.CenterVertically, modifier = Modifier diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayout.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayout.kt new file mode 100644 index 00000000..8b35207e --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayout.kt @@ -0,0 +1,139 @@ +/* + * Look4Sat. Amateur radio satellite tracker and pass predictor. + * Copyright (C) 2019-2026 Arty Bishop and contributors. + * + * 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 com.rtbishop.look4sat.core.domain.navigation + +/** + * Single source of truth for the navigation menu layout. + * + * The bottom bar holds at most [MAIN_SLOTS] pages; the rest live behind the More + * button. Both the bar and the settings editor resolve through here, so the list + * the user edits is exactly the list they get. + * + * Lives in `core:domain` (pure Kotlin) so it is unit-testable and KMP-ready. + */ +object MenuLayout { + + /** Bottom-bar capacity, including the Settings entry. */ + const val MAIN_SLOTS = 5 + + /** Must stay reachable from some menu, so the user cannot lock themselves out. */ + const val SETTINGS_ID = "Settings" + + /** Bar contents for a fresh install. */ + val defaultMainOrder = listOf("Satellites", "Passes", "Radar", "Map", SETTINGS_ID) + + /** More-menu contents for a fresh install. */ + val defaultMoreOrder = listOf("Mutual", "Roaming", "CwDecode", "WavelogLog", "AMSAT") + + /** What the bar and the More menu actually show. */ + data class Layout(val mainIds: List, val moreIds: List) + + /** A menu assignment ready to be persisted to settings. */ + data class Assignment(val screenOrder: List, val subMenuOrder: List) + + /** + * Map persisted preferences onto the two menus. + * + * A page named by neither persisted list is new to this install and follows + * the defaults, so upgrades never lose pages. Visible pages that overflow + * [MAIN_SLOTS] fall through to the More menu instead of disappearing, and + * [SETTINGS_ID] always survives the slot cut. + */ + fun resolve( + allScreenIds: List, + screenOrder: List, + subMenuOrder: List, + hiddenScreenIds: List + ): Layout { + val visible = allScreenIds.filter { it !in hiddenScreenIds || it == SETTINGS_ID } + val wantMain = ArrayList() + val wantMore = ArrayList() + for (id in visible) { + when { + id in screenOrder -> wantMain.add(id) + id in subMenuOrder -> wantMore.add(id) + id in defaultMainOrder -> wantMain.add(id) + else -> wantMore.add(id) + } + } + wantMain.sortBy { rank(it, screenOrder, defaultMainOrder) } + wantMore.sortBy { rank(it, subMenuOrder, defaultMoreOrder) } + + // Reserve the Settings slot before cutting so it cannot be truncated away. + val settingsOnBar = SETTINGS_ID in wantMain + val budget = if (settingsOnBar) MAIN_SLOTS - 1 else MAIN_SLOTS + val main = ArrayList(MAIN_SLOTS) + for (id in wantMain) { + if (id == SETTINGS_ID) continue + if (main.size == budget) break + main.add(id) + } + if (settingsOnBar) main.add(SETTINGS_ID) + + val overflow = wantMain.filter { it !in main } + return Layout(mainIds = main, moreIds = overflow + wantMore) + } + + /** Move [screenId] onto the bar, evicting the last movable page when full. */ + fun moveToMain( + screenId: String, + allScreenIds: List, + screenOrder: List, + subMenuOrder: List + ): Assignment { + val current = resolve(allScreenIds, screenOrder, subMenuOrder, emptyList()) + val main = current.mainIds.toMutableList() + val more = current.moreIds.toMutableList() + more.remove(screenId) + if (screenId !in main) { + val at = main.indexOf(SETTINGS_ID).let { if (it == -1) main.size else it } + main.add(at, screenId) + } + val movable = main.filter { it != SETTINGS_ID && it != screenId } + if (main.size > MAIN_SLOTS && movable.isNotEmpty()) { + val evicted = movable.last() + main.remove(evicted) + more.add(0, evicted) + } + return Assignment(screenOrder = main, subMenuOrder = more) + } + + /** Move [screenId] off the bar; Settings is refused so it stays reachable. */ + fun moveToMore( + screenId: String, + allScreenIds: List, + screenOrder: List, + subMenuOrder: List + ): Assignment { + if (screenId == SETTINGS_ID) return Assignment(screenOrder, subMenuOrder) + val current = resolve(allScreenIds, screenOrder, subMenuOrder, emptyList()) + val more = current.moreIds.toMutableList() + if (screenId !in more) more.add(screenId) + return Assignment( + screenOrder = current.mainIds.filter { it != screenId }, + subMenuOrder = more + ) + } + + private fun rank(id: String, persisted: List, fallback: List): Int { + val persistedIndex = persisted.indexOf(id) + if (persistedIndex != -1) return persistedIndex + val fallbackIndex = fallback.indexOf(id) + return if (fallbackIndex != -1) fallbackIndex else Int.MAX_VALUE + } +} diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayoutTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayoutTest.kt new file mode 100644 index 00000000..4090258a --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/navigation/MenuLayoutTest.kt @@ -0,0 +1,154 @@ +/* + * Look4Sat. Amateur radio satellite tracker and pass predictor. + * Copyright (C) 2019-2026 Arty Bishop and contributors. + * + * 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 com.rtbishop.look4sat.core.domain.navigation + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Menu layout rules. Every case here is a bug that shipped at least once, so + * treat a failure as a regression rather than a spec question. + */ +class MenuLayoutTest { + + private val all = listOf( + "Satellites", "Passes", "Radar", "Mutual", "Roaming", + "CwDecode", "WavelogLog", "AMSAT", "Map", "Settings" + ) + + private fun layout( + screenOrder: List = emptyList(), + subMenuOrder: List = emptyList(), + hidden: List = emptyList() + ) = MenuLayout.resolve(all, screenOrder, subMenuOrder, hidden) + + @Test + fun defaultsPutFivePagesOnTheBarAndTheRestBehindMore() { + val l = layout() + assertEquals(listOf("Satellites", "Passes", "Radar", "Map", "Settings"), l.mainIds) + assertEquals(listOf("Mutual", "Roaming", "CwDecode", "WavelogLog", "AMSAT"), l.moreIds) + } + + @Test + fun settingsIsAlwaysReachable() { + // Moving any page onto the bar used to push Settings out of BOTH menus, + // permanently locking the user out of the settings page. + for (page in listOf("Mutual", "Roaming", "CwDecode", "WavelogLog", "AMSAT")) { + val moved = MenuLayout.moveToMain(page, all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue( + "moving $page hid Settings: main=${l.mainIds} more=${l.moreIds}", + "Settings" in l.mainIds || "Settings" in l.moreIds + ) + } + } + + @Test + fun movingAmsatToMainActuallyTakesEffect() { + // The legacy migration re-appended AMSAT to the sub-menu unconditionally, + // silently undoing the user's choice. + val moved = MenuLayout.moveToMain("AMSAT", all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue("AMSAT missing from the bar: ${l.mainIds}", "AMSAT" in l.mainIds) + assertTrue("AMSAT still behind More: ${l.moreIds}", "AMSAT" !in l.moreIds) + } + + @Test + fun movingWavelogLogToMainActuallyTakesEffect() { + val moved = MenuLayout.moveToMain("WavelogLog", all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue("WavelogLog missing from the bar: ${l.mainIds}", "WavelogLog" in l.mainIds) + } + + @Test + fun newPagesUnknownToPersistedOrderDefaultToTheMoreMenu() { + // Upgrading from a build that predates AMSAT and WavelogLog: neither list + // mentions them, so both must land behind More rather than vanishing. + val l = layout( + screenOrder = listOf("Satellites", "Passes", "Radar", "Map", "Settings"), + subMenuOrder = listOf("Mutual", "Roaming", "CwDecode") + ) + assertTrue("AMSAT should land behind More", "AMSAT" in l.moreIds) + assertTrue("WavelogLog should land behind More", "WavelogLog" in l.moreIds) + } + + @Test + fun everyVisiblePageIsReachableFromSomeMenu() { + // Pages beyond the five slots used to vanish instead of overflowing. + val l = layout( + screenOrder = listOf("Satellites", "Passes", "Radar", "Mutual", "Roaming", "Map", "Settings"), + subMenuOrder = listOf("CwDecode", "WavelogLog", "AMSAT") + ) + assertEquals("every page must be reachable", all.toSet(), (l.mainIds + l.moreIds).toSet()) + } + + @Test + fun hiddenPagesAppearInNeitherMenu() { + val l = layout(hidden = listOf("Radar", "Map")) + assertTrue("Radar" !in l.mainIds && "Radar" !in l.moreIds) + assertTrue("Map" !in l.mainIds && "Map" !in l.moreIds) + } + + @Test + fun settingsCannotBeHidden() { + val l = layout(hidden = listOf("Settings")) + assertTrue("Settings" in l.mainIds || "Settings" in l.moreIds) + } + + @Test + fun theBarNeverExceedsFiveSlots() { + val l = layout(screenOrder = all) + assertTrue("bar had ${l.mainIds.size} slots: ${l.mainIds}", l.mainIds.size <= MenuLayout.MAIN_SLOTS) + } + + @Test + fun movingAPageOutOfTheBarPutsItBehindMore() { + val moved = MenuLayout.moveToMore("Radar", all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue("Radar" in l.moreIds) + assertTrue("Radar" !in l.mainIds) + } + + @Test + fun settingsCannotBeMovedOffTheBar() { + val moved = MenuLayout.moveToMore("Settings", all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue("Settings" in l.mainIds || "Settings" in l.moreIds) + } + + @Test + fun resolveIsStableWhenAppliedTwice() { + // Persisting what resolve() produced must not change the outcome, or the + // settings list and the bar drift apart on the next recomposition. + val first = layout() + val second = layout(first.mainIds, first.moreIds) + assertEquals(first.mainIds, second.mainIds) + assertEquals(first.moreIds, second.moreIds) + } + + @Test + fun movingAPageOntoAFullBarEvictsAnotherPageIntoMore() { + // The bar starts full (5 including Settings), so making room must push an + // existing page into More instead of dropping it. + val moved = MenuLayout.moveToMain("CwDecode", all, emptyList(), emptyList()) + val l = layout(moved.screenOrder, moved.subMenuOrder) + assertTrue("CwDecode" in l.mainIds) + assertEquals("nothing may be lost", all.toSet(), (l.mainIds + l.moreIds).toSet()) + } +} diff --git a/core/presentation/src/main/java/com/rtbishop/look4sat/core/presentation/Navigation.kt b/core/presentation/src/main/java/com/rtbishop/look4sat/core/presentation/Navigation.kt index a722be18..7414683f 100644 --- a/core/presentation/src/main/java/com/rtbishop/look4sat/core/presentation/Navigation.kt +++ b/core/presentation/src/main/java/com/rtbishop/look4sat/core/presentation/Navigation.kt @@ -54,12 +54,6 @@ sealed class Screen(val iconResId: Int, val titleResId: Int, val screenId: Strin data object Settings : Screen(R.drawable.ic_settings, R.string.nav_prefs, "Settings") } -// UI settings: default main-menu page order (5 bottom-bar slots) -val defaultScreenOrder = listOf("Satellites", "Passes", "Radar", "Map", "Settings") - -// UI settings: default More-menu order (pages behind "More") -val defaultSubMenuOrder = listOf("Mutual", "Roaming", "CwDecode", "WavelogLog", "AMSAT") - @Serializable data object RadarDestination : NavKey diff --git a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsScreen.kt b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsScreen.kt index 6e2fc858..45017dbd 100644 --- a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsScreen.kt +++ b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsScreen.kt @@ -105,8 +105,7 @@ import com.rtbishop.look4sat.core.presentation.R import com.rtbishop.look4sat.core.presentation.ScreenColumn import com.rtbishop.look4sat.core.presentation.TopBar import com.rtbishop.look4sat.core.presentation.infiniteMarquee -import com.rtbishop.look4sat.core.presentation.defaultScreenOrder -import com.rtbishop.look4sat.core.presentation.defaultSubMenuOrder +import com.rtbishop.look4sat.core.domain.navigation.MenuLayout import com.rtbishop.look4sat.core.presentation.isVerticalLayout import java.text.SimpleDateFormat import java.util.Date @@ -406,7 +405,6 @@ private fun SettingsScreen( screenOrder = uiState.otherSettings.screenOrder, subMenuOrder = uiState.otherSettings.subMenuOrder, onToggle = { name -> onAction(SettingsAction.ToggleScreen(name)) }, - onReorder = { order -> onAction(SettingsAction.ReorderScreens(order)) }, onResetOrder = { onAction(SettingsAction.ResetMenuOrder) }, onUpdateMenu = { main, sub -> onAction(SettingsAction.UpdateMenuOrder(main, sub)) } ) @@ -797,7 +795,6 @@ private fun UiSettingsCard( screenOrder: List, subMenuOrder: List, onToggle: (String) -> Unit, - onReorder: (List) -> Unit, onResetOrder: () -> Unit, onUpdateMenu: (List, List) -> Unit ) { @@ -813,25 +810,18 @@ private fun UiSettingsCard( R.string.nav_map to "Map", R.string.nav_prefs to "Settings" ) // name 必须与 Screen.screenId 一致 (R8 安全) - // Main menu items: exclude sub-menu items; Settings pinned last - val mainItems = remember(screens, screenOrder, subMenuOrder) { - val sub = subMenuOrder.ifEmpty { defaultSubMenuOrder } - screens.map { it.second } - .filter { it !in sub } - .sortedBy { name -> - screenOrder.indexOf(name).let { idx -> - if (idx != -1) idx else defaultScreenOrder.indexOf(name).let { - if (it != -1) it else Int.MAX_VALUE - } - } - } - .sortedWith(compareBy { if (it == "Settings") 1 else 0 }) - } - // More-menu items - val subItems = remember(screens, subMenuOrder) { - val sub = subMenuOrder.ifEmpty { defaultSubMenuOrder } - sub.filter { s -> screens.any { (_, name) -> name == s } } + // Resolved by the same core:domain rule the bottom bar uses, so this list is + // exactly what the user will see on the bar. + val menuLayout = remember(screens, screenOrder, subMenuOrder, hiddenScreens) { + MenuLayout.resolve( + allScreenIds = screens.map { it.second }, + screenOrder = screenOrder, + subMenuOrder = subMenuOrder, + hiddenScreenIds = hiddenScreens + ) } + val mainItems = menuLayout.mainIds + val subItems = menuLayout.moreIds ElevatedCard(modifier = Modifier.fillMaxWidth()) { Column( modifier = Modifier.padding(horizontal = 8.dp, vertical = 4.dp), @@ -868,12 +858,12 @@ private fun UiSettingsCard( moveLabel = stringResource(id = R.string.prefs_ui_move_out), moveEnabled = { name -> name != "Settings" }, onMove = { name -> - onUpdateMenu( - mainItems.filter { it != name }, - (subMenuOrder.ifEmpty { defaultSubMenuOrder } + name).distinct() + val moved = MenuLayout.moveToMore( + name, screens.map { it.second }, screenOrder, subMenuOrder ) + onUpdateMenu(moved.screenOrder, moved.subMenuOrder) }, - onReorder = { main -> onUpdateMenu(main, subMenuOrder) } + onReorder = { main -> onUpdateMenu(main, subItems) } ) // More-menu area (pages behind "More") Text( @@ -888,19 +878,12 @@ private fun UiSettingsCard( // Buttons always visible; when the main menu hits 5, the last non-Settings item moves to More (swap) moveEnabled = { true }, onMove = { name -> - val mainWithoutSettings = mainItems.filter { it != "Settings" } - val needSwap = mainWithoutSettings.size >= 5 - val evicted = if (needSwap) mainWithoutSettings.last() else null - val newMain = if (needSwap) { - (mainWithoutSettings.dropLast(1) + name).distinct() - } else { - (mainWithoutSettings + name).distinct() - } - val newSub = (subItems.filter { it != name } + - if (evicted != null) listOf(evicted) else emptyList()).distinct() - onUpdateMenu(newMain, newSub) + val moved = MenuLayout.moveToMain( + name, screens.map { it.second }, screenOrder, subMenuOrder + ) + onUpdateMenu(moved.screenOrder, moved.subMenuOrder) }, - onReorder = { sub -> onUpdateMenu(screenOrder, sub) } + onReorder = { sub -> onUpdateMenu(mainItems, sub) } ) TextButton(onClick = onResetOrder) { Text(text = stringResource(id = R.string.prefs_ui_order_reset)) diff --git a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsState.kt b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsState.kt index 1f09d805..4cd148ca 100644 --- a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsState.kt +++ b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsState.kt @@ -66,10 +66,6 @@ sealed interface SettingsAction { data class ToggleNightMode(val value: Boolean) : SettingsAction // UI settings: toggle a bottom-nav page's hidden state (Screen simpleName) data class ToggleScreen(val screenName: String) : SettingsAction - // UI settings: update page order - data class ReorderScreens(val order: List) : SettingsAction - // UI settings: reset to default order - data object ResetScreenOrder : SettingsAction // UI settings: update main + More menu order (4.5.1 foldable menu) data class UpdateMenuOrder(val mainOrder: List, val subOrder: List) : SettingsAction // UI settings: reset main + More menu order diff --git a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsViewModel.kt b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsViewModel.kt index 363c5584..03658f55 100644 --- a/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsViewModel.kt +++ b/feature/settings/src/main/java/com/rtbishop/look4sat/feature/settings/SettingsViewModel.kt @@ -134,8 +134,6 @@ class SettingsViewModel( if (action.screenName in hidden) hidden.remove(action.screenName) else hidden.add(action.screenName) current.copy(hiddenScreens = hidden) } - is SettingsAction.ReorderScreens -> settingsRepo.updateOtherSettings { it.copy(screenOrder = action.order) } - SettingsAction.ResetScreenOrder -> settingsRepo.updateOtherSettings { it.copy(screenOrder = emptyList()) } is SettingsAction.UpdateMenuOrder -> settingsRepo.updateOtherSettings { it.copy(screenOrder = action.mainOrder, subMenuOrder = action.subOrder) }