Address review: preserve unmatched themes, validate write path, lock blend direction · jeremy.herve.bzh/skypress@6da243a

+29

src/components/PublicationForm.test.tsx

··· 59 59 }; 60 60 expect( checkedValue( renderForm( existing ) ) ).toBe( 'dusk' ); 61 61 } ); 62 + 63 + it( 'surfaces and pre-selects a "Current" option when the stored theme matches no preset', () => { 64 + // A valid theme whose colours match no preset (e.g. a preset whose values later changed). 65 + // It must remain visible + selected so an unrelated edit doesn't silently erase it. 66 + const existing: Publication = { 67 + uri: 'at://x', 68 + rkey: 'r', 69 + slug: 'blog', 70 + name: 'Blog', 71 + basicTheme: { 72 + $type: 'site.standard.theme.basic', 73 + background: { r: 10, g: 20, b: 30 }, 74 + foreground: { r: 240, g: 240, b: 240 }, 75 + accent: { r: 100, g: 50, b: 200 }, 76 + accentForeground: { r: 255, g: 255, b: 255 }, 77 + }, 78 + }; 79 + const markup = renderForm( existing ); 80 + expect( markup ).toContain( 'Current' ); 81 + expect( radios( markup ) ).toHaveLength( THEME_PRESETS.length + 2 ); // none + custom + presets 82 + expect( checkedValue( markup ) ).toBe( 'custom' ); 83 + } ); 84 + 85 + it( 'shows no "Current" option for an unthemed publication', () => { 86 + const existing: Publication = { uri: 'at://x', rkey: 'r', slug: 'blog', name: 'Blog' }; 87 + const markup = renderForm( existing ); 88 + expect( radios( markup ) ).toHaveLength( THEME_PRESETS.length + 1 ); 89 + expect( checkedValue( markup ) ).toBe( '' ); 90 + } ); 62 91 } );

+55 -6

src/components/PublicationForm.tsx

··· 11 11 PUBLICATION_ICON_MAX_BYTES, 12 12 } from '../lib/media/uploadImage'; 13 13 import { buildGetBlobUrl, type BlobRefJson } from '../lib/media/blob'; 14 - import { THEME_PRESETS, findPresetByColors, type Rgb } from '../lib/publish/themes'; 14 + import { 15 + THEME_PRESETS, 16 + findPresetByColors, 17 + resolveSelectedTheme, 18 + CUSTOM_THEME_SLUG, 19 + type Rgb, 20 + } from '../lib/publish/themes'; 15 21 16 22 interface Props { 17 23 agent: Agent; ··· 49 55 ? buildGetBlobUrl( pdsUrl, did, existing.icon.ref.$link ) 50 56 : null 51 57 ); 52 - const [ themeSlug, setThemeSlug ] = useState< string | null >( 53 - () => findPresetByColors( existing?.basicTheme )?.slug ?? null 54 - ); 58 + // A stored theme that matches no current preset (e.g. a preset whose colours later changed): 59 + // keep it verbatim and offer it as a selectable "Custom" option, so editing an unrelated field 60 + // never silently erases the publisher's colours. 61 + const customTheme = 62 + existing?.basicTheme && ! findPresetByColors( existing.basicTheme ) 63 + ? existing.basicTheme 64 + : null; 65 + const [ themeSlug, setThemeSlug ] = useState< string | null >( () => { 66 + const matched = findPresetByColors( existing?.basicTheme )?.slug; 67 + if ( matched ) { 68 + return matched; 69 + } 70 + return customTheme ? CUSTOM_THEME_SLUG : null; 71 + } ); 55 72 const [ uploading, setUploading ] = useState( false ); 56 73 const [ saving, setSaving ] = useState( false ); 57 74 const [ error, setError ] = useState< string | null >( null ); ··· 106 123 setSaving( true ); 107 124 setError( null ); 108 125 try { 109 - const selectedPreset = THEME_PRESETS.find( ( preset ) => preset.slug === themeSlug ); 110 126 const input = { 111 127 name: name.trim(), 112 128 description: description.trim() || undefined, 113 129 icon: icon ?? undefined, 114 - basicTheme: selectedPreset?.colors, 130 + basicTheme: resolveSelectedTheme( themeSlug, customTheme ), 115 131 }; 116 132 const saved = existing 117 133 ? await updatePublication( agent, did, handle, existing, input ) ··· 193 209 /> 194 210 <span className="pubform__theme-label">No theme</span> 195 211 </label> 212 + { customTheme && ( 213 + <label 214 + className={ `pubform__theme${ 215 + themeSlug === CUSTOM_THEME_SLUG ? ' is-selected' : '' 216 + }` } 217 + > 218 + <input 219 + type="radio" 220 + name="pub-theme" 221 + value={ CUSTOM_THEME_SLUG } 222 + checked={ themeSlug === CUSTOM_THEME_SLUG } 223 + onChange={ () => setThemeSlug( CUSTOM_THEME_SLUG ) } 224 + disabled={ saving } 225 + /> 226 + <span 227 + className="pubform__theme-swatch" 228 + style={ { 229 + background: `rgb(${ customTheme.background.r }, ${ customTheme.background.g }, ${ customTheme.background.b })`, 230 + color: `rgb(${ customTheme.foreground.r }, ${ customTheme.foreground.g }, ${ customTheme.foreground.b })`, 231 + } } 232 + aria-hidden="true" 233 + > 234 + <span 235 + className="pubform__theme-dot" 236 + style={ { 237 + background: `rgb(${ customTheme.accent.r }, ${ customTheme.accent.g }, ${ customTheme.accent.b })`, 238 + } } 239 + /> 240 + Aa 241 + </span> 242 + <span className="pubform__theme-label">Current</span> 243 + </label> 244 + ) } 196 245 { THEME_PRESETS.map( ( preset ) => { 197 246 const swatch = ( color: Rgb ) => `rgb(${ color.r }, ${ color.g }, ${ color.b })`; 198 247 return (

+17

src/lib/publish/records.test.ts

··· 173 173 const bare = buildPublicationRecord( { handle: 'a.b', slug: 's', name: 'N' } ); 174 174 expect( 'basicTheme' in bare ).toBe( false ); 175 175 } ); 176 + 177 + it( 'drops an invalid basicTheme rather than persisting it (write-boundary validation)', () => { 178 + const record = buildPublicationRecord( { 179 + handle: 'a.b', 180 + slug: 's', 181 + name: 'N', 182 + // Out-of-range channel — must never reach storage. 183 + basicTheme: { 184 + $type: 'site.standard.theme.basic', 185 + background: { r: 300, g: 0, b: 0 }, 186 + foreground: { r: 0, g: 0, b: 0 }, 187 + accent: { r: 0, g: 0, b: 0 }, 188 + accentForeground: { r: 0, g: 0, b: 0 }, 189 + } as never, 190 + } ); 191 + expect( 'basicTheme' in record ).toBe( false ); 192 + } ); 176 193 } ); 177 194 178 195 describe( 'buildDocumentRecord', () => {

+5 -2

src/lib/publish/records.ts

··· 6 6 */ 7 7 import type { BlockNode } from '../blocks/render'; 8 8 import type { BlobRefJson } from '../media/blob'; 9 - import type { BasicTheme } from './themes'; 9 + import { parseBasicTheme, type BasicTheme } from './themes'; 10 10 11 11 /** 12 12 * Public origin for the stored publication + article URLs (and the Bluesky post link). ··· 164 164 } ): PublicationRecord { 165 165 const trimmedName = input.name?.trim(); 166 166 const description = input.description?.trim(); 167 + // Validate the theme at the write boundary too (symmetric with the read path): an invalid 168 + // theme is dropped rather than persisted, so no out-of-range channel can ever reach storage. 169 + const basicTheme = parseBasicTheme( input.basicTheme ); 167 170 return { 168 171 $type: 'site.standard.publication', 169 172 url: publicationHomeUrl( input.handle, input.slug ), 170 173 name: trimmedName || input.handle, 171 174 ...( description ? { description } : {} ), 172 175 ...( input.icon ? { icon: input.icon } : {} ), 173 - ...( input.basicTheme ? { basicTheme: input.basicTheme } : {} ), 176 + ...( basicTheme ? { basicTheme } : {} ), 174 177 }; 175 178 } 176 179

+48

src/lib/publish/themes.test.ts

··· 5 5 findPresetByColors, 6 6 themeToCssVars, 7 7 themeStyleBlock, 8 + resolveSelectedTheme, 9 + CUSTOM_THEME_SLUG, 8 10 type BasicTheme, 9 11 } from './themes'; 10 12 ··· 113 115 } ) 114 116 ).toBeNull(); 115 117 } ); 118 + 119 + it( 'round-trips a parsed theme back to its preset (the edit-prefill path)', () => { 120 + // Production prefill reads `existing.basicTheme`, which came through parseBasicTheme. 121 + const parsed = parseBasicTheme( THEME_PRESETS[ 6 ].colors ); // sunrise 122 + expect( findPresetByColors( parsed )?.slug ).toBe( 'sunrise' ); 123 + } ); 124 + } ); 125 + 126 + describe( 'resolveSelectedTheme', () => { 127 + const custom: BasicTheme = { 128 + $type: 'site.standard.theme.basic', 129 + background: { r: 1, g: 2, b: 3 }, 130 + foreground: { r: 4, g: 5, b: 6 }, 131 + accent: { r: 7, g: 8, b: 9 }, 132 + accentForeground: { r: 10, g: 11, b: 12 }, 133 + }; 134 + 135 + it( 'maps null to undefined (no theme)', () => { 136 + expect( resolveSelectedTheme( null, null ) ).toBeUndefined(); 137 + expect( resolveSelectedTheme( null, custom ) ).toBeUndefined(); 138 + } ); 139 + 140 + it( 'maps a preset slug to that preset’s colours', () => { 141 + expect( resolveSelectedTheme( 'dusk', null ) ).toEqual( THEME_PRESETS[ 2 ].colors ); 142 + } ); 143 + 144 + it( 'keeps the custom theme verbatim for the custom sentinel', () => { 145 + expect( resolveSelectedTheme( CUSTOM_THEME_SLUG, custom ) ).toEqual( custom ); 146 + // No custom theme to keep → no theme rather than a dangling sentinel. 147 + expect( resolveSelectedTheme( CUSTOM_THEME_SLUG, null ) ).toBeUndefined(); 148 + } ); 149 + 150 + it( 'maps an unknown slug to undefined (preset removed) rather than crashing', () => { 151 + expect( resolveSelectedTheme( 'no-such-preset', null ) ).toBeUndefined(); 152 + } ); 116 153 } ); 117 154 118 155 describe( 'themeToCssVars', () => { ··· 130 167 for ( const key of [ '--muted', '--line', '--line-strong', '--sun-tint', '--btn-primary-hover' ] ) { 131 168 expect( vars[ key ] ).toMatch( /^rgb$\d{1,3}, \d{1,3}, \d{1,3}$$/ ); 132 169 } 170 + } ); 171 + 172 + it( 'blends muted as foreground OVER background (direction locked)', () => { 173 + // evening: bg 27, fg 240 → mix(fg, bg, 0.55) = round(240*.55 + 27*.45) = round(144.15) = 144. 174 + // If the blend direction inverted (bg over fg) this would be ~123 — caught here. 175 + expect( vars[ '--muted' ] ).toBe( 'rgb(144, 144, 144)' ); 176 + } ); 177 + 178 + it( 'carries accentForeground into --btn-primary-fg', () => { 179 + const noon = themeToCssVars( THEME_PRESETS[ 1 ].colors ); // accentForeground = black 180 + expect( noon[ '--btn-primary-fg' ] ).toBe( 'rgb(0, 0, 0)' ); 133 181 } ); 134 182 } ); 135 183

+25

src/lib/publish/themes.ts

··· 99 99 }; 100 100 } 101 101 102 + /** 103 + * Sentinel slug for "keep the publication's current colours" — used when a stored theme doesn't 104 + * match any preset (e.g. a future preset whose colours changed). Keeps the picker from silently 105 + * erasing a theme it can't name. Not a real preset; never written as a slug. 106 + */ 107 + export const CUSTOM_THEME_SLUG = 'custom'; 108 + 109 + /** 110 + * Resolve the picker's selection to the colours to store. `null` slug → no theme (omit); 111 + * the custom sentinel → keep `customTheme` verbatim; any other slug → that preset's colours. 112 + * Pure, so the form's submit mapping is unit-testable without rendering. 113 + */ 114 + export function resolveSelectedTheme( 115 + slug: string | null, 116 + customTheme: BasicTheme | null | undefined 117 + ): BasicTheme | undefined { 118 + if ( slug === null ) { 119 + return undefined; 120 + } 121 + if ( slug === CUSTOM_THEME_SLUG ) { 122 + return customTheme ?? undefined; 123 + } 124 + return THEME_PRESETS.find( ( preset ) => preset.slug === slug )?.colors; 125 + } 126 + 102 127 const sameRgb = ( a: Rgb, b: Rgb ): boolean => a.r === b.r && a.g === b.g && a.b === b.b; 103 128 104 129 /** Reverse-match stored colours to a known preset (so the picker highlights it), or null. */

+47

src/lib/reader/theme-injection.test.ts

··· 1 + /** 2 + * Regression guard for the publication-theme injection (Decision 0012). 3 + * 4 + * The publication-home and article pages inject a `<style>` overriding the design tokens when a 5 + * publication has a `basicTheme`. Rendering these `.astro` pages through astro/container isn't 6 + * viable here (the runner is pinned to jsdom for the WordPress block suites, which breaks 7 + * esbuild's init invariant — see index.phase.test.ts), so these asserts pin the wiring at the 8 + * source level: each page must compute `themeStyleBlock(publication.basicTheme)` and inject it, 9 + * gated on a truthy result, via `set:html` (the value is app-built CSS, proven injection-safe by 10 + * themes.test.ts). A refactor that drops the gate or passes the wrong field ships green without it. 11 + * 12 + * Lives under src/lib/ (not src/pages/) on purpose: a `.test.ts` inside a `[param]` route dir is 13 + * treated by Astro as a prerendered endpoint and breaks the build. 14 + */ 15 + import { readFileSync } from 'node:fs'; 16 + import { fileURLToPath } from 'node:url'; 17 + import { describe, expect, it } from 'vitest'; 18 + 19 + const read = ( rel: string ) => 20 + readFileSync( fileURLToPath( new URL( rel, import.meta.url ) ), 'utf8' ); 21 + 22 + describe( 'publication theme injection wiring', () => { 23 + for ( const [ label, file ] of [ 24 + [ 'publication home', '../../pages/[author]/[slug]/index.astro' ], 25 + [ 'article', '../../pages/[author]/[slug]/[rkey].astro' ], 26 + ] as const ) { 27 + describe( label, () => { 28 + const src = read( file ); 29 + 30 + it( 'imports themeStyleBlock from the themes module', () => { 31 + expect( src ).toMatch( 32 + /import\s*\{\s*themeStyleBlock\s*\}\s*from\s*['"][^'"]*publish\/themes['"]/ 33 + ); 34 + } ); 35 + 36 + it( 'derives the style block from the publication’s basicTheme', () => { 37 + expect( src ).toMatch( /themeStyleBlock$\s*publication\.basicTheme\s*$/ ); 38 + } ); 39 + 40 + it( 'injects it via set:html, gated on a truthy theme', () => { 41 + expect( src ).toMatch( 42 + /\{\s*themeStyle\s*&&\s*<Fragment\s+set:html=\{\s*themeStyle\s*\}\s*\/>\s*\}/ 43 + ); 44 + } ); 45 + } ); 46 + } 47 + } );

Configure Feed

Configure Feed