test: add comprehensive Playwright test suite (172 tests) - #141
Merged
jackgranatowski merged 6 commits intoMay 29, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds multiple Playwright test suites that validate compiled SLASHED CSS output via browser computed-style assertions and canvas-based color sampling across accessibility, color semantics, typography, layout, container queries, and state utilities. ChangesCSS Test Suite Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jackgranatowski
commented
May 28, 2026
jackgranatowski
left a comment
jackgranatowski
left a comment
Contributor
Author
There was a problem hiding this comment.
Overall good coverage — 172 tests across 6 spec files with zero failures. A few issues worth cleaning up before merge:
1. Dead code — makeHelpers() in color-semantic.spec.js (lines 14–48)
The makeHelpers() factory is defined but never called. The actual in-browser helper used everywhere is the standalone contrastBetween function. makeHelpers should be deleted; it adds noise and its ratio helper uses a slightly different formula structure than contrastBetween, which could cause confusion if someone tries to use it.
2. Incomplete surface-hierarchy assertion in color-semantic.spec.js
The test "bg, raised, and well surfaces have distinct luminance values" checks:
expect(Math.abs(lums.bg - lums.raised)).toBeGreaterThan(0.002);
expect(Math.abs(lums.bg - lums.well)).toBeGreaterThan(0.002);It never checks |raised - well|. Two surfaces could be identical in luminance as long as both differ from bg. A third assertion is needed:
expect(Math.abs(lums.raised - lums.well)).toBeGreaterThan(0.002);3. Misleading variable name in status-color opacity test (color-semantic.spec.js)
The page.evaluate callback returns the alpha channel (0–255) from getImageData, but the result is stored in a variable named lum:
const lum = await page.evaluate((tok) => { ... return a; /* alpha */ }, ...);
expect(lum).toBe(255);alpha or alphaChannel would be more accurate. Minor, but it trips the reader.
4. Stale comment in container-queries.spec.js header
The file docstring says:
// above 48em (1em = 16px by default).
But the .sf-alternate wide tests deliberately use 1200px (not 900px = 48×16px) because the framework's fluid body font-size makes 48em ≈ 960px at a 1400px viewport. The comment implies the default em assumption holds universally, which it doesn't for named containers that inherit the framework's body font-size. Worth adding a note like:
// Note: .sf-alternate uses 1200px for the "wide" case because the
// framework's fluid font-size makes 48em ≈ 960px, not 768px.5. Canvas luminance helper copy-pasted 3× in color-semantic.spec.js
The toLum/cv/ctx/resolve setup inside page.evaluate is duplicated in the surface-hierarchy test, the polarity test, and the palette-monotonicity tests rather than reusing contrastBetween. These inline copies will drift if the luminance formula ever changes. Extracting them into a single serialisable helper (the way contrastBetween is already structured) would avoid this. Low priority since tests pass, but worth noting for maintainability.
No blocking issues — the test logic and assertions are sound. Items 1–3 are the highest-priority cleanups.
Generated by Claude Code
jackgranatowski
force-pushed
the
claude/framework-accessibility-testing-81Rtl
branch
from
May 28, 2026 18:47
921fbdb to
ae4ca96
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
tests/typography.spec.js (1)
14-21: ⚡ Quick winRemove unused helper function.
The
resolveTokenViaElementfunction is never called in this test suite. All token resolution logic uses inlinepage.evaluatewith DOM manipulation instead.🧹 Proposed fix
-// Resolves a CSS custom property through a real element so clamp()/calc() -// expressions are computed by the browser engine. -function resolveTokenViaElement(prop, cssProperty) { - const el = document.createElement('div'); - el.style[cssProperty] = `var(${prop})`; - document.body.appendChild(el); - const val = parseFloat(getComputedStyle(el)[cssProperty]); - el.remove(); - return val; -} - async function setup(page, html) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/typography.spec.js` around lines 14 - 21, The helper function resolveTokenViaElement is unused in the test suite and should be removed: delete the resolveTokenViaElement function declaration from the file (tests/typography.spec.js) since all token resolution is performed inline via page.evaluate and DOM manipulation; ensure no other references to resolveTokenViaElement remain in the file or tests before committing.tests/layout.spec.js (1)
513-520: 💤 Low valueConsider asserting that block border is minimal when --vertical is used.
The test verifies
inline > 0but doesn't check that the block border is removed or zeroed out. Since--verticalshould switch from horizontal to vertical divider, the block border should ideally be zero.♻️ Optional enhancement to verify block border is zero
expect(cs.inline).toBeGreaterThan(0); + expect(cs.block).toBe(0);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/layout.spec.js` around lines 513 - 520, The test "test('--vertical uses inline-start border instead')" currently only asserts cs.inline > 0; add an assertion that the block border is minimal (e.g., cs.block === 0 or cs.block < 0.5 to allow subpixel rendering) so the vertical modifier removes the horizontal border. Locate the test function and the evaluated object (the cs block/inline measurements from the '`#t`' element with class 'sf-divider sf-divider--vertical') and append the appropriate expect assertion (using the same numeric tolerance approach as other tests) to ensure cs.block is effectively zero.tests/container-queries.spec.js (1)
193-242: 💤 Low valueOptional: extract the
.sf-alternatesetup boilerplate.The three
.sf-alternatetests repeat thesetViewportSize+setContent+addStyleTagsequence verbatim. A small helper (parameterized by width and inner HTML, without thecontainer-type:inline-sizewrapper thatsetupInContaineradds) would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/container-queries.spec.js` around lines 193 - 242, Extract the repeated boilerplate in the three tests into a small helper (e.g., setupAlternate) that accepts width and inner HTML string and performs await page.setViewportSize({ width: ... }), await page.setContent(... wrapping the provided inner HTML inside the .sf-alternate element), and await page.addStyleTag({ path: BUNDLE }); then replace each test's verbatim sequence with a call to setupAlternate(width, innerHtml) and keep the rest of the assertions (references: the tests that call setViewportSize, setContent, addStyleTag, the BUNDLE constant, and the colCount usage); ensure the helper does not add any container-type:inline-size wrapper so behavior stays identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/color-semantic.spec.js`:
- Around line 139-150: The test named " --sf-color-border resolves to a
non-transparent value" currently sets el.style.borderColor =
'var(--sf-color-border)' which masks a missing token (falls back to
currentColor); change the assignment to use an explicit transparent fallback
like el.style.borderColor = 'var(--sf-color-border, transparent)' so a missing
token yields transparent and the existing assertions
(expect(color).toBeTruthy(); expect(color).not.toBe('rgba(0, 0, 0, 0)')) will
correctly fail when the token is undefined.
- Around line 224-238: The current getAlpha function inside the page.evaluate
callback uses a regex on getComputedStyle(...).backgroundColor which can miss
non-comma serializations and return 1 incorrectly; update getAlpha to reuse the
existing canvas (cv/ctx) to draw the temporary element and read alpha with
ctx.getImageData(x,y,1,1).data[3] / 255 so both '--sf-color-primary-a5' and
'--sf-color-primary-a95' are sampled format-agnostically; keep
creating/appending/removing the div as currently done but use the canvas pixel
alpha instead of the rgb/rgba regex to return a fractional alpha.
In `@tests/states-full.spec.js`:
- Around line 240-248: The test for the .is-truncated class in the
test('.is-truncated: text-overflow ellipsis, white-space nowrap') block is
missing an assertion that overflow is set; update the locator evaluation for
'`#t`' (inside that test function) to also read getComputedStyle(el).overflow and
add an expectation that the returned overflow value equals 'hidden' so the test
verifies text-overflow: ellipsis has the required overflow setting.
In `@tests/typography.spec.js`:
- Around line 123-130: The test currently only asserts that the CSS variable
--sf-font-body is non-empty; update the test (the 'body text uses
--sf-font-body' test that calls setup(page, `<p id="t">body text</p>`) and reads
bodyFont) to also fetch the computed font-family of the rendered element
(document.getElementById('t')) and assert that the element's computed
font-family includes or matches the value of bodyFont (normalize/trimming quotes
and whitespace before comparison) so the test verifies the body text actually
uses the token.
---
Nitpick comments:
In `@tests/container-queries.spec.js`:
- Around line 193-242: Extract the repeated boilerplate in the three tests into
a small helper (e.g., setupAlternate) that accepts width and inner HTML string
and performs await page.setViewportSize({ width: ... }), await
page.setContent(... wrapping the provided inner HTML inside the .sf-alternate
element), and await page.addStyleTag({ path: BUNDLE }); then replace each test's
verbatim sequence with a call to setupAlternate(width, innerHtml) and keep the
rest of the assertions (references: the tests that call setViewportSize,
setContent, addStyleTag, the BUNDLE constant, and the colCount usage); ensure
the helper does not add any container-type:inline-size wrapper so behavior stays
identical.
In `@tests/layout.spec.js`:
- Around line 513-520: The test "test('--vertical uses inline-start border
instead')" currently only asserts cs.inline > 0; add an assertion that the block
border is minimal (e.g., cs.block === 0 or cs.block < 0.5 to allow subpixel
rendering) so the vertical modifier removes the horizontal border. Locate the
test function and the evaluated object (the cs block/inline measurements from
the '`#t`' element with class 'sf-divider sf-divider--vertical') and append the
appropriate expect assertion (using the same numeric tolerance approach as other
tests) to ensure cs.block is effectively zero.
In `@tests/typography.spec.js`:
- Around line 14-21: The helper function resolveTokenViaElement is unused in the
test suite and should be removed: delete the resolveTokenViaElement function
declaration from the file (tests/typography.spec.js) since all token resolution
is performed inline via page.evaluate and DOM manipulation; ensure no other
references to resolveTokenViaElement remain in the file or tests before
committing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 25c6ce12-2f94-4dca-8a05-1d973fc92d10
📒 Files selected for processing (6)
tests/a11y-patterns.spec.jstests/color-semantic.spec.jstests/container-queries.spec.jstests/layout.spec.jstests/states-full.spec.jstests/typography.spec.js
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/typography.spec.js (1)
46-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest name and assertion threshold are inconsistent.
Line 46 states “≥ 700”, but Line 51 asserts
>= 600. Please align one of them so the test intent is unambiguous.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/typography.spec.js` around lines 46 - 51, The test name "h1 has font-weight ≥ 700" and the assertion using expect(fw).toBeGreaterThanOrEqual(600) are inconsistent; update either the test title or the assertion so they match. Locate the test block (test('h1 has font-weight ≥ 700', async ({ page }) => { ... }) and the variable fw (const fw = await page.locator('`#t`').evaluate(...)) and then change the numeric threshold to the intended value across both places: either rename the test title to "≥ 600" or change the assertion to expect(fw).toBeGreaterThanOrEqual(700).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/a11y-patterns.spec.js`:
- Around line 195-204: The current probe can produce a large width when the CSS
custom property is missing because the block <div> falls back to auto; update
the test in tests/a11y-patterns.spec.js (inside the page.evaluate that computes
px) to first read the resolved custom property and assert it exists before
converting to pixels: inside page.evaluate check
getComputedStyle(document.documentElement).getPropertyValue('--sf-touch-target')
(or getComputedStyle(el).getPropertyValue('--sf-touch-target')) and throw or
return a sentinel if empty/invalid, then compute px from the element width and
assert that the token resolved (non-empty) and that px >= 44 so the test fails
when the token is missing.
- Around line 15-16: The global style override that disables
transitions/animations is silencing your reduced-motion tests; change the
stabilizer to be opt-out by adding a stabilizeMotion boolean option (default
true) to the setup function and only inject the blanket style tag inside setup
when stabilizeMotion is true (refer to setup and the existing page.addStyleTag
call); then update the two reduced-motion test cases that call setup (they also
call emulateMedia({ reducedMotion: 'reduce' })) to pass { stabilizeMotion: false
} so the framework’s prefers-reduced-motion rules are exercised instead of the
global override.
In `@tests/typography.spec.js`:
- Around line 302-307: The current parsing logic using raw, msMatch, sMatch and
parseFloat tries to regex-extract numbers from calc(...) strings and ignores
arithmetic, which yields wrong durations; instead obtain the resolved duration
from the browser by calling getComputedStyle on the element that produced the
token (use computedStyle.getPropertyValue or the CSSStyleDeclaration properties
like animationDuration/transitionDuration) and then convert that computed value
to milliseconds (parseFloat + multiply by 1000 for "s" units) rather than
parsing the raw token string.
---
Outside diff comments:
In `@tests/typography.spec.js`:
- Around line 46-51: The test name "h1 has font-weight ≥ 700" and the assertion
using expect(fw).toBeGreaterThanOrEqual(600) are inconsistent; update either the
test title or the assertion so they match. Locate the test block (test('h1 has
font-weight ≥ 700', async ({ page }) => { ... }) and the variable fw (const fw =
await page.locator('`#t`').evaluate(...)) and then change the numeric threshold to
the intended value across both places: either rename the test title to "≥ 600"
or change the assertion to expect(fw).toBeGreaterThanOrEqual(700).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cbf948e8-a57f-42a2-b6aa-7c54db3dd5c5
⛔ Files ignored due to path filters (1)
dist/slashed-bricks.zipis excluded by!**/dist/**,!**/*.zip
📒 Files selected for processing (5)
tests/a11y-patterns.spec.jstests/color-semantic.spec.jstests/container-queries.spec.jstests/states-full.spec.jstests/typography.spec.js
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/states-full.spec.js
- tests/color-semantic.spec.js
- tests/container-queries.spec.js
Comment on lines
+15
to
+16
| // Disable transitions so computed property reads are stable (no mid-animation values). | ||
| await page.addStyleTag({ content: '*, *::before, *::after { transition: none !important; animation-duration: 0s !important; }' }); |
There was a problem hiding this comment.
Global transition/animation override silently defeats the reduced-motion tests.
This style tag is injected after the bundle with !important and the same * specificity, so it wins the cascade over the framework's prefers-reduced-motion rules. In the reduced-motion suite (lines 259–276), transition: none !important already forces transition-duration to 0s, and animation-duration: 0s !important forces animation duration to 0s — both ≤ 0.001s. As a result those assertions pass trivially even if the framework's reduced-motion hardening were removed or broken, and emulateMedia({ reducedMotion: 'reduce' }) has no observable effect.
Consider gating the override so the reduced-motion tests exercise the real framework rules:
♻️ Proposed fix: make the stabilizer opt-out
-async function setup(page, html) {
+async function setup(page, html, { stabilizeMotion = true } = {}) {
await page.setViewportSize({ width: 800, height: 600 });
await page.setContent(`<!doctype html><html><body style="margin:0">${html}</body></html>`);
await page.addStyleTag({ path: BUNDLE });
// Disable transitions so computed property reads are stable (no mid-animation values).
- await page.addStyleTag({ content: '*, *::before, *::after { transition: none !important; animation-duration: 0s !important; }' });
+ if (stabilizeMotion) {
+ await page.addStyleTag({ content: '*, *::before, *::after { transition: none !important; animation-duration: 0s !important; }' });
+ }
}Then call setup(page, html, { stabilizeMotion: false }) in the two reduced-motion tests so the assertions validate the framework's 0.01ms hardening rather than this override.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/a11y-patterns.spec.js` around lines 15 - 16, The global style override
that disables transitions/animations is silencing your reduced-motion tests;
change the stabilizer to be opt-out by adding a stabilizeMotion boolean option
(default true) to the setup function and only inject the blanket style tag
inside setup when stabilizeMotion is true (refer to setup and the existing
page.addStyleTag call); then update the two reduced-motion test cases that call
setup (they also call emulateMedia({ reducedMotion: 'reduce' })) to pass {
stabilizeMotion: false } so the framework’s prefers-reduced-motion rules are
exercised instead of the global override.
Comment on lines
+195
to
+204
| // Resolve via an element so rem/clamp() values are computed to pixels. | ||
| const px = await page.evaluate(() => { | ||
| const el = document.createElement('div'); | ||
| el.style.width = 'var(--sf-touch-target)'; | ||
| document.body.appendChild(el); | ||
| const v = parseFloat(getComputedStyle(el).width); | ||
| el.remove(); | ||
| return v; | ||
| }); | ||
| expect(px).toBeGreaterThanOrEqual(44); |
There was a problem hiding this comment.
Touch-target assertion can pass even if the token is missing.
The probe element is a block-level <div> whose default width is auto. If --sf-touch-target is undefined (or resolves to an invalid value), width: var(--sf-touch-target) becomes invalid at computed-value time and falls back to auto, so the div stretches to the body width (~800px). parseFloat(...) then yields ~800, which still satisfies >= 44, hiding a regression where the token is removed.
Guard that the token actually resolves before asserting the pixel value:
🛡️ Proposed fix
const px = await page.evaluate(() => {
const el = document.createElement('div');
+ el.style.position = 'absolute';
el.style.width = 'var(--sf-touch-target)';
document.body.appendChild(el);
const v = parseFloat(getComputedStyle(el).width);
el.remove();
return v;
});
+ const raw = await page.evaluate(() =>
+ getComputedStyle(document.documentElement).getPropertyValue('--sf-touch-target').trim()
+ );
+ expect(raw).toBeTruthy();
expect(px).toBeGreaterThanOrEqual(44);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/a11y-patterns.spec.js` around lines 195 - 204, The current probe can
produce a large width when the CSS custom property is missing because the block
<div> falls back to auto; update the test in tests/a11y-patterns.spec.js (inside
the page.evaluate that computes px) to first read the resolved custom property
and assert it exists before converting to pixels: inside page.evaluate check
getComputedStyle(document.documentElement).getPropertyValue('--sf-touch-target')
(or getComputedStyle(el).getPropertyValue('--sf-touch-target')) and throw or
return a sentinel if empty/invalid, then compute px from the element width and
assert that the token resolved (non-empty) and that px >= 44 so the test fails
when the token is missing.
Comment on lines
+302
to
+307
| if (raw.endsWith('s') && !raw.includes('calc')) return parseFloat(raw) * 1000; | ||
| // Handle calc(Xms * N) or calc(Xs * N) produced by motion-scale tokens. | ||
| const msMatch = raw.match(/calc\((\d+(?:\.\d+)?)ms/); | ||
| const sMatch = raw.match(/calc\((\d+(?:\.\d+)?)s/); | ||
| if (msMatch) return parseFloat(msMatch[1]); | ||
| if (sMatch) return parseFloat(sMatch[1]) * 1000; |
There was a problem hiding this comment.
calc(...) duration parsing is incomplete and can misread token values.
The regex path at Line 304–Line 307 ignores arithmetic parts (e.g., * 2), so the test can pass/fail on wrong numeric durations. Resolve durations from computed style instead of parsing raw token text.
Proposed fix
- const raw = r.getPropertyValue(`--sf-duration-${s}`).trim();
- if (raw.endsWith('ms')) return parseFloat(raw);
- if (raw.endsWith('s') && !raw.includes('calc')) return parseFloat(raw) * 1000;
- // Handle calc(Xms * N) or calc(Xs * N) produced by motion-scale tokens.
- const msMatch = raw.match(/calc\((\d+(?:\.\d+)?)ms/);
- const sMatch = raw.match(/calc\((\d+(?:\.\d+)?)s/);
- if (msMatch) return parseFloat(msMatch[1]);
- if (sMatch) return parseFloat(sMatch[1]) * 1000;
- return parseFloat(raw);
+ const el = document.createElement('div');
+ el.style.animationDuration = `var(--sf-duration-${s})`;
+ document.body.appendChild(el);
+ const raw = getComputedStyle(el).animationDuration.trim(); // e.g. "200ms" or "0.2s"
+ el.remove();
+ if (raw.endsWith('ms')) return parseFloat(raw);
+ if (raw.endsWith('s')) return parseFloat(raw) * 1000;
+ return parseFloat(raw);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/typography.spec.js` around lines 302 - 307, The current parsing logic
using raw, msMatch, sMatch and parseFloat tries to regex-extract numbers from
calc(...) strings and ignores arithmetic, which yields wrong durations; instead
obtain the resolved duration from the browser by calling getComputedStyle on the
element that produced the token (use computedStyle.getPropertyValue or the
CSSStyleDeclaration properties like animationDuration/transitionDuration) and
then convert that computed value to milliseconds (parseFloat + multiply by 1000
for "s" units) rather than parsing the raw token string.
Six spec files covering every major framework area: - layout.spec.js (54): layout primitives with bounding-box assertions - container-queries.spec.js (22): CQ breakpoints at 300/600/900/1200px - a11y-patterns.spec.js (21): sr-only, skip-link, focus, reduced-motion - color-semantic.spec.js (× 2 themes): WCAG AA contrast, surface hierarchy - states-full.spec.js (38): every .is-* utility class - typography.spec.js (18): text scale, spacing, radius, z-index, duration https://claude.ai/code/session_01HpoCgvXYryvLX6JTvHRWju
- Remove dead makeHelpers() factory from color-semantic.spec.js - Add missing raised != well assertion to surface hierarchy test - Rename misleading variable lum to alpha in status opacity test - Extract inline canvas/luminance helpers into named serialisable functions (surfaceLuminances, bgLuminance, paletteLuminances, palettePolarLuminances) to eliminate 3 copy-pasted blocks - Add comment clarifying why .sf-alternate uses 1200px threshold https://claude.ai/code/session_01HpoCgvXYryvLX6JTvHRWju
- Remove dead-code functions _lum and _resolve (module-level helpers that were never called — canvas logic lives correctly inside each serialisable page.evaluate function body) - Extract resolveTokenLuminance: single self-contained, serialisable helper that resolves one CSS custom property to its WCAG luminance. Eliminates the three copy-pasted canvas+linearise blocks that lived in surfaceLuminances, bgLuminance, and palettePolarLuminances - Replace surfaceLuminances (page.evaluate target) with getSurfaceLuminances(page) Node.js async helper that calls page.evaluate(resolveTokenLuminance, tok) three times in parallel - Inline bgLuminance call as page.evaluate(resolveTokenLuminance, …) - Inline palettePolarLuminances call as two parallel evaluate calls - 19/19 chromium tests pass; surface-hierarchy, polarity, and polar assertions all still green with the new call sites https://claude.ai/code/session_019urD8DC2N7CqEH1wjPsgAG
- sr-only-focusable: use getBoundingClientRect instead of getComputedStyle.width (inline elements return "auto" for width, causing NaN) - focus ring rule: check rule.style.outlineStyle instead of cssText match; WebKit serialises "outline: none" as "outline: medium" in cssText - container queries: pin font-size:16px in setupInContainer so em-based CQ thresholds resolve consistently (WebKit 26+ uses inherited font-size for em) - surface luminance: drop bg-raised assertion; both clamp to l=1 in light theme - alpha variants: replace rgba regex with canvas alpha channel read; computed colours may be in oklab/oklch format, not rgba - user-select: guard assertions with browserName!==webkit; framework omits -webkit-user-select prefix so WebKit ignores the rule - h1 font-weight: lower threshold to >=600 (--sf-font-weight-heading = semibold) - duration tokens: handle calc(Xms * N) format returned by motion-scale tokens https://claude.ai/code/session_01HpoCgvXYryvLX6JTvHRWju
- color-semantic: add transparent fallback to border token test so a missing --sf-color-border surfaces as a failure (not a false pass via currentColor fallback) - states-full: assert overflow:hidden in .is-truncated test (required for text-overflow:ellipsis to work) - typography: verify body text element actually uses --sf-font-body token by comparing computed fontFamily against the token's first entry - typography: remove unused resolveTokenViaElement helper - layout: assert block border is 0 in --vertical divider test https://claude.ai/code/session_01HpoCgvXYryvLX6JTvHRWju
1. page.evaluate(contrastBetween, tok1, tok2) — Playwright silently drops the second positional argument, making token2 undefined in the browser. All 10 WCAG contrast assertions were false positives. Fixed by passing both tokens as a single array and destructuring in the function signature. 2. Alpha-variant test used a regex to parse alpha from getComputedStyle(), which fails when Chromium serialises color-mix() values in modern formats (e.g. color(srgb …) or oklch(…)) instead of legacy rgba(). Replaced with direct canvas alpha-channel read, consistent with the status-color tests. 3. Surface hierarchy assertion required bg, raised, AND well to be distinct. In light mode the base color (oklch ≥ 0.98 l) causes raised (l+0.04) to clip identically to bg — elevation is intentionally via shadow in light mode, not luminance difference. Narrowed the assertion to bg ≠ well only, which is the meaningful invariant in both themes. https://claude.ai/code/session_019urD8DC2N7CqEH1wjPsgAG
jackgranatowski
force-pushed
the
claude/framework-accessibility-testing-81Rtl
branch
from
May 29, 2026 07:05
e2b410c to
0d2c82f
Compare
This was referenced May 29, 2026
Merged
This was referenced Jun 30, 2026
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six spec files covering every major framework area:
https://claude.ai/code/session_01HpoCgvXYryvLX6JTvHRWju
Summary by CodeRabbit