diff --git a/cli/engine/detect-antipatterns-browser.js b/cli/engine/detect-antipatterns-browser.js index 1926f4672..c3dba70f0 100644 --- a/cli/engine/detect-antipatterns-browser.js +++ b/cli/engine/detect-antipatterns-browser.js @@ -1441,12 +1441,17 @@ function scanCssTextForPseudoStripe(content) { decls.get('height') || decls.get('block-size') || '', customProps)); const verticalCandidate = widthPx != null && widthPx >= 3 && widthPx <= 12; // Horizontal variant (top/bottom stripe) carries extra exemptions: - // link/button underline affordances, tab strips, selected states, and - // state-conditional (:hover/:focus/...) affordances are not stripes. + // link/button underline affordances, selected-state indicators + // (aria-selected="true", aria-current, active/current/selected class + // hints), and state-conditional (:hover/:focus/...) affordances are + // not stripes. Tab-strip membership alone ([role=tab], .tabs, bare + // [aria-selected]) is NOT exempt — a stripe on every tab in the + // group is decoration; only the selected item's underline stays. const horizontalCandidate = heightPx != null && heightPx >= 3 && heightPx <= 12 && !/(?:^|[\s>+~,(])(?:a|button|summary|tr|td|th|table|li)(?![\w-])/i.test(selector) - && !/\[role=["']?tab|\[aria-selected/i.test(selector) - && !/(?:^|[\s._[-])(?:tabs?|tablist|tab-[\w-]*|btn[\w-]*|button[\w-]*|link[\w-]*)(?![\w])/i.test(selector) + && !/\[aria-selected\s*[*^$|~]?=\s*["']?true/i.test(selector) + && !/\[aria-current(?!\s*[*^$|~]?=\s*["']?false)/i.test(selector) + && !/(?:^|[\s._[-])(?:active|current|selected|btn[\w-]*|button[\w-]*|link[\w-]*)(?![\w])/i.test(selector) && !/:(?:hover|focus|focus-visible|focus-within|active|checked)\b/i.test(selector); if (!verticalCandidate && !horizontalCandidate) continue; @@ -1539,12 +1544,14 @@ function scanCssTextForInsetStripe(content) { let m; while ((m = ruleRe.exec(content)) !== null) { const selector = m[1].trim(); - // State/selection contexts: current-item markers, interaction states, - // explicit tab semantics. + // Selection-state contexts: current-item markers and interaction + // states. Tab-strip membership alone ([role=tab], .tabs, bare + // [aria-selected]) is NOT exempt — a stripe on every tab in the + // group is decoration; only the selected item's indicator stays. if (/:(?:hover|focus|focus-visible|focus-within|active|checked|target)\b/i.test(selector)) continue; - if (/\[aria-(?:current|selected)/i.test(selector)) continue; - if (/\[role=["']?tab/i.test(selector)) continue; - if (/(?:^|[\s._[-])(?:active|current|selected|tabs?)(?![\w])/i.test(selector)) continue; + if (/\[aria-selected\s*[*^$|~]?=\s*["']?true/i.test(selector)) continue; + if (/\[aria-current(?!\s*[*^$|~]?=\s*["']?false)/i.test(selector)) continue; + if (/(?:^|[\s._[-])(?:active|current|selected)(?![\w])/i.test(selector)) continue; // Structural tags where a single-edge inset shadow is depth/quoting, // not an accent stripe. if (/(?:^|[\s>+~,(])(?:button|hr|tr|td|th|table|blockquote|pre|code)(?![\w-])/i.test(selector)) continue; @@ -1646,7 +1653,7 @@ function scanCssTextForMarquee(content) { const decls = parseCssDeclBlock(m[2]); for (const name of infiniteAnimationNames(decls)) { if (!marqueeKeyframes.has(name)) continue; - const key = `${selector}${name}`; + const key = `${selector} ${name}`; if (seen.has(key)) continue; seen.add(key); findings.push({ id: 'marquee', snippet: `${selector} — infinite horizontal loop animation "${name}"` }); @@ -2181,21 +2188,23 @@ function resolveBorderRadiusPx(el, style, widthPx, win) { // Browser adapters — call getComputedStyle/getBoundingClientRect on live DOM -// Selected-state / tab-strip context for accent stripes. Explicit tab -// semantics ([role=tablist]/[role=tab]) and active/current-item markers -// (aria-selected, aria-current, active/current/selected class hints) -// exempt the stripe as a selection indicator. A bare nav/menu ancestor -// deliberately does NOT — the same stripe repeated unconditionally on -// every menu item is decoration, not state. +// Selected-state context for accent stripes. Only an actual selection +// marker exempts the stripe as the standard active-item indicator: +// aria-selected="true", aria-current (any non-false value), or an +// active/current/selected class hint. Tab-strip MEMBERSHIP alone +// ([role=tablist]/[role=tab]/.tabs ancestry, aria-selected="false") +// deliberately does not — a chromatic stripe repeated on every tab in +// the group, or on every menu item, is decoration, not state; the +// selected item's own underline stays legal. function isTabContextElement(el) { if (!el) return false; try { - if (el.closest?.('[role="tablist"], [role="tab"], [aria-selected], [aria-current]')) return true; + if (el.closest?.('[aria-selected="true"], [aria-current]:not([aria-current="false"])')) return true; } catch { /* selector engine differences — fall through to class scan */ } let cur = el, depth = 0; while (cur && cur.nodeType === 1 && depth < 6) { const cls = String(cur.getAttribute?.('class') || cur.className || ''); - if (/(?:^|[\s_-])(?:tabs?|active|current|selected)(?:$|[\s_-])/i.test(cls)) return true; + if (/(?:^|[\s_-])(?:active|current|selected)(?:$|[\s_-])/i.test(cls)) return true; cur = cur.parentElement; depth++; } diff --git a/cli/engine/rules/checks.mjs b/cli/engine/rules/checks.mjs index 4e20b080f..aec9f92fe 100644 --- a/cli/engine/rules/checks.mjs +++ b/cli/engine/rules/checks.mjs @@ -740,12 +740,17 @@ function scanCssTextForPseudoStripe(content) { decls.get('height') || decls.get('block-size') || '', customProps)); const verticalCandidate = widthPx != null && widthPx >= 3 && widthPx <= 12; // Horizontal variant (top/bottom stripe) carries extra exemptions: - // link/button underline affordances, tab strips, selected states, and - // state-conditional (:hover/:focus/...) affordances are not stripes. + // link/button underline affordances, selected-state indicators + // (aria-selected="true", aria-current, active/current/selected class + // hints), and state-conditional (:hover/:focus/...) affordances are + // not stripes. Tab-strip membership alone ([role=tab], .tabs, bare + // [aria-selected]) is NOT exempt — a stripe on every tab in the + // group is decoration; only the selected item's underline stays. const horizontalCandidate = heightPx != null && heightPx >= 3 && heightPx <= 12 && !/(?:^|[\s>+~,(])(?:a|button|summary|tr|td|th|table|li)(?![\w-])/i.test(selector) - && !/\[role=["']?tab|\[aria-selected/i.test(selector) - && !/(?:^|[\s._[-])(?:tabs?|tablist|tab-[\w-]*|btn[\w-]*|button[\w-]*|link[\w-]*)(?![\w])/i.test(selector) + && !/\[aria-selected\s*[*^$|~]?=\s*["']?true/i.test(selector) + && !/\[aria-current(?!\s*[*^$|~]?=\s*["']?false)/i.test(selector) + && !/(?:^|[\s._[-])(?:active|current|selected|btn[\w-]*|button[\w-]*|link[\w-]*)(?![\w])/i.test(selector) && !/:(?:hover|focus|focus-visible|focus-within|active|checked)\b/i.test(selector); if (!verticalCandidate && !horizontalCandidate) continue; @@ -838,12 +843,14 @@ function scanCssTextForInsetStripe(content) { let m; while ((m = ruleRe.exec(content)) !== null) { const selector = m[1].trim(); - // State/selection contexts: current-item markers, interaction states, - // explicit tab semantics. + // Selection-state contexts: current-item markers and interaction + // states. Tab-strip membership alone ([role=tab], .tabs, bare + // [aria-selected]) is NOT exempt — a stripe on every tab in the + // group is decoration; only the selected item's indicator stays. if (/:(?:hover|focus|focus-visible|focus-within|active|checked|target)\b/i.test(selector)) continue; - if (/\[aria-(?:current|selected)/i.test(selector)) continue; - if (/\[role=["']?tab/i.test(selector)) continue; - if (/(?:^|[\s._[-])(?:active|current|selected|tabs?)(?![\w])/i.test(selector)) continue; + if (/\[aria-selected\s*[*^$|~]?=\s*["']?true/i.test(selector)) continue; + if (/\[aria-current(?!\s*[*^$|~]?=\s*["']?false)/i.test(selector)) continue; + if (/(?:^|[\s._[-])(?:active|current|selected)(?![\w])/i.test(selector)) continue; // Structural tags where a single-edge inset shadow is depth/quoting, // not an accent stripe. if (/(?:^|[\s>+~,(])(?:button|hr|tr|td|th|table|blockquote|pre|code)(?![\w-])/i.test(selector)) continue; @@ -945,7 +952,7 @@ function scanCssTextForMarquee(content) { const decls = parseCssDeclBlock(m[2]); for (const name of infiniteAnimationNames(decls)) { if (!marqueeKeyframes.has(name)) continue; - const key = `${selector}${name}`; + const key = `${selector} ${name}`; if (seen.has(key)) continue; seen.add(key); findings.push({ id: 'marquee', snippet: `${selector} — infinite horizontal loop animation "${name}"` }); @@ -1480,21 +1487,23 @@ function resolveBorderRadiusPx(el, style, widthPx, win) { // Browser adapters — call getComputedStyle/getBoundingClientRect on live DOM -// Selected-state / tab-strip context for accent stripes. Explicit tab -// semantics ([role=tablist]/[role=tab]) and active/current-item markers -// (aria-selected, aria-current, active/current/selected class hints) -// exempt the stripe as a selection indicator. A bare nav/menu ancestor -// deliberately does NOT — the same stripe repeated unconditionally on -// every menu item is decoration, not state. +// Selected-state context for accent stripes. Only an actual selection +// marker exempts the stripe as the standard active-item indicator: +// aria-selected="true", aria-current (any non-false value), or an +// active/current/selected class hint. Tab-strip MEMBERSHIP alone +// ([role=tablist]/[role=tab]/.tabs ancestry, aria-selected="false") +// deliberately does not — a chromatic stripe repeated on every tab in +// the group, or on every menu item, is decoration, not state; the +// selected item's own underline stays legal. function isTabContextElement(el) { if (!el) return false; try { - if (el.closest?.('[role="tablist"], [role="tab"], [aria-selected], [aria-current]')) return true; + if (el.closest?.('[aria-selected="true"], [aria-current]:not([aria-current="false"])')) return true; } catch { /* selector engine differences — fall through to class scan */ } let cur = el, depth = 0; while (cur && cur.nodeType === 1 && depth < 6) { const cls = String(cur.getAttribute?.('class') || cur.className || ''); - if (/(?:^|[\s_-])(?:tabs?|active|current|selected)(?:$|[\s_-])/i.test(cls)) return true; + if (/(?:^|[\s_-])(?:active|current|selected)(?:$|[\s_-])/i.test(cls)) return true; cur = cur.parentElement; depth++; } diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index 69415e5d0..4edb8d164 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -1081,11 +1081,16 @@ describe('side-tab — pseudo-element stripe variant', () => { expect(scanCssTextForPseudoStripe(btn)).toHaveLength(0); }); - test('skips tab/selected-state underlines (horizontal variant)', () => { - const tab = '[role="tab"][aria-selected="true"]::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; - const tabs = '.tabs .item::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; - expect(scanCssTextForPseudoStripe(tab)).toHaveLength(0); - expect(scanCssTextForPseudoStripe(tabs)).toHaveLength(0); + test('skips selected-state underlines, flags all-tabs underlines (horizontal variant)', () => { + const selectedTab = '[role="tab"][aria-selected="true"]::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; + const activeItem = '.tabs .item.active::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; + expect(scanCssTextForPseudoStripe(selectedTab)).toHaveLength(0); + expect(scanCssTextForPseudoStripe(activeItem)).toHaveLength(0); + // The same stripe on EVERY tab in the group is decoration, not state. + const allTabs = '.tabs .item::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; + const roleTab = '[role="tab"]::after { position: absolute; bottom: 0; left: 0; right: 0; height: 3px; background: #4a7de0; }'; + expect(scanCssTextForPseudoStripe(allTabs)).toHaveLength(1); + expect(scanCssTextForPseudoStripe(roleTab)).toHaveLength(1); }); test('skips hover-state underline affordance (horizontal variant)', () => { @@ -1313,6 +1318,59 @@ describe('inset box-shadow stripe', () => { `; expect(scanCssTextForInsetStripe(css)).toHaveLength(0); }); + + test('flags all-tabs inset underlines, keeps the selected tab exempt', () => { + const allTabs = '.tab { box-shadow: inset 0 -3px 0 #f59e0b; }'; + const roleTab = '[role="tab"] { box-shadow: inset 0 -3px 0 #f59e0b; }'; + const selectedOnly = 'nav button[aria-selected="true"] { box-shadow: inset 0 -3px 0 #f59e0b; }'; + expect(scanCssTextForInsetStripe(allTabs)).toHaveLength(1); + expect(scanCssTextForInsetStripe(roleTab)).toHaveLength(1); + expect(scanCssTextForInsetStripe(selectedOnly)).toHaveLength(0); + }); + + test('static tab strip: chromatic border on every tab flags, selected-only underline stays silent', async () => { + // All-tabs variant: every tab in the group carries the stripe. + await withStaticFixture({ + 'index.html': ` +
+ + + +
+

Departures board

+

Enough body copy for the page-level scanners to treat this as a page.

+ `, + }, async ({ file }) => { + const findings = await detectHtml(file); + const stripes = findings.filter(f => f.antipattern === 'side-tab'); + // The two unselected tabs flag; the selected tab stays exempt. + expect(stripes).toHaveLength(2); + }); + + // Reserved-space variant: transparent on all, chromatic on selected only. + await withStaticFixture({ + 'index.html': ` +
+ + +
+

Departures board

+

Enough body copy for the page-level scanners to treat this as a page.

+ `, + }, async ({ file }) => { + const findings = await detectHtml(file); + expect(findings.filter(f => f.antipattern === 'side-tab')).toHaveLength(0); + }); + }); }); // ---------------------------------------------------------------------------