Address PR review comments for theming plan · malpercio.dev/atbb@604bc03

+79 -54

1 changed file

expand all

docs

+79 -54

docs/theming-plan.md

··· 21 21 ### How Themes Work 22 22 23 23 ``` 24 - Forum DID PDS Web Server (Hono) 25 - ┌──────────────────────────┐ ┌───────────────────────────────────┐ 26 - │ space.atbb.forum.theme │ │ │ 27 - │ (multiple records) │──cache─▶│ Theme resolution per request: │ 28 - │ │ │ 1. User pref (membership record) │ 29 - │ space.atbb.forum.forum │ │ 2. Color scheme (cookie/header) │ 30 - │ .themePolicy { │──cache─▶│ 3. Forum default (themePolicy) │ 31 - │ availableThemes │ │ 4. Hardcoded fallback │ 32 - │ defaultLightTheme │ │ │ │ 33 - │ defaultDarkTheme │ │ ▼ │ 34 - │ allowUserChoice │ │ <style>:root { --tokens }</style>│ 35 - │ } │ │ + /static/theme.css │ 36 - └──────────────────────────┘ └───────────────────────────────────┘ 24 + Forum DID PDS Web Server (Hono) 25 + ┌─────────────────────────────┐ ┌───────────────────────────────────┐ 26 + │ space.atbb.forum.theme │ │ │ 27 + │ (multiple records) │──cache─▶│ Theme resolution per request: │ 28 + │ │ │ 1. User pref (membership record) │ 29 + │ space.atbb.forum.themePolicy│ │ 2. Color scheme (cookie/header) │ 30 + │ (singleton) { │──cache─▶│ 3. Forum default (themePolicy) │ 31 + │ availableThemes │ │ 4. Hardcoded fallback │ 32 + │ defaultLightTheme │ │ │ │ 33 + │ defaultDarkTheme │ │ ▼ │ 34 + │ allowUserChoice │ │ <style>:root { --tokens }</style>│ 35 + │ } │ │ + /static/theme.css │ 36 + └─────────────────────────────┘ └───────────────────────────────────┘ 37 37 ``` 38 38 39 39 1. **Theme records** live on the Forum DID's PDS as `space.atbb.forum.theme` records. A forum can have many saved themes. 40 - 2. **Theme policy** on the `forum.forum` record controls which themes are available to users, which are the defaults for light/dark mode, and whether users can choose their own. 40 + 2. **Theme policy** is a separate singleton record (`space.atbb.forum.themePolicy`) that controls which themes are available to users, which are the defaults for light/dark mode, and whether users can choose their own. 41 41 3. **On each request**, the web server resolves which theme to render (see [Theme Resolution](#theme-resolution) below) and injects the winning theme's CSS custom properties into a `<style>` block in `<head>`. 42 42 4. **A single base stylesheet** (`theme.css`) references only custom properties — never hardcoded colors or sizes. Swapping property values completely changes the look. 43 43 5. **Optional per-theme CSS overrides** can extend the base for structural changes (e.g., sidebar layout vs. top-nav), stored as a `cssOverrides` string in the theme record. ··· 182 182 key: tid # Multiple themes per forum 183 183 fields: 184 184 name: string (required) # "Neobrutal Default", "Dark Mode", etc. 185 - colorScheme: string (required) # "light" | "dark" — which mode this theme targets 185 + colorScheme: 186 + type: string (required) 187 + knownValues: ["light", "dark"] # Which mode this theme targets (extensible) 186 188 tokens: map<string, string> # Design token key-value pairs 187 189 cssOverrides: string (optional)# Raw CSS for structural overrides 188 - fontUrls: array<string> (opt) # Google Fonts or self-hosted font URLs 190 + fontUrls: array<string> (opt) # HTTPS URLs for Google Fonts or self-hosted fonts 189 191 createdAt: datetime 190 192 updatedAt: datetime 191 193 ``` 192 194 193 - **Record ownership:** Forum DID (same as `forum.forum`, `forum.category`). 195 + **Record ownership:** Forum DID (same as `forum.forum`, `forum.category`). When implemented, `forum.theme` must be added to the canonical ownership list in CLAUDE.md. 194 196 195 197 **Why `tid` key?** Forums can have many saved themes (like phpBB's theme gallery). The admin curates which ones are available via the theme policy below. 196 198 197 - **Why `colorScheme` instead of `active`?** A single `active` boolean is too limiting. Forums need separate defaults for light and dark mode, plus a curated list of user-selectable themes. The `colorScheme` field tags each theme so the resolution logic knows which mode it serves. The `themePolicy` on the forum record handles the rest. 199 + **Why `colorScheme` instead of `active`?** A single `active` boolean is too limiting. Forums need separate defaults for light and dark mode, plus a curated list of user-selectable themes. The `colorScheme` field tags each theme so the resolution logic knows which mode it serves. The theme policy handles the rest. 198 200 199 - ### Extended: `space.atbb.forum.forum` 201 + ### New: `space.atbb.forum.themePolicy` 200 202 201 - Add a `themePolicy` object to the existing forum singleton record: 203 + A new singleton record on the Forum DID for theme configuration, separate from the main forum record to allow independent updates without invalidating `strongRef`s to the forum record. 202 204 203 205 ```yaml 204 - # New fields on forum.forum 205 - themePolicy: 206 - type: object 207 - properties: 208 - availableThemes: # AT-URIs of themes admins have enabled for users 209 - type: array 210 - items: string (at-uri) 211 - defaultLightTheme: # AT-URI of the default light-mode theme 212 - type: string (at-uri) 213 - defaultDarkTheme: # AT-URI of the default dark-mode theme 214 - type: string (at-uri) 215 - allowUserChoice: # Can users pick their own theme? 216 - type: boolean 217 - default: true 206 + lexiconId: space.atbb.forum.themePolicy 207 + key: literal:self # Singleton — one per forum 208 + 209 + # Named def for theme references 210 + defs: 211 + themeRef: 212 + type: object 213 + required: [theme] 214 + properties: 215 + theme: 216 + type: ref 217 + ref: com.atproto.repo.strongRef # CID integrity check for theme records 218 + 219 + fields: 220 + availableThemes: # Themes admins have enabled for users 221 + type: array 222 + items: 223 + type: ref 224 + ref: '#themeRef' 225 + defaultLightTheme: # Default light-mode theme 226 + type: ref 227 + ref: '#themeRef' 228 + defaultDarkTheme: # Default dark-mode theme 229 + type: ref 230 + ref: '#themeRef' 231 + allowUserChoice: # Can users pick their own theme? 232 + type: boolean 233 + default: true 234 + updatedAt: datetime 218 235 ``` 236 + 237 + **Record ownership:** Forum DID. 219 238 220 239 The admin's saved themes may outnumber the available list — `availableThemes` is the curated subset exposed to users. Both `defaultLightTheme` and `defaultDarkTheme` must be members of `availableThemes`. 221 240 ··· 226 245 ```yaml 227 246 # New optional field on membership 228 247 preferredTheme: 229 - type: string (at-uri, optional) # AT-URI of user's chosen theme, null = follow forum defaults 248 + type: ref (optional) 249 + ref: com.atproto.repo.strongRef # strongRef to space.atbb.forum.theme record 250 + # Null = follow forum defaults 230 251 ``` 231 252 232 - This lives on the **user's PDS** (they own their membership record), so theme preference is portable — leave a forum and rejoin, your preference is still there. 253 + This lives on the **user's PDS** (they own their membership record), so theme preference is portable — leave a forum and rejoin, your preference is still there. Uses `strongRef` for CID integrity — if the theme record is updated, stale preferences are detected. 233 254 234 255 --- 235 256 ··· 261 282 - The editor itself is an HTMX-driven form. Token changes POST to the server, which returns an updated `<style>` block for the preview panel via an `hx-swap`. 262 283 - No client-side JS framework needed — HTMX + server rendering is sufficient for the live preview workflow. 263 284 - Theme JSON import/export is just the `tokens` + `cssOverrides` + `fontUrls` + `colorScheme` fields serialized. 264 - - Theme policy changes (defaults, available list, allowUserChoice) write back to the `forum.forum` record on the Forum DID's PDS. 285 + - Theme policy changes (defaults, available list, allowUserChoice) write to the `space.atbb.forum.themePolicy` singleton on the Forum DID's PDS. 265 286 266 287 --- 267 288 268 289 ## AppView API Endpoints 269 290 270 - Theme data flows through the AppView like all other forum data. New endpoints: 291 + Theme data flows through the AppView like all other forum data. New endpoints (AppView REST, not XRPC — consistent with existing `/api/forum`, `/api/categories` patterns): 271 292 272 293 ### Read Endpoints 273 294 ··· 275 296 |----------|------|-------------| 276 297 | `GET /api/themes` | Public | List available themes (filtered by `themePolicy.availableThemes`). Returns name, colorScheme, and token summary for each. | 277 298 | `GET /api/themes/:rkey` | Public | Get a single theme's full token set, cssOverrides, and fontUrls. | 278 - | `GET /api/forum` (existing) | Public | Already returns forum metadata — now also includes `themePolicy` (defaults, available list, allowUserChoice). | 299 + | `GET /api/theme-policy` | Public | Get the forum's theme policy (available themes, defaults for light/dark, allowUserChoice). | 279 300 280 301 ### Write Endpoints 281 302 ··· 284 305 | `POST /api/themes` | Admin | Create a new theme record on Forum DID's PDS. | 285 306 | `PUT /api/themes/:rkey` | Admin | Update an existing theme's tokens, name, colorScheme, etc. | 286 307 | `DELETE /api/themes/:rkey` | Admin | Delete a theme. Fails if it's currently a default. | 287 - | `PUT /api/forum/theme-policy` | Admin | Update `themePolicy` on the forum record (available list, defaults, allowUserChoice). | 308 + | `PUT /api/theme-policy` | Admin | Update the `themePolicy` singleton (available list, defaults, allowUserChoice). | 288 309 | `PATCH /api/membership/theme` | User | Set `preferredTheme` on the caller's membership record (writes to their PDS). Pass `null` to clear. | 289 310 290 311 ### Caching 291 312 292 313 The web server caches resolved theme data aggressively since themes change rarely: 293 314 294 - - **Theme tokens:** Cached in-memory on the web server, keyed by AT-URI. Invalidated when the AppView receives a firehose event for `space.atbb.forum.theme` records. 315 + - **Theme tokens:** Cached in-memory on the web server, keyed by AT-URI **and resolved color scheme** (light/dark). Cache key must include color scheme to prevent serving a cached light response to dark-mode users. Use `Vary: Cookie` or equivalent for HTTP caching. 295 316 - **Theme policy:** Cached alongside forum metadata. Same invalidation path. 296 317 - **User preference:** Looked up from the AppView's indexed `membership` records (local DB query, not a PDS fetch per request). 297 318 ··· 425 446 Is the user logged in? 426 447 AND has a preferredTheme set on their membership record? 427 448 AND does the forum's themePolicy.allowUserChoice == true? 428 - AND is preferredTheme still in themePolicy.availableThemes? 449 + AND is preferredTheme.uri still in themePolicy.availableThemes? 450 + AND does preferredTheme.cid match current theme record (integrity check)? 429 451 → Use their preferred theme. 430 452 431 453 2. Color scheme default ··· 434 456 b. HTTP header: Sec-CH-Prefers-Color-Scheme (client hint) 435 457 c. Default: light 436 458 437 - → Use themePolicy.defaultDarkTheme or defaultLightTheme accordingly. 459 + → Use themePolicy.defaultDarkTheme or defaultLightTheme accordingly 460 + (with CID integrity check via strongRef). 438 461 439 462 3. Hardcoded fallback 440 463 If no theme policy exists or the resolved theme can't be loaded: ··· 487 510 - No admin editor, no dynamic themes — just ship a good-looking default 488 511 489 512 ### Theme Phase 2: Light/Dark + Token System 490 - - Define `space.atbb.forum.theme` lexicon (with `colorScheme` field) 491 - - Add `themePolicy` to `space.atbb.forum.forum` lexicon 513 + - Define `space.atbb.forum.theme` lexicon (with `colorScheme` field using `knownValues`) 514 + - Define `space.atbb.forum.themePolicy` lexicon as separate singleton (with `themeRef` strongRef wrapper) 492 515 - Build `tokensToCss()` utility 493 516 - Ship built-in preset JSON files (neobrutal light + dark, clean light + dark, classic) 494 517 - Load theme policy + resolved theme from Forum DID's PDS (with caching) 495 518 - Inject tokens dynamically in `BaseLayout` based on theme resolution waterfall 496 519 - Add light/dark toggle (cookie-based, vanilla JS, ~6 lines) 497 520 - Add `Sec-CH-Prefers-Color-Scheme` client hint support as fallback 498 - - AppView endpoints: `GET /api/themes`, `GET /api/themes/:rkey` 521 + - AppView endpoints: `GET /api/themes`, `GET /api/themes/:rkey`, `GET /api/theme-policy` (REST, not XRPC) 499 522 500 523 ### Theme Phase 3: Admin Theme Management 501 - - Admin endpoints: `POST/PUT/DELETE /api/themes`, `PUT /api/forum/theme-policy` 524 + - **CSS sanitization (mandatory gate):** Server-side sanitization for `cssOverrides` and freeform `tokens` map before rendering. Raw CSS in `<style>` tags is a real exfiltration vector via `url()`, `@import`, `@font-face`. Must sanitize before this phase ships. 525 + - Admin endpoints: `POST/PUT/DELETE /api/themes`, `PUT /api/forum/theme-policy` (AppView REST, not XRPC) 502 526 - Theme list management UI (create, duplicate, delete, availability toggles) 503 527 - Default light/dark assignment dropdowns 504 528 - `allowUserChoice` kill-switch toggle 505 529 - Token editor with grouped controls + live preview via HTMX 506 530 - Import/export (JSON download/upload) 531 + - Database additions: `themes` table following `(did, rkey, cid, indexed_at)` pattern 507 532 508 533 ### Theme Phase 4: User Choice 509 534 - Add `preferredTheme` field to `space.atbb.membership` lexicon 510 - - User endpoint: `PATCH /api/membership/theme` 535 + - User endpoint: `PATCH /api/membership/theme` (AppView REST, not XRPC) 511 536 - Theme picker UI for logged-in users (dropdown in settings or site header) 512 537 - "Auto (follow forum default)" option to clear preference 538 + - Database additions: `preferred_theme_uri` column on `memberships` table (nullable) 513 539 - AppView indexes `preferredTheme` from membership records for fast lookup 514 540 515 541 ### Theme Phase 5: Polish ··· 522 548 523 549 ## Open Questions 524 550 525 - 1. **Font loading strategy.** Google Fonts is easy but has privacy implications for self-hosters. Should we bundle a few default fonts, allow self-hosted uploads, or both? 526 - 2. **CSS override sandboxing.** Raw CSS in `cssOverrides` could break layouts or introduce XSS (via `url()`, `expression()`, etc.). Need a sanitization strategy — maybe a CSS parser that strips dangerous properties. 527 - 3. **Theme record size limits.** AT Proto records have size limits. If `cssOverrides` gets large, might need to store it as a blob reference instead of inline. 528 - 4. **Build-time vs. runtime tokens.** The plan above is fully runtime (no CSS build per theme). This is simpler but means we can't use tools like Tailwind for the base styles. Is that acceptable? 529 - 5. **Theme migration on updates.** When atBB ships new tokens in a release (e.g., a new component token), existing saved themes won't have values for them. Need a merge strategy — probably fall back to the preset's value for any missing token. 530 - 6. **Stale user preferences.** If an admin removes a theme from `availableThemes` while users have it as their `preferredTheme`, the resolution waterfall handles this gracefully (falls through to the color scheme default). But should we also notify affected users or silently degrade? 551 + 1. **Font loading strategy.** Google Fonts is easy but has privacy implications for self-hosters. Should we bundle a few default fonts, allow self-hosted uploads, or both? **Security note:** `fontUrls` must be constrained to HTTPS and consider an allowlist (Google Fonts, self-hosted paths) — arbitrary font URLs leak user IPs to third parties. 552 + 2. **Theme record size limits.** AT Proto records have size limits. If `cssOverrides` gets large, might need to store it as a blob reference instead of inline. 553 + 3. **Build-time vs. runtime tokens.** The plan above is fully runtime (no CSS build per theme). This is simpler but means we can't use tools like Tailwind for the base styles. Is that acceptable? 554 + 4. **Theme migration on updates.** When atBB ships new tokens in a release (e.g., a new component token), existing saved themes won't have values for them. Need a merge strategy — probably fall back to the preset's value for any missing token. 555 + 5. **Stale user preferences.** If an admin removes a theme from `availableThemes` while users have it as their `preferredTheme`, the resolution waterfall handles this gracefully (falls through to the color scheme default). But should we also notify affected users or silently degrade?

Configure Feed

Configure Feed