From e4a6adbf3aa898b68a1165f7f89b4d7974ca2afc Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 12:46:34 +0000 Subject: [PATCH] design-lint: tenth review cycle fixes tooltip-focus-delay splits the transition value at top-level commas, so a delay after cubic-bezier() or steps() is still read; nestSelector budgets one substitution per '&' before expanding; stripTags with a limit stops reading at the limit instead of after the next tag; accentedHeads reads each span's text once and covers classes from one map; a JSX close tag with no '>' is text, not a failure every ancestor rescans; selectorNames reads the tag that opens an :is()/:where() list; a linked sheet that cannot be read is unreadable, not unparseable; the space probes read their viewport from the page; the internal-error title covers a page or stylesheet the lint could not read. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/design-lint/css.ts | 5 +++-- lib/design-lint/html.ts | 7 +++---- lib/design-lint/render-probe-space.ts | 14 +++++++------- lib/design-lint/rules/ids.ts | 2 +- lib/design-lint/rules/motion.ts | 4 ++-- lib/design-lint/rules/type.ts | 10 ++++++---- lib/design-lint/static.ts | 5 +++-- 7 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/design-lint/css.ts b/lib/design-lint/css.ts index 4b38f3fe3..d539cc13b 100644 --- a/lib/design-lint/css.ts +++ b/lib/design-lint/css.ts @@ -80,7 +80,7 @@ function flatBody(src: string, s: number, e: number): string { export function selectorNames(selectors: string[]): { tags: Set; classes: Set } { const tags = new Set(), classes = new Set(); for (const sel of selectors) { - for (const m of sel.matchAll(/(?:^|[\s,>+~])([a-zA-Z]\w*)/g)) tags.add(m[1]); + for (const m of sel.matchAll(/(?:^|[\s,>+~(])([a-zA-Z]\w*)/g)) tags.add(m[1]); for (const m of sel.matchAll(/\.([\w-]+)/g)) classes.add(m[1]); } return { tags, classes }; @@ -93,7 +93,8 @@ const MAX_SELECTOR = 8192; function nestSelector(sel: string, parent: string): string { const p = parent.includes(',') ? `:is(${parent})` : parent; const parts = splitTop(sel).map((part) => part.trim()); - if (sel.length + p.length * parts.length > MAX_SELECTOR) return parts.map((part) => part.replace(/&/g, '').trim()).filter(Boolean).join(', ') || p; + const substitutions = parts.reduce((n, part) => n + Math.max(1, part.split('&').length - 1), 0); + if (sel.length + p.length * substitutions > MAX_SELECTOR) return parts.map((part) => part.replace(/&/g, '').trim()).filter(Boolean).join(', ') || p; return parts.map((part) => part.includes('&') ? part.replace(/&/g, p) : `${p} ${part}`).join(', '); } diff --git a/lib/design-lint/html.ts b/lib/design-lint/html.ts index 44cfc4339..8c4c23496 100644 --- a/lib/design-lint/html.ts +++ b/lib/design-lint/html.ts @@ -115,14 +115,13 @@ function blankRanges(s: string, ranges: Array<[number, number]>, cut = false): s export function stripTags(s: string, keep = false, limit = Infinity): string { let out = '', i = 0, seen = 0; for (;;) { + if (limit < Infinity) for (let k = i; k < s.length && s[k] !== '<'; k++) if (s[k] > ' ' && ++seen > limit) return out + s.slice(i, k + 1); const lt = s.indexOf('<', i); if (lt < 0) break; const gt = s.indexOf('>', lt + 1); if (gt < 0) break; - const text = s.slice(i, lt); - out += text + (keep ? pad(s.slice(lt, gt + 1)) : ' '); + out += s.slice(i, lt) + (keep ? pad(s.slice(lt, gt + 1)) : ' '); i = gt + 1; - if (limit < Infinity && (seen += (text.match(/\S/g) || []).length) > limit) return out; } return out + s.slice(i); } @@ -533,7 +532,7 @@ class JsxScanner { let j = k; while (j < n && /[\w$.:-]/.test(src[j])) j++; const e = this.ws(j); - if (src[e] !== '>') return -1; + if (src[e] !== '>') { i = e; continue; } const closer = src.slice(k, j); if (closer === name) { if (name) this.paint(i, e + 1); return e + 1; } if (this.open.lastIndexOf(closer) >= 0 && this.open.lastIndexOf(closer) < this.open.length - 1) return i; diff --git a/lib/design-lint/render-probe-space.ts b/lib/design-lint/render-probe-space.ts index 7c5c0bd4e..ba60a6185 100644 --- a/lib/design-lint/render-probe-space.ts +++ b/lib/design-lint/render-probe-space.ts @@ -158,8 +158,8 @@ export function glWords(P: Probes, ctx: HeroCtx): ProbeFinding[] { const w = Math.round(words.reduce((a, x) => { const h = Math.max(1, x.bottom - x.top); const ov = Math.max(0, Math.min(x.bottom, y + 900) - Math.max(x.top, y)); return a + x.c * Math.min(1, ov / h); }, 0)); if (w > maxWords) { maxWords = w; at = y; } } - if (maxWords > 95) out.push({ rule: 'screen-word-budget', grade: 'fail', evidence: maxWords + ' words on one 900px screen (from y=' + at + ')', fix: 'one idea per screen: cut to about 80 words and make the section an object' }); - else if (maxWords > 80) out.push({ rule: 'screen-word-budget', grade: 'warn', evidence: maxWords + ' words on one 900px screen (from y=' + at + '), close to the 80-word budget' }); + if (maxWords > 95) out.push({ rule: 'screen-word-budget', grade: 'fail', evidence: maxWords + ' words on one ' + innerHeight + 'px screen (from y=' + at + ')', fix: 'one idea per screen: cut to about 80 words and make the section an object' }); + else if (maxWords > 80) out.push({ rule: 'screen-word-budget', grade: 'warn', evidence: maxWords + ' words on one ' + innerHeight + 'px screen (from y=' + at + '), close to the 80-word budget' }); if (ctx.stack > 2) out.push({ rule: 'screen-word-budget', grade: 'fail', evidence: 'the hero carries ' + ctx.stack + ' text blocks before its actions', fix: 'headline, one sentence, actions; delete the rest', at: ctx.h1 ? P.H.at(ctx.h1) : undefined }); if (ctx.hero && ctx.h1) { const actions = Array.from(ctx.hero.querySelectorAll('a, button')).filter((c) => vis(c) && (c.textContent || '').trim() && !c.closest('nav, form, figure, footer')); @@ -335,8 +335,8 @@ export function glHeroSpace(P: Probes, ctx: HeroCtx): ProbeFinding[] { heroFoot = { cBottom, navBottom }; } if (heroEnd != null) { - if (heroEnd > vh + 8) out.push({ rule: 'hero-balance', grade: 'fail', evidence: 'the hero runs past the 1440x900 fold (it ends at ' + Math.round(heroEnd) + 'px)', fix: 'size the display and the visual so the hero is one screen', at: heroAt }); - else if (heroEnd < vh * 0.84 && secs.length > 1) out.push({ rule: 'hero-balance', grade: 'fail', evidence: 'the hero ends at ' + Math.round(heroEnd) + 'px of a 900px screen', fix: 'fill the first screen with the visual or the headline, not with copy', at: heroAt }); + if (heroEnd > vh + 8) out.push({ rule: 'hero-balance', grade: 'fail', evidence: 'the hero runs past the ' + innerWidth + 'x' + innerHeight + ' fold (it ends at ' + Math.round(heroEnd) + 'px)', fix: 'size the display and the visual so the hero is one screen', at: heroAt }); + else if (heroEnd < vh * 0.84 && secs.length > 1) out.push({ rule: 'hero-balance', grade: 'fail', evidence: 'the hero ends at ' + Math.round(heroEnd) + 'px of a ' + innerHeight + 'px screen', fix: 'fill the first screen with the visual or the headline, not with copy', at: heroAt }); } if (fig && textBox) { const off = Math.round((fig.top + fig.bottom) / 2 - (textBox.top + textBox.bottom) / 2); @@ -442,10 +442,10 @@ export function glWide(P: Probes, ctx: HeroCtx): ProbeFinding[] { const { vis, text, at } = P.H; const out: ProbeFinding[] = []; const vh = innerHeight; - if (ctx.textBox && ctx.textBox.left < 120) out.push({ rule: 'wide-container', grade: 'fail', evidence: 'the hero text starts ' + Math.round(ctx.textBox.left) + 'px from the edge at 1920', fix: 'cap the container at 1360 to 1520px and centre it', at: ctx.h1 ? at(ctx.h1) : undefined }); - if (ctx.heroEnd != null && ctx.heroEnd > vh + 8) out.push({ rule: 'wide-container', grade: 'fail', evidence: 'the hero runs past the 1920x1080 screen (it ends at ' + Math.round(ctx.heroEnd) + 'px)', fix: 'cap vw-sized visuals and display sizes', at: ctx.hero ? at(ctx.hero) : undefined }); + if (ctx.textBox && ctx.textBox.left < 120) out.push({ rule: 'wide-container', grade: 'fail', evidence: 'the hero text starts ' + Math.round(ctx.textBox.left) + 'px from the edge at ' + innerWidth, fix: 'cap the container at 1360 to 1520px and centre it', at: ctx.h1 ? at(ctx.h1) : undefined }); + if (ctx.heroEnd != null && ctx.heroEnd > vh + 8) out.push({ rule: 'wide-container', grade: 'fail', evidence: 'the hero runs past the ' + innerWidth + 'x' + innerHeight + ' screen (it ends at ' + Math.round(ctx.heroEnd) + 'px)', fix: 'cap vw-sized visuals and display sizes', at: ctx.hero ? at(ctx.hero) : undefined }); const inkWidth = (e: Element) => { const range = document.createRange(); range.selectNodeContents(e); return range.getBoundingClientRect().width; }; const wide = Array.from(document.querySelectorAll('p, h1, h2, h3, li')).filter((e) => vis(e) && !e.querySelector('p, h1, h2, h3, h4, div, figure, ul, ol, table, section') && text(e).length > 40 && inkWidth(e) > 1600); - if (wide.length) out.push({ rule: 'wide-container', grade: 'fail', evidence: wide.length + ' text block' + (wide.length > 1 ? 's run' : ' runs') + ' wider than 1600px at 1920 (first: ' + wide[0].tagName.toLowerCase() + ' "' + text(wide[0]).slice(0, 28) + '")', fix: 'cap the container; hold the measure', at: at(wide[0]) }); + if (wide.length) out.push({ rule: 'wide-container', grade: 'fail', evidence: wide.length + ' text block' + (wide.length > 1 ? 's run' : ' runs') + ' wider than 1600px at ' + innerWidth + ' (first: ' + wide[0].tagName.toLowerCase() + ' "' + text(wide[0]).slice(0, 28) + '")', fix: 'cap the container; hold the measure', at: at(wide[0]) }); return out; } diff --git a/lib/design-lint/rules/ids.ts b/lib/design-lint/rules/ids.ts index 26455db1b..89ee67eff 100644 --- a/lib/design-lint/rules/ids.ts +++ b/lib/design-lint/rules/ids.ts @@ -172,7 +172,7 @@ export const RULES: Record = { 'waiver': { tier: 'ask', gate: 'waiver', title: 'a waiver the lint refused', fix: 'fix the finding, or waive it with the rule id and a reason about this page' }, 'house-style': { tier: 'polish', gate: 'waiver', title: 'more than three rules waived on one page', fix: 'lock the choices into DESIGN.md so later pages inherit them instead of re-arguing them' }, 'render': { tier: 'blocks', gate: 'render', title: 'the page did not render, so its render rules did not run', fix: 'fix the page or the URL and rerun; an unrendered page is never a pass' }, - 'internal-error': { tier: 'ask', gate: 'lint', title: 'a rule family threw on this page, so its rules did not run', fix: 'report the error with the page that caused it; a page the lint could not read is not a pass' }, + 'internal-error': { tier: 'ask', gate: 'lint', title: 'a rule family threw, or a page or stylesheet could not be read, so the lint did not run on it', fix: 'report the error with the page that caused it; a page the lint could not read is not a pass' }, }; diff --git a/lib/design-lint/rules/motion.ts b/lib/design-lint/rules/motion.ts index 1c1e8b0b4..dda1e7135 100644 --- a/lib/design-lint/rules/motion.ts +++ b/lib/design-lint/rules/motion.ts @@ -5,7 +5,7 @@ * missing-states, missing-reduced-motion, autoplay-unmuted, lazy-lcp, video-without-poster, input-state-shift, * control-height, content-hidden-at-rest. */ -import { isTokenRule, pxOf, resolveVars, selectorIsInteractive, stripComments } from '../css'; +import { isTokenRule, pxOf, resolveVars, selectorIsInteractive, splitTop, stripComments } from '../css'; import { mergedRules, type LintContext, type SharedStyles } from '../context'; import { getAttr, hasAttr } from '../html'; import { trunc, type Reporter } from '../report'; @@ -162,7 +162,7 @@ function tooltipDelay(ctx: LintContext, r: Reporter): void { const delayed = (sel: RegExp): { file: string; line: number } | null => { for (const rule of tips) { if (!sel.test(rule.selector)) continue; - const d = rule.decls.find((x) => /^transition(-delay)?$/.test(x.prop) && x.value.split(',').some((part) => { + const d = rule.decls.find((x) => /^transition(-delay)?$/.test(x.prop) && splitTop(x.value).some((part) => { const ms = [...part.matchAll(/(? m[2] === 'ms' ? Number(m[1]) : Number(m[1]) * 1000); return (x.prop === 'transition-delay' ? ms[0] : ms[1]) >= 100; })); diff --git a/lib/design-lint/rules/type.ts b/lib/design-lint/rules/type.ts index 2500d6186..4c923f690 100644 --- a/lib/design-lint/rules/type.ts +++ b/lib/design-lint/rules/type.ts @@ -225,13 +225,15 @@ function accentedHeads(ctx: LintContext): CssRule[] { if (!inner) continue; const headLen = textOf(doc, i).replace(/\s+/g, '').length || 1; const spans = openTags(doc, /^(span|em|strong|b|i|mark)$/, i + 1, closeIndex(doc, i), (s) => !!(getAttr(s.attrs, 'class') || '').trim()) - .map((el) => ({ classes: getAttr(el.tag.attrs, 'class') || '', inner: el.inner })); - const cover = (cls: string) => spans.filter((x) => x.classes.split(/\s+/).includes(cls)).reduce((n, x) => n + plainText(x.inner).replace(/\s+/g, '').length, 0) / headLen; + .map((el) => ({ classes: (getAttr(el.tag.attrs, 'class') || '').split(/\s+/).filter(Boolean), text: plainText(el.inner) })); + const covered = new Map(); + for (const x of spans) for (const cls of x.classes) covered.set(cls, (covered.get(cls) ?? 0) + x.text.replace(/\s+/g, '').length); + const cover = (cls: string) => (covered.get(cls) ?? 0) / headLen; const headDecls = styleFor(ctx, getAttr(t.attrs, 'class'), t.name, true); const own = (prop: string) => valueOf(ctx, headDecls, prop) ?? (prop === 'color' ? resolveVars('var(--color-ink)', ctx.tokens) : null); for (const m of spans) { - if (!/[A-Za-z0-9]/.test(plainText(m.inner))) continue; - for (const cls of m.classes.split(/\s+/).filter(Boolean)) { + if (!/[A-Za-z0-9]/.test(m.text)) continue; + for (const cls of m.classes) { if (cover(cls) >= 0.8) continue; for (const rule of byClass.get(cls) ?? []) { if (seen.has(rule.selector)) continue; diff --git a/lib/design-lint/static.ts b/lib/design-lint/static.ts index 6ff310957..509f00c36 100644 --- a/lib/design-lint/static.ts +++ b/lib/design-lint/static.ts @@ -140,8 +140,9 @@ class CssCache { const raw = fs.readFileSync(f, 'utf8'); out = { raw, rules: parseCss(raw, f), imports: importsOf(raw, path.dirname(f), rootOf(f, this.roots)) }; } catch (e) { - if ((e as NodeJS.ErrnoException).code === 'ENOENT') this.warn(`design-lint: skipped ${f}, not found`); - else { const why = `it could not be parsed (${(e as Error).message})`; this.warn(`design-lint: skipped ${f}, ${why}`); this.skipped.push({ file: f, kind: 'unparseable', why }); } + const code = (e as NodeJS.ErrnoException).code; + if (code === 'ENOENT') this.warn(`design-lint: skipped ${f}, not found`); + else { const why = code ? `it cannot be read (${code})` : `it could not be parsed (${(e as Error).message})`; this.warn(`design-lint: skipped ${f}, ${why}`); this.skipped.push({ file: f, kind: code ? 'unreadable' : 'unparseable', why }); } } this.files.set(f, out); return out;