Merge pull request 'docs(website): theming phases 6-8 as built, and phase 9 cancelled' (#107) from docs/theming-nav-phase-6-8 into edge

Reviewed-on: #107

website/BACKEND_DESIGN.md

@@ -281,9 +281,25 @@ and [THEMING_AND_NAV.md](THEMING_AND_NAV.md) §2.
 
 Values are `TEXT`, so a JSON-valued key arrives as a **string** and every
 consumer parses it. Server side that is `utils/settingsJson.js`
-(`parseJsonSetting`), client side `parseLayout`/its callers; both treat a
-malformed or wrong-shaped value as **absent** rather than as an error, so a
-hand-edited row degrades to the default instead of rendering something broken.
+(`parseJsonSetting`), client side `client/src/lib/settingsJson.js` and
+`parseLayout`; both treat a malformed or wrong-shaped value as **absent** rather
+than as an error, so a hand-edited row degrades to the default instead of
+rendering something broken.
+
+**The three `nav_*` rows are presentation, never authorization.** An entry is
+keyed by an item's existing `to` and may carry only `label`, `order`, `hidden`
+and — admin nav only — `group`; `utils/navOverrides.js` rejects anything else on
+write, naming the key. It deliberately does **not** check that a `to` exists: the
+base `NAV` arrays are client constants, and duplicating them server-side would
+create a second source of truth for navigation that drifts the first time a route
+is added. `client/src/lib/navOverrides.js` drops an unknown `to` at merge time
+instead, which is also what makes deleting a route in code safe. The merge runs
+*before* the role and shard-feature filters in `SiteHeader.jsx` /
+`AdminLayout.jsx`, which are unchanged and remain the boundary — a stored
+`hidden: false` on a gated item shows nobody anything. `hidden: false` is
+accepted (the editor sends it mid-edit) but never stored, so hiding stays
+subtractive. `hidden` on `/admin/navigation` is dropped for `nav_admin`, because
+that screen is the only UI that can un-hide anything.
 
 **`theme_visual` is resolved server-side, not shipped raw to the browser.**
 `utils/themeResolve.js` layers `:root` ← preset ← custom, field by field, into
@@ -896,7 +912,7 @@ file a route sits in — that is the property the route manifest freezes.
 | POST | `/posts/upload` | multipart image upload (multer) → `{image_url}` for screenshots |
 | GET | `/wiki` · GET `/wiki/:slug` | read incl. unpublished |
 | POST | `/wiki` · PUT `/wiki/:slug` · DELETE `/wiki/:slug` | manage pages |
-| GET | `/settings` · PUT `/settings` | read all / update `{key:value,...}`. Enum-constrained keys are validated on the way in; `theme_visual` additionally has every value checked against the closed sets in `config/themePresets.js` (hex color, shortlisted font stack, bounded px radius, listed shadow) and is stored stringified, and `brand_assets` has every slot checked against `utils/brandAssets.js` — a same-origin path under `/uploads/`, `/brand/` or `/assets/`, never an off-origin or protocol-relative URL, since these values are written straight into the page as an `<img src>` / `<link rel=icon>` / `og:image`. Cleared slots are dropped rather than stored as `null`. A write to `brand_assets` or `theme_visual` invalidates the cached HTML shell. The read path drops bad fields anyway, so the `400` is about **feedback** — a save that appears to succeed and then does nothing is worse than a rejection |
+| GET | `/settings` · PUT `/settings` | read all / update `{key:value,...}`. Enum-constrained keys are validated on the way in; `theme_visual` additionally has every value checked against the closed sets in `config/themePresets.js` (hex color, shortlisted font stack, bounded px radius, listed shadow) and is stored stringified, and `brand_assets` has every slot checked against `utils/brandAssets.js` — a same-origin path under `/uploads/`, `/brand/` or `/assets/`, never an off-origin or protocol-relative URL, since these values are written straight into the page as an `<img src>` / `<link rel=icon>` / `og:image`. Cleared slots are dropped rather than stored as `null`. The three `nav_*` keys go through `utils/navOverrides.js` on the same path — shape only (`label`/`order`/`hidden`/`group` keyed by an app path), since whether a key names a route the nav declares is settled client-side at merge time; without this they would reach the store as `"[object Object]"` and read as absent for ever. A write to `brand_assets` or `theme_visual` invalidates the cached HTML shell (a nav write does not — nav is not in the shell). The read path drops bad fields anyway, so the `400` is about **feedback** — a save that appears to succeed and then does nothing is worse than a rejection |
 | DELETE | `/settings/:key` | reset one setting to its default by deleting the row. Allowlisted to the keys whose default lives outside the store (`theme_visual`, `brand_assets`, `nav_public`, `nav_admin`, `nav_player`, `hero_layout_draft`) — anything else is `400`. Idempotent: resetting a key that was never set succeeds |
 | POST | `/settings/brand-asset/:slot` | upload one brand asset (`logo` · `hero` · `favicon`) **and** point `brand_assets` at it, in one call → `{ url, brand_assets }`. One call rather than "upload, then PUT" so a half-completed save never leaves an unreferenced file in `/uploads`. Uses the shared `imageUpload.js` multer config — the mimetype allowlist is never widened, only tightened per slot: favicons are **PNG only** (§4.10 of [THEMING_AND_NAV.md](THEMING_AND_NAV.md)) and capped at 512 KB, logos at 1 MB, heroes at the shared 8 MB. A refused file is unlinked before the response. Merges into the existing overrides, so uploading a logo never clears a hero. `adminOnly` — tighter than the generic `POST /admin/uploads`, which editors may reach |
 | GET | `/activity?limit=&offset=` | paginated activity log |

website/THEMING_AND_NAV.md

@@ -538,15 +538,16 @@ today until the admin acts.
 | **3 — Theme engine** ✅ | Three presets, the combined Google Fonts link, `SiteContext` extension, and the effective-value resolution in `getPublic().brand` (§4.5) |
 | **4 — Admin theme UI** ✅ | `/admin/appearance` view + route in `App.jsx` + `NAV`/`TITLES` entries in `AdminLayout.jsx` |
 | **5 — Brand assets** ✅ | Cached-shell rewrite in `app.js` (§4.3); upload endpoint on the existing multer config; `<img>` logo slot beside `MoonDot` in the shells; `heroImage` chain extension |
-| **6 — Public nav wiring** | `SiteHeader.jsx` → `nav_public`. Lowest risk of the three: no roles, no groups |
-| **7 — Nav builder UI** | `NavEditor.jsx` with `@dnd-kit` (new dependency), **Public tab only** |
-| **8 — Admin + Player nav** | Wire the remaining two layouts, add the remaining two tabs, once the public pattern is validated in use |
-| **9 — Palette-following literals + Parchment (optional)** | Promote the hue-carrying `rgba()` literals of §4.8 so they follow the palette (the **dark** presets need this too — see "Phases 3–4 as landed"), then the light-mode port with its own contrast pass across every component |
+| **6 — Public nav wiring** ✅ | `SiteHeader.jsx` → `nav_public`. Lowest risk of the three: no roles, no groups |
+| **7 — Nav builder UI** ✅ | `NavEditor.jsx` with `@dnd-kit` (new dependency), **Public tab only** |
+| **8 — Admin + Player nav** ✅ | Wire the remaining two layouts, add the remaining two tabs, once the public pattern is validated in use |
+| **9 — Palette-following literals + Parchment** ❌ **cancelled** | Was: promote the hue-carrying `rgba()` literals of §4.8 so they follow the palette, then the light-mode port. Not scheduled — see "Phase 9, cancelled" below |
 
 Phases 0–2 are one PR pair (website + docs), 3–4 a second, 5 a third, 6–8 a
 fourth. **All four PR pairs target `edge`, not `main`** — the feature reaches
 `main` as one `edge` → `main` merge once every phase is in, so no release ever
-carries a half-wired theme engine.
+carries a half-wired theme engine. Phase 8 is the last one, so that merge is
+what closes the feature.
 
 ### Phases 0–2 as landed
 
@@ -756,6 +757,75 @@ admin panel still has the env name in its tab and its link previews. Fixing it
 would change the served shell for instances with no `brand_assets` row, which is
 exactly what §9 says must not change in this phase. It wants its own change.
 
+### Phases 6–8 as landed
+
+The nav half, wired end to end: the public header, the admin sidebar and the
+player portal all read their override row, and `/admin/navigation` writes them.
+Five things the design did not settle.
+
+**1. The server had no way to store a nav row, and would have stored garbage.**
+§8 described phases 6–8 as client work, and for the *merge* that is right. But
+`updateSettings` validates and stringifies `theme_visual` and `brand_assets` and
+lets everything else through to `settingsDb.set` — so a `nav_public` object would
+have been written as the string `"[object Object]"`, which `parseJsonSetting`
+then reads as absent. The save would have returned 200 and done nothing, for
+ever. `server/src/utils/navOverrides.js` mirrors `utils/brandAssets.js`:
+`validateNavOverrides` is strict on write and names the offending key,
+`resolveNavOverrides` is forgiving and drops fields that would do nothing.
+
+**2. The server cannot check that a `to` exists, and should not try.** The three
+base `NAV` arrays are client constants. Shipping a copy to the server would
+create a second source of truth for navigation that drifts the first time a route
+is added, and it would buy nothing: `applyNavOverrides` already drops an entry
+whose `to` the base array does not declare, which is the right place for it — a
+route deleted in code stops mattering immediately, with no migration. **The
+server validates shape; the client owns membership.** So the write path accepts
+any app-internal path as a key (absolute, no scheme, no `//host`, no whitespace)
+and rejects everything else, and it rejects any field that is not one of the
+four — a `roles` or `to` in the body is a 400, not something quietly stored.
+
+**3. `hidden: false` is accepted and never stored.** The editor sends it while a
+row is being edited, so rejecting it would be hostile; storing it would leave a
+row that reads like an instruction to *force* something visible, which this layer
+must never be able to express. It is dropped on the way in, and hiding stays
+subtractive.
+
+**4. The nav editor cannot be hidden, and that is enforced three times.** An
+admin who hid `/admin/navigation` would lose the only screen that can un-hide it.
+The row's eye toggle is disabled with a note saying why; `resolveNavOverrides`
+drops `hidden` on that one `to` for `nav_admin`; and `AdminLayout` strips it
+again before merging, which is what also covers a row edited straight in the
+database. Typing the URL still works regardless — the guard is about not
+stranding an admin who never learned it.
+
+**5. Orders are written only when something actually moved.** §7.1 says the
+editor writes an order for every item "the way drag-and-drop does", and it does —
+but only for a nav whose sequence differs from the code's. An admin who renames
+one item stores exactly one field, and a route added to `NAV` later still lands
+where the code puts it. The comparison is against the base **restricted to the
+rows that admin can see**, so a role- or feature-gated item missing from their
+palette is not mistaken for a reorder. An override for such an item is carried
+through their save untouched rather than quietly reset.
+
+Two smaller notes. The section dropdown offers "(no section)" only to rows coded
+into an untitled group (Dashboard, Account): for anything else it is a move an
+override cannot express (§6.4 allows an existing titled section or nothing), so
+offering it would silently do nothing. And `useNavOverrides` keeps one
+module-level copy of the two authenticated rows, which is what lets a save in the
+editor update the sidebar the admin is looking at without a reload — and stops
+the second layout to mount from flashing the coded nav first.
+
+### Phase 9, cancelled
+
+The §4.8 `rgba()` literal promotion and the Parchment light-mode port are **not
+scheduled**. The finding that motivated them stands and is worth keeping: those
+literals carry a *hue*, not merely a light/dark assumption — `.btn-ghost` is
+`rgba(11,22,48,0.45)`, so the portal quick-links read blue on Fantasy's warm
+page. It is a real rough edge in the three dark presets, not only a blocker for a
+hypothetical light one. It is simply not worth the contrast pass across every
+component right now. Anyone picking it up should start from the census in §4.8
+and the live observation in "Phases 3–4 as landed".
+
 ### 8.1 Admin builder UI notes
 
 - Tabbed control for the three navs; drag-and-drop reorderable list.