From 767fcfe318c184aaaa8b6fb0132d6ed0813c31b9 Mon Sep 17 00:00:00 2001 From: Justinvolved <64196420+Justinvolved@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:59:49 +0200 Subject: [PATCH] Fold demo-build learnings: mechanical visual-QA design gate + detector-authoring traps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Demo storefronts kept shipping polish defects a screenshot glance skips — towering uncapped image bands, list-mode PLPs rendering zero rows behind HTTP 200 — because the re-skin ladder treated image height as an eyeball check and "it renders" (200) was the only mechanical bar. Promotes those to a mechanical definition-of-done in dw-demo-base/references/visual-qa.md: a `tall` image-band-height detector (durable fix = Tier-1 theme-CSS cap, added to dw-demo-swift/references/re-skin.md), a PLP row-presence + per-row-content detector, and framing the existing overflow/gap/stretched detectors as a blocking gate rather than a skippable checklist. Adds a stamped, non-blocking design sign-off pattern (SKIP until a sign-off artifact exists, then PASS) to orchestrator.md so visual taste stays a human decision without a build-blocking pause. Documents three authoring traps that make such a gate pass silently — Playwright's single-arg page.evaluate, ConvertTo-Json unwrapping a single-element array to a scalar, and a PowerShell local case-aliasing a declared parameter — with the rule that a probe run emitting zero probes must never be PASS. --- .claude-plugin/marketplace.json | 2 +- CHANGELOG.md | 35 ++++++++++++++ README.md | 2 +- .../dw-demo-base/references/orchestrator.md | 15 +++++- skills/dw-demo-base/references/visual-qa.md | 48 +++++++++++++++++-- skills/dw-demo-swift/references/re-skin.md | 1 + 6 files changed, 95 insertions(+), 8 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 9b9ddd1..67a8a8c 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,7 +6,7 @@ }, "metadata": { "description": "Claude skills for Dynamicweb 10 — organized by task domain, bundled by role.", - "version": "4.11.0" + "version": "4.11.1" }, "plugins": [ { diff --git a/CHANGELOG.md b/CHANGELOG.md index b128ab8..d74571a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,41 @@ All notable changes to the Dynamicweb Skills plugin are recorded here. The `version` field in `.claude-plugin/marketplace.json` tracks these entries. +## [4.11.1] + +Folds a demo-build session's design-quality-gate learnings into the demo skills' visual-QA, +orchestrator, and re-skin references: a mechanical definition-of-done that catches the polish +defects a screenshot glance skips, plus the authoring traps that make such a gate pass silently. + +### Added +- **Image-band height is a Tier-1 visual-QA item, mechanically gated** + (`dw-demo-base/references/visual-qa.md` detector + eyeball + symptom table + DoD; + `dw-demo-swift/references/re-skin.md` verification): stock image components carry no + serialized height field, so a swapped-in portrait crop or slider cover-card renders at full + column-width height and dominates the fold — a defect distinct from a stretched image. A new + `tall` detector flags any image band taller than a configured fraction of the viewport; the + durable fix is a Tier-1 theme-CSS cap (`aspect-ratio` + `max-height` + `object-fit: cover`). +- **PLP list asserts row-presence AND per-row content, not HTTP 200** + (`dw-demo-base/references/visual-qa.md`): a list-mode product page can return 200 while + rendering zero rows (empty/not-yet-repopulated index, mis-scoped shop). A new detector asserts + row count ≥ `minRows` and that each row carries its required-field selectors (thumbnail / SKU / + price / add-to-cart); an empty or field-short list behind 200 is a named finding, never a pass. +- **The programmatic detectors are the mechanical definition-of-done** + (`dw-demo-base/references/visual-qa.md`): the `overflowX` / section-gap / stretched-image / + placeholder detectors are framed as a blocking pass/fail run before eyeballing, not a checklist + the agent may skip. +- **Design sign-off — taste stays human without blocking automation** + (`dw-demo-base/references/orchestrator.md` acceptance criteria; `visual-qa.md` DoD): a stamped, + non-blocking sign-off leg that reports SKIP ("awaiting human sign-off") until a sign-off + artifact exists, then PASS — so visual taste gets a human decision on the keeper screenshots + without a build-blocking pause (the one blocking human gate stays the impact sign-off). +- **Authoring detector/probe scripts — three silent-false-green traps** + (`dw-demo-base/references/visual-qa.md`): Playwright `page.evaluate` passes exactly one arg + (pass an options object); PowerShell `ConvertTo-Json` unwraps a single-element array to a scalar + (normalise scalar-or-array on the JS side); and a PowerShell local that is a case-variant of a + parameter silently aliases it (`$Body`/`$body` — name the local distinctly). A probe run that + emits zero probes must never be reported as PASS. + ## [4.11.0] Folds eight learnings from two 2026-07 demo-build runs (a customer build dispatch and a diff --git a/README.md b/README.md index 5327404..6563544 100644 --- a/README.md +++ b/README.md @@ -136,7 +136,7 @@ Browse Dynamicweb source code on GitHub to understand internal APIs, classes, an ### Demos (Presales) **[dw-demo-base](skills/dw-demo-base/SKILL.md)** -Foundation skill for all demos. Scaffolds the dw10-suite host, wires the Backend MCP and two-layer localhost TLS bypass, installs Playwright MCP, and drops the customisations and customer-context guardrails. Use this first. Also owns the **orchestrator abstraction** ([references/orchestrator.md](skills/dw-demo-base/references/orchestrator.md)) — how a build is driven, GSD primary or the native `/demo:*` command set. Owns the **hosted/cloud fork** ([references/online-mode.md](skills/dw-demo-base/references/online-mode.md)) — building on an install reached only by URL + Admin API key — and the **publish path** ([references/publish-to-hosted.md](skills/dw-demo-base/references/publish-to-hosted.md)) — migrating a locally-built demo onto one. +Foundation skill for all demos. Scaffolds the dw10-suite host, wires the Backend MCP and two-layer localhost TLS bypass, installs Playwright MCP, and drops the customisations and customer-context guardrails. Use this first. Also owns the **orchestrator abstraction** ([references/orchestrator.md](skills/dw-demo-base/references/orchestrator.md)) — how a build is driven, GSD primary or the native `/demo:*` command set. Owns the **hosted/cloud fork** ([references/online-mode.md](skills/dw-demo-base/references/online-mode.md)) — building on an install reached only by URL + Admin API key — and the **publish path** ([references/publish-to-hosted.md](skills/dw-demo-base/references/publish-to-hosted.md)) — migrating a locally-built demo onto one. Owns the **visual-QA design gate** ([references/visual-qa.md](skills/dw-demo-base/references/visual-qa.md)) — the mechanical definition-of-done (overflow, section-gap, image-band-height, PLP row-content detectors) plus a human taste sign-off. **[dw-demo-pim](skills/dw-demo-pim/SKILL.md)** PIM modelling from a blank DB — product data built from scratch via MCP. Use after `dw-demo-base`. diff --git a/skills/dw-demo-base/references/orchestrator.md b/skills/dw-demo-base/references/orchestrator.md index 9fa1262..2fbd45c 100644 --- a/skills/dw-demo-base/references/orchestrator.md +++ b/skills/dw-demo-base/references/orchestrator.md @@ -264,7 +264,20 @@ native single pass produced it. Per phase: the cart; the re-skin reads as the customer's brand; the `CUSTOMISATIONS.md` ledger accounts for every custom-code row. - **Polish** — no broken links on the storyline path; no placeholder/lorem content on visited - pages; the demo runs end-to-end in one pass without a dead end. + pages; the demo runs end-to-end in one pass without a dead end; the mechanical visual-QA gate + passes on every demo-critical page (see `references/visual-qa.md` "Definition of done") **and** + the design has a human taste sign-off. + +**Design sign-off — taste stays human without blocking automation.** Mechanical asserts prove +structure (image-band caps, PLP row-presence + content, no overflow/gaps — `references/visual-qa.md`); +they cannot judge visual hierarchy or brand fit, so an all-green mechanical run can still ship a +page that reads wrong. Reserve that judgement as a **stamped, non-blocking** leg rather than a +human pause: the design sign-off check reports **SKIP ("awaiting human sign-off")** until a +sign-off artifact (e.g. `home//design-signoff.json` — `{ signed_off_by, ts, notes }`, +written by an approve-design step) exists, then **PASS**. A SKIP is stamped, never a FAIL, so it +records "taste not yet reviewed" without failing the build or forcing an execution pause — the one +*blocking* human gate remains the impact sign-off. The keeper screenshots at both breakpoints are +the artifact the human signs off on. A demo that needs different criteria edits this list in its own roadmap; the orchestrator reads the project's copy, so both orchestrators stay in agreement. diff --git a/skills/dw-demo-base/references/visual-qa.md b/skills/dw-demo-base/references/visual-qa.md index 1754dd3..40a3a9a 100644 --- a/skills/dw-demo-base/references/visual-qa.md +++ b/skills/dw-demo-base/references/visual-qa.md @@ -7,6 +7,8 @@ A page that renders is not a page that is done. The recurring polish gaps on dem - [The mindset rule](#the-mindset-rule) - [Breakpoints — capture both, always](#breakpoints--capture-both-always) - [Programmatic detectors — run before eyeballing](#programmatic-detectors--run-before-eyeballing) +- [PLP list — assert rows AND per-row content](#plp-list--assert-rows-and-per-row-content) +- [Authoring detector scripts — traps that pass silently](#authoring-detector-scripts--traps-that-pass-silently) - [Interaction pass — a screenshot cannot verify behaviour](#interaction-pass--a-screenshot-cannot-verify-behaviour) - [The eyeball checklist](#the-eyeball-checklist) - [Symptom → owning fix (route findings, don't re-diagnose)](#symptom--owning-fix-route-findings-dont-re-diagnose) @@ -30,7 +32,8 @@ One `browser_evaluate` call returns the mechanical findings. Adjust the section ```js () => { const de = document.documentElement, vw = de.clientWidth; - const out = { overflowX: Math.max(0, de.scrollWidth - vw), offenders: [], broken: [], stretched: [], gaps: [] }; + const out = { overflowX: Math.max(0, de.scrollWidth - vw), offenders: [], broken: [], stretched: [], tall: [], gaps: [] }; + const vh = window.innerHeight, bandCap = 0.85 * vh; // 0.85 = the demo's configured band-cap fraction // 1. Horizontal-overflow offenders — the element whose right edge IS the scrollbar for (const el of document.querySelectorAll('body *')) { const r = el.getBoundingClientRect(); @@ -45,6 +48,11 @@ One `browser_evaluate` call returns the mechanical findings. Adjust the section const nat = img.naturalWidth / img.naturalHeight, ren = r.width / r.height; if (Math.abs(nat - ren) / nat > 0.15 && getComputedStyle(img).objectFit === 'fill') out.stretched.push(img.currentSrc || img.src); + // Oversized band — a tall crop rendered full-height (object-fit:cover, no cap) dominates the fold. + // This is a DIFFERENT defect from `stretched`: aspect is fine, the band is just too tall. + const wrap = img.closest('figure, picture') || img, wr = wrap.getBoundingClientRect(); + if (Math.max(r.height, wr.height) > bandCap) + out.tall.push({ src: (img.currentSrc || img.src).slice(-60), px: Math.round(Math.max(r.height, wr.height)), cap: Math.round(bandCap) }); } } // 3. Whitespace bands — gaps between consecutive top-level sections @@ -54,11 +62,13 @@ One `browser_evaluate` call returns the mechanical findings. Adjust the section const gap = Math.round(secs[i].top - secs[i - 1].bottom); if (gap > 120) out.gaps.push({ afterSectionIndex: i - 1, px: gap }); } - out.offenders = out.offenders.slice(0, 10); out.broken = out.broken.slice(0, 10); + out.offenders = out.offenders.slice(0, 10); out.broken = out.broken.slice(0, 10); out.tall = out.tall.slice(0, 10); return out; } ``` +`tall` catches the recurring oversized image band — a portrait crop or slider cover-card rendered at full column-width height, uncapped, so it dominates the fold. It is a **distinct** finding from `stretched`: the aspect ratio is correct, the band is simply too tall. The stock image components carry no serialized height field, so a swapped-in photo reproduces it on every demo. The durable fix is a theme-CSS cap (`aspect-ratio` + `max-height` + `object-fit: cover` on the image wrapper and the slider cover-card), a Tier-1 re-skin item — route to `dw-demo-swift/references/re-skin.md`. Tune `bandCap` to the band the demo wants (a full-bleed hero legitimately fills the fold; a content-band image should not). + Pair it with two tool calls that catch the invisible failures: - `browser_console_messages` — a template NRE or JS exception often renders as a *silently missing section* with no visual trace at all. @@ -66,6 +76,30 @@ Pair it with two tool calls that catch the invisible failures: Any non-empty finding is a defect until proven otherwise (the one sanctioned exception: `broken` images on a page that was **not** scroll-swept are a measurement artifact — sweep and re-run, per `browser-automation.md`). +## PLP list — assert rows AND per-row content + +A list-mode product-list page (PLP) can return HTTP 200 while rendering **zero product rows** — an empty index, a not-yet-repopulated segment, a mis-scoped shop. HTTP 200 is the *seeding* bar ("it renders"); a **filled** list is the *demo* bar, and nothing catches the gap between them unless you assert it. Make row-presence and per-row content a mechanical check on every PLP, never an eyeball: + +```js +(sel) => { + const rows = [...document.querySelectorAll(sel.rowSelector)]; + const missing = rows.map((row, i) => ({ + i, absent: sel.fieldSelectors.filter(f => !row.querySelector(f)) + })).filter(r => r.absent.length); + return { rows: rows.length, minRows: sel.minRows, empty: rows.length < sel.minRows, missing: missing.slice(0, 10) }; +} +``` + +Feed it the page's real selectors (e.g. `{ rowSelector: '.list-item', minRows: 1, fieldSelectors: ['.thumbnail', '.sku', '.price', '[data-add-to-cart]'] }`). `empty: true` (fewer than `minRows`) or any `missing` entry is a **named finding**, never a pass: an empty or field-short list ships a broken demo behind a green status code. This makes the eyeball checklist's "product grid rendering zero tiles" row deterministic. + +## Authoring detector scripts — traps that pass silently + +When these detectors move from an ad-hoc `browser_evaluate` into a scripted probe runner, three authoring traps each degrade to **"nothing to check" and report a false green** — the worst failure a gate can have, because it looks like success. A probe run that emits **zero probes must never be reported as PASS** — treat an empty probe set as a failure, so a mis-wired runner surfaces instead of silently passing. + +- **Playwright `page.evaluate` passes exactly one argument to the page function.** Calling `evaluate(fn, a, b, c)` throws "Too many arguments"; if that throw is caught as a page-load failure, every page assert is skipped and the leg passes with zero probes. Pass a single options object — `evaluate(fn, { rowSelector, minRows, bandCap })` — and destructure it inside. +- **PowerShell `ConvertTo-Json` unwraps a single-element array to a scalar.** A one-page probe config serializes `"pages": "/x"` (string) and a one-entry map as an object, so a JS `Array.isArray()` guard sees no pages and runs zero asserts. On the JS consumer side, normalise scalar-or-array — `const arr = v => v == null ? [] : Array.isArray(v) ? v : [v];` — so a single page or viewport is never dropped. (Force an array at the PowerShell edge with the unary `,` operator or `@(...)` when you control both ends.) +- **PowerShell variable names are case-insensitive, so a local that is a case-variant of a parameter silently aliases it.** `param($Body); … $body = …` overwrites the parameter — `$body` *is* `$Body`. Name the local distinctly (`$respBody`, `$reqBody`), never a re-cased copy of a declared parameter. The collision is invisible at a glance and blanks the value rather than erroring. + ## Interaction pass — a screenshot cannot verify behaviour A slider whose arrows do nothing looks perfect in a static shot. For every interactive control visible on the page — slider/carousel arrows, tabs, accordions, variant selectors, add-to-cart — **click it once and assert something changed** (a class/`aria-*` attribute flips, the visible slide index moves, the cart badge increments, a panel expands). One interaction per component type per page is enough to catch dead wiring; a control that changes nothing is a finding even when it *renders* flawlessly. @@ -78,7 +112,7 @@ Read each screenshot against these — every "no" is a finding to route: |---|---| | **Vertical rhythm** — are gaps between sections consistent? | One band 2–3× its neighbours (the classic 6rem-default stripe); sections touching with no breathing room | | **Alignment** — do stacked sections, card grids, and headings share grid lines? | A card row with ragged left edges or unequal card heights; one section indented differently from every other | -| **Images** — crops sensible, aspect ratios consistent per row, no letterboxing inside tiles? | A decapitated product subject; one portrait tile in a landscape row; a logo stretched wide; grey empty wells | +| **Images** — crops sensible, aspect ratios consistent per row, heights capped, no letterboxing inside tiles? | A decapitated product subject; one portrait tile in a landscape row; a towering image band or slider cover-card that eats the whole fold; a logo stretched wide; grey empty wells | | **Text** — complete and real? | Truncation mid-word, copy overflowing its card, lorem/placeholder strings, untranslated resource keys, headings in the wrong visual size order | | **Edges** — padding at the viewport, no horizontal scrollbar in frame? | Content flush against the screen edge; a scrollbar track visible at the bottom of the shot | | **Controls** — inside their containers and styled? | An arrow poking past the section edge; a browser-default button among styled ones; a CTA with no hover affordance | @@ -93,6 +127,8 @@ Most recurring findings have a *known* cause with a documented fix — route the |---|---|---| | Horizontal scrollbar; slider arrow at/past the viewport edge | `NavigationPlacement: slider-nav-outside-expand` on a full-width slider | [`foundational/swift-building.md`](foundational/swift-building.md) §3 symptom table | | Uniform oversized whitespace bands between sections | GridRow `NULL` spacing → Swift 6rem default; layout columns are SQL-only and reverted by later MCP saves | [`foundational/data-access.md`](foundational/data-access.md) "SQL-direct content seeding" + [`foundational/cache-invalidation.md`](foundational/cache-invalidation.md) | +| Towering image band / slider cover-card eating the fold (`tall` detector) | Stock image component has no serialized height field; a tall crop renders full column-width height, uncapped | theme-CSS cap (`aspect-ratio` + `max-height` + `object-fit: cover`), Tier-1 in [`dw-demo-swift/re-skin.md`](../../dw-demo-swift/references/re-skin.md) | +| PLP list renders zero rows behind HTTP 200 (`empty`/`missing` finding) | Empty or not-yet-repopulated index, or a mis-scoped shop; 200 proves the shell, not the fill | [`foundational/commerce-catalog.md`](foundational/commerce-catalog.md) + [`foundational/search-indexing.md`](foundational/search-indexing.md) (rebuild/repopulate the index) | | ~192px dead grey band inside a section | Bootstrap `.ratio` aspect-ratio token vs CSS custom-property | [`foundational/swift-building.md`](foundational/swift-building.md) §3 | | Blank image wells in a `fullPage` capture | Lazy-load, page not scroll-swept — measurement artifact, not a defect | [`browser-automation.md`](browser-automation.md) verify-flow step 5 | | Blank cells in spec/attribute components (admin shows values) | Stored list-field value is the display name, not `FieldOptionValue` | [`foundational/pim-modelling.md`](foundational/pim-modelling.md) §2.8 | @@ -115,10 +151,12 @@ Findings are data/content defects — fix them through the build-phase action su Batch at most a handful of fixes between re-checks, and never declare a page done from a pre-fix screenshot. -## Definition of done (per demo-critical page) +The detectors above are the **mechanical gate**, not a checklist the agent may skip — every one of them is deterministic, so run them as a hard pass/fail before any eyeballing. The eyeball checklist and interaction pass sit on top; they never substitute for a clean detector run. -- Detectors: `overflowX` 0, `broken`/`stretched` empty, no unexplained gap > 120px, console free of errors, no 404 assets. +- Detectors: `overflowX` 0, `broken`/`stretched`/`tall` empty, no unexplained gap > 120px, console free of errors, no 404 assets. No image band taller than the configured fraction of the viewport (`tall`). +- PLP pages: row count ≥ the demo's `minRows` AND every row carries its required-field selectors (thumbnail / SKU / price / add-to-cart) — an empty or field-short list behind HTTP 200 is a failure, not a pass. - **Theme gate:** the page `` emits all three `Files/System/Styles/{ColorSchemes,Buttons,Typography}` links and the computed body font is the theme's, not the browser's serif fallback. A serif-fallback page renders "almost right" and still fails — that is the silent Style-asset empty-state (`foundational/swift-building.md` §7); the full-page screenshot must read as a *designed* page before the host counts as ready. - Interaction pass: every visible control changes state when used. - Eyeball checklist: pass at desktop AND mobile widths. - Keeper screenshots (both breakpoints) saved under `\notes\qa\` (the canonical QA-evidence home — see `SKILL.md` "Artifact hygiene"; never the demo root). +- **Human sign-off on taste.** The mechanical gate proves structure (caps, rows, no overflow/gaps); it cannot judge hierarchy or brand fit, so an all-green run can still read wrong. Reserve one human decision on the keeper screenshots as the last step. Under an orchestrator this is a stamped sign-off leg (see [`orchestrator.md`](orchestrator.md) "Acceptance criteria"); standalone it is an explicit "does this read as the customer's brand?" review before the page counts as done. Taste stays human without blocking the mechanical gate. diff --git a/skills/dw-demo-swift/references/re-skin.md b/skills/dw-demo-swift/references/re-skin.md index a3d63a3..066f304 100644 --- a/skills/dw-demo-swift/references/re-skin.md +++ b/skills/dw-demo-swift/references/re-skin.md @@ -83,6 +83,7 @@ Operates on a deserialized Swift 2.4 composition (framework-only `base` + `surfa - Run `git status` in `\` -- verify NO `.cs` files changed in `Controllers/` or `Providers/` (would have tripped the customisations-ledger preflight in base) and NO `.scss` / `.ts` files changed (recompilation drift). - `.cshtml` changes are NOT automatically a problem -- new content layouts alongside standard templates are part of the §Pixel-perfect escalation ladder ([`swift-building.md`](../../dw-demo-base/references/foundational/swift-building.md) §9). The thing to avoid is **modifications** to existing standard `.cshtml`. Use `git diff` to confirm `.cshtml` changes are net-new files, not modifications to baseline files. - If a `_custom.css` was edited: that's the doc-canonical override slot, expected. Verify it lives at `Files/Templates/Designs/Swift-v2/Custom/_custom.css` and is loaded by a `Custom/HeadInclude.cshtml` wired to the Master area's `CustomHeadInclude` field. Stock `Custom/custom.css` must remain the hotpink placeholder — run `git diff --name-only -- '*custom.css'` and confirm the only hit is `_custom.css`. +- **Image-band height is a Tier-1 (hard) re-skin item — cap it, do not eyeball it.** The stock `Swift-v2_Image` band and the slider cover-card carry no serialized height field, so a swapped-in photo renders at full column-width height, uncapped, and towers over the fold. Every re-skin that changes photography reproduces this. The durable fix is a Tier-1 (`_custom.css`) CSS cap on the image wrapper and the slider cover-card — `aspect-ratio` + `max-height: min(60vh, 640px)` + `object-fit: cover`. A full-bleed hero may legitimately fill the fold; a content-band image must not. Definition of done: no image band taller than the configured fraction of the viewport, measured by the `tall` detector in [`visual-qa.md`](../../dw-demo-base/references/visual-qa.md) — that mechanical check is the sign-off, not a screenshot glance. ## What this recipe does NOT do