diff --git a/android/THEMING_AND_NAV.md b/android/THEMING_AND_NAV.md index ef8d8a1..cff1318 100644 --- a/android/THEMING_AND_NAV.md +++ b/android/THEMING_AND_NAV.md @@ -410,43 +410,73 @@ are unchanged by it. An override cannot introduce an app route, cannot touch role or the shard's visibility config would otherwise withhold. Hiding is subtractive, exactly as `applyNavOverrides` has it. +> **Decision (2026-08-08): an admin may hide the Home row.** The website lets `/` +> be hidden from the public header — the brand link still goes home — and the app +> mirrors it rather than inventing a policy the site does not have. Home remains +> the `NavHost`'s start destination and remains reachable by back-press; unlike +> the website's `/admin/navigation`, which has three guards because hiding it +> would strip the only way to *undo* an override, nothing about a hidden Home row +> is unrecoverable. Hiding therefore stays one uniform rule with no special cases. + ### 6.2 Path → app route The public nav is keyed by **website** paths. The app needs a mapping table, and it is the one new piece of cross-repo coupling this milestone introduces — so it lives in one file with the website's `NAV` array quoted beside it. -| website `to` | app route | note | -|---|---|---| -| `/` | `Routes.HOME` | | -| `/site/news` | `Routes.NEWS` | | -| `/site/five-on-friday` | `Routes.news(FIVE_ON_FRIDAY)` | the app's News screen already has all four categories as tabs — these three select one | -| `/site/newsletter` | `Routes.news(NEWSLETTER)` | | -| `/site/screenshots` | `Routes.news(SCREENSHOTS)` | | -| `/wiki` | `Routes.WIKI` | | -| `/site/shard` | `Routes.SHARD` | `feature: status` | -| `/site/champs` | `Routes.SHARD_CHAMPS` | **not in `APP_MENU` today** — reached via the Shard hub | -| `/site/guilds` | `Routes.SHARD_GUILDS` | as above | -| `/site/governors` | `Routes.SHARD_GOVERNORS` | as above | -| `/site/houses` | `Routes.SHARD_HOUSES` | as above | -| `/site/rules` | `Routes.SHARD_RULES` | | -| `/site/atlas` | `Routes.ATLAS` | | -| `/site/leaderboards` | `Routes.SHARD_LEADERBOARDS` | | -| `/site/market` | `Routes.SHARD_MARKET` | | -| `/site/about` | `Routes.page("about")` | | +**The table is in the website's order, and that order is load-bearing.** A stored +`order` is an index into the site's nav, so a row the admin never moved has to +take its sort key from the same list or explicit and implicit keys sit on two +incomparable number lines (see "Phase 5 as landed"). The rows below are verbatim +from `website/client/src/components/SiteHeader.jsx` — note that the three news +categories are **Screenshots, Five on Friday, Newsletter** in that sequence, +between News and Wiki. + +| # | website `to` | app route | note | +|---|---|---|---| +| 0 | `/` | `Routes.HOME` | | +| 1 | `/site/news` | `Routes.NEWS` | | +| 2 | `/site/screenshots` | `Routes.news(SCREENSHOTS)` | the app's News screen already has all four categories as tabs — these three select one, and none has an `APP_MENU` row | +| 3 | `/site/five-on-friday` | `Routes.news(FIVE_ON_FRIDAY)` | as above | +| 4 | `/site/newsletter` | `Routes.news(NEWSLETTER)` | as above | +| 5 | `/wiki` | `Routes.WIKI` | | +| 6 | `/site/shard` | `Routes.SHARD` | `feature: status` | +| 7 | `/site/champs` | `Routes.SHARD_CHAMPS` | **not in `APP_MENU` today** — reached via the Shard hub | +| 8 | `/site/guilds` | `Routes.SHARD_GUILDS` | as above | +| 9 | `/site/governors` | `Routes.SHARD_GOVERNORS` | as above | +| 10 | `/site/houses` | `Routes.SHARD_HOUSES` | as above | +| 11 | `/site/rules` | `Routes.SHARD_RULES` | | +| 12 | `/site/atlas` | `Routes.ATLAS` | | +| 13 | `/site/leaderboards` | `Routes.SHARD_LEADERBOARDS` | | +| 14 | `/site/market` | `Routes.SHARD_MARKET` | | +| 15 | `/site/about` | `Routes.page("about")` | | + +**The `feature` values are not mirrored into this table**, though the website's +array carries one on nine of these rows. `APP_MENU` stays the app's own source of +truth for gating: a second copy of a security-relevant value that drifts silently +is worth more than it costs. The table carries the mapping and nothing else. Three asymmetries to resolve rather than paper over: - **`Routes.NEWS` takes no category argument today.** It gains an optional one so - the three category entries can land on the right tab. This is a small route - change with its own test, not a nav concern. -- **Four web entries have no `APP_MENU` row** (champs / guilds / governors / - houses — the app puts them behind the Shard hub, which is the better phone - shape and stays). An override for one of them therefore has a mapped route but - no menu entry. **Rule: an override for a path the app does not surface in its - menu is ignored**, exactly as the web drops an override for an unknown `to`. - It is *not* an invitation to add the entry — the hub is a deliberate design - choice, and a nav override may not introduce navigation. + a link to one of the three category pages can land on the right tab. This is a + small route change with its own test, not a nav concern. +- **Seven web entries have no `APP_MENU` row** — champs / guilds / governors / + houses (the app puts them behind the Shard hub, which is the better phone shape + and stays) and the three news categories (tabs on one screen). An override for + one of them therefore has a mapped route but no menu entry. **Rule: an override + for a path the app does not surface in its menu is ignored**, exactly as the web + drops an override for an unknown `to`. It is *not* an invitation to add the + entry — the hub and the tab strip are deliberate design choices, and a nav + override may not introduce navigation. + + > **Decision (2026-08-08):** this rule covers the **news categories too**, not + > only the hub four. The mapping in the table above is what phase 6's added + > links resolve against — and *there* a category tab is a perfectly good + > destination, because the admin named it by path — but neither a relabel nor a + > reorder of `/site/screenshots` puts a new row in the drawer. The alternative + > considered and rejected was surfacing such a row only when overridden, which + > keeps AC-1 but lets an override introduce navigation after all. - **Ten app entries have no `nav_public` counterpart** — Contact, Account and Notifications, the three player groups, and the four staff rows. They are unaffected by `nav_public` and keep their coded order, appended after the @@ -585,7 +615,7 @@ PR against `main`. | **2** ✅ | **Radii & shadow** | Ratio-scaled `Shapes` (§5.2), elevation map (§5.4) | | **3** ✅ | **Fonts** | Seven bundled families + licenses; stack → `FontFamily` resolution; `Type.kt` takes its three families from the resolved theme. APK size recorded | | **4** ✅ | **Brand assets** | Logo in the drawer header and top bar, hero on Home (§5.6). Coil + `LocalAssetResolver` already exist; renders nothing when unset | -| **5** | **Public nav: label / order / hidden** | The path→route table (§6.2), `Routes.news(category)`, the merge, drawer wiring. AC-3 | +| **5** ✅ | **Public nav: label / order / hidden** | The path→route table (§6.2), `Routes.news(category)`, the merge, drawer wiring. AC-3 | | **6** | **Public nav: sections & added links** | Drawer groups, `pruneNav` port, link path validation, native-route resolution + Custom Tab fallback (§6.3) | | **7** | **Authenticated navs** | ❌ **cancelled** — was: `GET /api/v1/settings/nav` behind a session-keyed repository; label/hidden for the four mapped rows (§6.4). See "Phase 7, cancelled" below | | **8** | **Docs, coverage & cutover** | This doc's "as landed" notes and any amendments the build forces, the `PLAN.md` §9 M12 entry refreshed, Sonar coverage for the new modules, AC-5 on-device walk, then `edge` → `main` | @@ -813,6 +843,69 @@ proven not to have moved anything. cropped hero, a synthesised bold, and a logo's contrast against the top bar are all things only a screen shows. +### Phase 5 as landed + +- **An untouched instance gets `APP_MENU` back by identity, not by equality.** + `applyNavOverrides` returns the *same list instance* when there is no stored row, + an empty one, or one with nothing usable in it — so AC-1's claim for the drawer + is a one-line `assertSame` rather than a structural comparison, and there is no + path where an unedited nav is rebuilt and could come out different. "Nothing + usable" is a real case worth the check: a blank label, `hidden: false`, an + unknown path, and a path the app maps but does not surface all say nothing. +- **An untouched row's implicit sort key is its index in the WEBSITE's nav, not + the app's** — the one thing the spec did not settle and the whole sort turns on. + A stored `order` is a position in the site's sixteen-row list, so a key taken + from the app's nine-row block would put explicit and implicit keys on two + incomparable number lines: an unmoved About (app index 8) would sort ahead of an + unmoved Market (web order 14) the moment any row carried an explicit order. This + is why §6.2's table is now numbered and why its order is called load-bearing. + Both tie-breaks are the web's — an explicit order beats a coincidental index, + and two explicit orders keep code order because the sort is stable. + - In practice the editor writes an order for *every* visible row when the admin + drags anything, so a mix of explicit and implicit keys is the stale-row case + rather than the normal one. It still has to resolve predictably. +- **The app's own rows are partitioned off, not sorted.** Contact, Account, + Notifications, the three player groups and the four staff rows have no website + counterpart to be reordered against, so they keep their coded order after the + public block. They already sit there today, which is what makes the partition + the current layout rather than a new one — but it does mean an `APP_MENU` that + interleaved an app-only row *among* the public ones would see it moved to the + tail. Nothing does today; a future row should be added with that in mind. +- **`group` and `section` are read and dropped.** The app renders no sections in + this phase (phase 6) and never renders the admin sidebar's groups at all, and a + value that cannot be honored is better dropped than half-applied. Both stored + shapes are read, though: website phase 10's `{items, sections, links}` and the + bare map phases 6-8 stored, which is unambiguous because every key in it is a + path and so can never be the string `items`. +- **`Routes.NEWS_ROUTE` is declared beside `Routes.NEWS` rather than replacing + it**, because the two are used for different things: the pattern is what + `composable()` and `destination.route` speak, the bare route is what callers + navigate to. Navigating to plain `Routes.NEWS` matches the pattern with no + argument, so the drawer row and the push deep-link (`Routes.forStream`) are + untouched. The consequence to remember: **`destination.route` is now a pattern + carrying a query**, so `RunicApp` compares on `substringBefore('?')` for both + the top-level check and the selected-row check. A future route that takes an + optional argument inherits that for free; one that does not, and is compared + against by hand, will not. +- **`Routes.news()` takes the `PostCategory` enum, not a slug string** (unlike the + existing `Routes.post()`), so an unmapped category cannot reach the NavHost — + the tab strip *is* the enum's entries, and a slug it does not know would select + nothing. `NewsViewModel` falls back to the default feed for an unknown slug + anyway, since a hand-edited settings row can carry one. +- **Phase 5 ships `Routes.news(category)`; phase 6 is its first caller.** Nothing + in the drawer navigates to a category tab, by the §6.2 decision above. It ships + here because it is the table's route builder and the table is this phase's. +- Tests: `NavOverridesTest` (19) + `NavPathsTest` (11) + 2 new `NewsViewModel` + cases (and three existing ones rewritten onto a `SavedStateHandle`). **442 unit tests green** (410 + 32), `lintDebug` and + `assembleDebug` clean. AC-3 is covered three ways — a feature-gated row that an + override relabels, moves to the front and marks `hidden: false` is still not + shown to an admin whose shard does not publish it; role-gated rows stay hidden + from an anonymous caller whatever the row says; and hiding composes with the + gates rather than competing with them. What the JVM still cannot reach is the + same limit phase 4 hit: that the reordered drawer *draws* in the new order, and + that navigating to plain `news` really does match the optional-argument pattern + at runtime, are AC-5's. + ### Phase 7, cancelled Reading `nav_admin` / `nav_player` is **not scheduled**. The measurement in §6.4