From 0f3db191e00cdc6776a4ff060b67a60f868d9809 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tao=20Bojl=C3=A9n?= <git@btao.org>
Date: Sat, 20 Jun 2026 16:52:22 +0100
Subject: [PATCH] fix(e2e): address final review (allowlist hover_card demo,
 state-gated locators, docs)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- hover_card: its 'shown' findings are demo-page artifacts (text-xs muted span,
  <h4> heading jump in the demo), so allowlist them instead of fixme — this keeps
  the shown state SCANNED for any other violation. Drop the FOLLOWUPS component
  entry.
- radio_group/slider: return state-gated locators (:checked, :focus, value-changed)
  so a no-op interaction fails the waitFor instead of scanning the wrong state.
- README: correct CI to Tangled (.tangled/workflows/e2e.yml) and Node to 24.

(Reviewer's 'command filtered/option-active will fail' was verified false: role=
combobox requires aria-expanded, which IS present; aria-controls is not required.
Those tests pass — confirmed by running them.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 FOLLOWUPS.md                               |  6 -----
 test/e2e/README.md                         |  5 ++--
 test/e2e/support/allowlist.mjs             |  7 +++++
 test/e2e/support/scenarios/hover_card.mjs  | 31 +++++++---------------
 test/e2e/support/scenarios/radio_group.mjs | 12 +++++----
 test/e2e/support/scenarios/slider.mjs      | 24 ++++++++++++-----
 6 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/FOLLOWUPS.md b/FOLLOWUPS.md
index d90a717..6b6f0d2 100644
--- a/FOLLOWUPS.md
+++ b/FOLLOWUPS.md
@@ -188,12 +188,6 @@ the full rationale in its scenario file's comments. Summary:
   `priv/templates/theme.css`, `website/components/docs/alert_dialog.ex`.
   Scenario: `support/scenarios/alert_dialog.mjs`.
 
-- **[P2] hover_card / shown — `color-contrast` + `heading-order`:** the demo content
-  (`website/components/docs/hover_card.ex`) uses a `text-xs text-muted-foreground` span
-  (~3.46:1) and an `<h4>` that jumps from the page `<h1>`. Both are demo-authoring
-  issues — fix the demo (higher-contrast class; `<p>`/`<strong>` instead of `<h4>`).
-  Scenario: `support/scenarios/hover_card.mjs`.
-
 - **[P1] command / no-results — `aria-required-children` (critical):** like combobox
   (§10), when the query matches nothing the `command_empty` element leaves
   `role="listbox"` with no `role="option"` children. Affects:
diff --git a/test/e2e/README.md b/test/e2e/README.md
index 648d6fa..f036f33 100644
--- a/test/e2e/README.md
+++ b/test/e2e/README.md
@@ -7,7 +7,7 @@ Playwright-based end-to-end test suite for the Shadix docs site. Runs two suites
 
 ## Prerequisites
 
-- Node.js 18+ (managed by mise; `mise install` from repo root)
+- Node.js 24 (managed by mise; `mise install` from repo root)
 - A running Shadix docs server:
   - **Local:** `mix dev` from repo root (serves `http://localhost:4001`)
   - **CI:** a production release build served on `:8080` (set via `PLAYWRIGHT_BASE_URL`)
@@ -81,7 +81,8 @@ Keep regression tests focused on a single interaction flow. Use web-first assert
 
 ## CI
 
-CI runs the full suite against a production release build. The GitHub Actions workflow:
+CI runs the full suite against a production release build. The Tangled workflow
+(`.tangled/workflows/e2e.yml`, nixery engine):
 
 1. Builds the Elixir release with `MIX_ENV=docs mix release shadix_docs`
 2. Starts the release server and waits for it to be healthy
diff --git a/test/e2e/support/allowlist.mjs b/test/e2e/support/allowlist.mjs
index b0ca2c3..8d5b79e 100644
--- a/test/e2e/support/allowlist.mjs
+++ b/test/e2e/support/allowlist.mjs
@@ -62,6 +62,13 @@ export const ALLOW = {
   // Two <nav aria-label="breadcrumb"> demos render on one page; real apps have a
   // single breadcrumb, so the duplicate-landmark finding is a demo artifact.
   breadcrumb: ["landmark-unique"],
+
+  // The hover_card "shown" scenario surfaces two DEMO-authored issues in
+  // website/components/docs/hover_card.ex (not component markup): a
+  // `text-xs text-muted-foreground` span (~3.46:1) and an <h4> in the card body
+  // that jumps heading levels (h1 → h4). Allowlisted (rather than fixme'd) so the
+  // shown state is still scanned for any other violation.
+  hover_card: ["color-contrast", "heading-order"],
 };
 
 export function unexpected(slug, violations) {
diff --git a/test/e2e/support/scenarios/hover_card.mjs b/test/e2e/support/scenarios/hover_card.mjs
index 2d75b03..208241b 100644
--- a/test/e2e/support/scenarios/hover_card.mjs
+++ b/test/e2e/support/scenarios/hover_card.mjs
@@ -7,20 +7,16 @@
 // The content div starts as `hidden` (display:none) and becomes visible on
 // hover, so waitFor() must use { state: "visible" }.
 //
-// One state is exercised:
-//   shown — hover the trigger button, wait for the card content to appear.
+// One state is exercised: shown — hover the trigger, wait for the card content.
 //
-// FIXME defects in the "shown" state:
-//   1. color-contrast (serious): the card body uses `text-xs text-muted-foreground`
-//      (#898989 on #fefefe) which has a contrast ratio of 3.46:1 — below the WCAG
-//      AA requirement of 4.5:1 for small text. The `text-muted-foreground` token
-//      needs to darken to ~#767676 or the text size must become ≥14px bold.
-//      Source: website/components/docs/hover_card.ex (demo content).
-//   2. heading-order (moderate): the card demo uses <h4> directly inside the card
-//      body which, after the page h1, causes a heading-level jump (h1 → h4,
-//      skipping h2/h3). The card content heading should be <p> or <strong> unless
-//      the page hierarchy warrants h4.
-//      Source: website/components/docs/hover_card.ex (demo content).
+// The "shown" state has two DEMO-PAGE artifacts (not component defects), handled
+// via the allowlist rather than fixme — that keeps this state SCANNED for any
+// other violation while suppressing the known demo noise:
+//   - color-contrast: the demo body uses a `text-xs text-muted-foreground` span
+//     (~3.46:1). It is demo-authored text, not component markup.
+//   - heading-order: the demo uses an <h4> in the card body (h1 → h4 jump).
+// Both live in website/components/docs/hover_card.ex. See support/allowlist.mjs
+// (hover_card entry).
 
 const preview = (page) => page.locator("[data-shadix-preview]").first();
 const trigger = (page) =>
@@ -30,16 +26,7 @@ const content = (page) =>
 
 export default [
   {
-    // FIXME: Two genuine a11y defects appear when the card is open:
-    //   1. color-contrast (serious) — text-xs text-muted-foreground contrast is
-    //      3.46:1, below the 4.5:1 WCAG AA minimum for small text.
-    //   2. heading-order (moderate) — <h4> in the card body jumps heading levels
-    //      (h1 → h4) on the docs page, producing an invalid heading order.
-    // See FOLLOWUPS.md for details.
     name: "shown",
-    fixme: true,
-    fixmeReason:
-      "color-contrast: text-xs text-muted-foreground (#898989) has 3.46:1 contrast ratio, below 4.5:1 WCAG AA; heading-order: <h4> in card body jumps from h1 directly to h4",
     run: async (page) => {
       await trigger(page).hover();
       await content(page).waitFor({ state: "visible" });
diff --git a/test/e2e/support/scenarios/radio_group.mjs b/test/e2e/support/scenarios/radio_group.mjs
index 5bdeff1..9d5abc3 100644
--- a/test/e2e/support/scenarios/radio_group.mjs
+++ b/test/e2e/support/scenarios/radio_group.mjs
@@ -9,21 +9,23 @@
 // selectors sourced from lib/shadix/components/radio_group.ex.
 
 const preview = (page) => page.locator("[data-shadix-preview]").first();
-const group = (page) => preview(page).locator("[data-slot='radio-group']").first();
 // The "pro" option is the second radio in the configurator preview; it is NOT
 // pre-selected, so clicking it produces a genuine state change.
 const proRadio = (page) =>
   preview(page).locator("[data-slot='radio-group-item'][value='pro']").first();
+// The same radio in its :checked state — only matches AFTER the click succeeds,
+// so the driver's waitFor() fails loudly if the selection didn't take.
+const proRadioChecked = (page) =>
+  preview(page).locator("[data-slot='radio-group-item'][value='pro']:checked").first();
 
 export default [
   {
     name: "option-selected",
     run: async (page) => {
-      // Click a radio that is not pre-selected to drive the "selected" state.
       await proRadio(page).click();
-      // Return the group container — it is always visible and its children's
-      // :checked state reflects the selection.
-      return group(page);
+      // Return the :checked locator so the scan only runs once the selection
+      // actually landed (a state-gated locator, per the scenario contract).
+      return proRadioChecked(page);
     },
   },
 ];
diff --git a/test/e2e/support/scenarios/slider.mjs b/test/e2e/support/scenarios/slider.mjs
index 1987f35..624f19f 100644
--- a/test/e2e/support/scenarios/slider.mjs
+++ b/test/e2e/support/scenarios/slider.mjs
@@ -14,6 +14,11 @@
 
 const preview = (page) => page.locator("[data-shadix-preview]").first();
 const slider = (page) => preview(page).locator("[data-slot='slider']").first();
+// The slider in its :focus state — only matches once the input is actually
+// focused, so a failed focus() fails the driver's waitFor() rather than silently
+// scanning an unfocused slider.
+const sliderFocused = (page) =>
+  preview(page).locator("[data-slot='slider']:focus").first();
 
 export default [
   {
@@ -22,20 +27,25 @@ export default [
       // Focus the native range input — the thumb becomes keyboard-active and
       // aria-valuenow / aria-valuemin / aria-valuemax are exposed to AT.
       await slider(page).focus();
-      await slider(page).waitFor({ state: "visible" });
-      return slider(page);
+      return sliderFocused(page);
     },
   },
   {
     name: "value-changed",
     run: async (page) => {
-      // Focus then press ArrowRight to increment the value by one step.
-      // The native input updates its value attribute synchronously; we wait
-      // for the element to remain visible (it never hides) and return it.
+      // Focus then press ArrowRight to increment the value by one step. Wait for
+      // the native value to actually change so a no-op key press is caught.
       await slider(page).focus();
+      const before = await slider(page).inputValue();
       await slider(page).press("ArrowRight");
-      await slider(page).waitFor({ state: "visible" });
-      return slider(page);
+      await page.waitForFunction(
+        ([sel, prev]) => {
+          const el = document.querySelector(sel);
+          return el && el.value !== prev;
+        },
+        ["[data-shadix-preview] [data-slot='slider']", before],
+      );
+      return sliderFocused(page);
     },
   },
 ];
-- 
2.51.2