mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
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>
This commit is contained in:
1 parent
50b1190241
commit
e4a6adbf3a
7 files changed
+25
-22
No files matched your search
@@ -80,7 +80,7 @@ function flatBody(src: string, s: number, e: number): string {
|
||||
export function selectorNames(selectors: string[]): { tags: Set<string>; classes: Set<string> } {
|
||||
const tags = new Set<string>(), classes = new Set<string>();
|
||||
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(', ');
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -172,7 +172,7 @@ export const RULES: Record<string, RuleMeta> = {
|
||||
'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' },
|
||||
};
|
||||
|
||||
|
||||
|
||||
@@ -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(/(?<![\w.-])(\d*\.?\d+)(ms|s)\b/g)].map((m) => m[2] === 'ms' ? Number(m[1]) : Number(m[1]) * 1000);
|
||||
return (x.prop === 'transition-delay' ? ms[0] : ms[1]) >= 100;
|
||||
}));
|
||||
|
||||
@@ -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<string, number>();
|
||||
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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user