Merge pull request 'docs(android): M12 phase 5 as landed' (#117) from docs/android-theming-nav-phase-5 into edge
Reviewed-on: #117
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user