docs(website): theming phases 6-8 as built, and phase 9 cancelled
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>
This commit is contained in:
@@ -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
|
Values are `TEXT`, so a JSON-valued key arrives as a **string** and every
|
||||||
consumer parses it. Server side that is `utils/settingsJson.js`
|
consumer parses it. Server side that is `utils/settingsJson.js`
|
||||||
(`parseJsonSetting`), client side `parseLayout`/its callers; both treat a
|
(`parseJsonSetting`), client side `client/src/lib/settingsJson.js` and
|
||||||
malformed or wrong-shaped value as **absent** rather than as an error, so a
|
`parseLayout`; both treat a malformed or wrong-shaped value as **absent** rather
|
||||||
hand-edited row degrades to the default instead of rendering something broken.
|
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.**
|
**`theme_visual` is resolved server-side, not shipped raw to the browser.**
|
||||||
`utils/themeResolve.js` layers `:root` ← preset ← custom, field by field, into
|
`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 |
|
| POST | `/posts/upload` | multipart image upload (multer) → `{image_url}` for screenshots |
|
||||||
| GET | `/wiki` · GET `/wiki/:slug` | read incl. unpublished |
|
| GET | `/wiki` · GET `/wiki/:slug` | read incl. unpublished |
|
||||||
| POST | `/wiki` · PUT `/wiki/:slug` · DELETE `/wiki/:slug` | manage pages |
|
| 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 |
|
| 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 |
|
| 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 |
|
| GET | `/activity?limit=&offset=` | paginated activity log |
|
||||||
|
|||||||
@@ -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) |
|
| **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` |
|
| **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 |
|
| **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 |
|
| **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** |
|
| **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 |
|
| **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 |
|
| **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
|
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
|
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
|
`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
|
### 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
|
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.
|
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
|
### 8.1 Admin builder UI notes
|
||||||
|
|
||||||
- Tabbed control for the three navs; drag-and-drop reorderable list.
|
- Tabbed control for the three navs; drag-and-drop reorderable list.
|
||||||
|
|||||||
Reference in New Issue
Block a user