fix(nav): resolve the menu layout in core:domain so page order actually applies

用户报告"把 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> 分支不动 —— 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 个页面逐一移入主菜单, 设置入口全部保住、页面无丢失; 隐藏页 + 移动组合无页面丢失; 幂等性通过
2026-08-13 16:10:41 +00:00
1 parent 8adf861ed7
commit 6fc2f560b7
@@ -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 <https://www.gnu.org/licenses/>.
+ */
+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<String>, val moreIds: List<String>)
+
+    /** A menu assignment ready to be persisted to settings. */
+    data class Assignment(val screenOrder: List<String>, val subMenuOrder: List<String>)
+
+    /**
+     * 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<String>,
+        screenOrder: List<String>,
+        subMenuOrder: List<String>,
+        hiddenScreenIds: List<String>
+    ): Layout {
+        val visible = allScreenIds.filter { it !in hiddenScreenIds || it == SETTINGS_ID }
+        val wantMain = ArrayList<String>()
+        val wantMore = ArrayList<String>()
+        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<String>(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<String>,
+        screenOrder: List<String>,
+        subMenuOrder: List<String>
+    ): 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<String>,
+        screenOrder: List<String>,
+        subMenuOrder: List<String>
+    ): 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<String>, fallback: List<String>): 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
+    }
+}
@@ -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 <https://www.gnu.org/licenses/>.
+ */
+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<String> = emptyList(),
+        subMenuOrder: List<String> = emptyList(),
+        hidden: List<String> = 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())
+    }
+}