Merge pull request 'docs(android): M12 phase 6 as landed' (#118) from docs/android-theming-nav-phase-6 into edge
Reviewed-on: #118
This commit is contained in:
@@ -507,22 +507,69 @@ off to a Custom Tab otherwise. The patterns the app can resolve:
|
|||||||
|
|
||||||
```
|
```
|
||||||
/ → HOME
|
/ → HOME
|
||||||
/site/news|five-on-friday|newsletter|screenshots → NEWS (category)
|
/site/news → NEWS
|
||||||
/site/news/<idOrSlug> → POST
|
/site/{screenshots,five-on-friday,newsletter}
|
||||||
|
→ NEWS, that category's tab
|
||||||
|
/site/newsletter/<id> → POST
|
||||||
/wiki → WIKI
|
/wiki → WIKI
|
||||||
/wiki/<slug> → WIKI_PAGE
|
/wiki/<slug> → WIKI_PAGE
|
||||||
/site/<shard surface> → the mapped shard route (per §6.2)
|
/site/<shard surface> → the mapped shard route (per §6.2)
|
||||||
/site/about, /page/<slug> → PAGE
|
/site/atlas/<slug> → ATLAS_CREATURE
|
||||||
/contact → CONTACT
|
/site/market/vendors/<serial> → SHARD_MARKET_VENDOR
|
||||||
|
/site/about → PAGE("about")
|
||||||
|
/<slug> → PAGE(slug), unless <slug> is reserved
|
||||||
anything else → WebHandoff (Custom Tab), M3's existing hand-off
|
anything else → WebHandoff (Custom Tab), M3's existing hand-off
|
||||||
```
|
```
|
||||||
|
|
||||||
|
> **Correction (2026-08-08).** The table this replaces named three paths the
|
||||||
|
> website does not serve, and was written from the app's routes rather than the
|
||||||
|
> site's. Checked against `website/client/src/App.jsx`: there is **no
|
||||||
|
> `/site/news/<idOrSlug>`** — news items render on their category page and the
|
||||||
|
> site's one post-detail route is the newsletter's `/site/newsletter/:id`; there
|
||||||
|
> is **no `/page/<slug>`** — CMS pages are served from a top-level `/<slug>`; and
|
||||||
|
> there is **no `/contact`** at all, the app's contact form being app-only (§6.2
|
||||||
|
> says as much). The site's real detail routes for the atlas and the market were
|
||||||
|
> missing. A link resolver that does not read the site's own route table cannot be
|
||||||
|
> right by accident, so it now quotes it, next to §6.2's table and for the same
|
||||||
|
> reason.
|
||||||
|
>
|
||||||
|
> **Decision (2026-08-08): the `/<slug>` catch-all is in**, with the site's
|
||||||
|
> non-CMS top-level segments (`admin`, `account`, `player`, `site`, `wiki`,
|
||||||
|
> `invite`, `preview`, `api`, `uploads`) excluded from it. React Router ranks its
|
||||||
|
> static routes above `/:slug` and the app has to do the same, or a link to the
|
||||||
|
> admin panel would open an in-app 404 instead of the real thing in a browser.
|
||||||
|
> The reserved list is a second piece of cross-repo coupling and it lives beside
|
||||||
|
> the first. It buys the case that matters most: a page the admin wrote — the
|
||||||
|
> likeliest added link there is — opens natively.
|
||||||
|
>
|
||||||
|
> **A path carrying a query or a fragment hands off**, whatever its route part
|
||||||
|
> says. No app route takes either, so a native match would quietly drop what the
|
||||||
|
> admin typed; the browser honors it exactly.
|
||||||
|
|
||||||
A native match still passes through the app's own gates: an added link to
|
A native match still passes through the app's own gates: an added link to
|
||||||
`/site/market` on a shard that does not publish the market lands on the Market
|
`/site/market` on a shard that does not publish the market lands on the Market
|
||||||
screen's honest "not published here" state (M11's `FEATURE_UNAVAILABLE`), which
|
screen's honest "not published here" state (M11's `FEATURE_UNAVAILABLE`), which
|
||||||
is what typing the URL on the web does too. The link itself is not gated — that
|
is what typing the URL on the web does too. The link itself is not gated — that
|
||||||
is the website's decision and the app does not second-guess it.
|
is the website's decision and the app does not second-guess it.
|
||||||
|
|
||||||
|
**In the drawer** (all four settled by the org lead before code, 2026-08-08):
|
||||||
|
|
||||||
|
- **A section is a static header with its rows indented beneath it**, always
|
||||||
|
open. A collapsible group was considered and rejected: it costs remembered
|
||||||
|
state per section and can hide the row the admin meant to surface.
|
||||||
|
- **A link that hands off carries a trailing icon**
|
||||||
|
(`Icons.AutoMirrored.Filled.ExitToApp`, the only "leaves the app" glyph in
|
||||||
|
`material-icons-core` — the extended artifact is not a dependency and phase 3
|
||||||
|
already spent the APK budget). A link the app resolves natively is deliberately
|
||||||
|
indistinguishable from a coded row; that is the point of resolving it.
|
||||||
|
- **The Custom Tab's absolute URL comes from `LocalAssetResolver`**, which
|
||||||
|
already resolves any site-relative path against the configured base URL and is
|
||||||
|
already provided at the app root. A nav path travelling through something named
|
||||||
|
"asset resolver" is the cost; zero new plumbing is the benefit.
|
||||||
|
- **A resolved link opens like any other drawer row** — `navigateTopLevel`,
|
||||||
|
detail screen or not — rather than pushing onto the current screen. One rule,
|
||||||
|
and back-press lands on Home exactly as it does from every other row.
|
||||||
|
|
||||||
### 6.4 `nav_admin` and `nav_player` — label and hidden only
|
### 6.4 `nav_admin` and `nav_player` — label and hidden only
|
||||||
|
|
||||||
Both are bare maps and neither carries sections or links. The app honors
|
Both are bare maps and neither carries sections or links. The app honors
|
||||||
@@ -616,7 +663,7 @@ PR against `main`.
|
|||||||
| **3** ✅ | **Fonts** | Seven bundled families + licenses; stack → `FontFamily` resolution; `Type.kt` takes its three families from the resolved theme. APK size recorded |
|
| **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 |
|
| **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) |
|
| **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 |
|
| **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` |
|
| **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` |
|
||||||
|
|
||||||
@@ -906,6 +953,51 @@ proven not to have moved anything.
|
|||||||
that navigating to plain `news` really does match the optional-argument pattern
|
that navigating to plain `news` really does match the optional-argument pattern
|
||||||
at runtime, are AC-5's.
|
at runtime, are AC-5's.
|
||||||
|
|
||||||
|
### Phase 6 as landed
|
||||||
|
|
||||||
|
- **The spec's link table was wrong about the website, and that is the finding to
|
||||||
|
remember.** Three of its rows named paths the site does not serve and two of the
|
||||||
|
site's real detail routes were missing — see the correction in §6.3. Phase 5 hit
|
||||||
|
the same class of error in §6.2's row order. Both times the fix was to quote the
|
||||||
|
website's own source beside the table, so `NavPaths.kt` now carries `App.jsx`'s
|
||||||
|
route list next to `SiteHeader.jsx`'s `NAV`.
|
||||||
|
- **Phase 6 does not re-implement phase 5, and AC-1 is unchanged because of it.**
|
||||||
|
`buildNavTree` hands straight to `applyNavOverrides` when the stored row carries
|
||||||
|
no sections and no links, so an untouched instance still gets `APP_MENU` back by
|
||||||
|
identity — the tree build only runs when the admin actually created structure.
|
||||||
|
The regression test is that an items-only row through the tree equals the same
|
||||||
|
row through the flat merge.
|
||||||
|
- **`section` on an item override is read but not counted as "usable".** A
|
||||||
|
section-only override says nothing to a flat list, so `NavOverride.isEmpty`
|
||||||
|
deliberately excludes it and an instance that only ever grouped rows still gets
|
||||||
|
its coded list back by identity from `applyNavOverrides`. The tree adds its own
|
||||||
|
check. This is the one place the two consumers of a stored row disagree about
|
||||||
|
what "empty" means, and they have to.
|
||||||
|
- **A dangling reference degrades to "no grouping", never to "no row".** A member
|
||||||
|
of a section that does not exist — because the admin deleted it, or its label
|
||||||
|
was blank and it was dropped — is an ordinary top-level row, not a row that
|
||||||
|
vanished with its section. Same for a link naming an unknown section: its
|
||||||
|
destination is still good, only the grouping was wrong.
|
||||||
|
- **`visibleEntries` was split into `isEntryVisible`.** `pruneNav` applies the
|
||||||
|
predicate inside a section as well as at the top level, and both callers must
|
||||||
|
ask exactly one question, or a sectioned row could end up gated by a rule its
|
||||||
|
top-level twin is not. The list form now just filters with it.
|
||||||
|
- **Sort keys extend phase 5's number line rather than starting a new one.** A
|
||||||
|
coded row keys on its index in the website's sixteen-row nav; an admin-created
|
||||||
|
section or link with no stored order keys on `16, 17, …` in creation order, so
|
||||||
|
it appends after the coded rows instead of jumping to the front on a `0`
|
||||||
|
default. Both tie-breaks are still the web's.
|
||||||
|
- **What is not gated is as load-bearing as what is.** An added link carries no
|
||||||
|
role or feature check — the screen behind it enforces its own — so a section
|
||||||
|
holding one is never emptied by the caller's role, while a section holding only
|
||||||
|
gated rows is dropped rather than drawn as a header over nothing. Both are
|
||||||
|
tested; the second is the case `pruneNav` exists for.
|
||||||
|
- Tests: `NavTreeTest` (27) + 7 new `NavPathsTest` cases. **476 unit tests green**
|
||||||
|
(442 + 34), `lintDebug` and `assembleDebug` clean, no new lint findings.
|
||||||
|
Phase 4's limit still holds — no Robolectric and no `androidTest` source set —
|
||||||
|
so the section header's indent, the hand-off icon, and that a Custom Tab really
|
||||||
|
opens are AC-5's, not the JVM's.
|
||||||
|
|
||||||
### Phase 7, cancelled
|
### Phase 7, cancelled
|
||||||
|
|
||||||
Reading `nav_admin` / `nav_player` is **not scheduled**. The measurement in §6.4
|
Reading `nav_admin` / `nav_player` is **not scheduled**. The measurement in §6.4
|
||||||
|
|||||||
Reference in New Issue
Block a user