mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-18 17:16:46 +03:00
Merge pull request #412 from pbakaus/static-named-color-borders
Fix side-tab false negative on named colors in the static-html engine
This commit is contained in:
Paul Bakaus
@@ -2,7 +2,7 @@ import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
import { profileStep, recordProfileEvent } from '../../profile/profiler.mjs';
|
||||
import { collectCssCustomProps, cssLengthToPx, parseAnyColor, resolveLengthPx, resolveVarRefs } from '../../rules/checks.mjs';
|
||||
import { CSS_NAMED_COLORS, collectCssCustomProps, cssLengthToPx, parseAnyColor, resolveLengthPx, resolveVarRefs } from '../../rules/checks.mjs';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// jsdom CSS-variable border override map
|
||||
@@ -344,18 +344,29 @@ const STATIC_PROP_MAP = {
|
||||
'overflow-y': 'overflowY',
|
||||
};
|
||||
|
||||
// parseStaticColor tries parseAnyColor first, which already resolves every
|
||||
// name in the shared CSS_NAMED_COLORS table. This fallback only carries the
|
||||
// keywords parseAnyColor deliberately returns null for: the cascade needs
|
||||
// `transparent` to read as an actual zero-alpha color.
|
||||
const STATIC_NAMED_COLORS = {
|
||||
black: { r: 0, g: 0, b: 0, a: 1 },
|
||||
white: { r: 255, g: 255, b: 255, a: 1 },
|
||||
transparent: { r: 0, g: 0, b: 0, a: 0 },
|
||||
gray: { r: 128, g: 128, b: 128, a: 1 },
|
||||
grey: { r: 128, g: 128, b: 128, a: 1 },
|
||||
silver: { r: 192, g: 192, b: 192, a: 1 },
|
||||
red: { r: 255, g: 0, b: 0, a: 1 },
|
||||
green: { r: 0, g: 128, b: 0, a: 1 },
|
||||
blue: { r: 0, g: 0, b: 255, a: 1 },
|
||||
};
|
||||
|
||||
// Named-color alternation for plucking a color token out of shorthand values
|
||||
// (issue #359: a hardcoded 9-name list here silently dropped `purple`,
|
||||
// `crimson`, `teal`, ... from border shorthands, so the side defaulted to
|
||||
// neutral black and side-tab never fired on .html files). Derived from the
|
||||
// same table parseAnyColor resolves against, so extraction and parsing can't
|
||||
// drift apart. Longest-first so names containing other names as substrings
|
||||
// (rebeccapurple) are matched whole.
|
||||
const NAMED_COLOR_TOKENS = [...Object.keys(CSS_NAMED_COLORS), ...Object.keys(STATIC_NAMED_COLORS)]
|
||||
.sort((a, b) => b.length - a.length)
|
||||
.join('|');
|
||||
const STATIC_COLOR_TOKEN_RE = new RegExp(
|
||||
`(?:rgba?\\([^)]+\\)|oklch\\([^)]+\\)|oklab\\([^)]+\\)|lch\\([^)]+\\)|lab\\([^)]+\\)|hsla?\\([^)]+\\)|hwb\\([^)]+\\)|#[0-9a-f]{3,8}\\b|\\b(?:${NAMED_COLOR_TOKENS})\\b)`,
|
||||
'i'
|
||||
);
|
||||
|
||||
function splitCssList(value) {
|
||||
const parts = [];
|
||||
let depth = 0, quote = '', start = 0;
|
||||
@@ -441,7 +452,7 @@ function extractStaticColor(value) {
|
||||
}
|
||||
return '';
|
||||
}
|
||||
const colorLike = raw.match(/(?:rgba?\([^)]+\)|oklch\([^)]+\)|oklab\([^)]+\)|lch\([^)]+\)|lab\([^)]+\)|hsla?\([^)]+\)|hwb\([^)]+\)|#[0-9a-f]{3,8}\b|\b(?:black|white|gray|grey|silver|red|green|blue|transparent)\b)/i);
|
||||
const colorLike = raw.match(STATIC_COLOR_TOKEN_RE);
|
||||
if (!colorLike) return '';
|
||||
return colorLike[0];
|
||||
}
|
||||
|
||||
@@ -5369,6 +5369,7 @@ function checkFirstViewportColumnOverflowDOM() {
|
||||
}
|
||||
|
||||
export {
|
||||
CSS_NAMED_COLORS,
|
||||
checkBorders,
|
||||
isEmojiOnlyText,
|
||||
checkColors,
|
||||
|
||||
@@ -366,6 +366,42 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('named-color-borders: named-color side-tabs are flagged, neutral names pass', async () => {
|
||||
// Regression for issue #359: the static cascade's shorthand color
|
||||
// extraction recognized only 9 named colors, so `border-left: 4px solid
|
||||
// purple` (or any of the other named colors parseAnyColor understands)
|
||||
// lost its color during expansion, defaulted to neutral black, and never
|
||||
// fired side-tab — while the same declaration in a .css file was flagged
|
||||
// by the regex engine. The extraction list is now derived from the same
|
||||
// CSS_NAMED_COLORS table the parser uses, so the two can't drift apart.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'named-color-borders.html'));
|
||||
const sideTabs = f.filter(r => r.antipattern === 'side-tab').map(r => r.snippet).sort();
|
||||
// Six FLAG cases, each with a unique width/radius signature so every
|
||||
// finding attributes to exactly one case (an offsetting miss + false
|
||||
// positive can't cancel out in an aggregate count):
|
||||
// purple 4px + radius 8 (the issue reproducer), rebeccapurple 5px +
|
||||
// radius 4 (contains "purple" as a substring — whole-token matching),
|
||||
// crimson 4px top stripe, bare 3px teal, var() resolving to a named
|
||||
// color at 6px + radius 4, and a 7px inline style attribute.
|
||||
// The PASS column (neutral named colors at 3-4px, 1px thin, uniform)
|
||||
// must contribute nothing — dimgray/gainsboro/black have to parse AND
|
||||
// read as neutral rather than being dropped as unknown colors, and none
|
||||
// of its shapes can produce any of the signatures below.
|
||||
assert.deepEqual(sideTabs, [
|
||||
'border-left: 3px',
|
||||
'border-left: 4px + border-radius: 8px',
|
||||
'border-left: 5px + border-radius: 4px',
|
||||
'border-left: 6px + border-radius: 4px',
|
||||
'border-left: 7px',
|
||||
'border-top: 4px',
|
||||
]);
|
||||
const borderAccent = f.filter(r => r.antipattern === 'border-accent-on-rounded');
|
||||
assert.equal(
|
||||
borderAccent.length, 0,
|
||||
`expected 0 border-accent-on-rounded, got ${borderAccent.length}: ${borderAccent.map(r => r.snippet).join('; ')}`
|
||||
);
|
||||
});
|
||||
|
||||
it('modern-color-borders: regex fallback skips neutral 1px oklch dividers', () => {
|
||||
const css = `
|
||||
.flag-side-tab {
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
<!DOCTYPE html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="UTF-8">
|
||||
<title>Side-Tab with CSS Named Colors (purple/rebeccapurple/crimson/teal)</title>
|
||||
<style>
|
||||
/* Two-column fixture: left col = should-flag, right col = should-pass.
|
||||
Regression for issue #359: `border-left: 4px solid purple` in a .html
|
||||
file never fired side-tab because the static cascade's shorthand color
|
||||
extraction only recognized 9 named colors — every other spec name
|
||||
(purple, rebeccapurple, crimson, teal, ...) was dropped from the
|
||||
shorthand, leaving the side at the default black, which reads as
|
||||
neutral and silently skips the check. The same declaration in a .css
|
||||
file was flagged by the regex engine, so the two engines disagreed. */
|
||||
|
||||
:root {
|
||||
--accent: orange; /* named color behind a var() — must resolve and flag */
|
||||
}
|
||||
|
||||
body { font-family: system-ui, sans-serif; margin: 0; padding: 24px; background: #fafafa; }
|
||||
.grid { display: grid; grid-template-columns: 1fr 1fr; gap: 32px; max-width: 1120px; margin: 0 auto; }
|
||||
.col h2 { font-size: 14px; text-transform: uppercase; letter-spacing: 0.05em; margin: 0 0 16px; color: #475569; }
|
||||
.case { padding: 12px 16px; margin-bottom: 16px; }
|
||||
.case h3 { font-size: 14px; margin: 0 0 4px; }
|
||||
.case p { font-size: 13px; margin: 0; color: #64748b; }
|
||||
|
||||
/* ── FLAG cases: colored side-tab borders using CSS named colors ── */
|
||||
|
||||
/* 1: the exact reproducer from issue #359 — named purple in a <style>
|
||||
rule with background + padding + radius */
|
||||
#flag-named-purple {
|
||||
width: 400px;
|
||||
background: #f6f6f6;
|
||||
padding: 16px;
|
||||
border-left: 4px solid purple;
|
||||
border-radius: 8px;
|
||||
}
|
||||
|
||||
/* 2: rebeccapurple — a longer name that contains another color name
|
||||
("purple") as a substring; must match whole-token. Width 5px so the
|
||||
finding snippet is unique to this case. */
|
||||
#flag-named-rebecca {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 5px solid rebeccapurple;
|
||||
}
|
||||
|
||||
/* 3: horizontal stripe variant — named crimson riding the top edge */
|
||||
#flag-named-crimson-top {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-top: 4px solid crimson;
|
||||
}
|
||||
|
||||
/* 4: named teal, 3px, no radius — the bare w >= 3 arm */
|
||||
#flag-named-teal {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-left: 3px solid teal;
|
||||
}
|
||||
|
||||
/* 5: named color behind a var() in the shorthand. Width 6px so the
|
||||
finding snippet is unique to this case. */
|
||||
#flag-named-var {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 6px solid var(--accent);
|
||||
}
|
||||
|
||||
/* ── PASS cases: neutral named colors and non-side-tab shapes ── */
|
||||
|
||||
/* 1: dimgray — chromatic-looking name, neutral value; must NOT fire */
|
||||
#pass-named-dimgray {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 4px solid dimgray;
|
||||
}
|
||||
|
||||
/* 2: gainsboro — light neutral named color */
|
||||
#pass-named-gainsboro {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 3px solid gainsboro;
|
||||
}
|
||||
|
||||
/* 3: named black side border — neutral, not a colored stripe */
|
||||
#pass-named-black {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 4px solid black;
|
||||
}
|
||||
|
||||
/* 4: 1px named purple — too thin to qualify */
|
||||
#pass-named-thin {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border-radius: 4px;
|
||||
border-left: 1px solid purple;
|
||||
}
|
||||
|
||||
/* 5: uniform named purple border on all four sides — not a side-tab */
|
||||
#pass-named-allsides {
|
||||
width: 400px;
|
||||
background: #ffffff;
|
||||
border: 3px solid purple;
|
||||
border-radius: 4px;
|
||||
}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<div class="grid">
|
||||
<div class="col" data-col="flag">
|
||||
<h2>Should flag</h2>
|
||||
<div class="case" id="flag-named-purple"><h3>named purple</h3><p>border-left 4px solid purple + radius</p></div>
|
||||
<div class="case" id="flag-named-rebecca"><h3>rebeccapurple</h3><p>border-left 4px + radius</p></div>
|
||||
<div class="case" id="flag-named-crimson-top"><h3>crimson top stripe</h3><p>border-top 4px, horizontal variant</p></div>
|
||||
<div class="case" id="flag-named-teal"><h3>named teal</h3><p>border-left 3px, no radius</p></div>
|
||||
<div class="case" id="flag-named-var"><h3>var() to named</h3><p>border-left 4px solid var(--accent)</p></div>
|
||||
<div class="case" style="width: 400px; background: #ffffff; border-left: 7px solid purple">
|
||||
<h3>inline named purple</h3><p>style attribute, issue #359 case (a); width 7px keeps the snippet unique</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="col" data-col="pass">
|
||||
<h2>Should pass</h2>
|
||||
<div class="case" id="pass-named-dimgray"><h3>dimgray</h3><p>neutral named color</p></div>
|
||||
<div class="case" id="pass-named-gainsboro"><h3>gainsboro</h3><p>light neutral named color</p></div>
|
||||
<div class="case" id="pass-named-black"><h3>named black</h3><p>neutral side border</p></div>
|
||||
<div class="case" id="pass-named-thin"><h3>1px purple</h3><p>too thin to qualify</p></div>
|
||||
<div class="case" id="pass-named-allsides"><h3>uniform purple</h3><p>all four sides, not a stripe</p></div>
|
||||
</div>
|
||||
</div>
|
||||
<script src="/js/detect-antipatterns-browser.js"></script>
|
||||
</body>
|
||||
</html>
|
||||
Reference in New Issue
Block a user