From 252733644b5f8cb7253cfdf23f00c32ef5183173 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Fri, 7 Aug 2026 18:15:50 -0500 Subject: [PATCH 1/5] docs(website): theming & nav phases 0-2 as built Matches RunicGateway/website's phases 0-2 of THEMING_AND_NAV.md. BACKEND_DESIGN.md: - The new /settings router group and its one route, plus why it is a fifth group rather than a route on an existing one. - DELETE /admin/settings/:key in the admin route table, with the allowlist and why reset deletes instead of writing. - The five unseeded theming/nav keys under the settings schema: absence of the row is the "use the default" state, values are TEXT so consumers parse, and malformed reads as absent. - Route count 215 -> 225. THEMING_AND_NAV.md: - Phases 0-2 marked landed, with an "as landed" section recording the three things the design left open: where /settings/nav lives, where parseJsonSetting lives, and the exact 23-declaration radius promotion. - The nav merge util's ordering rules, settled by the implementation: an untouched item keeps its index as its sort key, an explicit order wins a tie against a coincidental index, equal explicit orders keep code order, and `group` is honored only when it names an existing section. - All four PR pairs target `edge`; the feature reaches `main` as one merge. api-route-inventory.json: resynced from server/routes.manifest.json. Picks up the two new routes plus eight that were already missing from the mirror since the Protocol 3.0 cutover (shard clilocs, market, points). Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 47 ++++++++++++++++++++++-- website/THEMING_AND_NAV.md | 62 +++++++++++++++++++++++++++++--- website/api-route-inventory.json | 40 +++++++++++++++++++++ 3 files changed, 143 insertions(+), 6 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 409569c..133ffb2 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -119,6 +119,17 @@ server/ vendors, chars, sales, houses appeals.router.js (4) /player/appeals shard.controller.js + appeals.controller.js + settings/ index.js owns the shared `noindex, requireAuth` gate + (authenticated, ANY role) and the mount table. + A fifth group, for site-wide settings that + need a login but no particular role — /public + is anonymous, /admin/settings is adminOnly + while AdminLayout renders for editors and + moderators, and /player is self-scoped data + nav.router.js (1) /settings/nav — the nav_admin and + nav_player overrides, read by the + layouts that render them + nav.controller.js admin/ index.js mounts the capability routers below at their own prefixes; owns the shared `noindex, isLoggedIn, staffOnly` gate and @@ -151,7 +162,12 @@ server/ email.router.js (6) /admin/email — Gmail OAuth2 delivery — adminOnly discordBot.router.js (2) /admin/discord-bot — adminOnly - settings.router.js (2) /admin/settings — adminOnly + settings.router.js (3) /admin/settings — adminOnly. The + DELETE /:key is "reset to default" + and carries its own key allowlist + (theming/nav keys + the hero draft) + so it can never drop site_mode or + the uo-link config dashboard.router.js (2) GET /dashboard (staff-wide) and PUT /site-mode (adminOnly) — the two singletons owning no path @@ -248,6 +264,21 @@ Seeded keys: `site_mode` (default `maintenance`), `site_mode_changed_at`, `site_mode_changed_by`, `maintenance_message`, `status_message`, `homepage_teaser`, `contact_email` (=UOMysticmoon@gmail.com), `site_title`. +**Deliberately unseeded keys** — the theming & navigation overrides +(`theme_visual`, `brand_assets`, `nav_public`, `nav_admin`, `nav_player`). All +five are JSON strings, and **the absence of the row is the "use the default" +state**: colors/fonts/radii fall back to `theme.css`, assets to `BRAND_*`, navs +to the hardcoded `NAV` arrays. No migration writes defaults into them, because a +stored copy of a default would stop tracking the default. Resetting one is +therefore a `DELETE`, not a write — see `DELETABLE_KEYS` in `settings.model.js` +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. + ### activity_log — append-only | col | type | notes | |---|---|---| @@ -602,7 +633,7 @@ are authoritative, and they answer different questions: | Artifact | Source of truth for | Generated by | |---|---|---| -| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 215 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | +| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 225 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | | `server/swagger/swagger-output.json` — served at `/api/docs` | **What each route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations | The split is deliberate: Swagger is annotation-derived, so an unannotated route is invisible in it and @@ -802,6 +833,17 @@ from the per-route **siteMode** middleware (§5), never from an auth gate. Public content GETs pass through the **siteMode** gate (§5). +### /settings (settings/index.js → §2) — behind `requireAuth` + `noindex`, no role gate + +Site-wide settings that need a login but no particular role. It exists because the +other four groups each answer a different question: `/public` is anonymous, +`/admin/settings` is `adminOnly`, and `/player` is data scoped to `req.user.id`. +These rows are configuration that happens to need a login. + +| Method | Path | Purpose | +|---|---|---| +| GET | `/settings/nav` | `{ nav_admin, nav_player }` — the stored nav overrides as raw JSON strings (or `null`), for the two authenticated layouts that render them. Deliberately not public: an anonymous visitor has no use for either, and the admin nav's labels describe the shape of the admin surface. Open to **any** role because `AdminLayout` renders for editors and moderators and `PlayerPortalLayout` for players, none of whom can read `GET /admin/settings`. Presentation-only — the role/feature filters in those layouts still decide what is shown, and an override can never un-hide a gated item (see [THEMING_AND_NAV.md](THEMING_AND_NAV.md) §7) | + ### /admin (admin/index.js → the capability routers in §2) — all behind `isLoggedIn` + `noindex` + `staffOnly` `admin/index.js` applies the shared gate and mounts each capability router at the prefix it owns; @@ -837,6 +879,7 @@ file a route sits in — that is the property the route manifest freezes. | 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,...}` | +| 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 | | GET | `/activity?limit=&offset=` | paginated activity log | | GET | `/users` · POST `/users` · PUT `/users/:id` · DELETE `/users/:id` | user mgmt (can't delete self / last admin; password hashed on write) | | GET | `/users/:id/trusted-devices` | list a user's active trusted devices (never tokens) | diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index 7454e2c..276f2c6 100644 --- a/website/THEMING_AND_NAV.md +++ b/website/THEMING_AND_NAV.md @@ -479,6 +479,31 @@ function applyNavOverrides(baseNav, overrides) { `overrides` absent → return `baseNav` unchanged. This is the "respect defaults" path and is the single most important case to test. +**As built (Phase 1).** Two shapes are handled by the one function — flat +(`SiteHeader`, `PlayerPortalLayout`) and grouped (`AdminLayout`) — detected by +whether every entry carries an `items` array. Three rules the doc left open, +settled by the implementation and locked by tests: + +- **Ordering.** An item the admin never reordered keeps its index in the base + array as its sort key, so setting one `order` does not scramble the rest. + Explicit and implicit keys therefore share one number line and can collide; + ties break **explicit first** (an admin who said "0" means first, not + "wherever the untouched item at index 0 already sits"), and two explicit + equal orders keep code order via a stable sort. The editor writes an order for + every item in a list the way drag-and-drop does, so ties are the stale-row + case, not the normal one — they just have to resolve predictably. +- **`group`.** Accepted only when it names a title the base nav already + declares; anything else is dropped, so an item can never land under a header + that does not exist. Group *order* is not overridable — sections stay in code + order, only membership and within-group order move. +- **Field-by-field validation.** A bad `label` does not discard a good `order` + beside it, and `hidden` is honored only as the literal boolean `true`. + Everything unrecognized is ignored rather than rejected, so a hand-edited row + degrades to the code default instead of rendering a broken nav. + +`hidden: false` cannot un-hide anything: hiding here is subtractive only, and +the role/feature filters still run afterward, unchanged. + ## 8. Build phases Each phase is independently shippable and leaves the site rendering identically to @@ -486,9 +511,9 @@ today until the admin acts. | Phase | Work | |---|---| -| **0 — Settings-store groundwork** | `settingsDb.remove()`; `DELETE /admin/settings/:key` with key allowlist; `GET /settings/nav` (§4.2); `parseJsonSetting()` helper; register the five keys; three into `PUBLIC_KEYS`. Swagger + route-manifest regen | -| **1 — `navOverrides.js` + tests** | The pure merge util, unit-tested in isolation. **The one piece with real correctness risk** | -| **2 — Radius/shadow token groundwork** | Promote the literals in `theme.css` to the four tokens of §4.7, values unchanged. Verify zero visual diff before any admin UI exists | +| **0 — Settings-store groundwork** ✅ | `settingsDb.remove()`; `DELETE /admin/settings/:key` with key allowlist; `GET /settings/nav` (§4.2); `parseJsonSetting()` helper; register the five keys; three into `PUBLIC_KEYS`. Swagger + route-manifest regen | +| **1 — `navOverrides.js` + tests** ✅ | The pure merge util, unit-tested in isolation. **The one piece with real correctness risk** | +| **2 — Radius/shadow token groundwork** ✅ | Promote the literals in `theme.css` to the four tokens of §4.7, values unchanged. Verify zero visual diff before any admin UI exists | | **3 — Theme engine** | Three preset blocks, 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; `` logo slot beside `MoonDot` in the three shells; `heroImage` chain extension | @@ -498,7 +523,36 @@ today until the admin acts. | **9 — Parchment (optional)** | Light-mode port per §4.8 — its own contrast pass across every component | Phases 0–2 are one PR pair (website + docs), 3–4 a second, 5 a third, 6–8 a -fourth. +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. + +### Phases 0–2 as landed + +- **`/api/v1/settings` is a fifth router group**, not a route bolted onto an + existing one. §4.2 named the URL but not where it lives, and the domain split + leaves no group it fits: `/public` is anonymous, `/admin/settings` is + `adminOnly` while `AdminLayout` renders for editors and moderators, and + `/player` is data scoped to `req.user.id`. The group carries + `noindex, requireAuth` and no role gate. The route-manifest guard test that + asserts every `/admin/**` and `/player/**` route sits behind `requireAuth` now + covers `/settings/**` too. +- **Reset is `DELETE /api/v1/admin/settings/:key`** with the allowlist in + `settings.model.js` (`DELETABLE_KEYS`), which is what stops a stray request + from dropping `site_mode` or the uo-link config. It is admin-only and + idempotent, and a test asserts it never writes a row. +- **`parseJsonSetting` lives at `server/src/utils/settingsJson.js`.** Non-object + JSON (`4`, `"x"`, `null`, `[]`) is treated as absent alongside syntax errors, + and a validator rejection discards the whole object rather than half-applying + it. The client keeps `parseLayout`; a client-side counterpart arrives with its + first consumer in Phase 3. +- **Phase 2 was a 23-declaration promotion** — 14×`8px` → `--radius-input`, + 4×`999px` → `--radius-pill`, 4×`10px` → `--radius-card`, 1×`12px` → + `--radius-panel` — matching the §4.7 census exactly. The `7px`/`6px` editor + chrome and the two `50%` circles stay literal. `--shadow-card` and + `--panel-grad` were **already** tokens and already derived, so the shadow half + of the phase was a no-op; the only two `box-shadow` declarations in + `theme.css` both already read `var(--shadow-card)`. ### 8.1 Admin builder UI notes diff --git a/website/api-route-inventory.json b/website/api-route-inventory.json index f42fcb4..ad7d5d3 100644 --- a/website/api-route-inventory.json +++ b/website/api-route-inventory.json @@ -249,6 +249,10 @@ "method": "PUT", "path": "/api/v1/admin/settings" }, + { + "method": "DELETE", + "path": "/api/v1/admin/settings/:key" + }, { "method": "POST", "path": "/api/v1/admin/shard/account" @@ -293,6 +297,18 @@ "method": "GET", "path": "/api/v1/admin/shard/char/:serial" }, + { + "method": "GET", + "path": "/api/v1/admin/shard/clilocs" + }, + { + "method": "POST", + "path": "/api/v1/admin/shard/clilocs/import" + }, + { + "method": "PUT", + "path": "/api/v1/admin/shard/clilocs/path" + }, { "method": "GET", "path": "/api/v1/admin/shard/houses" @@ -817,10 +833,30 @@ "method": "GET", "path": "/api/v1/public/shard/idoc" }, + { + "method": "GET", + "path": "/api/v1/public/shard/market" + }, + { + "method": "GET", + "path": "/api/v1/public/shard/market/meta" + }, + { + "method": "GET", + "path": "/api/v1/public/shard/market/vendors/:serial" + }, { "method": "GET", "path": "/api/v1/public/shard/online" }, + { + "method": "GET", + "path": "/api/v1/public/shard/points" + }, + { + "method": "GET", + "path": "/api/v1/public/shard/points/:system" + }, { "method": "GET", "path": "/api/v1/public/shard/presence" @@ -860,6 +896,10 @@ { "method": "GET", "path": "/api/v1/public/wiki/tags" + }, + { + "method": "GET", + "path": "/api/v1/settings/nav" } ], "internal": [ -- 2.49.1 From e669243aca7f61046426c6cb669b73d4af1e931c Mon Sep 17 00:00:00 2001 From: wtclaude Date: Fri, 7 Aug 2026 19:16:37 -0500 Subject: [PATCH 2/5] docs(website): theming phases 3-4 as built Records where the build diverged from the design and why. - The presets do not live in theme.css as [data-theme] blocks. Section 6.2 is marked superseded and a "Phases 3-4 as landed" section explains the inline --accent precedence problem that forced server-side resolution. - Section 4.5's accentInt fix is struck through: getPublic() never exposed accentInt, and Discord embeds are colored by a separate process reading env, so there was nothing per-request to recompute. Replaced with what was actually done -- the bot fetching the effective accent. - Phase 9 re-scoped. Applying Fantasy on a live instance showed the section 4.8 rgba literals carry a hue, not just a light/dark assumption, so the dark presets need that promotion too. - BACKEND_DESIGN.md: the new /settings/theme/options route, the `theme` block on /public/settings, effective values in the brand block, theme_visual validation on PUT /admin/settings, route count 225 -> 226. - api-route-inventory.json regenerated from the manifest. Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 23 +++++- website/THEMING_AND_NAV.md | 130 ++++++++++++++++++++++++++++--- website/api-route-inventory.json | 4 + 3 files changed, 144 insertions(+), 13 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 133ffb2..c4e2143 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -129,7 +129,10 @@ server/ nav.router.js (1) /settings/nav — the nav_admin and nav_player overrides, read by the layouts that render them - nav.controller.js + theme.router.js (1) /settings/theme/options — the closed + sets the admin appearance form is + built from. Static; no DB read + nav.controller.js + theme.controller.js admin/ index.js mounts the capability routers below at their own prefixes; owns the shared `noindex, isLoggedIn, staffOnly` gate and @@ -279,6 +282,17 @@ consumer parses it. Server side that is `utils/settingsJson.js` 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. +**`theme_visual` is resolved server-side, not shipped raw to the browser.** +`utils/themeResolve.js` layers `:root` ← preset ← custom, field by field, into +the CSS custom properties `getPublic()` returns as `theme`; the SPA's only job +is to write them onto `` and take back what it wrote last time +(`client/src/lib/themeVars.js`). One authority for the merge means the effective +accent in `brand.accent` — the cross-repo contract the Android app and the +Discord bot theme themselves from — always agrees with what the website paints. +Values reaching a CSS variable are checked against closed sets on both paths: +strictly on write (400, naming the field) and forgivingly on read (drop the bad +field, keep its neighbours). + ### activity_log — append-only | col | type | notes | |---|---|---| @@ -633,7 +647,7 @@ are authoritative, and they answer different questions: | Artifact | Source of truth for | Generated by | |---|---|---| -| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 225 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | +| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 226 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | | `server/swagger/swagger-output.json` — served at `/api/docs` | **What each route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations | The split is deliberate: Swagger is annotation-derived, so an unannotated route is invisible in it and @@ -809,7 +823,7 @@ from the per-route **siteMode** middleware (§5), never from an auth gate. | Method | Path | Notes | |---|---|---| -| GET | `/settings` | whitelisted public keys, derived `registration`/`gameAccountSignup` flags, the per-shard **`brand`** block (name, `accent` color, logo/hero/favicon) a client themes itself from — one image runs as any shard, asset fields may be site-relative paths (resolve against the base URL) — and a **`push`** block `{ ntfyUrl }` (M7): the client-facing ntfy relay URL the app's embedded distributor registers its device topic against, from `NTFY_PUBLIC_URL` / first `NTFY_ALLOWED_ORIGINS` (never the internal `NTFY_BASE_URL`); `null` when push isn't configured for the shard. | +| GET | `/settings` | whitelisted public keys, derived `registration`/`gameAccountSignup` flags, the per-shard **`brand`** block (name, `accent` color, logo/hero/favicon) a client themes itself from — one image runs as any shard, asset fields may be site-relative paths (resolve against the base URL); these are **effective** values, so an admin theme (`theme_visual`) beats `BRAND_ACCENT_COLOR` and an uploaded `brand_assets` asset beats its `BRAND_*` path — an optional **`theme`** block, the resolved CSS custom properties for that admin theme (absent when the instance was never themed, which is what makes it render from the shipped stylesheet unchanged) — and a **`push`** block `{ ntfyUrl }` (M7): the client-facing ntfy relay URL the app's embedded distributor registers its device topic against, from `NTFY_PUBLIC_URL` / first `NTFY_ALLOWED_ORIGINS` (never the internal `NTFY_BASE_URL`); `null` when push isn't configured for the shard. | | GET | `/status` | status message + current mode, **plus a `version` block** (`{ service:'runic-gateway', api, server }`) so a client first-run probe recognizes the backend and can run a version-mismatch guard | | GET | `/version` | lightweight, **DB-free** backend identity/version (`{ service, api, server }`) — the canonical target for the version guard and a cheap liveness check | | GET | `/posts/:category` | published only; `category` ∈ news\|five-on-friday\|newsletter\|screenshots | @@ -843,6 +857,7 @@ These rows are configuration that happens to need a login. | Method | Path | Purpose | |---|---|---| | GET | `/settings/nav` | `{ nav_admin, nav_player }` — the stored nav overrides as raw JSON strings (or `null`), for the two authenticated layouts that render them. Deliberately not public: an anonymous visitor has no use for either, and the admin nav's labels describe the shape of the admin surface. Open to **any** role because `AdminLayout` renders for editors and moderators and `PlayerPortalLayout` for players, none of whom can read `GET /admin/settings`. Presentation-only — the role/feature filters in those layouts still decide what is shown, and an override can never un-hide a gated item (see [THEMING_AND_NAV.md](THEMING_AND_NAV.md) §7) | +| GET | `/settings/theme/options` | The closed sets an admin may pick from when theming the site: the presets (each with its **full token map**, so a form can show what an unset field currently resolves to), the curated Google Fonts shortlist per role, the shadow depths, the editable color/radius field names paired with the CSS variable each drives, and `shippedTokens` (what `theme.css`'s `:root` declares). Static — derived from `config/themePresets.js`, no DB read. Served rather than duplicated in client code so the options the form **offers** can never drift from the ones `PUT /admin/settings` **accepts** | ### /admin (admin/index.js → the capability routers in §2) — all behind `isLoggedIn` + `noindex` + `staffOnly` @@ -878,7 +893,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,...}` | +| 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. 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 | | GET | `/activity?limit=&offset=` | paginated activity log | | GET | `/users` · POST `/users` · PUT `/users/:id` · DELETE `/users/:id` | user mgmt (can't delete self / last admin; password hashed on write) | diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index 276f2c6..4993625 100644 --- a/website/THEMING_AND_NAV.md +++ b/website/THEMING_AND_NAV.md @@ -50,6 +50,7 @@ Every new setting is an *override layer*, never a replacement: | # | Decision | |---|---| | Brand contract | **`getPublic().brand` returns effective values** (override → env). The Android app and Discord embeds track admin theming for free — see §4.5 | +| Theme delivery | **The server resolves the whole effective token set** and the client writes it as CSS custom properties. No `[data-theme]` blocks — see §6.2 | | Structural tokens | **Radius + shadow depth only.** `spacingUnit` and `borderWeight` are **cut**, not deferred — see §4.6 | | Radius token values | **Seeded at today's real values** (four tokens, not three), so the promotion step is a true no-op — see §4.7 | | Presets in v1 | **Three dark presets** — Runic Gateway, Modern, Fantasy. Parchment (light) is Phase 9 — see §4.8 | @@ -156,8 +157,19 @@ The web client needs **no change** for this — its existing ([`SiteContext.jsx:30-32`](../../website/client/src/contexts/SiteContext.jsx)) simply receives a better value. Consequences to handle: -- `brand.accentInt` must be **recomputed from the effective accent** per request - rather than read from the boot-time constant, or Discord embeds drift. +- ~~`brand.accentInt` must be **recomputed from the effective accent** per + request rather than read from the boot-time constant, or Discord embeds + drift.~~ **Corrected in Phase 3 — this fix as written was a no-op.** + `getPublic().brand` never exposes `accentInt` (`publicBrand.test.js` asserts + it is `undefined`, deliberately: it is a Discord-only integer form), and the + server-side `brand.accentInt` has no consumer at all. Discord embeds are + colored by **`bot/src/brand.js`, in a separate process**, reading + `BRAND_ACCENT_COLOR` from env at boot — so there was nothing per-request to + recompute, and the drift the note describes was real but unfixable from the + server. What Phase 3 actually did: the bot now fetches + `GET /public/settings` → `brand.accent` (it already has a public-API client) + behind a 10-minute cached getter, keeping env as the fallback. See + "Phases 3–4 as landed" below. - `publicBrand.test.js` gains cases: no rows → env values unchanged (the existing assertions must still pass verbatim); `theme_visual` accent set → effective accent returned; `brand_assets.favicon` set → favicon overridden while `logo` @@ -362,10 +374,19 @@ null a field out to "clear" it** — remove it from the object. ### 6.2 Preset blocks -`:root` (no `data-theme` attribute set at all) stays the **Runic Gateway** default -— today's actual values — so an instance with no `theme_visual` row renders -exactly as it does now. `runic-gateway` is *also* declared as a named preset so -that switching back to it after trying another is the same code path. +> **Superseded in Phase 3.** The presets below are correct as *values* and were +> built as specified, but they do **not** live in `theme.css` as `[data-theme]` +> blocks. They live in `server/src/config/themePresets.js`, and the server +> resolves the effective token set into `getPublic().theme` for the client to +> write onto ``. See "Phases 3–4 as landed" for why, and note two +> corrections the build made to the palettes: each preset carries the **full** +> color set (fifteen tokens, not the eight below), and `--shadow-card` is +> themed alongside the radii. + +`:root` stays the **Runic Gateway** default — today's actual values — so an +instance with no `theme_visual` row renders exactly as it does now. +`runic-gateway` is *also* declared as a named preset so that switching back to +it after trying another is the same code path. ```css [data-theme="runic-gateway"] { @@ -514,13 +535,13 @@ today until the admin acts. | **0 — Settings-store groundwork** ✅ | `settingsDb.remove()`; `DELETE /admin/settings/:key` with key allowlist; `GET /settings/nav` (§4.2); `parseJsonSetting()` helper; register the five keys; three into `PUBLIC_KEYS`. Swagger + route-manifest regen | | **1 — `navOverrides.js` + tests** ✅ | The pure merge util, unit-tested in isolation. **The one piece with real correctness risk** | | **2 — Radius/shadow token groundwork** ✅ | Promote the literals in `theme.css` to the four tokens of §4.7, values unchanged. Verify zero visual diff before any admin UI exists | -| **3 — Theme engine** | Three preset blocks, 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` | +| **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; `` logo slot beside `MoonDot` in the three 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 — Parchment (optional)** | Light-mode port per §4.8 — its own contrast pass across every component | +| **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 | 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 @@ -554,6 +575,97 @@ carries a half-wired theme engine. of the phase was a no-op; the only two `box-shadow` declarations in `theme.css` both already read `var(--shadow-card)`. +### Phases 3–4 as landed + +Four things the design settled differently once it met the code. + +**1. The server resolves the whole token set; there are no `[data-theme]` +blocks.** §6.2 put the presets in `theme.css` and had the client set a +`data-theme` attribute. That does not work as written: `SiteContext` writes +`--accent` as an **inline style on ``** (`SiteContext.jsx:31`), and an +inline property beats any attribute-selector block. An admin who picked Fantasy +without also setting a custom accent would have had Fantasy's `#c9973f` painted +over by `brand.accent` from env — and §4.5's whole point is that +`getPublic().brand.accent` is what the phone app themes itself from, so the two +surfaces would have disagreed about the accent while both being "right". + +The fix removes the conflict rather than sequencing around it. Presets live in +`server/src/config/themePresets.js`; `server/src/utils/themeResolve.js` layers +`:root` ← preset ← custom **per field** into a token map; `getPublic()` returns +it as `theme`; `client/src/lib/themeVars.js` writes it onto ``. One +authority for the merge, `brand.accent` is by construction the accent the site +actually paints, and `theme.css`'s `:root` is untouched — an instance with no +row gets no `theme` block, the client writes nothing, and the page renders +byte-for-byte as today. + +The client half's real logic is *removal*: inline properties are not cleared by +writing a smaller object over them, so `applyThemeTokens` tracks what it set +last time and `removeProperty`s whatever the new payload no longer mentions. +Without that, "Reset to defaults" would look broken until a reload. + +**2. Presets carry the full fifteen-token palette, and theme `--shadow-card`.** +§6.2's blocks set eight colors. Applied literally, Fantasy's warm brown page +would have kept `--line: #2a3544` and `--blue: #13243c` — dark blue-grey borders +and a blue-grey active nav row — because those tokens are not in the list. +Every preset now sets `--panel-flat`, `--line`, `--line-soft`, `--head`, +`--muted`, `--dim` and `--blue` as well. The admin *form* still exposes only +§6.1's eight; the rest are supporting shades a preset gets right coherently but +that are not worth hand-picking. `--mode-live` / `--mode-maint` stay fixed +across every preset (green means live) and `--panel-grad` stays derived, both +locked by tests. + +**3. The option catalog is served, not duplicated.** +`GET /api/v1/settings/theme/options` returns the presets (with their full token +maps, so a control can show what an unset field currently resolves to), the font +shortlist, the shadow depths, and the editable field names paired with the CSS +variable each drives. Duplicating those lists in client code would mean the form +could offer a font the server rejects, which surfaces as a save 400ing for no +visible reason. A test asserts every offered option validates. + +Validation is deliberately asymmetric: **strict on write** (`PUT +/admin/settings` 400s and names the offending field) and **forgiving on read** +(a bad field is dropped, its neighbours keep applying). Strict-on-write gives +feedback; forgiving-on-read means a row hand-edited in the DB degrades to the +shipped default instead of rendering a broken site. + +One addition to §5.1's twelve font options: **Georgia in the serif list.** The +shortlist gave the sans role a "today's default" option (Arial, byte-identical +to `--sans`) but left serif with no way back to `Georgia, "Times New Roman", +serif` short of resetting the whole theme. It pulls in no web family, so §5.2's +combined URL is unchanged. + +**4. The Discord bot fetches the accent; §4.5's `accentInt` note was a no-op.** +See the correction in §4.5. `bot/src/brand.js` now reads +`GET /public/settings` → `brand.accent` through the public-API client it already +had, behind getters with a 10-minute TTL — so `brand.accentInt` stays a plain +property read at every existing call site, an embed never awaits a network call, +and any failure (site down, maintenance, malformed body) keeps the last known +good value with `BRAND_ACCENT_COLOR` as the floor. + +**Found while smoke-testing: §4.8's rgba literals are not only a light-mode +problem.** The 28 dark-assuming `rgba()` literals were scoped to Phase 9 on the +reasoning that they break a *light* preset. Applying **Fantasy** on a live +instance shows they also carry a **hue**: `.btn-ghost`'s +`background: rgba(11, 22, 48, 0.45)` (essentially `--blue` at 45%) leaves the +portal's quick-link buttons reading blue on a warm brown page, and the hero +overlay stack in `heroLayout.js` is `rgba(11,15,20,…)` regardless of preset. +Nothing is broken or unreadable — it is a visible seam, not a bug — but Phase 9 +should be re-scoped from "light-mode port" to "make the hue-carrying literals +follow the palette", which the dark presets need too. Not fixed here: it is the +23-declaration-style promotion Phase 2 was, and folding it into the phase that +introduced the presets would have hidden it inside an unrelated diff. + +**Still open, by decision:** the theme arrives with the `/public/settings` fetch, +so a themed instance paints the shipped palette for one frame before repainting. +Phase 5 has to rewrite `renderIndexHtml` into a cached, invalidated shell anyway +(§4.3) — injecting a `` block, last +in `` so it follows the built stylesheet and wins the equal-specificity tie. It exists only to +stop a themed instance painting the shipped palette for one frame; `SiteContext` removes it once the +`/public/settings` payload has arrived and applied — gated on a **successful** fetch, since dropping +it after a failed one would strip a themed instance back to the shipped colors. Token names and +values are re-checked against conservative patterns on the way into the block: everything there comes +from a closed set already, and this keeps that a property of the HTML writer rather than of a +validator three modules away. + --- ## 5. Site mode (LIVE / MAINTENANCE) diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index 4993625..24e53f6 100644 --- a/website/THEMING_AND_NAV.md +++ b/website/THEMING_AND_NAV.md @@ -537,7 +537,7 @@ today until the admin acts. | **2 — Radius/shadow token groundwork** ✅ | Promote the literals in `theme.css` to the four tokens of §4.7, values unchanged. Verify zero visual diff before any admin UI exists | | **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; `` logo slot beside `MoonDot` in the three shells; `heroImage` chain extension | +| **5 — Brand assets** ✅ | Cached-shell rewrite in `app.js` (§4.3); upload endpoint on the existing multer config; `` 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 | @@ -655,17 +655,107 @@ follow the palette", which the dark presets need too. Not fixed here: it is the 23-declaration-style promotion Phase 2 was, and folding it into the phase that introduced the presets would have hidden it inside an unrelated diff. -**Still open, by decision:** the theme arrives with the `/public/settings` fetch, -so a themed instance paints the shipped palette for one frame before repainting. -Phase 5 has to rewrite `renderIndexHtml` into a cached, invalidated shell anyway -(§4.3) — injecting a ``, injected last in +`` so it follows the built stylesheet and wins the equal-specificity tie. +`SiteContext` removes that block once the `/public/settings` payload has arrived +and been applied — otherwise a later reset would remove the inline properties +only to reveal the stale block underneath. The removal is gated on a +**successful** fetch, not merely a finished one: a failed request leaves the app +with no theme at all, and dropping the block then would strip a themed instance +back to the shipped palette for no reason. + +**The logo went into all six MoonDot surfaces, not three.** §8 named the three +persistent shells (site header, admin sidebar, portal sidebar); the admin login, +the player login/register card and the maintenance page carry the same mark and +an operator who uploads a logo means their instance, not three of its pages. +`components/BrandLogo.jsx` renders **nothing** when `brand.logo` is empty — which +is the shipped default — so every one of those surfaces is unchanged on an +untouched instance. On the three centered layouts the logo is stacked *above* the +moon rather than beside it, because turning that block into a flex row would have +changed its height on instances with no logo. + +The footer's "powered by Runic Gateway" emblem is deliberately untouched (§4.11): +it is the project's badge, not the instance's. + +**The hero chain needed no code.** §4.9's real order — +`hero_layout.background.image_url` → `brand_assets.hero` → `BRAND_HERO` → +`/assets/img/runic-emblem.png` — already holds, because Phase 3 resolved +`brand_assets` into `getPublic().brand.hero` and `SiteContext.heroImage` reads +that. What was missing was saying so: the hero row in the admin panel now states +that a hero-editor background wins over the uploaded one, so "I uploaded a hero +and the portal ignored it" does not become a bug report against a working system. + +**Observed and left alone:** the shell's `` and description still come +from `BRAND_NAME`/`BRAND_DESCRIPTION`, not from the admin-set `site_title` that +`getPublic().brand.name` prefers, so an instance that renamed itself through the +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. + ### 8.1 Admin builder UI notes - Tabbed control for the three navs; drag-and-drop reorderable list. diff --git a/website/api-route-inventory.json b/website/api-route-inventory.json index f91067f..3e97f78 100644 --- a/website/api-route-inventory.json +++ b/website/api-route-inventory.json @@ -253,6 +253,10 @@ "method": "DELETE", "path": "/api/v1/admin/settings/:key" }, + { + "method": "POST", + "path": "/api/v1/admin/settings/brand-asset/:slot" + }, { "method": "POST", "path": "/api/v1/admin/shard/account" -- 2.49.1 From 5c24ff137802c7152c32d721c5bf45ede7c3ef14 Mon Sep 17 00:00:00 2001 From: wtclaude <claude@whitlocktech.net> Date: Sat, 8 Aug 2026 00:02:46 -0500 Subject: [PATCH 4/5] docs(website): theming phases 6-8 as built, and phase 9 cancelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marks the nav wiring and the builder UI landed, and records the five places the build differed from the design: - The server had no way to store a nav row. The design scoped 6-8 as client work, but updateSettings would have written a nav object as "[object Object]" — a save that 200s and does nothing, for ever. - The server deliberately cannot check that a `to` exists: the base NAV arrays are client constants, and a server-side copy would be a second source of truth for navigation. Shape is the server's question, membership the client's. - `hidden: false` is accepted and never stored, so hiding stays subtractive. - The nav editor cannot be hidden, enforced in three places. - Orders are written only when something actually moved, compared against the base restricted to the rows the editing admin can see. Phase 9 (hue-carrying rgba literals + Parchment) is cancelled rather than deferred. The finding that motivated it is kept as the record: those literals carry a hue, so they are a rough edge in the three dark presets and not only a blocker for a hypothetical light one. Co-Authored-By: Claude <noreply@anthropic.com> --- website/BACKEND_DESIGN.md | 24 ++++++++++-- website/THEMING_AND_NAV.md | 80 +++++++++++++++++++++++++++++++++++--- 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 676f71f..3b1654f 100644 --- a/website/BACKEND_DESIGN.md +++ b/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 | diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index 24e53f6..d782c78 100644 --- a/website/THEMING_AND_NAV.md +++ b/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. -- 2.49.1 From 2784cad6e401638534c9d038e3a46901b6a4d9c6 Mon Sep 17 00:00:00 2001 From: wtclaude <claude@whitlocktech.net> Date: Sat, 8 Aug 2026 00:42:39 -0500 Subject: [PATCH 5/5] =?UTF-8?q?docs(website):=20phase=2010=20=E2=80=94=20p?= =?UTF-8?q?ublic=20nav=20sections=20and=20added=20links,=20and=20the=20?= =?UTF-8?q?=C2=A77=20amendment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the capability asked for before the edge -> main cutover: dropdown sections in the public header, with the coded entries organised into them and admin-authored links alongside. §7 is amended rather than quietly contradicted. It said the override layer "cannot introduce a `to` that is not already in the hardcoded NAV array"; that remains true of every CODED entry, and the restated constraint spells out what an added link may be — a same-origin path, carrying no gate of its own, advertising a route rather than granting one — plus why the property is structural: coded entries live in a map keyed by routes the base array declares, and everything that can name an arbitrary path lives in `links`, where the rule is applied. §6.4 gains the { items, sections, links } wrapper, including the two properties worth knowing: a bare map still reads as the items map, and a nav with no sections still stores one. §9 gains three acceptance criteria — the empty dropdown does not render, an added link cannot leave the origin, and deleting a section returns its entries to the top level rather than removing them. "Phase 10 as landed" records why the Public tab needed its own tree editor, why moving between containers stayed a dropdown rather than a cross-container drag, why the menu opens on click and its trigger is not a link, and the palette-vs-full-nav bug this surfaced in the phase 6-8 save path. Co-Authored-By: Claude <noreply@anthropic.com> --- website/BACKEND_DESIGN.md | 13 ++++ website/THEMING_AND_NAV.md | 132 ++++++++++++++++++++++++++++++++++--- 2 files changed, 137 insertions(+), 8 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 3b1654f..7328a98 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -301,6 +301,19 @@ 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. +**`nav_public` may also carry dropdown sections and admin-authored links**, as +`{ items, sections, links }` — a bare map still reads as `items`, and a nav with +no sections still stores one. A **section** has a label and a position and no +route at all: it only opens, so it adds no reachable surface. A **link** is the +one place a path may be named that the code does not declare, and is therefore +the one place the path rule applies: same-origin only, no scheme and no +protocol-relative `//host`. A link carries no gate of its own and needs none — +the page behind it enforces its own access, so an added link advertises a route +and never grants one. Coded entries stay in `items`, keyed by a route the base +array must declare, which is what keeps "an override cannot introduce a route" +structurally true. Sections and links are dropped for `nav_admin` / `nav_player`, +whose layouts cannot render them. + **`theme_visual` is resolved server-side, not shipped raw to the browser.** `utils/themeResolve.js` layers `:root` ← preset ← custom, field by field, into the CSS custom properties `getPublic()` returns as `theme`; the SPA's only job diff --git a/website/THEMING_AND_NAV.md b/website/THEMING_AND_NAV.md index d782c78..3611d54 100644 --- a/website/THEMING_AND_NAV.md +++ b/website/THEMING_AND_NAV.md @@ -455,27 +455,86 @@ Any field absent for a given `to` falls back to the code default — label from honored**, so removing a route in code can never leave a dangling override that does something unexpected. +**`nav_public` may also be a wrapper** (Phase 10), because the public header is +the one nav an admin can restructure rather than only reorder: + +```json +{ + "items": { "/site/champs": { "order": 0, "section": "sec_a1b2" } }, + "sections": [ { "id": "sec_a1b2", "label": "The World", "order": 4 } ], + "links": [ { "id": "lnk_c3d4", "label": "Player Guide", + "to": "/wiki/new-player-guide", "order": 1, "section": "sec_a1b2" } ] +} +``` + +- **A bare map is still read as the items map.** Every item key is a path + starting with `/`, so it can never collide with the literal key `items` — the + detection is unambiguous, and a nav with no sections still *stores* the bare + map, so this feature changed nothing for one that does not use it. +- `nav_admin` / `nav_player` keep the bare map; `sections` and `links` are + dropped for them, since neither layout can render an admin-created section. +- Top-level order is one number line shared by ungrouped entries **and + sections**; within a section, by its members. An admin-created entity with no + stored order appends after the coded ones rather than jumping to the front. +- **One level only.** No menu inside a menu. +- An `items[].section` or `links[].section` naming no declared section falls back + to the top level, mirroring the "group must name an existing title" rule. + ## 7. Navigation: hard constraint -The override system can **only** affect `label`, `order`, `hidden`, and — admin -nav only — `group` (which *existing* titled section an item sits under). +> **Amended in Phase 10.** This section originally said the override layer +> "cannot introduce a `to` that is not already in the corresponding hardcoded +> `NAV` array". That is still true of every **coded** entry, but the public +> header now also lets an admin add links of their own, so the constraint is +> restated below in the narrower form that survives. Nothing about the *gates* +> changed. + +The override system can affect a **coded** entry's `label`, `order`, `hidden`, +and which container it sits in — `group` on the admin nav (an *existing* titled +section) or `section` on the public header (an admin-created dropdown). It **cannot**: -- introduce a `to` that is not already in the corresponding hardcoded `NAV` array; -- change or remove an item's `roles` (admin nav) or `feature` (public nav) gate; -- un-hide an item for a viewer whose role or feature check would otherwise fail. +- change a coded entry's `to`, or introduce a new one in its place; +- change or remove an entry's `roles` (admin nav) or `feature` (public nav) gate; +- un-hide an entry for a viewer whose role or feature check would otherwise fail. + +**The public header may additionally carry admin-created `sections` and +admin-authored `links`** (§6.4, §7.2). This is a genuine widening and is worth +stating plainly: + +- A **section** is a container with a label and a position. It has no `to` and is + never itself a link — it only opens — so it adds no reachable surface at all. +- A **link** is the one thing an admin may add to a nav, and the only place a path + is not required to already exist in code. It is restricted to a **same-origin + path**: no scheme, no protocol-relative `//host`, no whitespace or quotes. The + nav is not a place to send visitors to an origin the operator does not control. +- A link carries **no `roles` or `feature` of its own, and needs none**: the page + behind it enforces its own access, so a link to somewhere the viewer cannot + reach behaves exactly as typing that address would. Adding a link advertises a + route; it never grants one. + +The property this rests on is structural rather than a check someone has to +remember: coded entries live in an `items` map whose keys **must** be routes the +base array declares, so that map can never introduce a route, while everything +that *can* name an arbitrary path lives in `links`, where the path rule is +applied on both the write and the read path. The existing filters in -[`SiteHeader.jsx:43`](../../website/client/src/components/SiteHeader.jsx) and -[`AdminLayout.jsx:155-164`](../../website/client/src/routes/admin/AdminLayout.jsx) +[`SiteHeader.jsx`](../../website/client/src/components/SiteHeader.jsx) and +[`AdminLayout.jsx`](../../website/client/src/routes/admin/AdminLayout.jsx) run **after** the override merge, unchanged, and remain the actual security boundary. The override layer is presentation-only. This is the same "server-enforced gate, client-side is only about not advertising a dead end" principle already documented in `SiteHeader.jsx`'s comments, and this feature must not weaken it. -Two existing behaviors the merge must not disturb: +Three existing behaviors the merge must not disturb: + +- **Empty dropdowns.** A section whose every entry is filtered out by a shard + feature must not render at all — a menu that opens onto nothing is worse than + no menu. `pruneNav` applies the gate inside a section and then drops one it + leaves empty. - **Moderator confinement.** `AdminLayout` restricts moderators to `MOD_PATHS` and redirects them out of anything else. Overrides apply before that filter, so a @@ -542,6 +601,7 @@ today until the admin acts. | **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 | +| **10 — Public nav sections + added links** ✅ | Admin-created dropdown sections in the public header, coded entries organised into them, and admin-authored same-origin links. `NavDropdown.jsx`, `buildPublicNav`/`pruneNav`, the `nav_public` wrapper of §6.4, and the Public tab's own tree editor. **Amends §7** | 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 @@ -815,6 +875,54 @@ module-level copy of the two authenticated rows, which is what lets a save in th 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 10 as landed + +Asked for after phases 6–8 were built and before the `edge` → `main` cutover: +the public site should support dropdown sections with links organised inside +them. Scoped to the **public header only** — the admin sidebar keeps its four +coded sections and the player portal its three flat rows — and to **same-origin +links**, which is what makes §7's amendment a narrowing rather than an opening. + +**The shape change was free because nothing had shipped.** `nav_public` grew a +`{items, sections, links}` wrapper. Had this landed after the cutover it would +have needed a migration or a version field; before it, a forgiving read of the +bare map is enough, and that read is kept anyway as insurance for a row written +during review. + +**Sections are entries in the top-level order, which is why the Public tab has +its own editor.** The admin sidebar's groups are a fixed frame the code declares: +only membership moves. A public section is something the admin created and can +drag among the pills. That is a tree, not a list of groups, so +`PublicNavTree.jsx` renders it with a nested `SortableContext` per section, while +the other two tabs keep the phase-7 grouped editor. The shared `Row` was +generalised — its destination `<select>` takes a list of choices instead of +knowing about admin group titles. + +**Moving between containers is still the dropdown, not a drag**, exactly as on +the Admin tab. Cross-container dragging is a lot of interaction surface for +something an admin does once, and keeping every drag a simple reorder is what +lets the nested contexts stay independent. + +**Deleting a section does not delete what is inside it.** The entries move back +to the top level. It is the one destructive act this screen could commit — those +are coded pages and the admin's own links — so it is locked by a test. + +**The dropdown opens on click, never hover, and the trigger is not a link.** A +hover menu is unusable on touch, and making the trigger navigate means tapping to +open takes you somewhere instead. A section is a container, not a destination. +`NavDropdown.jsx` carries the rest of the contract: Escape closes and returns +focus, an outside press closes, navigating closes, Arrow Up/Down walk the items, +and `aria-haspopup`/`aria-expanded` let it be announced as a menu. + +**A bug the palette filter had, found by the test for it:** `buildNavOverrides` +judged "does this route still exist?" against the *palette* — the base array +already filtered to what the editing admin can see. For the admin nav that is +harmless (an admin sees every row), but on the public header a shard-feature-gated +row is filtered out, so the guard meant to carry its override through could never +fire, and their save would have quietly reset it. Membership is now judged against +the **full** coded nav while the rows still come from the palette: they are two +different questions. + ### Phase 9, cancelled The §4.8 `rgba()` literal promotion and the Parchment light-mode port are **not @@ -861,5 +969,13 @@ and the live observation in "Phases 3–4 as landed". role/feature gate would otherwise hide. Verified by overriding `hidden: false` on a role-gated item as a lower-privileged test admin and confirming the filter still hides it. +- A dropdown section whose every entry is hidden by shard visibility **does not + render at all**, rather than opening onto an empty menu. +- An added link cannot leave the origin: a `to` carrying a scheme, a + protocol-relative `//host`, whitespace or quotes is refused on write and dropped + on read. An added link never grants access — the page behind it still gates + itself. +- Deleting a dropdown section returns its entries to the top level; it never + removes a coded page or an admin's own link. - Deleting a theme/asset/nav row returns that surface to env/code defaults, not to a stored copy of the defaults. -- 2.49.1