fix(showcase): break wn-breadcrumb slotchange infinite loop (WHYNOT-WP-0002 T11)
WnBreadcrumb._onSlot inserted separator <span>s into its own light DOM on slotchange but cleaned up in the shadow DOM, so they were never removed — each insertion re-fired slotchange, looping the main thread and wedging the showcase page. Made _onSlot idempotent: exclude own separators when reading items, and mutate only when separators are not already correct. - Un-fixme the showcase visual test; add a warm-up full-page capture so deviceScaleFactor-2 sub-pixel snapping settles before the assertion. All 5 visual tests pass. - Remove the dead Google-Fonts @import from colors_and_type.css (token stacks are system-ui; webfont unused + a CI-flake source; no visual change). - Unblocks WHYNOT-WP-0003 T08 (showcase = per-version visual catalog); both T11 and T08 marked done. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
17
CHANGELOG.md
17
CHANGELOG.md
@@ -6,6 +6,23 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Version
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Showcase page no longer wedges the renderer** (WHYNOT-WP-0002 T11). `<wn-breadcrumb>`
|
||||
inserted separator elements into its own light DOM on `slotchange` while cleaning up
|
||||
in the shadow DOM, so separators were never removed — each insertion re-fired
|
||||
`slotchange` and the main thread looped forever, so `examples/showcase/index.html`
|
||||
never finished rendering. `WnBreadcrumb._onSlot` is now idempotent (excludes its own
|
||||
separators; mutates only when they are not already correct). The showcase visual test
|
||||
is un-`fixme`'d and captures a stable `showcase.png`, unblocking WHYNOT-WP-0003 T08
|
||||
(showcase as per-version visual catalog).
|
||||
|
||||
### Removed
|
||||
|
||||
- **Dead Google-Fonts `@import`** from `src/styles/colors_and_type.css`. Every token
|
||||
font stack is system-ui based, so the imported IBM Plex webfont was unused; it was
|
||||
also a documented source of CI flakiness. No visual change (system fonts unchanged).
|
||||
|
||||
### Added
|
||||
|
||||
- **Versioned IR manifest + consumer drift-check** (WHYNOT-WP-0003, Phase 1–4). The
|
||||
|
||||
@@ -236,11 +236,23 @@ export class WnEmptyState extends WnBase {
|
||||
export class WnBreadcrumb extends WnBase {
|
||||
_onSlot(e) {
|
||||
const slot = e.target;
|
||||
const items = slot.assignedElements({ flatten: true });
|
||||
// Build the rendered tree: each item + a separator after it.
|
||||
const wrapper = this.shadowRoot?.querySelector('.wn-breadcrumb__list');
|
||||
if (!wrapper) return;
|
||||
wrapper.querySelectorAll('.wn-breadcrumb__sep').forEach(s => s.remove());
|
||||
// Separators are inserted into the LIGHT DOM (so they sit in document order
|
||||
// between the slotted items), which re-fires this slotchange. We must
|
||||
// therefore be idempotent: exclude our own separators when reading items,
|
||||
// and skip all mutation once the separators are already correct — otherwise
|
||||
// each insertion retriggers slotchange and the main thread loops forever.
|
||||
const items = slot.assignedElements({ flatten: true })
|
||||
.filter((el) => !el.classList.contains("wn-breadcrumb__sep"));
|
||||
const existing = [...this.querySelectorAll(":scope > .wn-breadcrumb__sep")];
|
||||
|
||||
if (existing.length === Math.max(0, items.length - 1)) {
|
||||
// Structure already correct — only refresh the "current" marker, do not
|
||||
// touch the child list (no mutation ⇒ no slotchange re-fire ⇒ loop ends).
|
||||
items.forEach((el, i) => el.classList.toggle("wn-breadcrumb__current", i === items.length - 1));
|
||||
return;
|
||||
}
|
||||
|
||||
existing.forEach((s) => s.remove());
|
||||
items.forEach((el, i) => {
|
||||
el.classList.toggle("wn-breadcrumb__current", i === items.length - 1);
|
||||
if (i > 0) {
|
||||
@@ -248,8 +260,6 @@ export class WnBreadcrumb extends WnBase {
|
||||
sep.className = "wn-breadcrumb__sep";
|
||||
sep.setAttribute("aria-hidden", "true");
|
||||
sep.textContent = "/";
|
||||
// Use light-DOM-relative insertion: items are still in light DOM,
|
||||
// so DOM-order separators between them belong in light DOM too.
|
||||
el.parentNode.insertBefore(sep, el);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -7,8 +7,11 @@
|
||||
artefacts over heavy fills.
|
||||
============================================================ */
|
||||
|
||||
/* ---------- Webfonts (Google Fonts, see /fonts for offline) ---------- */
|
||||
@import url("https://fonts.googleapis.com/css2?family=IBM+Plex+Mono:wght@400;500;600&family=IBM+Plex+Sans:wght@300;400;500;600;700&family=IBM+Plex+Serif:ital,wght@0,400;0,500;1,400&display=swap");
|
||||
/* No webfont is loaded. Every token font stack is system-ui based
|
||||
(ui-sans-serif / ui-monospace / ui-serif), so the design ships with zero
|
||||
network font dependency. (Historically this imported IBM Plex from Google
|
||||
Fonts; that webfont was unused and a source of CI flakiness, so it was
|
||||
dropped.) */
|
||||
|
||||
/* @generated tokens — regenerated by `make adapt-lit` from ir/tokens.json. DO NOT EDIT. */
|
||||
:root {
|
||||
|
||||
@@ -8,28 +8,30 @@ import { test, expect } from "@playwright/test";
|
||||
//
|
||||
// To update intentionally: pnpm test:visual:update
|
||||
|
||||
// The design-tokens stylesheet (colors_and_type.css) @imports IBM Plex from
|
||||
// Google Fonts, but every token font stack is system-ui based — the webfont is
|
||||
// unused. Left live it intermittently hangs in CI, blocking the page's module
|
||||
// <script> (a pending stylesheet defers script execution) so custom elements
|
||||
// never register. Abort the font CDNs so baselines are deterministic & offline.
|
||||
// Defensive: no webfont is loaded anymore (token font stacks are system-ui
|
||||
// based), but abort the font CDNs anyway so a re-introduced webfont can never
|
||||
// make a baseline non-deterministic or network-dependent.
|
||||
test.beforeEach(async ({ page }) => {
|
||||
await page.route(/fonts\.(googleapis|gstatic)\.com/, (route) => route.abort());
|
||||
});
|
||||
|
||||
test.describe("showcase — every component", () => {
|
||||
// KNOWN BROKEN — tracked as adhoc against WHYNOT-WP-0002. The showcase page
|
||||
// (every component on one page) wedges the renderer main thread when its
|
||||
// module executes: components + vendored lit render fine in isolation, but
|
||||
// one demo composition on this page infinite-loops, so the page never
|
||||
// reaches `load` and no `showcase.png` baseline can be captured. The four
|
||||
// whynot-control baselines are unaffected. Remove `.fixme` once the looping
|
||||
// component is fixed and regenerate the baseline.
|
||||
test.fixme("renders", async ({ page }) => {
|
||||
// Previously KNOWN BROKEN (WHYNOT-WP-0002 T11): the page wedged the renderer
|
||||
// main thread because <wn-breadcrumb> inserted separators into its own light
|
||||
// DOM on slotchange, re-firing slotchange in an infinite loop. Fixed by making
|
||||
// WnBreadcrumb._onSlot idempotent (src/elements/layout.js).
|
||||
test("renders", async ({ page }) => {
|
||||
await page.goto("/examples/showcase/index.html");
|
||||
// Wait for custom elements to register + Lit to render.
|
||||
await page.waitForFunction(() => !!customElements.get("wn-button"));
|
||||
await page.waitForTimeout(800);
|
||||
// Warm-up capture: the first full-page screenshot resizes the viewport to
|
||||
// the full content height, and at deviceScaleFactor 2 that first layout pass
|
||||
// snaps several sections by ≤4px. Discard it so the page is settled before
|
||||
// the real assertion — otherwise toHaveScreenshot fails its "two consecutive
|
||||
// stable screenshots" check on this long page.
|
||||
await page.screenshot({ fullPage: true });
|
||||
await page.waitForTimeout(200);
|
||||
await expect(page).toHaveScreenshot("showcase.png", { fullPage: true });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -216,11 +216,27 @@ gate that confirms Lit actually matches the designbook appearance.
|
||||
|
||||
```task
|
||||
id: WHYNOT-WP-0002-T11
|
||||
status: todo
|
||||
status: done
|
||||
priority: medium
|
||||
state_hub_task_id: "7435338d-702a-43d7-9c86-49531fe0d8e4"
|
||||
```
|
||||
|
||||
**Resolved 2026-06-27.** Root cause: `WnBreadcrumb._onSlot` (`src/elements/layout.js`)
|
||||
inserted separator `<span>`s into its own **light DOM** on `slotchange`, but its
|
||||
cleanup queried the **shadow** wrapper for separators — so they were never removed.
|
||||
Each insertion mutated the breadcrumb's children, re-firing `slotchange`, inserting
|
||||
more separators, looping the main thread forever. (The earlier "specific demo
|
||||
composition" hypothesis was a misdiagnosis — the minimal repro simply omitted
|
||||
`<wn-breadcrumb>`; a minimal page *with* it reproduces the wedge, *without* it
|
||||
registers in ~300ms.) Fix: made `_onSlot` idempotent — it excludes its own
|
||||
separators when reading items and skips all DOM mutation once separators are
|
||||
already correct, so the self-triggered `slotchange` is a no-op and the loop ends.
|
||||
Also removed the dead Google-Fonts `@import` from `colors_and_type.css` (token
|
||||
font stacks are system-ui; the webfont was unused and a documented CI-flake source).
|
||||
`test.fixme` removed in `tests/visual/ui-kit.spec.mjs`; the showcase now renders and
|
||||
captures a stable `showcase.png` (a warm-up full-page capture settles deviceScaleFactor-2
|
||||
sub-pixel snapping before the assertion). All 5 visual tests pass.
|
||||
|
||||
Discovered 2026-06-26 while regenerating visual baselines after the T06 token
|
||||
regen. The `examples/showcase/index.html` "every component" page wedges the
|
||||
renderer main thread when its module executes — the page never reaches `load`
|
||||
|
||||
@@ -225,7 +225,7 @@ anything, complementing the machine-readable manifest.
|
||||
|
||||
```task
|
||||
id: WHYNOT-WP-0003-T08
|
||||
status: wait
|
||||
status: done
|
||||
priority: low
|
||||
state_hub_task_id: "a0886a4f-cf27-44ef-b8c6-8e61ceda1f84"
|
||||
```
|
||||
@@ -236,6 +236,13 @@ just to confirm, once T11 lands, that the showcase is deployable/inspectable per
|
||||
version (e.g. served from a tag) — no new build, reuse the existing page. Blocked on
|
||||
WP-0002-T11.
|
||||
|
||||
**Done 2026-06-27.** WP-0002-T11 landed (breadcrumb infinite-loop fixed); the
|
||||
showcase page now renders deterministically and is covered by a passing visual test
|
||||
(`showcase.png` baseline). No new build — the existing page *is* the per-version
|
||||
visual catalog: served statically from any checkout/tag (`pnpm showcase`), it pairs
|
||||
with the machine-readable `ir/manifest.json` + `ir/INDEX.md` as the visual side of a
|
||||
version's inventory. Nothing further to build.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Deferred: live contract-parity mode (design-only)
|
||||
|
||||
Reference in New Issue
Block a user