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 `` / `` / `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 `` / `` / `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; `` 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.