mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-21 10:36:27 +03:00
Fix: do not flag Roboto in system font stacks (#671)
Overused-font primary selection skipped platform UI faces, so a canonical system stack reported the Android fallback as the primary. AI-assisted (Cursor). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
import { GENERIC_FONTS, OVERUSED_FONTS } from '../../shared/constants.mjs';
|
||||
import { OVERUSED_FONTS, primaryFontFace } from '../../shared/constants.mjs';
|
||||
import {
|
||||
checkSourceDesignSystem,
|
||||
collectStaticDesignSystemFindings,
|
||||
@@ -51,9 +51,7 @@ function checkStaticPageTypography(document, window) {
|
||||
for (const el of document.querySelectorAll('p, h1, h2, h3, h4, h5, h6, li, td, th, dd, blockquote, figcaption, a, button, label, span, div')) {
|
||||
const hasText = el.childNodes.some(n => n.nodeType === 3 && n.textContent.trim().length > 0);
|
||||
if (!hasText) continue;
|
||||
const ff = window.getComputedStyle(el).fontFamily || '';
|
||||
const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
|
||||
const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
|
||||
const primary = primaryFontFace(window.getComputedStyle(el).fontFamily);
|
||||
if (!primary) continue;
|
||||
fonts.add(primary);
|
||||
if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
WCAG_LARGE_BOLD_TEXT_PX,
|
||||
WCAG_LARGE_TEXT_PX,
|
||||
isBrandFontOnOwnDomain,
|
||||
primaryFontFace,
|
||||
} from '../shared/constants.mjs';
|
||||
import {
|
||||
CSS_NAMED_COLORS,
|
||||
@@ -331,7 +332,7 @@ function checkIconTile(opts) {
|
||||
function resolveSerif(fontFamily) {
|
||||
if (!fontFamily) return { primary: null, isSerif: false };
|
||||
const tokens = fontFamily.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
|
||||
const primary = tokens.find(f => f && !GENERIC_FONTS.has(f)) || null;
|
||||
const primary = primaryFontFace(fontFamily, GENERIC_FONTS);
|
||||
if (!primary) return { primary: null, isSerif: false };
|
||||
if (KNOWN_SERIF_FONTS.has(primary)) return { primary, isSerif: true };
|
||||
if (tokens.includes('serif')) return { primary, isSerif: true };
|
||||
@@ -3930,8 +3931,7 @@ function checkTypography() {
|
||||
const style = getComputedStyle(el);
|
||||
const ff = style.fontFamily;
|
||||
if (!ff) continue;
|
||||
const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
|
||||
const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
|
||||
const primary = primaryFontFace(ff);
|
||||
if (!primary) continue;
|
||||
fontUsage.set(primary, (fontUsage.get(primary) || 0) + 1);
|
||||
totalTextElements++;
|
||||
@@ -4176,8 +4176,7 @@ function checkPageTypography(doc, win) {
|
||||
if (rule.type !== 1) continue;
|
||||
const ff = rule.style?.fontFamily;
|
||||
if (!ff) continue;
|
||||
const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase());
|
||||
const primary = stack.find(f => f && !GENERIC_FONTS.has(f));
|
||||
const primary = primaryFontFace(ff);
|
||||
if (primary) {
|
||||
fonts.add(primary);
|
||||
if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
|
||||
@@ -4196,11 +4195,10 @@ function checkPageTypography(doc, win) {
|
||||
const ffRe = /font-family\s*:\s*([^;}]+)/gi;
|
||||
let fm;
|
||||
while ((fm = ffRe.exec(html)) !== null) {
|
||||
for (const f of fm[1].split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())) {
|
||||
if (f && !GENERIC_FONTS.has(f)) {
|
||||
fonts.add(f);
|
||||
if (OVERUSED_FONTS.has(f)) overusedFound.add(f);
|
||||
}
|
||||
const primary = primaryFontFace(fm[1]);
|
||||
if (primary) {
|
||||
fonts.add(primary);
|
||||
if (OVERUSED_FONTS.has(primary)) overusedFound.add(primary);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -56,13 +56,27 @@ function isBrandFontOnOwnDomain(font) {
|
||||
return allowed.some(suffix => host === suffix || host.endsWith('.' + suffix));
|
||||
}
|
||||
|
||||
const GENERIC_FONTS = new Set([
|
||||
// Overused-font primary selection skips only CSS generics so a system stack
|
||||
// keeps the system face as primary; GENERIC_FONTS still includes platform
|
||||
// faces for design-system/serif resolution.
|
||||
const CSS_GENERIC_FONTS = new Set([
|
||||
'serif', 'sans-serif', 'monospace', 'cursive', 'fantasy',
|
||||
'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
|
||||
'-apple-system', 'blinkmacsystemfont', 'segoe ui',
|
||||
'inherit', 'initial', 'unset', 'revert',
|
||||
]);
|
||||
|
||||
const GENERIC_FONTS = new Set([
|
||||
...CSS_GENERIC_FONTS,
|
||||
'system-ui', 'ui-serif', 'ui-sans-serif', 'ui-monospace', 'ui-rounded',
|
||||
'-apple-system', 'blinkmacsystemfont', 'segoe ui',
|
||||
]);
|
||||
|
||||
function primaryFontFace(fontFamily, skip = CSS_GENERIC_FONTS) {
|
||||
return String(fontFamily || '')
|
||||
.split(',')
|
||||
.map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase())
|
||||
.find(f => f && !skip.has(f)) || null;
|
||||
}
|
||||
|
||||
// WCAG large text thresholds are defined in points: 18pt normal text and
|
||||
// 14pt bold text. Browsers expose font-size in CSS pixels at 96px per inch.
|
||||
const WCAG_LARGE_TEXT_PX = 18 * (96 / 72);
|
||||
@@ -104,6 +118,7 @@ export {
|
||||
BRAND_FONT_DOMAINS,
|
||||
isBrandFontOnOwnDomain,
|
||||
GENERIC_FONTS,
|
||||
primaryFontFace,
|
||||
WCAG_LARGE_TEXT_PX,
|
||||
WCAG_LARGE_BOLD_TEXT_PX,
|
||||
EM_DASH_FLOOR,
|
||||
|
||||
@@ -674,6 +674,60 @@ describe('detectText — overused fonts', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectHtml — overused fonts system stack', () => {
|
||||
const systemStackCss = `body { font: 15px/1.5 -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; }
|
||||
h1 { font-size: 34px; }
|
||||
p { font-size: 15px; }
|
||||
small { font-size: 12px; }`;
|
||||
const systemStackPage = `<!DOCTYPE html><html><head><style>${systemStackCss}</style></head><body><h1>Hello</h1><p>world</p><small>meta</small></body></html>`;
|
||||
|
||||
test('font shorthand system stack does not flag Roboto as overused', async () => {
|
||||
await withStaticFixture({ 'index.html': systemStackPage }, async ({ file }) => {
|
||||
const f = await detectHtml(file);
|
||||
expect(f.filter(r => r.antipattern === 'overused-font')).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
test('font-family system stack does not flag Roboto as overused', async () => {
|
||||
const page = `<!DOCTYPE html><html><head><style>
|
||||
body { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; }
|
||||
h1 { font-size: 34px; }
|
||||
p { font-size: 15px; }
|
||||
small { font-size: 12px; }
|
||||
</style></head><body><h1>Hello</h1><p>world</p><small>meta</small></body></html>`;
|
||||
await withStaticFixture({ 'index.html': page }, async ({ file }) => {
|
||||
const f = await detectHtml(file);
|
||||
expect(f.filter(r => r.antipattern === 'overused-font')).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
test('Inter before system stack still flags overused-font', async () => {
|
||||
const page = `<!DOCTYPE html><html><head><style>
|
||||
body { font-family: Inter, -apple-system, BlinkMacSystemFont, sans-serif; }
|
||||
h1 { font-size: 34px; }
|
||||
p { font-size: 15px; }
|
||||
</style></head><body><h1>Hello</h1><p>world</p></body></html>`;
|
||||
await withStaticFixture({ 'index.html': page }, async ({ file }) => {
|
||||
const f = await detectHtml(file);
|
||||
expect(f.some(r => r.antipattern === 'overused-font' && /inter/i.test(r.snippet))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
test('checkPageTypography regex path skips Roboto in system stack', () => {
|
||||
const html = `<!DOCTYPE html><html><head><style>
|
||||
body { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; }
|
||||
</style></head><body><h1>Hello</h1><p>world</p></body></html>`;
|
||||
const doc = {
|
||||
styleSheets: [],
|
||||
documentElement: { outerHTML: html },
|
||||
querySelectorAll() { return []; },
|
||||
};
|
||||
const win = { getComputedStyle() { return { fontSize: '16px' }; } };
|
||||
const f = checkPageTypography(doc, win);
|
||||
expect(f.filter(r => r.id === 'overused-font')).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectText — flat type hierarchy', () => {
|
||||
test('flags sizes too close together', () => {
|
||||
const page = '<!DOCTYPE html><html><style>h1{font-size:18px}h2{font-size:16px}h3{font-size:15px}p{font-size:14px}.s{font-size:13px}</style></html>';
|
||||
|
||||
Reference in New Issue
Block a user