C1: HtmlBlock's PURIFY_CONFIG omitted 'style' from ALLOWED_ATTR, so the toolbar colour picker added in this branch was silently deleted by DOMPurify -- issue #2 was regressed, not fixed. Adds style/id plus table tags, with tests pinning the markup path in both render and toHtml. I3: PagesPanel's three confirmation states were not mutually exclusive; cancelling delete revealed an unbidden reset prompt on a destructive action. I5: orphan repair logged at console.warn, which the new console buffer cannot see -- the reporter would never capture the most diagnostic signal for the still-unreproduced drop bug. Also aligns useWhpApi's initial-load failure handling with loadState's fallback. I7: corrects comments (and the design spec) that asserted an orphan "renders somewhere on the canvas", which a mid-plan audit disproved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
297 lines
21 KiB
Markdown
297 lines
21 KiB
Markdown
# Site Builder — Five User-Reported Issues
|
||
|
||
**Date:** 2026-08-08
|
||
**Status:** Approved design, ready for implementation planning
|
||
**Repos touched:** `cloud-hosting-platform/site-builder` (`craft/`) and `cloud-hosting-platform/whp` (`web-files/`, `sql/`, `scripts/`)
|
||
|
||
## Background
|
||
|
||
Five issues came in from people using the WHP site builder:
|
||
|
||
1. Elements can be dropped outside the main page; once there they cannot be selected or deleted.
|
||
2. The HTML element exposes colour options that show in the builder but do not reach the live page.
|
||
3. Users want a way to reset a page to blank.
|
||
4. The Layers tab sometimes does not show everything.
|
||
5. There is no way to report a builder problem from inside the builder.
|
||
|
||
Two of these are already explained by the source:
|
||
|
||
- **#2 is a render/export mismatch, not just a stray control.** `HtmlBlock`'s React render applies its `style` prop, but `HtmlBlock.toHtml()` returns only the DOMPurify-cleaned `code` and ignores `style` entirely. Every style control the panel offers is therefore dead on export. The controls themselves come from `GenericPropsEditor`, the catch-all panel that `GuidedStyles` routes HTML to via its `isUtility` branch (shared with Divider and Spacer).
|
||
- **#4 is a data-model consequence.** `FeaturesGrid`, `Tabs`, `Accordion`, `PricingTable`, `Testimonials`, `Gallery`, `ContentSlider`, `NumberCounter`, `Menu`, `SocialLinks`, `Navbar` and `ContactForm` all hold their content in **array props**, not Craft child nodes, so they are genuinely leaf nodes in the Craft tree and `LayersPanel` has nothing to nest under them. `ColumnLayout` is the exception — it uses real `<Element canvas>` children, which is why columns nest correctly and nothing else does.
|
||
|
||
#1 is **not** diagnosed. Reproducing it is the first task of implementation, and the prevention fix follows the evidence rather than this document.
|
||
|
||
## Scope
|
||
|
||
In scope: the five items above, across the Craft.js editor, the WHP site-builder API, one new database table, and one new root-only admin page.
|
||
|
||
Out of scope: converting composite components from array props to real Craft children; any change to publish/export output beyond `HtmlBlock` no longer applying `style` in the editor; CSRF enforcement anywhere other than the one new endpoint.
|
||
|
||
---
|
||
|
||
## Item 1 — Elements dropped outside the page
|
||
|
||
### Invariant
|
||
|
||
**Every node in editor state is reachable from `ROOT`.** Enforced at three independent points so no single failure strands a block.
|
||
|
||
### 1a. Prevention (drop time)
|
||
|
||
A drop whose resolved target is not a `ROOT` descendant is rejected: a new block from `BlocksPanel` is never created, and a moved block returns to its origin.
|
||
|
||
The mechanism is deliberately left open pending a live reproduction. Two structural candidates, both to be tested against a real editor session before anything is written:
|
||
|
||
- Craft.js's `DefaultEventHandlers` retains the **last valid drop indicator** when the pointer leaves every droppable region. The gutter beside `.canvas-device-frame` (visible at tablet/mobile device widths) and the `pointer-events: none` header/footer `ZonePreview` bands are both places where the pointer is over the canvas area but over no Craft node.
|
||
- `Canvas.tsx` wraps `<Frame>` in a plain positioned `<div>`; drops landing on that wrapper resolve to no node.
|
||
|
||
**The plan's first task is reproduction, and its acceptance criterion is a written description of the actual mechanism.** No fix is written before that.
|
||
|
||
**Status (2026-08-08, final review pass): NOT implemented.** The reproduction task did not land during this branch — the drop-time mechanism described above (stale drop indicator vs. wrapper-`<div>` drop target, or something else) was never confirmed against a live editor session, so no prevention fix was written, per the acceptance criterion above. What shipped instead is 1b (repair) and 1c (recovery), plus the in-builder issue reporter from Item 5, whose plan was to capture a real repro of this specific bug from customers going forward. A code-review pass on this branch found that `useWhpApi.ts` and `PageContext.tsx`'s orphan-repair log lines used `console.warn`, which the reporter's console buffer does not capture (it patches `console.error` only) — fixed to `console.error` so a future in-the-wild repro of this exact symptom is actually captured. Reproducing the drop-time mechanism and writing the 1a prevention fix remains open work.
|
||
|
||
### 1b. Repair (load time)
|
||
|
||
`PageContext`'s deserialization path runs an orphan sweep before handing state to Craft:
|
||
|
||
- Parse the serialized craft state, walk every node's parent chain.
|
||
- Any node whose chain does not terminate at `ROOT` is reattached to the end of `ROOT`'s children.
|
||
- The repair is logged to the console with node id and type.
|
||
|
||
This is a **pure function** over serialized state — `repairOrphanNodes(serialized: string): { state: string; repaired: string[] }` — unit-testable with no browser and no Craft instance, following the precedent of `PageContext.treeToState.test.ts`.
|
||
|
||
### 1c. Recovery (always available)
|
||
|
||
`LayersPanel` gains an **Unplaced** group, rendered after the `ROOT` tree, listing any node in `state.nodes` not reachable from `ROOT`. Rows behave like normal layer rows: click to select, and the existing delete paths (context menu, Delete key) then work on them.
|
||
|
||
The group renders only when the set is non-empty, so ordinary sessions see no new UI.
|
||
|
||
### Error handling
|
||
|
||
If `repairOrphanNodes` throws on malformed state, it returns the input unchanged and logs — a page must never fail to load because the repair pass could not parse it.
|
||
|
||
---
|
||
|
||
## Item 2 — HTML block
|
||
|
||
### 2a. Only the Edit HTML control
|
||
|
||
- `GuidedStyles` gains an `isHtml` branch (`/^html$/i`) evaluated **before** `isUtility`, and `html` is removed from the `isUtility` regex so the two cannot both match.
|
||
- New `panels/right/styles/HtmlStylePanel.tsx` renders the Edit HTML button and nothing else.
|
||
- `HtmlCodeField` moves out of `GenericPropsEditor.tsx` into `panels/right/styles/HtmlCodeField.tsx` and is imported by `HtmlStylePanel`. `GenericPropsEditor` keeps `code` in `SKIP_PROPS` (harmless, and guards any future component carrying a `code` prop).
|
||
|
||
### 2b. Render must match export
|
||
|
||
`HtmlBlock`'s render stops applying the `style` prop. It keeps the selection outline and `minHeight: 40px` — both are editor affordances, not content.
|
||
|
||
This is the fix for the actual complaint. Removing the controls alone would leave already-styled blocks still rendering their dead styling in the editor and still not on the live page.
|
||
|
||
Existing stored `style` props are left in place: they are already ignored by `toHtml()`, so they have no output effect, and stripping them would mean touching every saved site.
|
||
|
||
### 2c. Smarter HTML editor
|
||
|
||
`ui/CodeEditor.tsx` already provides CodeMirror 6 with autocompletion, auto-closing tags and brackets, bracket matching, a fold gutter, `indentWithTab` and the one-dark theme. Nothing new is needed for tab-complete — but the component **lazy-loads CodeMirror and silently falls back to a plain `<textarea>`** when its chunks fail to load, which is a failure mode this project has shipped before. Verification is therefore a required task, not an assumption.
|
||
|
||
Additions:
|
||
|
||
- **Imperative handle on `CodeEditor`.** `insertAtCursor(text: string)` and `getValue()`, exposed via `forwardRef` + `useImperativeHandle`, dispatching a CodeMirror transaction at the current selection. In textarea-fallback mode the same handle operates on the textarea's `selectionStart`/`selectionEnd`, so the toolbar keeps working when CodeMirror is unavailable.
|
||
- **Toolbar** above the editor in the Edit HTML modal: insert `div`, `section`, heading, paragraph, link, unordered list, image. Each inserts a snippet at the cursor and places the caret inside it.
|
||
- **Colour control** in the toolbar: a swatch picker that inserts `style="color: #rrggbb"` at the cursor, so colour becomes part of the user's own markup — which does survive to the live page — instead of a dead panel prop.
|
||
- **Format button** running a small built-in prettifier: `formatHtml(src: string): string`, a pure indent-only formatter (block-level tags on their own lines, two-space indent, inline tags untouched, contents of `<pre>` preserved verbatim). No `js-beautify` dependency; unit-tested directly.
|
||
|
||
The Head Code modal is **not** changed by this work. It uses the same `CodeEditor`, so it inherits the imperative handle harmlessly, but no toolbar is added there.
|
||
|
||
---
|
||
|
||
## Item 3 — Reset
|
||
|
||
Both actions affect the editor/staging state only. The live site changes on **Publish**, and both confirmation dialogs say so.
|
||
|
||
### 3a. Reset Page
|
||
|
||
- Entry point: the per-page row in `PagesPanel`, alongside the existing rename/delete controls. The TopBar is already crowded and collapses to an overflow menu at ≤768px.
|
||
- Confirmation dialog naming the page.
|
||
- Implementation: `actions.deserialize(EMPTY_PAGE_STATE)` where `EMPTY_PAGE_STATE` is a serialized single empty `Container` matching what `Canvas.tsx` mounts. Going through `deserialize` puts the reset in Craft's history, so **Ctrl+Z restores the page**. The dialog says so.
|
||
- Only the active page's canvas is cleared. Header, footer, other pages, per-page SEO and design tokens are untouched.
|
||
- The control is offered on **page rows only**. The Header and Footer entries in `PagesPanel` do not get it — a blank header or footer is what the existing zone editing already allows, and a reset there would silently change every page on the site.
|
||
|
||
### 3b. Reset Entire Site
|
||
|
||
- Entry point: a danger zone at the bottom of `SiteDesignPanel`.
|
||
- Guard: the user types the site domain to enable the button, matching the pattern used elsewhere in WHP for destructive actions.
|
||
- Effect: all pages replaced by a single empty page named Home (slug `index`), header and footer cleared, design tokens restored to `SiteDesignContext` defaults, `headCode` cleared.
|
||
- **Not undoable.** The dialog states this explicitly and states that the live site remains as-published until the user publishes again.
|
||
- Per-page SEO is cleared with its page. Uploaded assets are **not** deleted — they are shared, referenced by URL, and deleting them is a separate destructive action with its own blast radius.
|
||
|
||
Auto-save runs every 30 seconds, so a reset persists to staging shortly after it happens. This is stated in the Reset Entire Site dialog.
|
||
|
||
---
|
||
|
||
## Item 4 — Layers tab completeness
|
||
|
||
### 4a. Virtual rows for array-prop content
|
||
|
||
A registry maps a component `displayName` to the prop holding its items and the per-item field to use as a label:
|
||
|
||
```ts
|
||
// panels/left/layers-virtual-rows.ts
|
||
export const VIRTUAL_CHILD_PROPS: Record<string, { prop: string; label: string; fallback: string }> = {
|
||
'Features Grid': { prop: 'features', label: 'title', fallback: 'Feature' },
|
||
Tabs: { prop: 'tabs', label: 'title', fallback: 'Tab' },
|
||
Accordion: { prop: 'items', label: 'title', fallback: 'Item' },
|
||
'Pricing Table': { prop: 'plans', label: 'name', fallback: 'Plan' },
|
||
Testimonials: { prop: 'testimonials', label: 'name', fallback: 'Testimonial' },
|
||
Gallery: { prop: 'images', label: 'alt', fallback: 'Image' },
|
||
'Content Slider': { prop: 'slides', label: 'title', fallback: 'Slide' },
|
||
'Number Counter': { prop: 'counters', label: 'label', fallback: 'Counter' },
|
||
Menu: { prop: 'links', label: 'text', fallback: 'Link' },
|
||
'Social Links': { prop: 'links', label: 'platform', fallback: 'Link' },
|
||
Navbar: { prop: 'links', label: 'text', fallback: 'Link' },
|
||
'Contact Form': { prop: 'fields', label: 'label', fallback: 'Field' },
|
||
};
|
||
```
|
||
|
||
Exact prop and label keys are **verified against each component's `craft.props` defaults during implementation**; the table above is the starting point, not the contract. A registry entry whose prop is absent or not an array yields no rows rather than throwing.
|
||
|
||
`deriveVirtualRows(displayName, props)` is a pure function returning `{ index, label }[]`, unit-tested independently of React.
|
||
|
||
Rendering: virtual rows appear as children of their node, visually distinguished from real nodes (dimmer text, an item glyph rather than a component icon, no disclosure triangle) so they do not read as separately draggable or deletable.
|
||
|
||
### 4b. Selecting a virtual row
|
||
|
||
Clicking one selects the parent node (so the right panel opens the correct array editor) **and** requests that item's card be scrolled into view. The request travels through a small React context (`{ nodeId, prop, index }`), which the array editors (`ArrayItemFields`, `FeaturesEditor`, `SectionTypePanel`) consume to `scrollIntoView` the matching card. If an editor does not consume it, the click still selects the parent — the scroll is an enhancement, never a dependency.
|
||
|
||
### 4c. Unplaced group
|
||
|
||
As described in Item 1c.
|
||
|
||
### 4d. Panel scrolling
|
||
|
||
`LayersPanel`'s tree gets an explicit scroll container so deep or long trees remain fully reachable. This was a secondary hypothesis for the report; it is cheap and correct regardless.
|
||
|
||
### Explicitly not doing
|
||
|
||
Converting composites to real Craft children. That is a stored-state migration across every existing customer site, for cosmetic tree parity, with real risk to published output.
|
||
|
||
---
|
||
|
||
## Item 5 — In-builder issue reporting
|
||
|
||
### 5a. Editor UI
|
||
|
||
- A bug icon in the TopBar (and in `TopBarOverflowMenu` for ≤768px) opens a Report an Issue modal.
|
||
- Fields: category (`bug` / `confusing` / `feature`), a required description, and a default-on checkbox **"Include this page's contents to help debugging"** with a one-line plain-language note about what that sends. Unchecking it omits the canvas state; everything else in the payload still goes.
|
||
- On success the modal shows **`Report #SB-1234 submitted`** and closes on acknowledgement.
|
||
- On failure it keeps the user's text and offers retry — a report must never be silently lost.
|
||
|
||
### 5b. Captured context
|
||
|
||
| Field | Source |
|
||
|---|---|
|
||
| `username` | server-side from `AUTH_USER`; never accepted from the client |
|
||
| `site_id`, `site_domain` | `WHP_CONFIG` |
|
||
| `page_id`, `page_slug` | `PageContext` |
|
||
| `editor_version` | build stamp (below) |
|
||
| `user_agent`, `viewport` | `navigator` / `window` |
|
||
| `device_mode` | current `DeviceMode` |
|
||
| `selected_type` | `displayName` of the selected node, or null |
|
||
| `console_errors` | last 20 entries from a ring buffer |
|
||
| `canvas_state` | `query.serialize()` of the current page, omitted if the checkbox is unchecked |
|
||
|
||
**Build stamp.** `vite.config.ts` gains a `define` for `__EDITOR_BUILD__`, set from the short git SHA plus build date at build time, so a report identifies exactly which bundle produced it. `craft/package.json`'s `2.0.0` is not build-specific and is not sufficient.
|
||
|
||
**Console error ring buffer.** Installed in `main.tsx` at boot: patches `console.error` and adds `window.onerror` / `unhandledrejection` listeners, retaining the last 20 entries as `{ ts, message }` with each message truncated to 500 characters. It always chains to the original handler. Message text only — no stack traces, which are minified and would leak file paths for little benefit.
|
||
|
||
**Cap.** The whole JSON payload is capped at 512 KB. When `canvas_state` would breach it, the canvas state is dropped and a `canvas_state_omitted: "size"` marker is sent so the report is not silently truncated into something misleading.
|
||
|
||
### 5c. Endpoint
|
||
|
||
`action=report_issue` added to `web-files/api/site-builder.php`, handled by `handleReportIssue($pdo, $isRoot)`:
|
||
|
||
- POST only.
|
||
- Session auth via the existing `AUTH_USER` / `HOME_DIR` gate at the top of the file.
|
||
- **CSRF validated** with `validate_csrf_token()` from `auto-prepend.php`. `WHP_CONFIG` already ships `csrfToken` to the editor. This is a new state-changing endpoint, so it validates now rather than waiting on the fleet-wide CSRF rollout.
|
||
- Ownership: a non-root user may only file against a site they own, checked the same way the other handlers in this file check site ownership.
|
||
- **Rate limit:** at most 5 reports per user per hour, enforced by counting recent rows in the reports table itself (the pattern `Sitesmith.php` uses). Over the limit returns HTTP 429 with a plain message.
|
||
- Validation: description 1–5000 characters after trimming; category must be one of the three; oversized `canvas_state` rejected rather than truncated.
|
||
- Response: `{ success: true, reference: "SB-1234", id: 1234 }`.
|
||
|
||
### 5d. Schema
|
||
|
||
New table, idempotent migration in `sql/migrations/staging/`, canonical schema regenerated per the `whp-add-migration` skill:
|
||
|
||
```sql
|
||
CREATE TABLE IF NOT EXISTS whp.site_builder_reports (
|
||
id INT AUTO_INCREMENT PRIMARY KEY,
|
||
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||
username VARCHAR(64) NOT NULL,
|
||
site_id INT NULL,
|
||
site_domain VARCHAR(255) NULL,
|
||
page_slug VARCHAR(255) NULL,
|
||
category ENUM('bug','confusing','feature') NOT NULL DEFAULT 'bug',
|
||
description TEXT NOT NULL,
|
||
editor_version VARCHAR(64) NULL,
|
||
user_agent VARCHAR(512) NULL,
|
||
viewport VARCHAR(32) NULL,
|
||
device_mode VARCHAR(16) NULL,
|
||
selected_type VARCHAR(64) NULL,
|
||
console_errors JSON NULL,
|
||
canvas_state LONGTEXT NULL,
|
||
status ENUM('new','triaged','fixed','wontfix') NOT NULL DEFAULT 'new',
|
||
admin_notes TEXT NULL,
|
||
INDEX idx_status_created (status, created_at),
|
||
INDEX idx_user_created (username, created_at)
|
||
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
|
||
```
|
||
|
||
`AUTO_INCREMENT` and the `PRIMARY KEY` are stated explicitly and must survive any future migration of this table.
|
||
|
||
### 5e. Admin page
|
||
|
||
`web-files/pages/site-builder-reports.php`, registered per the `whp-admin-page-registration` skill: `$allowed_pages`, `$page_permissions` (root only), a sidebar entry under the existing site-builder grouping, and a documentation entry.
|
||
|
||
- List view: reference, date, user, site, category, status, first line of the description. Filter by status and category.
|
||
- Detail view: full description and all captured context.
|
||
- Status and notes are editable inline.
|
||
- **"Copy for Claude"** button emitting one compact JSON object (context plus description, canvas state included only when present) for pasting straight into a session. This is what makes the feature pay off operationally.
|
||
|
||
---
|
||
|
||
## Testing
|
||
|
||
**Vitest (`craft/`), pure functions first:**
|
||
|
||
- `repairOrphanNodes` — orphan reattached, healthy state unchanged, malformed input returns input.
|
||
- `deriveVirtualRows` — each registry entry, missing prop, non-array prop, missing label field falls back.
|
||
- `formatHtml` — nesting, inline tags untouched, `<pre>` preserved, idempotent on already-formatted input.
|
||
- `buildReportPayload` — checkbox off omits canvas state, oversize drops canvas state and sets the marker, all context fields present.
|
||
- Console ring buffer — retains 20, truncates long messages, chains to the original handler.
|
||
|
||
**React component tests:** `HtmlStylePanel` renders only the Edit HTML control; `GuidedStyles` routes HTML to it and not to `GenericPropsEditor`; Reset Page dialog wiring; Unplaced group appears only when orphans exist; virtual rows select their parent.
|
||
|
||
**Export test:** an `HtmlBlock.toHtml` case asserting `style` never reaches output, and a render test asserting the editor no longer applies it — the two sides of the reported mismatch, locked down together.
|
||
|
||
**PHP:** a validation test script for `handleReportIssue` in the style of `scripts/test-site-builder-asset-urls.php` — category validation, description bounds, size cap, rate-limit counting. Follows the project's "verify through the web path, not bare `php -r`" rule for anything touching `open_basedir`-sensitive paths.
|
||
|
||
**Manual, on the canary (192.168.1.148) before any prod host:**
|
||
|
||
- `curl -s -o /dev/null -w '%{http_code}' <host>/site-builder/js/index2.js` returns 200 — CodeMirror chunks reachable, editor not degraded to the textarea fallback.
|
||
- File a report end to end and confirm it lands in `whp.site_builder_reports` and renders on the admin page.
|
||
- Reproduce the original off-canvas drop and confirm it is now impossible.
|
||
|
||
## Deployment
|
||
|
||
Standard path, owned by the `whp-deploy` skill:
|
||
|
||
1. `cd /workspace/site-builder/craft && npm run build`
|
||
2. Copy `dist/index.html`, `dist/css/editor.css` and **all** `dist/js/*.js` into `web-files/site-builder/` — copying only `editor.js` breaks the code editor at runtime.
|
||
3. Commit both repos (site-builder source, whp bundle).
|
||
4. `build-release.sh` on the build server.
|
||
5. Roll out 192.168.1.148 → whp02 → sdbees → whp01, verifying each before the next.
|
||
|
||
Published sites are static snapshots. None of these changes alter published output, so no site needs re-publishing as a result of this work.
|
||
|
||
## Risks
|
||
|
||
- **Item 1 root cause is unknown at design time.** If reproduction fails, prevention cannot be written responsibly. Repair (1b) and recovery (1c) still ship and still solve the user-visible problem — a stranded element becomes deletable — so the item is not blocked, but the plan must not pretend prevention landed if it did not.
|
||
- **`canvas_state` is customer content.** It is stored in the WHP database and readable by root only. The checkbox and its note exist so this is never a surprise.
|
||
- **Ring buffer patches `console.error` globally.** It must always chain to the original, or it silently swallows diagnostics for everyone.
|