From aaa305cc3e635b4548eca386641aab0d300d98fd Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 11:14:00 -0700 Subject: [PATCH 01/61] Add design spec: unified image/asset picker for site builder Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-07-12-site-builder-asset-picker-design.md | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-12-site-builder-asset-picker-design.md diff --git a/docs/superpowers/specs/2026-07-12-site-builder-asset-picker-design.md b/docs/superpowers/specs/2026-07-12-site-builder-asset-picker-design.md new file mode 100644 index 0000000..35748e9 --- /dev/null +++ b/docs/superpowers/specs/2026-07-12-site-builder-asset-picker-design.md @@ -0,0 +1,149 @@ +# Site Builder — unified image/asset picker (2026-07-12) + +Wherever an image is entered by URL in the Craft.js site builder +(`/workspace/site-builder/craft/`), the user should also be able to **pick an +image already uploaded to the site builder** (and upload a new one). Today this +"Browse uploaded assets" affordance exists but is copy-pasted into ~6 components +and missing entirely from ~7 other image fields. Scope decisions were made with +the user before writing this spec. + +## Problem + +The "select an uploaded image" capability (a **Browse** button opening an inline +thumbnail grid backed by `?action=list_assets`) is implemented — six independent +times: + +- `craft/src/components/media/ImageBlock.tsx` +- `craft/src/components/basic/Logo.tsx` +- `craft/src/components/basic/Navbar.tsx` (logo) +- `craft/src/panels/right/styles/ImageStylePanel.tsx` +- `craft/src/panels/right/styles/HeroStylePanel.tsx` (`AssetBrowser` sub-component) +- `craft/src/components/media/VideoBlock.tsx` (video assets) + +…and is **absent** (plain URL `` only) from: + +- `craft/src/panels/right/styles/BackgroundSectionStylePanel.tsx` (`bgImage`) +- `craft/src/panels/right/styles/MediaStylePanel.tsx` (two image fields) +- `craft/src/panels/right/styles/FeaturesEditor.tsx` (per-feature image — has + upload + URL, no browse) +- `craft/src/components/sections/FeaturesGrid.tsx` (per-feature image, settings) +- `craft/src/components/sections/Gallery.tsx` (per-image `src`) +- `craft/src/components/sections/ContentSlider.tsx` (per-slide `imageSrc`) +- `craft/src/components/layout/Container.tsx` (background image URL) + +Same capability, inconsistent presence — a UX gap for users and 6 divergent +copies to maintain. + +## Solution — one reusable component, two presentation variants + +### New: `craft/src/ui/AssetPicker.tsx` + +Single source of truth for choosing an image (or, later, video). Lives in +`src/ui/` — the existing neutral home for reusable settings controls +(`BorderControl`, `SpacingInput`, `AnchorIdField`, …), importable by BOTH +`src/components/**` and `src/panels/**`. (No component currently imports from +`panels/right/styles`, so the shared piece must not live there.) + +It composes the three affordances that already exist today: +1. **Upload** a new file (drag-drop zone + button) +2. **Browse** uploaded site assets (inline thumbnail grid) +3. **Paste a URL** (escape hatch for external images) + +```ts +interface AssetPickerProps { + value: string; // current url ('' = none) + onChange: (url: string) => void; + mediaType?: 'image' | 'video' | 'any'; // default 'image' — filters grid + file accept + variant?: 'full' | 'compact'; // default 'full' + placeholder?: string; // URL input placeholder +} +``` + +- **`variant="full"`** — the rich layout: preview thumbnail with remove-✕, dashed + drag-drop zone when empty, Upload + Browse buttons, inline grid, URL apply row. + Mirrors today's `ImageStylePanel` layout. Consumers: **ImageStylePanel, Logo + settings, Navbar logo, BackgroundSectionStylePanel, MediaStylePanel, Hero bg + image, Container bg image**. +- **`variant="compact"`** — one tight row (small thumbnail + Upload + Browse + + inline URL input; grid expands below on Browse) that fits inside array-editor + item cards. Consumers: **FeaturesEditor, FeaturesGrid settings, Gallery rows, + ContentSlider slides**. + +Both variants share ALL logic (upload, browse fetch + grid, URL state, WHP/ +standalone gating); only the markup differs, selected by `variant`. + +### Supporting change: `craft/src/utils/assets.ts` (de-duplicate) + +`uploadToWhp` is duplicated in at least `panels/right/styles/shared.tsx`, +`panels/right/styles/FeaturesEditor.tsx`, and inline in several components; every +Browse re-implements the `list_assets` fetch + `type` filter. Add a neutral util: + +```ts +// src/utils/assets.ts +export async function uploadAsset(file: File): Promise; +export async function listAssets(mediaType?: 'image' | 'video' | 'any'): Promise; +export interface Asset { name: string; url: string; type: string; } +``` + +- `uploadAsset` = the existing `uploadToWhp` body (POST `upload_asset`, blob + fallback when no `WHP_CONFIG`). +- `listAssets` = fetch `list_assets`, return `[]` when no `WHP_CONFIG`, filter by + `mediaType` (`type` startsWith `image`/`video`; `any` = no filter). + +`AssetPicker` uses both. `shared.tsx` keeps re-exporting `uploadToWhp` (thin +wrapper delegating to `uploadAsset`) so nothing else breaks; local copies in +components are removed as each is migrated to `AssetPicker`. + +## Behavior details + +- **Standalone mode** (no `window.WHP_CONFIG`): Browse is hidden (no assets API + to call); Upload still works via `blob:` fallback. Matches today's behavior. +- **Video-ready**: `mediaType` drives both the grid filter and the file-input + `accept`. Image fields are wired now with the default `'image'`. VideoBlock / + Hero bg-video may adopt the same component with `mediaType="video"` — done in + this work if cheap, otherwise a clean follow-up. +- **No `toHtml` changes.** This is purely settings-panel input UX; published HTML + is byte-for-byte unaffected, so all existing `*.toHtml.test.ts` stay green. +- **Prop wiring unchanged.** Each consumer keeps writing to its own prop + (`src`, `imageSrc`, `logoImage`, `bgImage`, per-item `image`/`imageSrc`) — only + the input UI is swapped. `AssetPicker` is controlled via `value`/`onChange`. + +## Migration order (each independently verifiable) + +1. `utils/assets.ts` + `AssetPicker.tsx` + unit test (no consumers yet). +2. Migrate the "full" panel fields: ImageStylePanel, MediaStylePanel, + BackgroundSectionStylePanel, HeroStylePanel (image), Container bg. +3. Migrate the "full" component settings: Logo, Navbar logo. +4. Migrate ImageBlock to the shared component. +5. Migrate the "compact" array editors: FeaturesEditor, FeaturesGrid, Gallery, + ContentSlider. +6. Remove now-dead duplicated upload/browse state and local `uploadToWhp` copies. + +## Testing + +- New `craft/src/ui/AssetPicker.test.tsx`: + - renders the current `value` (thumbnail/URL shown), + - `onChange` fires with the asset URL on grid-select and on URL "Apply", + - grid requests + shows only assets matching `mediaType`, + - Browse hidden when `window.WHP_CONFIG` is absent + (mock `fetch` + toggle `window.WHP_CONFIG`). +- New `craft/src/utils/assets.test.ts`: `uploadAsset` blob fallback + POST shape; + `listAssets` filter + empty-without-config. +- Regression: full `cd craft && npx vitest run` and `npm run build` (tsc) green. + +## Out of scope / follow-ups + +- Rewriting the asset grid UX itself (search, pagination, folders) — this reuses + the current grid look, just makes it universal. +- Migrating VideoBlock/Hero video pickers is optional in this pass (component is + built video-ready; wire if cheap, else follow-up). +- A separate, parallel Fable-agent audit of the whole site builder may surface + additional improvements; those are tracked independently, not in this spec. + +## Deploy + +Ships inside the WHP release, per the standard site-builder pipeline +(build craft bundle → copy `dist/{index.html,js,css}` into +`whp/web-files/site-builder/` → `build-release.sh` → `download-update.sh` across +the fleet). Handled at ship time via the `whp-deploy-release` skill, after +implementation and review. From 02e99f76233bfb279af40247324c2e3478a4fda7 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 11:27:57 -0700 Subject: [PATCH 02/61] Add design spec: site builder security & data-loss hardening Covers Critical (stored-XSS escaping cluster) + High (copy/paste id reuse, header/footer save corruption, AI-boundary validation) findings from the 2026-07-12 audit. All claims verified against code. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-site-builder-security-hardening-design.md | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-12-site-builder-security-hardening-design.md diff --git a/docs/superpowers/specs/2026-07-12-site-builder-security-hardening-design.md b/docs/superpowers/specs/2026-07-12-site-builder-security-hardening-design.md new file mode 100644 index 0000000..b197bc4 --- /dev/null +++ b/docs/superpowers/specs/2026-07-12-site-builder-security-hardening-design.md @@ -0,0 +1,128 @@ +# Site Builder — security & data-loss hardening (2026-07-12) + +Fixes the **Critical** (stored-XSS / HTML-breakout) and **High** (state-corruption +and save data-loss) findings from the 2026-07-12 Fable audit of the Craft.js site +builder (`/workspace/site-builder/craft/`). Medium/Low findings are deferred to the +audit backlog (in memory), not this spec. Sequenced **before** the asset-picker +work per user decision. Every claim below was verified against the code before +writing this spec. + +## 1 — Escaping cluster (Critical, do first: closes the most with one change) + +**Verified root cause:** `escapeHtml` exists once (`utils/html-export.ts:201`) but is +**not exported** and no component uses it; there are **27** local `esc`/`escapeHtml` +definitions across `src/`, in three incompatible variants (some omit `&`, some omit +quotes entirely, none escape `'`), and several land in attribute or JS-string +contexts where HTML-entity escaping is wrong anyway. + +### 1a — Shared escaping/URL util +New `craft/src/utils/escape.ts` (exported, single source of truth): + +```ts +export function escapeHtml(s: string): string; // & < > " — text/content context +export function escapeAttr(s: string): string; // escapeHtml + ' — attribute context +export function safeUrl(s: string): string; // returns '' (or '#') for javascript:/data:text/html/vbscript: schemes +``` + +- Move the correct body from `html-export.ts` into `escape.ts`; `html-export.ts` + imports it. Delete all 27 local copies and import from `escape.ts`. +- `safeUrl` wraps every exported `href`/`src`/`action`/`url()` value: `ButtonLink`, + `Icon`, `SocialLinks`, `Logo`, `Menu`, `Navbar`, `_cta-helpers`, `ContentSlider`, + `FeaturesGrid`, `PricingTable`, `ImageBlock`, `VideoBlock`, `Gallery`, background + images. Attribute values use `escapeAttr`; text nodes use `escapeHtml`. + +### 1b — JS-string / raw-HTML contexts (entity escaping is NOT enough here) +- **HtmlBlock** (`basic/HtmlBlock.tsx:144`): `toHtml` returns `props.code` raw while + the render sanitizes via the file's own `purifyHtml`. Fix: `return { html: + purifyHtml(props.code || '') }`. (DOMPurify already a dep.) +- **Countdown** (`sections/Countdown.tsx:298`): `new Date("${targetDate}")` inside an + inline `'})` output is DOMPurify-sanitized (no raw `.mp4' }, '').html; + expect(html2).not.toContain(''); + }); + + test('VideoBlock youtube embed still works after safeUrl pass', () => { + const html = toHtmlOf(VideoBlock)({ videoUrl: 'https://www.youtube.com/watch?v=abc123' }, '').html; + expect(html).toContain('https://www.youtube.com/embed/abc123'); + }); + + test('Gallery img src', () => { + const html = toHtmlOf(Gallery)({ images: [{ src: XSS, alt: 'a' }] }, '').html; + expect(html).not.toContain('javascript:'); + const html2 = toHtmlOf(Gallery)({ images: [{ src: QUOTE_BREAKOUT, alt: 'a' }] }, '').html; + expect(html2).not.toContain('onerror="alert(1)"'); + }); + + test('BackgroundSection bg image url()', () => { + const html = toHtmlOf(BackgroundSection)({ bgImage: XSS }, '').html; + expect(html).not.toContain('javascript:'); + }); + + test('HeroSimple bg image url() and bg video src', () => { + const html = toHtmlOf(HeroSimple)({ bgType: 'image', bgImage: XSS }, '').html; + expect(html).not.toContain('javascript:'); + const html2 = toHtmlOf(HeroSimple)({ bgType: 'video', bgVideo: XSS }, '').html; + expect(html2).not.toContain('javascript:'); + const html3 = toHtmlOf(HeroSimple)({ bgType: 'video', bgVideo: QUOTE_BREAKOUT }, '').html; + expect(html3).not.toContain('onerror="alert(1)"'); + }); + + test('CallToAction bg image url()', () => { + const html = toHtmlOf(CallToAction)({ bgType: 'image', bgValue: XSS }, '').html; + expect(html).not.toContain('javascript:'); + }); + + test('MapEmbed iframe src (found beyond the brief-listed sites via grep sweep)', () => { + const html = toHtmlOf(MapEmbed)({ address: 'New York, NY', zoom: 14 }, '').html; + expect(html).toContain('maps.google.com'); + expect(html).not.toContain('javascript:'); + }); + + test('FormContainer legacy form action is safeUrl-filtered (found via grep sweep)', () => { + const html = toHtmlOf(FormContainer)({ action: XSS, method: 'GET' }, '').html; + expect(html).not.toContain('javascript:'); + // non-relay legacy path (no recipientEmail) still works normally + const html2 = toHtmlOf(FormContainer)({ action: '/legacy', method: 'POST' }, '').html; + expect(html2).toContain('action="/legacy"'); + }); +}); diff --git a/craft/src/utils/form-relay-wiring.ts b/craft/src/utils/form-relay-wiring.ts index 9831807..b80ef37 100644 --- a/craft/src/utils/form-relay-wiring.ts +++ b/craft/src/utils/form-relay-wiring.ts @@ -9,7 +9,7 @@ * HTML — see PR #47 review). */ -import { escapeAttr } from './escape'; +import { escapeAttr, safeUrl } from './escape'; export interface RelayWiring { /** true when a recipient is set (relay path); false = legacy formAction fallback */ @@ -33,7 +33,7 @@ export function relayFormWiring( fallbackAction: string | undefined, ): RelayWiring { if (!recipientEmail) { - return { useRelay: false, marker: '', actionAttr: escapeAttr(fallbackAction || '#'), honeypot: '' }; + return { useRelay: false, marker: '', actionAttr: escapeAttr(safeUrl(fallbackAction || '#')), honeypot: '' }; } const fid = 'F' + Math.random().toString(36).slice(2, 8); return { From 71792870872d1907d7bfe4ce022edfc9f246dde1 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:06:07 -0700 Subject: [PATCH 07/61] fix(builder): sanitize HtmlBlock/Countdown/Gallery JS contexts Entity-escaping alone doesn't protect JS-string or raw-HTML sinks: - HtmlBlock.toHtml exported props.code raw; now runs it through the same purifyHtml (DOMPurify) config already used for the live editor preview, so

hi

' }, ''); + expect(html).not.toContain('hi

'); + }); +}); diff --git a/craft/src/components/basic/HtmlBlock.tsx b/craft/src/components/basic/HtmlBlock.tsx index b0fd034..a5b05df 100644 --- a/craft/src/components/basic/HtmlBlock.tsx +++ b/craft/src/components/basic/HtmlBlock.tsx @@ -140,6 +140,7 @@ HtmlBlock.craft = { /* ---------- HTML export ---------- */ (HtmlBlock as any).toHtml = (props: HtmlBlockProps, _childrenHtml: string) => { - // Output the raw code as-is - return { html: props.code || '' }; + // Run through the same DOMPurify config used for the live editor preview + // so exported pages can't carry `; } return { html: ` -
+ ${items}
${lightboxHtml} `, From fb4e9f87be492f069960a40e61d610af888b8084 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:06:16 -0700 Subject: [PATCH 08/61] fix(builder): sanitize style-string emission cssPropsToString() joined raw CSSProperties values into a style="..." attribute with zero escaping, so any component spreading user-controlled values into inline styles (background-image url(), etc.) could break out of the attribute or inject a second declaration -- this is what made BackgroundSection/HeroSimple/CallToAction/Section's bg-image url() sites (flagged in the A3 brief) safe without needing a per-call- site fix, since they already route through this helper. Each string value is now sanitized: url(...) contents are validated through safeUrl and re-wrapped escaped, stray `;` (the only way to inject a second live declaration) is stripped, and any raw `"` is entity-encoded so it can't terminate the attribute early. Legitimate multi-part values (box-shadow, gradients) that contain none of these characters pass through byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/utils/style-helpers.test.ts | 38 +++++++++++++++++++++++++++ craft/src/utils/style-helpers.ts | 25 +++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 craft/src/utils/style-helpers.test.ts diff --git a/craft/src/utils/style-helpers.test.ts b/craft/src/utils/style-helpers.test.ts new file mode 100644 index 0000000..6b10569 --- /dev/null +++ b/craft/src/utils/style-helpers.test.ts @@ -0,0 +1,38 @@ +import { describe, test, expect } from 'vitest'; +import { cssPropsToString } from './style-helpers'; + +describe('cssPropsToString sanitizes emitted values (A5)', () => { + test('quote/semicolon breakout cannot inject a second property', () => { + const out = cssPropsToString({ color: 'red";background:url(javascript:alert(1))"' } as any); + // must not contain a raw double-quote (would break out of style="...") + expect(out).not.toContain('"'); + // The only `;` allowed to survive is the one embedded inside an HTML + // entity we generated ourselves (e.g. `"`, `'`) -- decode those + // away and confirm no *live* semicolon (a real declaration separator) + // remains, i.e. no second property was injected via the breakout. + const withoutEntities = out.replace(/&(?:quot|amp|#39|#96|#x[0-9a-f]+|#[0-9]+);/gi, ''); + expect(withoutEntities).not.toContain(';'); + expect(out).not.toContain('javascript:'); + }); + + test('javascript: inside url() is neutralized', () => { + const out = cssPropsToString({ backgroundImage: 'url(javascript:alert(1))' } as any); + expect(out).not.toContain('javascript:'); + }); + + test('normal box-shadow value is unchanged', () => { + const out = cssPropsToString({ boxShadow: '0 2px 4px rgba(0,0,0,.1)' } as any); + expect(out).toBe('box-shadow:0 2px 4px rgba(0,0,0,.1)'); + }); + + test('normal gradient value is unchanged', () => { + const out = cssPropsToString({ background: 'linear-gradient(135deg, #667eea 0%, #764ba2 100%)' } as any); + expect(out).toBe('background:linear-gradient(135deg, #667eea 0%, #764ba2 100%)'); + }); + + test('legitimate background-image url is preserved (quoted)', () => { + const out = cssPropsToString({ backgroundImage: 'url(https://example.com/img.jpg)' } as any); + expect(out).toContain('https://example.com/img.jpg'); + expect(out).not.toContain('javascript:'); + }); +}); diff --git a/craft/src/utils/style-helpers.ts b/craft/src/utils/style-helpers.ts index d84e586..d419fb2 100644 --- a/craft/src/utils/style-helpers.ts +++ b/craft/src/utils/style-helpers.ts @@ -1,13 +1,36 @@ import { CSSProperties } from 'react'; +import { escapeAttr, safeUrl } from './escape'; const camelToKebab = (str: string): string => str.replace(/[A-Z]/g, (m) => '-' + m.toLowerCase()); +const URL_RE = /url\(\s*(['"]?)([\s\S]*?)\1\s*\)/gi; + +/** + * Sanitizes a single CSS declaration value so it can never terminate the + * `style="..."` attribute early, inject an extra declaration via a stray + * `;`, or smuggle a `javascript:`/`vbscript:`/`data:text/html` URL through + * a `url(...)` reference. Legitimate multi-part values (box-shadow, + * gradients, etc.) that contain none of these characters pass through + * unchanged. + */ +function sanitizeCssValue(raw: string): string { + // Neutralize url(...) references: validate/strip the scheme and re-wrap + // in single quotes with the contents escaped for attribute safety. + let val = raw.replace(URL_RE, (_m, _q, inner) => `url('${escapeAttr(safeUrl(inner.trim()))}')`); + // A `;` in a CSS value is never legitimate (declarations are separated by + // it) -- stray semicolons are how a breakout injects a second property. + val = val.replace(/;/g, ''); + // Any remaining raw `"` would close the `style="..."` attribute early. + val = val.replace(/"/g, '"'); + return val; +} + export function cssPropsToString(style: CSSProperties | undefined): string { if (!style) return ''; return Object.entries(style) .filter(([, v]) => v !== undefined && v !== null && v !== '') - .map(([k, v]) => `${camelToKebab(k)}:${v}`) + .map(([k, v]) => `${camelToKebab(k)}:${typeof v === 'string' ? sanitizeCssValue(v) : v}`) .join(';'); } From 4c001e1af42e2faf9faca4cc439eb1998424c580 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:15:23 -0700 Subject: [PATCH 09/61] fix(builder): preserve data-URI semicolons in css value sanitizer sanitizeCssValue's blanket `;` strip ran on the whole value AFTER url(...) content was already safely re-wrapped, corrupting legitimate data:image/png;base64,... URLs pasted into Background Image fields (the MIME/base64 separator `;` was deleted, breaking the data URI in exported/published HTML). Scope the `;`/`"` breakout sanitization to the segments outside url(...) matches only -- the url() branch is already fully safe via escapeAttr(safeUrl(...)) and must not be re-stripped. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/utils/style-helpers.test.ts | 18 +++++++++++++ craft/src/utils/style-helpers.ts | 37 +++++++++++++++++++-------- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/craft/src/utils/style-helpers.test.ts b/craft/src/utils/style-helpers.test.ts index 6b10569..d79a2f6 100644 --- a/craft/src/utils/style-helpers.test.ts +++ b/craft/src/utils/style-helpers.test.ts @@ -35,4 +35,22 @@ describe('cssPropsToString sanitizes emitted values (A5)', () => { expect(out).toContain('https://example.com/img.jpg'); expect(out).not.toContain('javascript:'); }); + + test('data-URI background image survives with its semicolon separator intact (A3)', () => { + const out = cssPropsToString({ + backgroundImage: 'url(data:image/png;base64,iVBORw0KGgo=)', + } as any); + // The `;` between the MIME type and `base64` is a required part of the + // data-URI syntax -- it must not be stripped by breakout sanitization. + expect(out).toContain('data:image/png;base64,iVBORw0KGgo='); + }); + + test('quote/semicolon breakout is still neutralized alongside a url()', () => { + const out = cssPropsToString({ + color: 'red";background:url(x)"', + } as any); + expect(out).not.toContain('"'); + const withoutEntities = out.replace(/&(?:quot|amp|#39|#96|#x[0-9a-f]+|#[0-9]+);/gi, ''); + expect(withoutEntities).not.toContain(';'); + }); }); diff --git a/craft/src/utils/style-helpers.ts b/craft/src/utils/style-helpers.ts index d419fb2..9a84418 100644 --- a/craft/src/utils/style-helpers.ts +++ b/craft/src/utils/style-helpers.ts @@ -6,24 +6,41 @@ const camelToKebab = (str: string): string => const URL_RE = /url\(\s*(['"]?)([\s\S]*?)\1\s*\)/gi; +// Outside of a url(...) reference, a `;` is never legitimate (declarations +// are separated by it) -- stray semicolons are how a breakout injects a +// second property -- and a raw `"` would close the `style="..."` attribute +// early. Inside url('...') the content has already been made safe via +// escapeAttr(safeUrl(...)), including any `;` required by data-URI syntax +// (`data:;base64,`), so this must never be applied there. +const sanitizeBreakoutChars = (s: string): string => s.replace(/;/g, '').replace(/"/g, '"'); + /** * Sanitizes a single CSS declaration value so it can never terminate the * `style="..."` attribute early, inject an extra declaration via a stray * `;`, or smuggle a `javascript:`/`vbscript:`/`data:text/html` URL through * a `url(...)` reference. Legitimate multi-part values (box-shadow, * gradients, etc.) that contain none of these characters pass through - * unchanged. + * unchanged, and legitimate `;`-containing data-URIs inside url(...) are + * preserved intact. */ function sanitizeCssValue(raw: string): string { - // Neutralize url(...) references: validate/strip the scheme and re-wrap - // in single quotes with the contents escaped for attribute safety. - let val = raw.replace(URL_RE, (_m, _q, inner) => `url('${escapeAttr(safeUrl(inner.trim()))}')`); - // A `;` in a CSS value is never legitimate (declarations are separated by - // it) -- stray semicolons are how a breakout injects a second property. - val = val.replace(/;/g, ''); - // Any remaining raw `"` would close the `style="..."` attribute early. - val = val.replace(/"/g, '"'); - return val; + let out = ''; + let lastIndex = 0; + URL_RE.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = URL_RE.exec(raw)) !== null) { + // Sanitize breakout characters only in the segment before this url(...) + // reference -- never inside the reference itself. + out += sanitizeBreakoutChars(raw.slice(lastIndex, m.index)); + // Neutralize the url(...) reference: validate/strip the scheme and + // re-wrap in single quotes with the contents escaped for attribute + // safety. This is already fully safe, `;` and all. + const inner = m[2]; + out += `url('${escapeAttr(safeUrl(inner.trim()))}')`; + lastIndex = URL_RE.lastIndex; + } + out += sanitizeBreakoutChars(raw.slice(lastIndex)); + return out; } export function cssPropsToString(style: CSSProperties | undefined): string { From 97123c4c581ec46c9a9f5b4872129dc779e82bff Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:18:44 -0700 Subject: [PATCH 10/61] fix(builder): regenerate node ids on duplicate/paste to prevent state corruption Craft.js duplicate (ContextMenu + keyboard shortcut) and paste were reusing the original node's toNodeTree() output verbatim, so addNodeTree() inserted duplicate node ids into the editor tree. Added regenerateTreeIds() which deep-clones a NodeTree and remaps rootNodeId, node map keys, node.id, internal node.data.parent, node.data.nodes, and node.data.linkedNodes via Craft.js's own getRandomId(). Also fixed pasteNode to insert as a sibling of the right-clicked node (using its parent) instead of using a leaf node as the new parent, which previously threw. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/hooks/useKeyboardShortcuts.ts | 3 +- craft/src/panels/context-menu/ContextMenu.tsx | 30 ++-- craft/src/utils/craft-tree.test.ts | 132 ++++++++++++++++++ craft/src/utils/craft-tree.ts | 64 +++++++++ 4 files changed, 219 insertions(+), 10 deletions(-) create mode 100644 craft/src/utils/craft-tree.test.ts create mode 100644 craft/src/utils/craft-tree.ts diff --git a/craft/src/hooks/useKeyboardShortcuts.ts b/craft/src/hooks/useKeyboardShortcuts.ts index b920f5a..78a14e9 100644 --- a/craft/src/hooks/useKeyboardShortcuts.ts +++ b/craft/src/hooks/useKeyboardShortcuts.ts @@ -1,6 +1,7 @@ import { useEffect } from 'react'; import { useEditor } from '@craftjs/core'; import { findDeletableTarget } from '../utils/craft-helpers'; +import { regenerateTreeIds } from '../utils/craft-tree'; function isInputFocused(): boolean { const el = document.activeElement; @@ -73,7 +74,7 @@ export function useKeyboardShortcuts() { const node = query.node(nodeId).get(); const parentId = node?.data?.parent; if (parentId) { - const tree = query.node(nodeId).toNodeTree(); + const tree = regenerateTreeIds(query.node(nodeId).toNodeTree()); actions.addNodeTree(tree, parentId); } } diff --git a/craft/src/panels/context-menu/ContextMenu.tsx b/craft/src/panels/context-menu/ContextMenu.tsx index 284f99d..ef55d77 100644 --- a/craft/src/panels/context-menu/ContextMenu.tsx +++ b/craft/src/panels/context-menu/ContextMenu.tsx @@ -3,6 +3,7 @@ import { useEditor } from '@craftjs/core'; import { findDeletableTarget } from '../../utils/craft-helpers'; import { useSitesmithModal } from '../../state/SitesmithContext'; import { buildSitesmithTarget } from '../../utils/sitesmith-target'; +import { regenerateTreeIds } from '../../utils/craft-tree'; interface ContextMenuProps { visible: boolean; @@ -65,15 +66,11 @@ export const ContextMenu: React.FC = ({ const duplicate = useCallback(() => { if (!nodeId || nodeId === 'ROOT') return; try { - const tree = query.node(nodeId).toSerializedNode(); const parentId = getParentId(); if (!parentId) return; - // Get the full subtree - const freshTree = query.node(nodeId).toNodeTree(); - const clonedTree = query.parseSerializedNode(freshTree.nodes[freshTree.rootNodeId].data).toNode(); - - actions.addNodeTree(freshTree, parentId); + const tree = regenerateTreeIds(query.node(nodeId).toNodeTree()); + actions.addNodeTree(tree, parentId); } catch (e) { console.error('Duplicate failed:', e); } @@ -92,10 +89,25 @@ export const ContextMenu: React.FC = ({ const pasteNode = useCallback(() => { const sourceId = clipboardRef.current; - if (!sourceId) return; + if (!sourceId) { + onClose(); + return; + } try { - const targetParent = nodeId || 'ROOT'; - const tree = query.node(sourceId).toNodeTree(); + if (!query.node(sourceId).get()) { + onClose(); + return; + } + + // Paste as a SIBLING of the right-clicked node, not as its child -- + // using the clicked node itself as the parent throws when it's a leaf. + let targetParent = 'ROOT'; + if (nodeId && nodeId !== 'ROOT') { + const clickedNode = query.node(nodeId).get(); + targetParent = clickedNode?.data?.parent || 'ROOT'; + } + + const tree = regenerateTreeIds(query.node(sourceId).toNodeTree()); actions.addNodeTree(tree, targetParent); } catch (e) { console.error('Paste failed:', e); diff --git a/craft/src/utils/craft-tree.test.ts b/craft/src/utils/craft-tree.test.ts new file mode 100644 index 0000000..859d786 --- /dev/null +++ b/craft/src/utils/craft-tree.test.ts @@ -0,0 +1,132 @@ +import { describe, test, expect } from 'vitest'; +import type { NodeTree, Node } from '@craftjs/core'; +import { regenerateTreeIds } from './craft-tree'; + +function makeNode(overrides: Partial): Node { + return { + id: 'placeholder', + data: { + props: {}, + type: 'div', + name: 'div', + displayName: 'div', + isCanvas: false, + parent: null, + linkedNodes: {}, + nodes: [], + hidden: false, + }, + info: {}, + events: { selected: false, dragged: false, hovered: false }, + dom: null, + related: {}, + rules: { + canDrag: () => true, + canDrop: () => true, + canMoveIn: () => true, + canMoveOut: () => true, + }, + _hydrationTimestamp: 0, + ...overrides, + } as Node; +} + +function makeTree(): NodeTree { + // root -> [childA, childB]; root also has a linkedNodes entry -> childB + const root = makeNode({ + id: 'root-1', + data: { + ...makeNode({}).data, + parent: null, + nodes: ['child-a-1'], + linkedNodes: { slot: 'child-b-1' }, + }, + }); + const childA = makeNode({ + id: 'child-a-1', + data: { + ...makeNode({}).data, + parent: 'root-1', + nodes: [], + linkedNodes: {}, + }, + }); + const childB = makeNode({ + id: 'child-b-1', + data: { + ...makeNode({}).data, + parent: 'root-1', + nodes: [], + linkedNodes: {}, + }, + }); + + return { + rootNodeId: 'root-1', + nodes: { + 'root-1': root, + 'child-a-1': childA, + 'child-b-1': childB, + }, + }; +} + +describe('regenerateTreeIds', () => { + test('produces an id set fully disjoint from the input', () => { + const input = makeTree(); + const inputIds = Object.keys(input.nodes); + const output = regenerateTreeIds(input); + const outputIds = Object.keys(output.nodes); + + for (const id of outputIds) { + expect(inputIds).not.toContain(id); + } + }); + + test('rootNodeId is a key in nodes and equals that node id', () => { + const output = regenerateTreeIds(makeTree()); + expect(output.nodes[output.rootNodeId]).toBeDefined(); + expect(output.nodes[output.rootNodeId].id).toBe(output.rootNodeId); + }); + + test('every child data.parent points to a NEW id that exists in the output', () => { + const output = regenerateTreeIds(makeTree()); + for (const id of Object.keys(output.nodes)) { + const node = output.nodes[id]; + if (node.data.parent !== null) { + expect(output.nodes[node.data.parent]).toBeDefined(); + } + } + // specifically the two children under the (new) root + const newRoot = output.nodes[output.rootNodeId]; + expect(newRoot.data.nodes.length).toBe(1); + const newChildAId = newRoot.data.nodes[0]; + expect(output.nodes[newChildAId].data.parent).toBe(output.rootNodeId); + }); + + test('linkedNodes values are remapped to existing new ids', () => { + const output = regenerateTreeIds(makeTree()); + const newRoot = output.nodes[output.rootNodeId]; + const linkedId = newRoot.data.linkedNodes.slot; + expect(linkedId).toBeDefined(); + expect(output.nodes[linkedId]).toBeDefined(); + // linked node's parent should still reference the new root + expect(output.nodes[linkedId].data.parent).toBe(output.rootNodeId); + }); + + test('structure/count is preserved', () => { + const input = makeTree(); + const output = regenerateTreeIds(input); + expect(Object.keys(output.nodes).length).toBe(Object.keys(input.nodes).length); + expect(output.nodes[output.rootNodeId].data.nodes.length).toBe( + input.nodes[input.rootNodeId].data.nodes.length + ); + }); + + test('does not mutate the input tree', () => { + const input = makeTree(); + const snapshot = JSON.parse(JSON.stringify(input)); + regenerateTreeIds(input); + expect(JSON.parse(JSON.stringify(input))).toEqual(snapshot); + }); +}); diff --git a/craft/src/utils/craft-tree.ts b/craft/src/utils/craft-tree.ts new file mode 100644 index 0000000..a4c3d72 --- /dev/null +++ b/craft/src/utils/craft-tree.ts @@ -0,0 +1,64 @@ +import type { NodeTree, Node, NodeId } from '@craftjs/core'; +import { getRandomId } from '@craftjs/utils'; + +/** + * Returns a deep-cloned NodeTree where every node id in the tree + * (rootNodeId, the `nodes` map keys, each `node.id`, each internal + * `node.data.parent`, every `node.data.nodes` entry, and every + * `node.data.linkedNodes` value) has been remapped to a fresh, + * collision-free id generated by Craft.js's own id generator. + * + * The subtree root's `data.parent` is deliberately left untouched -- + * `actions.addNodeTree(tree, parentId)` sets that itself when the tree is + * attached to its new parent. + * + * The input tree is not mutated. + */ +export function regenerateTreeIds(tree: NodeTree): NodeTree { + const oldIds = Object.keys(tree.nodes); + + // Build old -> new id map first so all cross-references can be rewritten + // consistently regardless of iteration order. + const idMap = new Map(); + for (const oldId of oldIds) { + idMap.set(oldId, getRandomId()); + } + + const remapId = (id: NodeId): NodeId => idMap.get(id) ?? id; + + const newNodes: Record = {}; + for (const oldId of oldIds) { + const oldNode = tree.nodes[oldId]; + const newId = idMap.get(oldId)!; + + const newParent = + oldId === tree.rootNodeId + ? oldNode.data.parent // subtree root's parent is set by the caller + : oldNode.data.parent !== null && oldNode.data.parent !== undefined + ? remapId(oldNode.data.parent) + : oldNode.data.parent; + + const newLinkedNodes: Record = {}; + for (const [slot, linkedId] of Object.entries(oldNode.data.linkedNodes || {})) { + newLinkedNodes[slot] = remapId(linkedId); + } + + const newNode: Node = { + ...oldNode, + id: newId, + data: { + ...oldNode.data, + parent: newParent, + nodes: (oldNode.data.nodes || []).map(remapId), + linkedNodes: newLinkedNodes, + }, + }; + + newNodes[newId] = newNode; + } + + return { + rootNodeId: remapId(tree.rootNodeId), + nodes: newNodes, + }; +} From 1a88baa95ded3662e1e68ac162e30a7bef83cede Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:25:22 -0700 Subject: [PATCH 11/61] fix(builder): deep-clone node data on id regeneration to avoid shared props regenerateTreeIds shallow-copied each node's data, leaving data.props (and data.custom) as the same object reference between the original node and its duplicate/pasted copy. Craft.js's setProp mutates data.props in place, so editing the duplicate's props silently mutated the original too. Deep-clone data via structuredClone before applying id remaps so no mutable sub-object is shared between original and regenerated nodes. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/utils/craft-tree.test.ts | 17 +++++++++++++++++ craft/src/utils/craft-tree.ts | 9 ++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/craft/src/utils/craft-tree.test.ts b/craft/src/utils/craft-tree.test.ts index 859d786..fe6ce06 100644 --- a/craft/src/utils/craft-tree.test.ts +++ b/craft/src/utils/craft-tree.test.ts @@ -129,4 +129,21 @@ describe('regenerateTreeIds', () => { regenerateTreeIds(input); expect(JSON.parse(JSON.stringify(input))).toEqual(snapshot); }); + + test('output node data.props is a deep clone, not shared with the input node', () => { + const input = makeTree(); + input.nodes['root-1'].data.props = { alignment: 'left' }; + + const output = regenerateTreeIds(input); + const outputRoot = output.nodes[output.rootNodeId]; + + // Clone must preserve content at the time of cloning. + expect(outputRoot.data.props).toEqual({ alignment: 'left' }); + + // Mutating the output's props (simulating Craft.js's in-place setProp) + // must NOT affect the input node's props object. + (outputRoot.data.props as { alignment: string }).alignment = 'right'; + + expect(input.nodes['root-1'].data.props).toEqual({ alignment: 'left' }); + }); }); diff --git a/craft/src/utils/craft-tree.ts b/craft/src/utils/craft-tree.ts index a4c3d72..aa08659 100644 --- a/craft/src/utils/craft-tree.ts +++ b/craft/src/utils/craft-tree.ts @@ -43,11 +43,18 @@ export function regenerateTreeIds(tree: NodeTree): NodeTree { newLinkedNodes[slot] = remapId(linkedId); } + // Deep-clone data so mutable sub-objects (props, custom, etc.) are never + // shared by reference between the original node and its regenerated + // copy. Craft.js's setProp mutates data.props in place, so a shallow + // copy here would let edits to the duplicate silently corrupt the + // original. + const clonedData = structuredClone(oldNode.data); + const newNode: Node = { ...oldNode, id: newId, data: { - ...oldNode.data, + ...clonedData, parent: newParent, nodes: (oldNode.data.nodes || []).map(remapId), linkedNodes: newLinkedNodes, From 4b36ce0d6aa2eab4870bb14020a5aa3b1c8b9af2 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:28:38 -0700 Subject: [PATCH 12/61] fix(builder): route live header/footer edits correctly on save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editing the Header/Footer sets activePageId to '__header__'/'__footer__', which matches no entry in `pages`. save() was serializing the live canvas into the top-level page slots (mislabeled as page content, matching no page) while exporting header/footer from stale stored state — auto-save every 30s silently dropped header/footer edits. Extract buildSavePayload() as a pure, unit-tested helper: header/footer craft state now comes from the live canvas when that zone is being edited (else stored state), and the top-level page fields fall back to the landing page's stored state when a header/footer zone is active, so page content is never clobbered or mislabeled. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/hooks/useWhpApi.save.test.ts | 90 +++++++++ craft/src/hooks/useWhpApi.ts | 263 +++++++++++++++++-------- 2 files changed, 269 insertions(+), 84 deletions(-) create mode 100644 craft/src/hooks/useWhpApi.save.test.ts diff --git a/craft/src/hooks/useWhpApi.save.test.ts b/craft/src/hooks/useWhpApi.save.test.ts new file mode 100644 index 0000000..d8447af --- /dev/null +++ b/craft/src/hooks/useWhpApi.save.test.ts @@ -0,0 +1,90 @@ +import { describe, test, expect } from 'vitest'; +import { buildSavePayload } from './useWhpApi'; +import { PageData } from '../types'; + +const pageA: PageData = { id: 'home', name: 'Home', slug: 'index', craftState: 'STORED_HOME', headCode: '' }; +const pageB: PageData = { id: 'page_2', name: 'About', slug: 'about', craftState: 'STORED_ABOUT', headCode: '' }; +const headerPage: PageData = { id: '__header__', name: 'Header', slug: '__header__', craftState: 'STALE_HEADER', headCode: '' }; +const footerPage: PageData = { id: '__footer__', name: 'Footer', slug: '__footer__', craftState: 'STALE_FOOTER', headCode: '' }; + +describe('buildSavePayload', () => { + test('editing header: live serialize lands in header_craft_state, not in any page slot', () => { + const payload = buildSavePayload({ + siteId: 1, + siteName: 'Test Site', + liveCraftState: 'LIVE_HEADER', + pages: [pageA, pageB], + headerPage, + footerPage, + activePageId: '__header__', + isEditingHeader: true, + isEditingFooter: false, + }); + + // The fresh live canvas must be reflected in header_craft_state, not the stale stored one. + expect(payload.header_craft_state).toBe('LIVE_HEADER'); + expect(payload.header_craft_state).not.toBe('STALE_HEADER'); + + // Footer must remain untouched (stored state, since we're not editing it). + expect(payload.footer_craft_state).toBe('STALE_FOOTER'); + + // Live header content must never leak into a page slot or the top-level page fields. + expect(payload.craft_state).not.toBe('LIVE_HEADER'); + for (const p of payload.pages_craft_state) { + expect(p.craftState).not.toBe('LIVE_HEADER'); + } + // Top-level page fields should fall back to the landing page's own stored state. + expect(payload.craft_state).toBe('STORED_HOME'); + + // Per-page slots must reflect each page's own stored state, untouched. + expect(payload.pages_craft_state.find((p) => p.id === 'home')?.craftState).toBe('STORED_HOME'); + expect(payload.pages_craft_state.find((p) => p.id === 'page_2')?.craftState).toBe('STORED_ABOUT'); + }); + + test('editing footer: live serialize lands in footer_craft_state, not in any page slot', () => { + const payload = buildSavePayload({ + siteId: 1, + siteName: 'Test Site', + liveCraftState: 'LIVE_FOOTER', + pages: [pageA, pageB], + headerPage, + footerPage, + activePageId: '__footer__', + isEditingHeader: false, + isEditingFooter: true, + }); + + expect(payload.footer_craft_state).toBe('LIVE_FOOTER'); + expect(payload.footer_craft_state).not.toBe('STALE_FOOTER'); + expect(payload.header_craft_state).toBe('STALE_HEADER'); + + expect(payload.craft_state).not.toBe('LIVE_FOOTER'); + for (const p of payload.pages_craft_state) { + expect(p.craftState).not.toBe('LIVE_FOOTER'); + } + }); + + test('editing a real page: behavior unchanged — live serialize goes to that page + top-level, header/footer come from stored state', () => { + const payload = buildSavePayload({ + siteId: 1, + siteName: 'Test Site', + liveCraftState: 'LIVE_PAGE_HOME', + pages: [pageA, pageB], + headerPage, + footerPage, + activePageId: 'home', + isEditingHeader: false, + isEditingFooter: false, + }); + + // Live canvas goes to the active page and top-level slots. + expect(payload.craft_state).toBe('LIVE_PAGE_HOME'); + expect(payload.pages_craft_state.find((p) => p.id === 'home')?.craftState).toBe('LIVE_PAGE_HOME'); + // Other pages keep their stored state. + expect(payload.pages_craft_state.find((p) => p.id === 'page_2')?.craftState).toBe('STORED_ABOUT'); + + // Header/footer come from stored (stale-but-correct, since we're not editing them) state. + expect(payload.header_craft_state).toBe('STALE_HEADER'); + expect(payload.footer_craft_state).toBe('STALE_FOOTER'); + }); +}); diff --git a/craft/src/hooks/useWhpApi.ts b/craft/src/hooks/useWhpApi.ts index 6492721..c76bf45 100644 --- a/craft/src/hooks/useWhpApi.ts +++ b/craft/src/hooks/useWhpApi.ts @@ -3,21 +3,87 @@ import { useEditor } from '@craftjs/core'; import { useEditorConfig } from '../state/EditorConfigContext'; import { usePages } from '../state/PageContext'; import { exportBodyHtml } from '../utils/html-export'; +import { PageData } from '../types'; -export function useWhpApi() { - const { query, actions } = useEditor(); - const { whpConfig, isWHP } = useEditorConfig(); - const { pages, headerPage, footerPage, activePageId, setHeaderCraftState, setFooterCraftState, setPagesCraftState } = usePages(); +export interface BuildSavePayloadInput { + siteId: number; + siteName: string; + /** query.serialize() of whatever is currently on the live canvas. */ + liveCraftState: string; + pages: PageData[]; + headerPage: PageData; + footerPage: PageData; + activePageId: string; + /** True when the live canvas is showing the header zone (activePageId === '__header__'). */ + isEditingHeader: boolean; + /** True when the live canvas is showing the footer zone (activePageId === '__footer__'). */ + isEditingFooter: boolean; +} - const save = useCallback(async () => { - if (!isWHP || !whpConfig) return null; +/** + * Pure payload builder for the save() API call. Extracted so the + * header/footer-vs-page routing logic can be unit-tested without mounting + * React/Craft.js. + * + * Bug this fixes: when the user is editing the Header or Footer, + * `activePageId` is `'__header__'`/`'__footer__'` — which matches no entry + * in `pages`. The live canvas serialization must be routed into + * `header_craft_state`/`footer_craft_state` in that case, NOT into a page + * slot or the top-level `craft_state`/`html` (which must always represent + * an actual page). Conversely, header/footer must be sourced from the FRESH + * live state when that zone is being edited, not from the stale stored + * `headerPage.craftState`/`footerPage.craftState`. + */ +export function buildSavePayload(input: BuildSavePayloadInput) { + const { + siteId, + siteName, + liveCraftState, + pages, + headerPage, + footerPage, + activePageId, + isEditingHeader, + isEditingFooter, + } = input; - // Serialize the current canvas state (whatever page is active) - const currentCraftState = query.serialize(); + const isPageActive = !isEditingHeader && !isEditingFooter; - // Export body HTML for the current page - let currentHtml = ''; - let css = ''; + // Fresh header/footer state: the live canvas wins when that zone is the + // one currently being edited; otherwise fall back to the last-committed + // stored state (updated on zone switch by PageContext's saveCurrentState). + const headerCraftState = isEditingHeader ? liveCraftState : (headerPage.craftState || null); + const footerCraftState = isEditingFooter ? liveCraftState : (footerPage.craftState || null); + + let headerHtml = ''; + try { + if (headerCraftState) { + headerHtml = exportBodyHtml(headerCraftState).html; + } + } catch (e) { + console.error('Header HTML export failed:', e); + } + + let footerHtml = ''; + try { + if (footerCraftState) { + footerHtml = exportBodyHtml(footerCraftState).html; + } + } catch (e) { + console.error('Footer HTML export failed:', e); + } + + // The top-level `craft_state`/`html` fields (and the matching per-page + // slot below) must always represent an actual PAGE. When editing the + // header/footer, activePageId matches no page — use the landing page's + // own stored state instead of leaking the live header/footer canvas into + // a page slot or mislabeling it as page content. + let currentCraftState: string | null; + let currentHtml = ''; + let css = ''; + + if (isPageActive) { + currentCraftState = liveCraftState; try { const result = exportBodyHtml(currentCraftState); currentHtml = result.html; @@ -25,84 +91,113 @@ export function useWhpApi() { } catch (e) { console.error('HTML export failed, saving state only:', e); } - - // Export header HTML from its craft state - let headerHtml = ''; + } else { + const landingPage = pages[0] ?? null; + currentCraftState = landingPage?.craftState ?? null; try { - if (headerPage.craftState) { - const hResult = exportBodyHtml(headerPage.craftState); - headerHtml = hResult.html; + if (currentCraftState) { + const result = exportBodyHtml(currentCraftState); + currentHtml = result.html; + css = result.css; } } catch (e) { - console.error('Header HTML export failed:', e); + console.error('HTML export failed, saving state only:', e); + } + } + + // Build the pages array with HTML for each page. For the active page (only + // possible when a real page is active), use the freshly exported HTML from + // the canvas; for others, export from their stored craft state. + const pagesPayload = pages.map((page, i) => { + // The first page is ALWAYS the landing page → publishes to index.html + // regardless of the page name/slug. Apache serves '/' from index.html, + // and renaming the first page should not break the root URL. + const filename = i === 0 ? 'index.html' : page.slug + '.html'; + let pageHtml = ''; + + if (isPageActive && page.id === activePageId) { + // Active page: use the current canvas HTML (already exported above) + pageHtml = currentHtml; + } else if (page.craftState) { + try { + pageHtml = exportBodyHtml(page.craftState).html; + } catch (e) { + console.error(`HTML export failed for page ${page.name}:`, e); + } } - // Export footer HTML from its craft state - let footerHtml = ''; - try { - if (footerPage.craftState) { - const fResult = exportBodyHtml(footerPage.craftState); - footerHtml = fResult.html; - } - } catch (e) { - console.error('Footer HTML export failed:', e); - } - - // Build the pages array with HTML for each page - // For the active page, use the freshly exported HTML from the canvas; - // for others, export from their stored craft state - const pagesPayload = pages.map((page, i) => { - // The first page is ALWAYS the landing page → publishes to index.html - // regardless of the page name/slug. Apache serves '/' from index.html, - // and renaming the first page should not break the root URL. - const filename = i === 0 ? 'index.html' : page.slug + '.html'; - let pageHtml = ''; - - if (page.id === activePageId) { - // Active page: use the current canvas HTML (already exported above) - pageHtml = currentHtml; - } else if (page.craftState) { - try { - const pResult = exportBodyHtml(page.craftState); - pageHtml = pResult.html; - } catch (e) { - console.error(`HTML export failed for page ${page.name}:`, e); - } - } - - return { - filename, - title: page.name, - html: pageHtml, - }; - }); - - // Build pages_craft_state array: for each page, store its craft state - // For the currently active page, always use the fresh canvas state (currentCraftState) - // since page.craftState may be stale (not updated until page switch) - const pagesGrapesjs = pages.map((page, i) => ({ - id: page.id, - name: page.name, - // Pin the landing page's slug to 'index' on the wire too, so that on - // reload the editor's clean-URL routing (.htaccess rewrite of /name → - // name.html) lines up with the file we just wrote (index.html). - slug: i === 0 ? 'index' : page.slug, - craftState: page.id === activePageId ? currentCraftState : (page.craftState || null), - })); - - const payload = { - site_id: whpConfig.siteId, - name: whpConfig.siteName, - html: currentHtml, - css, - pages: pagesPayload, - header_html: headerHtml, - footer_html: footerHtml, - craft_state: currentCraftState, - header_craft_state: headerPage.craftState || null, - footer_craft_state: footerPage.craftState || null, - pages_craft_state: pagesGrapesjs, + return { + filename, + title: page.name, + html: pageHtml, }; + }); + + // Build pages_craft_state array: for each page, store its craft state. + // For the currently active page (only when a real page is active), always + // use the fresh canvas state since page.craftState may be stale (not + // updated until page switch). When editing header/footer, activePageId + // matches no page, so every page correctly falls back to its own stored + // state below. + const pagesGrapesjs = pages.map((page, i) => ({ + id: page.id, + name: page.name, + // Pin the landing page's slug to 'index' on the wire too, so that on + // reload the editor's clean-URL routing (.htaccess rewrite of /name → + // name.html) lines up with the file we just wrote (index.html). + slug: i === 0 ? 'index' : page.slug, + craftState: (isPageActive && page.id === activePageId) ? liveCraftState : (page.craftState || null), + })); + + return { + site_id: siteId, + name: siteName, + html: currentHtml, + css, + pages: pagesPayload, + header_html: headerHtml, + footer_html: footerHtml, + craft_state: currentCraftState, + header_craft_state: headerCraftState, + footer_craft_state: footerCraftState, + pages_craft_state: pagesGrapesjs, + }; +} + +export function useWhpApi() { + const { query, actions } = useEditor(); + const { whpConfig, isWHP } = useEditorConfig(); + const { + pages, + headerPage, + footerPage, + activePageId, + isEditingHeader, + isEditingFooter, + setHeaderCraftState, + setFooterCraftState, + setPagesCraftState, + } = usePages(); + + const save = useCallback(async () => { + if (!isWHP || !whpConfig) return null; + + // Serialize whatever is currently on the live canvas (a page, the + // header, or the footer — depending on activePageId/isEditingHeader/ + // isEditingFooter). + const liveCraftState = query.serialize(); + + const payload = buildSavePayload({ + siteId: whpConfig.siteId, + siteName: whpConfig.siteName, + liveCraftState, + pages, + headerPage, + footerPage, + activePageId, + isEditingHeader, + isEditingFooter, + }); const resp = await fetch(`${whpConfig.apiUrl}?action=save`, { method: 'POST', @@ -113,7 +208,7 @@ export function useWhpApi() { body: JSON.stringify(payload), }); return resp.json(); - }, [isWHP, whpConfig, query, pages, activePageId, headerPage, footerPage]); + }, [isWHP, whpConfig, query, pages, activePageId, headerPage, footerPage, isEditingHeader, isEditingFooter]); const publish = useCallback(async () => { if (!isWHP || !whpConfig) return null; From b46b6915a5f9e421552356eb9fdad217b0ee306e Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:37:25 -0700 Subject: [PATCH 13/61] fix(builder): validate AI response before deserializing into craft state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an AI-boundary validation/sanitization pass in apply-ai-response.ts so malformed or malicious Sitesmith output can never crash addNodeTree or corrupt live craft state: - resolvedName allowlist: any node whose type.resolvedName is not a registered component in componentResolver is dropped (with its subtree) and a console.warn is logged; buildNodeTree throws before touching Craft.js if the tree ROOT itself is invalid, which existing call sites already catch and warn on (fail soft, never throws into addNodeTree). - update_props protected-key list (node_id) — an AI-supplied node_id can no longer overwrite the target node's real id; style is merged shallowly instead of replaced wholesale so unrelated existing style keys survive. - id policy: regenerate, never skip. Any AI-supplied node id that is 'ROOT', empty/non-string, or collides with an existing/already-used id is replaced with a fresh ai-auto-N id; the node itself is kept. Extends apply-ai-response.test.ts with coverage for all of the above plus regression tests proving fully-valid AI responses still apply exactly as before. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/utils/apply-ai-response.test.ts | 181 +++++++++++++++++++++- craft/src/utils/apply-ai-response.ts | 125 ++++++++++++++- 2 files changed, 298 insertions(+), 8 deletions(-) diff --git a/craft/src/utils/apply-ai-response.test.ts b/craft/src/utils/apply-ai-response.test.ts index e2e37fb..13bc8e7 100644 --- a/craft/src/utils/apply-ai-response.test.ts +++ b/craft/src/utils/apply-ai-response.test.ts @@ -1,5 +1,5 @@ -import { describe, test, expect } from 'vitest'; -import { serializeTreeForCraft, __test } from './apply-ai-response'; +import { describe, test, expect, vi } from 'vitest'; +import { serializeTreeForCraft, sanitizeAiTree, __test } from './apply-ai-response'; describe('serializeTreeForCraft', () => { test('flattens nested tree', () => { @@ -88,3 +88,180 @@ describe('findNodeIdByAiNodeId', () => { expect(__test.findNodeIdByAiNodeId({ getNodes: () => ({}) }, 'missing')).toBe(null); }); }); + +// --------------------------------------------------------------------------- +// Helpers for the tests below: fake @craftjs/core `query` / `actions` objects +// good enough to exercise buildNodeTree/applyPatch without a real Editor. +// --------------------------------------------------------------------------- + +function makeFakeQuery(existingIds: string[] = ['ROOT']) { + return { + getNodes: () => Object.fromEntries(existingIds.map((id) => [id, { data: { props: {} } }])), + parseFreshNode: (input: any) => ({ toNode: () => input }), + node: (id: string) => ({ + get: () => ({ data: { parent: 'ROOT' } }), + childNodes: () => [id], + }), + }; +} + +/** A fake query+actions pair backed by a single existing node, wired so + * findNodeIdByAiNodeId can resolve `aiNodeId` back to `craftId`. */ +function makeSingleNodeHarness(craftId: string, aiNodeId: string, initialProps: Record) { + const propsById: Record = { [craftId]: { node_id: aiNodeId, ...initialProps } }; + const query = { + getNodes: () => + Object.fromEntries(Object.entries(propsById).map(([id, p]) => [id, { data: { props: p } }])), + parseFreshNode: (input: any) => ({ toNode: () => input }), + node: (id: string) => ({ + get: () => ({ data: { parent: 'ROOT' } }), + childNodes: () => [craftId], + }), + }; + const actions = { + setProp: (id: string, fn: (p: any) => void) => { + if (propsById[id]) fn(propsById[id]); + }, + delete: (id: string) => { + delete propsById[id]; + }, + addNodeTree: () => {}, + }; + return { query, actions, propsById }; +} + +describe('sanitizeAiTree', () => { + test('unknown resolvedName at the root is rejected (returns null)', () => { + const tree = { type: { resolvedName: 'NotARealComponent' }, props: {}, nodes: [] }; + expect(sanitizeAiTree(tree, new Set())).toBeNull(); + }); + + test('unknown resolvedName on a nested child drops only that subtree; valid siblings survive', () => { + const tree = { + type: { resolvedName: 'Section' }, + props: { node_id: 's1' }, + nodes: [ + { type: { resolvedName: 'NotARealComponent' }, props: {}, nodes: [] }, + { type: { resolvedName: 'Heading' }, props: { node_id: 'h1' }, nodes: [] }, + ], + }; + const out = sanitizeAiTree(tree, new Set()); + expect(out).not.toBeNull(); + expect(out!.nodes!.length).toBe(1); + expect(out!.nodes![0].props.node_id).toBe('h1'); + }); + + test('a node id of "ROOT" is regenerated, never left as ROOT', () => { + const tree = { type: { resolvedName: 'Heading' }, props: { node_id: 'ROOT' }, nodes: [] }; + const out = sanitizeAiTree(tree, new Set(['ROOT'])); + expect(out).not.toBeNull(); + expect(out!.props.node_id).not.toBe('ROOT'); + }); + + test('an empty/non-string id is regenerated to a non-empty string', () => { + const tree = { type: { resolvedName: 'Heading' }, props: { node_id: '' }, nodes: [] }; + const out = sanitizeAiTree(tree, new Set()); + expect(typeof out!.props.node_id).toBe('string'); + expect(out!.props.node_id).toBeTruthy(); + }); + + test('an id colliding with an existing node is regenerated, not reused', () => { + const tree = { type: { resolvedName: 'Heading' }, props: { node_id: 'existing-1' }, nodes: [] }; + const out = sanitizeAiTree(tree, new Set(['existing-1'])); + expect(out!.props.node_id).not.toBe('existing-1'); + }); + + test('duplicate ids within the same tree are de-duplicated (no two nodes share an id)', () => { + const tree = { + type: { resolvedName: 'Section' }, + props: { node_id: 'dup-1' }, + nodes: [ + { type: { resolvedName: 'Heading' }, props: { node_id: 'dup-1' }, nodes: [] }, + ], + }; + const out = sanitizeAiTree(tree, new Set()); + expect(out!.props.node_id).toBe('dup-1'); + expect(out!.nodes![0].props.node_id).not.toBe('dup-1'); + }); + + test('a fully-valid tree passes through with structure and ids preserved (regression)', () => { + const tree = { + type: { resolvedName: 'Section' }, + props: { node_id: 'sec-1', aiName: 'Hero' }, + nodes: [ + { type: { resolvedName: 'Heading' }, props: { node_id: 'h-1', text: 'Welcome' }, nodes: [] }, + ], + }; + const out = sanitizeAiTree(tree, new Set()); + expect(out!.props.node_id).toBe('sec-1'); + expect(out!.nodes!.length).toBe(1); + expect(out!.nodes![0].props.node_id).toBe('h-1'); + expect(out!.nodes![0].props.text).toBe('Welcome'); + }); +}); + +describe('buildNodeTree', () => { + test('builds a NodeTree preserving structure for a fully valid AI tree (regression)', () => { + const tree = { + type: { resolvedName: 'Section' }, + props: { node_id: 'sec-1' }, + nodes: [ + { type: { resolvedName: 'Heading' }, props: { node_id: 'h-1', text: 'Hi' }, nodes: [] }, + ], + }; + const result = __test.buildNodeTree(makeFakeQuery(), tree); + expect(result.rootNodeId).toBe('sec-1'); + expect(result.nodes['sec-1']).toBeDefined(); + // Section wraps direct children in a pre-created section-inner linkedNode. + const innerId = result.nodes['sec-1'].data.linkedNodes['section-inner']; + expect(innerId).toBeDefined(); + expect(result.nodes[innerId].data.nodes).toEqual(['h-1']); + expect(result.nodes['h-1'].data.parent).toBe(innerId); + }); + + test('throws when the tree root has an unknown resolvedName (caught by callers, never reaches addNodeTree)', () => { + const tree = { type: { resolvedName: 'NotARealComponent' }, props: {}, nodes: [] }; + expect(() => __test.buildNodeTree(makeFakeQuery(), tree)).toThrow(); + }); +}); + +describe('applyPatch', () => { + test('update_props: does not overwrite node_id, applies a legit prop, merges style shallowly', () => { + const { query, actions, propsById } = makeSingleNodeHarness('craft-1', 'ai-node-1', { + text: 'Old', + style: { color: 'red', fontSize: '16px' }, + }); + const ops = [ + { + op: 'update_props', + node_id: 'ai-node-1', + props: { node_id: 'ai-hacked', text: 'New', style: { color: 'blue' } }, + }, + ]; + const result = __test.applyPatch(actions, query, ops); + expect(result.ok).toBe(true); + expect(propsById['craft-1'].node_id).toBe('ai-node-1'); // protected, unchanged + expect(propsById['craft-1'].text).toBe('New'); // legit prop applied + // style merges shallowly: color overwritten, fontSize preserved + expect(propsById['craft-1'].style).toEqual({ color: 'blue', fontSize: '16px' }); + }); + + test('an op with an unknown resolvedName tree is skipped without throwing; other ops in the batch still apply', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { query, actions, propsById } = makeSingleNodeHarness('craft-1', 'ai-node-1', {}); + const ops = [ + { + op: 'insert_after', + node_id: 'ai-node-1', + tree: { type: { resolvedName: 'NotARealComponent' }, props: {}, nodes: [] }, + }, + { op: 'delete_node', node_id: 'ai-node-1' }, + ]; + let result: { ok: boolean; message?: string } | undefined; + expect(() => { result = __test.applyPatch(actions, query, ops); }).not.toThrow(); + expect(result!.ok).toBe(true); + expect(propsById['craft-1']).toBeUndefined(); // delete_node (op 2) still applied + expect(warnSpy).toHaveBeenCalled(); // op 1's failure was logged, not thrown + warnSpy.mockRestore(); + }); +}); diff --git a/craft/src/utils/apply-ai-response.ts b/craft/src/utils/apply-ai-response.ts index c61f5d9..1a2894f 100644 --- a/craft/src/utils/apply-ai-response.ts +++ b/craft/src/utils/apply-ai-response.ts @@ -2,6 +2,7 @@ import { useEditor } from '@craftjs/core'; import type { NodeTree } from '@craftjs/core'; import { usePages } from '../state/PageContext'; import { SitesmithResponse, SerializedTreeNode } from '../types/sitesmith'; +import { componentResolver } from '../components/resolver'; /** Only Container is a "real" Craft.js canvas in serialized state. Layout * shells (Section/HeroSimple/ColumnLayout/etc) use linkedNodes @@ -16,6 +17,81 @@ const SHELL_INNER: Record = { FormContainer: 'form-inner', }; +/** + * update_props may never touch these keys, regardless of what the AI sends. + * `node_id` is the stable identifier other ops (and findNodeIdByAiNodeId) + * rely on to re-target a node in later turns of the conversation — letting + * the AI clobber it would silently orphan future patches. + */ +const PROTECTED_UPDATE_PROPS_KEYS = new Set(['node_id']); + +/** True if `name` is a registered component in the resolver (i.e. safe to + * hand to `actions.addNodeTree` without it throwing "does not exist in + * resolver" at render time). */ +function isKnownResolvedName(name: unknown): name is string { + return typeof name === 'string' && Object.prototype.hasOwnProperty.call(componentResolver, name); +} + +/** + * Validate + sanitize an AI-supplied tree before it is handed to + * `buildNodeTree`/`query.parseFreshNode`. Fails SOFT — never throws: + * + * - **resolvedName allowlist**: any node whose `type.resolvedName` is not a + * key of `componentResolver` is dropped (it and its subtree), with a + * `console.warn`. If the tree's ROOT is itself invalid, this returns + * `null` and the caller must skip the whole op/tree. + * - **id policy — REGENERATE (not skip)**: a node id that is `'ROOT'`, + * empty/non-string, or collides with an id already in `usedIds` is + * replaced with a fresh `ai-auto-N` id (and a warning is logged). The + * node itself is kept — only structurally invalid *types* are dropped; + * a merely-bad *id* is repaired so we never lose otherwise-valid AI + * content over an id collision. + * + * `usedIds` should be seeded with the live document's existing node ids + * (see `buildNodeTree`) so AI-supplied ids can never collide with content + * already on the canvas; it is also used internally to keep ids unique + * within the tree being sanitized. + */ +export function sanitizeAiTree( + tree: SerializedTreeNode, + usedIds: Set = new Set(), + idCounter: { n: number } = { n: 0 }, +): SerializedTreeNode | null { + const resolvedName = tree?.type?.resolvedName; + if (!isKnownResolvedName(resolvedName)) { + console.warn(`sitesmith: dropping node with unknown component "${String(resolvedName)}"`); + return null; + } + + const rawId = tree.props?.node_id; + let id = typeof rawId === 'string' ? rawId.trim() : ''; + if (!id) { + id = `ai-auto-${idCounter.n++}`; + } else if (id === 'ROOT') { + console.warn('sitesmith: rejecting AI-supplied node id "ROOT" (reserved); regenerating'); + id = `ai-auto-${idCounter.n++}`; + } else if (usedIds.has(id)) { + console.warn(`sitesmith: AI-supplied node id "${id}" collides with an existing id; regenerating`); + id = `ai-auto-${idCounter.n++}`; + } + // Belt-and-suspenders: keep drawing fresh ids in the (pathological) case + // the freshly generated id itself collides. + while (usedIds.has(id)) id = `ai-auto-${idCounter.n++}`; + usedIds.add(id); + + const children: SerializedTreeNode[] = []; + for (const child of tree.nodes ?? []) { + const sanitizedChild = sanitizeAiTree(child, usedIds, idCounter); + if (sanitizedChild) children.push(sanitizedChild); + } + + return { + type: tree.type, + props: { ...tree.props, node_id: id }, + nodes: children, + }; +} + /** * Flatten a SerializedTreeNode tree into a Craft.js node map ready for * `actions.deserialize()`. @@ -68,11 +144,25 @@ export function serializeTreeForCraft(tree: SerializedTreeNode): { rootNodeId: s * inserting/replacing sections or individual nodes. */ function buildNodeTree(query: any, tree: SerializedTreeNode): NodeTree { - const idCounter = { n: 0 }; + // Validate + sanitize at the AI boundary before touching Craft.js at all: + // unknown resolvedNames are dropped (never reach parseFreshNode/addNodeTree, + // which would throw), and ids are repaired ('ROOT'/empty/colliding → + // regenerated) against the live document's existing node ids so a new + // AI-supplied id can never clobber or duplicate one already on the canvas. + const existingIds = new Set( + typeof query?.getNodes === 'function' ? Object.keys(query.getNodes()) : [], + ); + const sanitized = sanitizeAiTree(tree, existingIds); + if (!sanitized) { + throw new Error('sitesmith: AI tree root has an unknown/invalid component type; nothing to build'); + } + const craftNodes: Record = {}; const walk = (node: SerializedTreeNode, parent: string | null): string => { - const id = (node.props.node_id as string | undefined) || `ai-auto-${idCounter.n++}`; + // sanitizeAiTree() has already guaranteed every node here has a valid, + // unique, non-'ROOT' node_id — no fallback/collision handling needed. + const id = node.props.node_id as string; const craftNode = (query.parseFreshNode({ id, data: { @@ -138,7 +228,7 @@ function buildNodeTree(query: any, tree: SerializedTreeNode): NodeTree { return id; }; - const rootId = walk(tree, null); + const rootId = walk(sanitized, null); return { rootNodeId: rootId, nodes: craftNodes }; } @@ -156,7 +246,7 @@ export function findNodeIdByAiNodeId(query: any, aiNodeId: string): string | nul } /** Exported for unit tests */ -export const __test = { findNodeIdByAiNodeId }; +export const __test = { findNodeIdByAiNodeId, buildNodeTree, applyPatch }; /** * React hook that returns an `apply` function. @@ -243,9 +333,32 @@ function applyPatch( } switch (op.op) { - case 'update_props': - actions.setProp(id, (p: any) => { Object.assign(p, op.props); }); + case 'update_props': { + const patchProps = op.props; + if (!patchProps || typeof patchProps !== 'object' || Array.isArray(patchProps)) { + console.warn('sitesmith patch: update_props with invalid props payload, skipping', op.node_id); + break; + } + actions.setProp(id, (p: any) => { + for (const [key, value] of Object.entries(patchProps)) { + if (PROTECTED_UPDATE_PROPS_KEYS.has(key)) { + console.warn(`sitesmith patch: refusing to overwrite protected prop "${key}"`); + continue; + } + // Merge `style` shallowly instead of blindly replacing it — the + // AI usually only means to change one or two style keys, and a + // blind Object.assign would clobber every other style the user + // (or a previous AI turn) had already set. + if (key === 'style' && value && typeof value === 'object' && !Array.isArray(value)) { + const existingStyle = p.style && typeof p.style === 'object' ? p.style : {}; + p.style = { ...existingStyle, ...(value as Record) }; + } else { + p[key] = value; + } + } + }); break; + } case 'replace_node': { try { From 296c10d019463a6c6b53b49eab1de5b4f14cc3c9 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:43:59 -0700 Subject: [PATCH 14/61] feat(builder): shared asset upload/list util Extract uploadAsset/listAssets into utils/assets.ts, lifting the exact uploadToWhp body and list_assets fetch pattern already duplicated across shared.tsx/ImageBlock/Logo/Navbar/etc. shared.tsx's uploadToWhp is now a thin re-export (`export const uploadToWhp = uploadAsset`) so its ~6 existing callers are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/panels/right/styles/shared.tsx | 23 +-- .../right/styles/shared.uploadToWhp.test.ts | 21 +++ craft/src/utils/assets.test.ts | 133 ++++++++++++++++++ craft/src/utils/assets.ts | 51 +++++++ 4 files changed, 211 insertions(+), 17 deletions(-) create mode 100644 craft/src/panels/right/styles/shared.uploadToWhp.test.ts create mode 100644 craft/src/utils/assets.test.ts create mode 100644 craft/src/utils/assets.ts diff --git a/craft/src/panels/right/styles/shared.tsx b/craft/src/panels/right/styles/shared.tsx index 6a5fba4..8b7c853 100644 --- a/craft/src/panels/right/styles/shared.tsx +++ b/craft/src/panels/right/styles/shared.tsx @@ -3,6 +3,7 @@ import { useEditor } from '@craftjs/core'; import { GRADIENTS, } from '../../../constants/presets'; +import { uploadAsset } from '../../../utils/assets'; /* ---------- Helper: auto text color for bg ---------- */ export function autoTextColor(bg: string): string { @@ -17,23 +18,11 @@ export function autoTextColor(bg: string): string { return '#ffffff'; } -/* ---------- Helper: upload to WHP ---------- */ -export async function uploadToWhp(file: File): Promise { - const cfg = (window as any).WHP_CONFIG; - if (!cfg) return URL.createObjectURL(file); - const formData = new FormData(); - formData.append('file', file); - try { - const resp = await fetch(`${cfg.apiUrl}?action=upload_asset&site_id=${cfg.siteId}`, { - method: 'POST', - headers: { 'X-CSRF-Token': cfg.csrfToken }, - body: formData, - }); - const data = await resp.json(); - if (data.success && data.url) return data.url; - return null; - } catch { return null; } -} +/* ---------- Helper: upload to WHP ---------- + Thin re-export over the shared `uploadAsset` util (`@/utils/assets`) so + the existing callers importing `uploadToWhp` from here keep working + unchanged. */ +export const uploadToWhp = uploadAsset; /* ---------- Shared inline styles ---------- */ export const labelStyle: CSSProperties = { fontSize: 11, color: '#a1a1aa', display: 'block', marginBottom: 4, textTransform: 'capitalize' }; diff --git a/craft/src/panels/right/styles/shared.uploadToWhp.test.ts b/craft/src/panels/right/styles/shared.uploadToWhp.test.ts new file mode 100644 index 0000000..4e75f65 --- /dev/null +++ b/craft/src/panels/right/styles/shared.uploadToWhp.test.ts @@ -0,0 +1,21 @@ +import { describe, test, expect, vi, afterEach } from 'vitest'; +import { uploadToWhp } from './shared'; +import { uploadAsset } from '../../../utils/assets'; + +describe('shared.uploadToWhp', () => { + afterEach(() => { + vi.unstubAllGlobals(); + delete (window as any).WHP_CONFIG; + }); + + test('is a thin re-export of uploadAsset (same function reference)', () => { + expect(uploadToWhp).toBe(uploadAsset); + }); + + test('still works standalone (no WHP_CONFIG -> blob: URL) for existing callers', async () => { + (URL as any).createObjectURL = vi.fn(() => 'blob:mock-url'); + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + const url = await uploadToWhp(file); + expect(url).toBe('blob:mock-url'); + }); +}); diff --git a/craft/src/utils/assets.test.ts b/craft/src/utils/assets.test.ts new file mode 100644 index 0000000..69bcc7c --- /dev/null +++ b/craft/src/utils/assets.test.ts @@ -0,0 +1,133 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import { uploadAsset, listAssets, Asset } from './assets'; + +const CFG = { + apiUrl: 'https://panel.example.com/api/site-builder', + siteId: 42, + csrfToken: 'tok-123', +}; + +function jsonResponse(body: any, ok = true) { + return { ok, json: async () => body } as Response; +} + +describe('uploadAsset', () => { + let createObjectURLSpy: ReturnType; + + beforeEach(() => { + createObjectURLSpy = vi.fn(() => 'blob:mock-url'); + (URL as any).createObjectURL = createObjectURLSpy; + vi.stubGlobal('fetch', vi.fn()); + delete (window as any).WHP_CONFIG; + }); + + afterEach(() => { + vi.unstubAllGlobals(); + delete (window as any).WHP_CONFIG; + }); + + test('without window.WHP_CONFIG returns a blob: URL and does not call fetch', async () => { + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + const url = await uploadAsset(file); + expect(url).toBe('blob:mock-url'); + expect(createObjectURLSpy).toHaveBeenCalledWith(file); + expect(fetch).not.toHaveBeenCalled(); + }); + + test('with config, POSTs to upload_asset with X-CSRF-Token and returns data.url', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockResolvedValue(jsonResponse({ success: true, url: '/storage/assets/photo.png' })); + + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + const url = await uploadAsset(file); + + expect(url).toBe('/storage/assets/photo.png'); + expect(fetchMock).toHaveBeenCalledTimes(1); + const [calledUrl, opts] = fetchMock.mock.calls[0]; + expect(calledUrl).toBe(`${CFG.apiUrl}?action=upload_asset&site_id=${CFG.siteId}`); + expect(opts.method).toBe('POST'); + expect(opts.headers['X-CSRF-Token']).toBe(CFG.csrfToken); + expect(opts.body).toBeInstanceOf(FormData); + }); + + test('with config, returns null when the server reports failure', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockResolvedValue(jsonResponse({ success: false })); + + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + expect(await uploadAsset(file)).toBeNull(); + }); + + test('with config, returns null when fetch throws', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockRejectedValue(new Error('network down')); + + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + expect(await uploadAsset(file)).toBeNull(); + }); +}); + +describe('listAssets', () => { + beforeEach(() => { + vi.stubGlobal('fetch', vi.fn()); + delete (window as any).WHP_CONFIG; + }); + + afterEach(() => { + vi.unstubAllGlobals(); + delete (window as any).WHP_CONFIG; + }); + + const ASSETS: Asset[] = [ + { name: 'a.png', url: '/storage/assets/a.png', type: 'image/png' }, + { name: 'b.mp4', url: '/storage/assets/b.mp4', type: 'video/mp4' }, + { name: 'c.pdf', url: '/storage/assets/c.pdf', type: 'application/pdf' }, + ]; + + test('without window.WHP_CONFIG returns [] and does not call fetch', async () => { + const assets = await listAssets('image'); + expect(assets).toEqual([]); + expect(fetch).not.toHaveBeenCalled(); + }); + + test('with config, filters by type prefix for the requested mediaType', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockResolvedValue(jsonResponse({ success: true, assets: ASSETS })); + + const images = await listAssets('image'); + expect(images).toEqual([ASSETS[0]]); + + const videos = await listAssets('video'); + expect(videos).toEqual([ASSETS[1]]); + + const [calledUrl] = fetchMock.mock.calls[0]; + expect(calledUrl).toBe(`${CFG.apiUrl}?action=list_assets&site_id=${CFG.siteId}`); + }); + + test('mediaType "any" (or omitted) returns all assets unfiltered', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockResolvedValue(jsonResponse({ success: true, assets: ASSETS })); + + expect(await listAssets('any')).toEqual(ASSETS); + expect(await listAssets()).toEqual(ASSETS); + }); + + test('with config, returns [] when the server reports failure or a non-array', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockResolvedValue(jsonResponse({ success: false })); + expect(await listAssets()).toEqual([]); + }); + + test('with config, returns [] when fetch throws', async () => { + (window as any).WHP_CONFIG = CFG; + const fetchMock = fetch as unknown as ReturnType; + fetchMock.mockRejectedValue(new Error('network down')); + expect(await listAssets()).toEqual([]); + }); +}); diff --git a/craft/src/utils/assets.ts b/craft/src/utils/assets.ts new file mode 100644 index 0000000..4d78276 --- /dev/null +++ b/craft/src/utils/assets.ts @@ -0,0 +1,51 @@ +/* ---------- Shared asset upload/list util ---------- + Single source of truth for talking to the WHP asset endpoints + (`upload_asset` / `list_assets`). Behavior is lifted verbatim from the + `uploadToWhp` helper duplicated across `shared.tsx` / `ImageBlock.tsx` / + `Logo.tsx` / `Navbar.tsx` / `VideoBlock.tsx` / `FeaturesGrid.tsx` and the + `list_assets` fetch pattern used in `ImageStylePanel.tsx` / `Logo.tsx` / + `Navbar.tsx` / `HeroStylePanel.tsx` / `ImageBlock.tsx` / `VideoBlock.tsx`, + so callers see identical behavior once switched over to this module. */ + +export interface Asset { + name: string; + url: string; + type: string; +} + +/** Upload a file to the WHP asset store. Falls back to a local `blob:` URL + * in standalone mode (no `window.WHP_CONFIG`). Returns `null` on failure. */ +export async function uploadAsset(file: File): Promise { + const cfg = (window as any).WHP_CONFIG; + if (!cfg) return URL.createObjectURL(file); + const formData = new FormData(); + formData.append('file', file); + try { + const resp = await fetch(`${cfg.apiUrl}?action=upload_asset&site_id=${cfg.siteId}`, { + method: 'POST', + headers: { 'X-CSRF-Token': cfg.csrfToken }, + body: formData, + }); + const data = await resp.json(); + if (data.success && data.url) return data.url; + return null; + } catch { return null; } +} + +/** List assets uploaded for the current site, optionally filtered by + * media type (matched against the asset's `type` prefix, e.g. + * `image/png`.startsWith('image')). Returns `[]` in standalone mode or + * on any failure. */ +export async function listAssets(mediaType: 'image' | 'video' | 'any' = 'any'): Promise { + const cfg = (window as any).WHP_CONFIG; + if (!cfg) return []; + try { + const resp = await fetch(`${cfg.apiUrl}?action=list_assets&site_id=${cfg.siteId}`); + const data = await resp.json(); + if (!data.success || !Array.isArray(data.assets)) return []; + if (mediaType === 'any') return data.assets; + return data.assets.filter((a: any) => (a.type || '').startsWith(mediaType)); + } catch { + return []; + } +} From c50b3856610b0698f254e40ab69e6865998b76f3 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 12:49:34 -0700 Subject: [PATCH 15/61] feat(builder): reusable AssetPicker (full/compact, media-typed) Introduces src/ui/AssetPicker.tsx, a single component that replaces the ~6 copy-pasted image-source UIs (upload/browse/URL) with one that also supports video via `mediaType`. Full variant mirrors ImageStylePanel's existing look; compact variant is a tight single row for array-editor item cards. Both share all state/handler logic, built on the C1 uploadAsset/listAssets util. Co-Authored-By: Claude Opus 4.8 (1M context) --- craft/src/ui/AssetPicker.test.tsx | 204 ++++++++++++++++++ craft/src/ui/AssetPicker.tsx | 337 ++++++++++++++++++++++++++++++ 2 files changed, 541 insertions(+) create mode 100644 craft/src/ui/AssetPicker.test.tsx create mode 100644 craft/src/ui/AssetPicker.tsx diff --git a/craft/src/ui/AssetPicker.test.tsx b/craft/src/ui/AssetPicker.test.tsx new file mode 100644 index 0000000..b7f908c --- /dev/null +++ b/craft/src/ui/AssetPicker.test.tsx @@ -0,0 +1,204 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { AssetPicker, acceptFor, matchesMediaType, isStandalone } from './AssetPicker'; +import type { Asset } from '../utils/assets'; + +vi.mock('../utils/assets', () => ({ + uploadAsset: vi.fn(), + listAssets: vi.fn(), +})); + +import { uploadAsset, listAssets } from '../utils/assets'; + +const uploadAssetMock = uploadAsset as unknown as ReturnType; +const listAssetsMock = listAssets as unknown as ReturnType; + +const GRID_ASSETS: Asset[] = [ + { name: 'a.png', url: '/storage/assets/a.png', type: 'image/png' }, + { name: 'b.mp4', url: '/storage/assets/b.mp4', type: 'video/mp4' }, +]; + +/* ---------- DOM test harness (no @testing-library/react in this repo, see + src/utils/assets.test.ts / navColorFields.test.ts for the plain-logic + pattern used elsewhere; component-level interaction here uses + react-dom/client + react-dom/test-utils `act`, both already transitive + deps of `react-dom`, so no new devDependency is introduced). ---------- */ +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { root.unmount(); }); + container.remove(); +} + +function click(el: Element | null) { + if (!el) throw new Error('element not found'); + act(() => { (el as HTMLElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); }); +} + +function setValue(input: HTMLInputElement, value: string) { + const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')!.set!; + setter.call(input, value); + input.dispatchEvent(new Event('input', { bubbles: true })); +} + +function q(testId: string): T | null { + return container.querySelector(`[data-testid="${testId}"]`); +} + +describe('AssetPicker pure helpers', () => { + test('acceptFor maps mediaType to the file input accept attribute', () => { + expect(acceptFor('image')).toBe('image/*'); + expect(acceptFor('video')).toBe('video/*'); + expect(acceptFor('any')).toBeUndefined(); + }); + + test('matchesMediaType guards drag-drop uploads by mediaType', () => { + expect(matchesMediaType('image/png', 'image')).toBe(true); + expect(matchesMediaType('video/mp4', 'image')).toBe(false); + expect(matchesMediaType('video/mp4', 'video')).toBe(true); + expect(matchesMediaType('application/pdf', 'any')).toBe(true); + }); + + test('isStandalone reflects presence of window.WHP_CONFIG', () => { + delete (window as any).WHP_CONFIG; + expect(isStandalone()).toBe(true); + (window as any).WHP_CONFIG = { apiUrl: 'x', siteId: 1, csrfToken: 't' }; + expect(isStandalone()).toBe(false); + delete (window as any).WHP_CONFIG; + }); +}); + +describe('AssetPicker component', () => { + beforeEach(() => { + uploadAssetMock.mockReset(); + listAssetsMock.mockReset(); + listAssetsMock.mockResolvedValue(GRID_ASSETS); + delete (window as any).WHP_CONFIG; + }); + + afterEach(() => { + if (container) unmount(); + delete (window as any).WHP_CONFIG; + }); + + test('renders the current value as a preview thumbnail (full variant)', () => { + render(); + const preview = q('asset-picker-preview'); + expect(preview).not.toBeNull(); + expect(preview!.src).toContain('/storage/assets/existing.png'); + expect(q('asset-picker-dropzone')).toBeNull(); + }); + + test('renders a dropzone (no preview) when value is empty', () => { + render(); + expect(q('asset-picker-dropzone')).not.toBeNull(); + expect(q('asset-picker-preview')).toBeNull(); + }); + + test('remove button clears the value via onChange("")', () => { + const onChange = vi.fn(); + render(); + click(q('asset-picker-remove')); + expect(onChange).toHaveBeenCalledWith(''); + }); + + test('Browse control is absent when window.WHP_CONFIG is undefined (standalone)', () => { + render(); + expect(q('asset-picker-browse')).toBeNull(); + }); + + test('Browse control is present and lists assets filtered by mediaType when WHP_CONFIG is set', async () => { + (window as any).WHP_CONFIG = { apiUrl: 'https://x', siteId: 1, csrfToken: 't' }; + render(); + const browseBtn = q('asset-picker-browse'); + expect(browseBtn).not.toBeNull(); + + await act(async () => { click(browseBtn); await Promise.resolve(); }); + + expect(listAssetsMock).toHaveBeenCalledWith('video'); + expect(q('asset-picker-grid')).not.toBeNull(); + }); + + test('selecting a grid asset invokes onChange with that asset url and closes the grid', async () => { + (window as any).WHP_CONFIG = { apiUrl: 'https://x', siteId: 1, csrfToken: 't' }; + const onChange = vi.fn(); + render(); + + await act(async () => { click(q('asset-picker-browse')); await Promise.resolve(); }); + expect(q('asset-picker-grid')).not.toBeNull(); + + click(q(`asset-picker-thumb-${GRID_ASSETS[0].name}`)); + + expect(onChange).toHaveBeenCalledWith(GRID_ASSETS[0].url); + expect(q('asset-picker-grid')).toBeNull(); + }); + + test('URL apply invokes onChange with the typed url (full variant Apply button)', () => { + const onChange = vi.fn(); + render(); + const input = q('asset-picker-url-input')!; + setValue(input, 'https://example.com/pic.jpg'); + click(q('asset-picker-apply')); + expect(onChange).toHaveBeenCalledWith('https://example.com/pic.jpg'); + }); + + test('uploading a file calls uploadAsset and applies the resolved url via onChange', async () => { + uploadAssetMock.mockResolvedValue('/storage/assets/uploaded.png'); + const onChange = vi.fn(); + render(); + + const fileInput = q('asset-picker-file-input')!; + const file = new File(['x'], 'photo.png', { type: 'image/png' }); + const dt = { files: [file] }; + Object.defineProperty(fileInput, 'files', { value: dt.files }); + + await act(async () => { + fileInput.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(uploadAssetMock).toHaveBeenCalledWith(file); + expect(onChange).toHaveBeenCalledWith('/storage/assets/uploaded.png'); + }); + + test('variant="compact" renders the compact single-row structure instead of the full structure', () => { + render(); + expect(q('asset-picker-compact')).not.toBeNull(); + expect(q('asset-picker-full')).toBeNull(); + expect(q('asset-picker-dropzone')).toBeNull(); + expect(q('asset-picker-url-input')).not.toBeNull(); + expect(q('asset-picker-upload')).not.toBeNull(); + }); + + test('compact variant: URL input applies onChange on blur', () => { + const onChange = vi.fn(); + render(); + const input = q('asset-picker-url-input')!; + setValue(input, 'https://example.com/vid.mp4'); + // React delegates onBlur via the bubbling 'focusout' event (blur itself doesn't bubble). + act(() => { input.dispatchEvent(new FocusEvent('focusout', { bubbles: true })); }); + expect(onChange).toHaveBeenCalledWith('https://example.com/vid.mp4'); + }); + + test('mediaType="video" sets file input accept to video/* and mediaType="any" omits accept', () => { + render(); + expect(q('asset-picker-file-input')!.accept).toBe('video/*'); + unmount(); + + render(); + expect(q('asset-picker-file-input')!.accept).toBe(''); + }); +}); diff --git a/craft/src/ui/AssetPicker.tsx b/craft/src/ui/AssetPicker.tsx new file mode 100644 index 0000000..1975e37 --- /dev/null +++ b/craft/src/ui/AssetPicker.tsx @@ -0,0 +1,337 @@ +import React, { useState, useCallback, useRef, useEffect, CSSProperties } from 'react'; +import { uploadAsset, listAssets, Asset } from '../utils/assets'; + +export interface AssetPickerProps { + value: string; + onChange: (url: string) => void; + /** Drives the file `` and the `listAssets` grid filter. Default 'image'. */ + mediaType?: 'image' | 'video' | 'any'; + /** 'full' mirrors ImageStylePanel's source UI; 'compact' is a single tight row for array-editor cards. */ + variant?: 'full' | 'compact'; + placeholder?: string; +} + +/* ---------- Pure helpers (exported for unit testing) ---------- */ + +/** File `` value for a given mediaType; `undefined` (any file) for 'any'. */ +export function acceptFor(mediaType: 'image' | 'video' | 'any'): string | undefined { + if (mediaType === 'image') return 'image/*'; + if (mediaType === 'video') return 'video/*'; + return undefined; +} + +/** Whether a dropped/selected file's MIME type is acceptable for the given mediaType. */ +export function matchesMediaType(fileType: string, mediaType: 'image' | 'video' | 'any'): boolean { + if (mediaType === 'any') return true; + return fileType.startsWith(mediaType); +} + +/** True when running outside WHP (no `window.WHP_CONFIG`) -- Browse should be hidden. */ +export function isStandalone(): boolean { + return typeof window === 'undefined' || !(window as any).WHP_CONFIG; +} + +const previewBoxStyle: CSSProperties = { + marginBottom: 8, + borderRadius: 6, + overflow: 'hidden', + border: '1px solid #3f3f46', + position: 'relative', +}; + +const removeBtnStyle: CSSProperties = { + position: 'absolute', + top: 4, + right: 4, + width: 24, + height: 24, + borderRadius: '50%', + background: 'rgba(0,0,0,0.7)', + border: 'none', + color: '#fff', + cursor: 'pointer', + fontSize: 12, + display: 'flex', + alignItems: 'center', + justifyContent: 'center', +}; + +const dropZoneStyle = (active: boolean): CSSProperties => ({ + padding: '20px 12px', + border: `2px dashed ${active ? '#3b82f6' : '#3f3f46'}`, + borderRadius: 6, + textAlign: 'center', + color: '#71717a', + fontSize: 12, + cursor: 'pointer', + marginBottom: 8, + transition: 'border-color 0.15s', +}); + +const uploadBtnStyle: CSSProperties = { + flex: 1, + padding: '8px 10px', + fontSize: 12, + borderRadius: 4, + cursor: 'pointer', + border: '1px solid #3f3f46', + background: '#3b82f6', + color: '#fff', + fontWeight: 500, +}; + +const browseBtnStyle = (active: boolean): CSSProperties => ({ + flex: 1, + padding: '8px 10px', + fontSize: 12, + borderRadius: 4, + cursor: 'pointer', + border: '1px solid #3f3f46', + background: active ? '#3b82f6' : '#27272a', + color: active ? '#fff' : '#e4e4e7', +}); + +const gridStyle: CSSProperties = { + maxHeight: 200, + overflowY: 'auto', + display: 'grid', + gridTemplateColumns: 'repeat(3, 1fr)', + gap: 4, + marginTop: 8, + background: '#18181b', + borderRadius: 6, + padding: 4, +}; + +export const AssetPicker: React.FC = ({ + value, + onChange, + mediaType = 'image', + variant = 'full', + placeholder = 'Or paste URL...', +}) => { + const fileInputRef = useRef(null); + const [showBrowser, setShowBrowser] = useState(false); + const [browserAssets, setBrowserAssets] = useState([]); + const [browserLoading, setBrowserLoading] = useState(false); + const [urlValue, setUrlValue] = useState(value || ''); + const [dragOver, setDragOver] = useState(false); + const standalone = isStandalone(); + + useEffect(() => { + setUrlValue(value || ''); + }, [value]); + + const handleUpload = useCallback(async (file: File) => { + const url = await uploadAsset(file); + if (url) { + onChange(url); + setUrlValue(url); + } + }, [onChange]); + + const handleBrowseToggle = useCallback(async () => { + if (showBrowser) { + setShowBrowser(false); + return; + } + setBrowserLoading(true); + try { + const assets = await listAssets(mediaType); + setBrowserAssets(assets); + setShowBrowser(true); + } finally { + setBrowserLoading(false); + } + }, [showBrowser, mediaType]); + + const handleSelectAsset = useCallback((asset: Asset) => { + onChange(asset.url); + setUrlValue(asset.url); + setShowBrowser(false); + }, [onChange]); + + const handleApplyUrl = useCallback(() => { + onChange(urlValue); + }, [urlValue, onChange]); + + const handleRemove = useCallback(() => { + onChange(''); + setUrlValue(''); + }, [onChange]); + + const handleDrop = useCallback(async (e: React.DragEvent) => { + e.preventDefault(); + setDragOver(false); + const file = e.dataTransfer.files?.[0]; + if (file && matchesMediaType(file.type, mediaType)) { + await handleUpload(file); + } + }, [handleUpload, mediaType]); + + const accept = acceptFor(mediaType); + const hasValue = !!value; + + const fileInput = ( + { + const file = e.target.files?.[0]; + if (file) handleUpload(file); + e.target.value = ''; + }} + data-testid="asset-picker-file-input" + /> + ); + + const grid = showBrowser && ( +
+ {browserAssets.map((asset) => ( +
handleSelectAsset(asset)} + data-testid={`asset-picker-thumb-${asset.name}`} + style={{ cursor: 'pointer', borderRadius: 4, overflow: 'hidden', border: '2px solid transparent', aspectRatio: '1' }} + > + {asset.name} +
+ ))} + {browserAssets.length === 0 && ( +

+ No assets uploaded yet. Use Upload above. +

+ )} +
+ ); + + const browseButton = !standalone && ( + + ); + + if (variant === 'compact') { + return ( +
+
+ {hasValue && ( +
+ + +
+ )} + + {browseButton} + { setUrlValue(e.target.value); }} + onBlur={handleApplyUrl} + onKeyDown={(e) => { if (e.key === 'Enter') handleApplyUrl(); }} + data-testid="asset-picker-url-input" + style={{ flex: 1, fontSize: 11, minWidth: 0 }} + /> +
+ {grid} + {fileInput} +
+ ); + } + + return ( +
+ {hasValue ? ( +
+ + +
+ ) : ( +
{ e.preventDefault(); setDragOver(true); }} + onDragLeave={() => setDragOver(false)} + onDrop={handleDrop} + onClick={() => fileInputRef.current?.click()} + data-testid="asset-picker-dropzone" + > + + Drop {mediaType === 'video' ? 'video' : mediaType === 'any' ? 'file' : 'image'} here or click to upload +
+ )} + +
+ + {browseButton} +
+ + {grid} + {fileInput} + +
+
+ setUrlValue(e.target.value)} + data-testid="asset-picker-url-input" + style={{ fontSize: 11 }} + /> + +
+
+
+ ); +}; From 4674a99ec9000de12799a57c74f7d6ce460b1cf3 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 13:00:10 -0700 Subject: [PATCH 16/61] feat(builder): unify StylePanel image fields on AssetPicker Replace the ad-hoc upload/browse/URL blocks in ImageStylePanel, HeroStylePanel (bgImage + bgVideo), BackgroundSectionStylePanel, NavStylePanel (standalone Logo imageSrc + Navbar logoImage), MediaStylePanel (Gallery/ContentSlider array items), and FeaturesEditor (per-feature image) with the shared AssetPicker component, dropping each panel's duplicated local showBrowser/browserAssets/handleUpload state and inline asset-browser JSX. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../styles/BackgroundSectionStylePanel.tsx | 30 +--- .../panels/right/styles/FeaturesEditor.tsx | 42 +---- .../panels/right/styles/HeroStylePanel.tsx | 132 ++------------- .../panels/right/styles/ImageStylePanel.tsx | 153 +----------------- .../panels/right/styles/MediaStylePanel.tsx | 37 +++-- .../src/panels/right/styles/NavStylePanel.tsx | 9 +- 6 files changed, 56 insertions(+), 347 deletions(-) diff --git a/craft/src/panels/right/styles/BackgroundSectionStylePanel.tsx b/craft/src/panels/right/styles/BackgroundSectionStylePanel.tsx index f72b17f..ba6c09f 100644 --- a/craft/src/panels/right/styles/BackgroundSectionStylePanel.tsx +++ b/craft/src/panels/right/styles/BackgroundSectionStylePanel.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useRef } from 'react'; +import React, { useCallback } from 'react'; import { useEditor } from '@craftjs/core'; import { BG_COLORS, @@ -12,18 +12,15 @@ import { PresetButtonGrid, CollapsibleSection, ColorPickerField, - uploadToWhp, labelStyle, inputStyle, - smallInputStyle, - btnActiveStyle, sectionGap, } from './shared'; +import { AssetPicker } from '../../../ui/AssetPicker'; /* ---------- BACKGROUND SECTION ---------- */ export const BackgroundSectionStylePanel: React.FC = ({ selectedId, nodeProps }) => { const { actions } = useEditor(); - const fileInputRef = useRef(null); const setProp = useCallback((key: string, value: any) => { actions.setProp(selectedId, (props: any) => { props[key] = value; }); @@ -35,34 +32,13 @@ export const BackgroundSectionStylePanel: React.FC = ({ selecte }); }, [actions, selectedId]); - const handleUpload = useCallback(async (file: File) => { - const url = await uploadToWhp(file); - if (url) setProp('bgImage', url); - }, [setProp]); - const style = nodeProps.style || {}; return ( <> {/* Background Image */} - {nodeProps.bgImage && ( -
- - -
- )} -
- -
- { const f = e.target.files?.[0]; if (f) handleUpload(f); e.target.value = ''; }} /> - setProp('bgImage', e.target.value)} style={{ ...smallInputStyle, width: '100%', marginTop: 4 }} /> + setProp('bgImage', url)} variant="full" /> {/* Background Color */} diff --git a/craft/src/panels/right/styles/FeaturesEditor.tsx b/craft/src/panels/right/styles/FeaturesEditor.tsx index f1ab0f3..42db943 100644 --- a/craft/src/panels/right/styles/FeaturesEditor.tsx +++ b/craft/src/panels/right/styles/FeaturesEditor.tsx @@ -1,22 +1,7 @@ -import React, { useRef } from 'react'; +import React from 'react'; import { useEditor } from '@craftjs/core'; import { labelStyle, inputStyle, sectionGap } from './shared'; - -/* Upload helper (same contract as ImageBlock/Navbar): uploads to WHP if a - WHP_CONFIG is present, otherwise falls back to a local blob URL. */ -async function uploadToWhp(file: File): Promise { - const cfg = (window as any).WHP_CONFIG; - if (!cfg) return URL.createObjectURL(file); - const fd = new FormData(); - fd.append('file', file); - try { - const resp = await fetch(`${cfg.apiUrl}?action=upload_asset&site_id=${cfg.siteId}`, { - method: 'POST', headers: { 'X-CSRF-Token': cfg.csrfToken }, body: fd, - }); - const data = await resp.json(); - return data.success && data.url ? data.url : null; - } catch { return null; } -} +import { AssetPicker } from '../../../ui/AssetPicker'; interface Feature { title?: string; description?: string; icon?: string; @@ -33,7 +18,6 @@ interface Feature { export const FeaturesEditor: React.FC<{ selectedId: string; features: unknown }> = ({ selectedId, features }) => { const { actions } = useEditor(); const list: Feature[] = Array.isArray(features) ? (features as Feature[]) : []; - const fileRefs = useRef>({}); const mutate = (fn: (arr: Feature[]) => Feature[]) => actions.setProp(selectedId, (p: any) => { p.features = fn([...(p.features || [])]); }); @@ -42,9 +26,6 @@ export const FeaturesEditor: React.FC<{ selectedId: string; features: unknown }> const add = () => mutate((arr) => [...arr, { title: 'New Feature', description: 'Describe this feature.', icon: '🔧', image: '', imageAlt: '', buttonText: '', buttonUrl: '' }]); const remove = (i: number) => mutate((arr) => { arr.splice(i, 1); return arr; }); - const onUpload = async (i: number, file: File) => { const url = await uploadToWhp(file); if (url) update(i, 'image', url); }; - - const smallBtn: React.CSSProperties = { padding: '4px 8px', fontSize: 11, borderRadius: 4, cursor: 'pointer', border: '1px solid #3f3f46', background: '#27272a', color: '#e4e4e7' }; return (
@@ -57,21 +38,12 @@ export const FeaturesEditor: React.FC<{ selectedId: string; features: unknown }> `; + inputHtml = ``; } else if (field.type === 'select') { const opts = (field.options || []).map((o) => ``).join(''); - inputHtml = ``; + inputHtml = ``; } else { - inputHtml = ``; + inputHtml = ``; } return `
${labelHtml}${inputHtml}
`; }).join('\n '); diff --git a/craft/src/components/forms/InputField.toHtml.test.ts b/craft/src/components/forms/InputField.toHtml.test.ts new file mode 100644 index 0000000..bdd83cc --- /dev/null +++ b/craft/src/components/forms/InputField.toHtml.test.ts @@ -0,0 +1,29 @@ +import { describe, test, expect } from 'vitest'; +import { InputField } from './InputField'; + +const toHtml = (InputField as any).toHtml; + +describe('InputField.toHtml accessibility (F2.1)', () => { + test('label for= matches input id=', () => { + const { html } = toHtml({ label: 'Your Name', name: 'name' }, ''); + const forMatch = html.match(/
`, }; }; diff --git a/craft/src/components/forms/TextareaField.toHtml.test.ts b/craft/src/components/forms/TextareaField.toHtml.test.ts new file mode 100644 index 0000000..6a9be42 --- /dev/null +++ b/craft/src/components/forms/TextareaField.toHtml.test.ts @@ -0,0 +1,21 @@ +import { describe, test, expect } from 'vitest'; +import { TextareaField } from './TextareaField'; + +const toHtml = (TextareaField as any).toHtml; + +describe('TextareaField.toHtml accessibility (F2.1)', () => { + test('label for= matches textarea id=', () => { + const { html } = toHtml({ label: 'Message', name: 'message' }, ''); + const forMatch = html.match(/