mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
Fix flat type hierarchy false positives (#702)
* Fix flat type hierarchy false positives Use rendered semantic roles and dominant size frequency, align the adjacent-step guidance, and abstain in source-only scans.\n\nAI assistance: prepared with Codex under maintainer direction. * Fix static hidden typography filtering Honor the hidden attribute in the static wrapper and use raw browser findings in regression coverage. AI assistance: prepared with Codex under maintainer direction. * Align typography sampling with painted content Count visibly painted aria-hidden text and exclude content-visibility hidden subtrees in both static and browser scans. AI assistance: prepared with Codex under maintainer direction.
This commit is contained in:
@@ -159,7 +159,7 @@ const ANTIPATTERNS = [
|
||||
scopes: ['type'],
|
||||
name: 'Flat type hierarchy',
|
||||
description:
|
||||
'Font sizes are too close together — no clear visual hierarchy. Use fewer sizes with more contrast (aim for at least a 1.25 ratio between steps).',
|
||||
'Dominant heading and body roles are separated by less than 1.25× at every step, leaving the size hierarchy flat. Add at least one stronger size step.',
|
||||
skillSection: 'Typography',
|
||||
skillGuideline: 'flat type hierarchy',
|
||||
},
|
||||
@@ -5184,6 +5184,88 @@ function checkElementGlow(tag, style, effectiveBg) {
|
||||
|
||||
// ─── Section 6: Page-Level Checks ───────────────────────────────────────────
|
||||
|
||||
const TYPE_HIERARCHY_SELECTOR = 'h1,h2,h3,h4,h5,h6,p,li,td,th,dd,blockquote,figcaption';
|
||||
const TYPE_HIERARCHY_MIN_ROLES = 3;
|
||||
const TYPE_HIERARCHY_MIN_STEP_RATIO = 1.25;
|
||||
|
||||
function typeHierarchyRole(el) {
|
||||
const tag = String(el?.tagName || el?.nodeName || '').toLowerCase();
|
||||
return /^h[1-6]$/.test(tag) ? tag : 'body';
|
||||
}
|
||||
|
||||
function hasTextContent(el) {
|
||||
return String(el?.textContent || '').trim().length > 0;
|
||||
}
|
||||
|
||||
function isRenderedTypeElement(el, getStyle) {
|
||||
for (let current = el; current; current = current.parentElement) {
|
||||
const hiddenAttr = typeof current.getAttribute === 'function' && current.getAttribute('hidden') !== null;
|
||||
if (current.hidden || hiddenAttr) return false;
|
||||
const style = getStyle(current);
|
||||
if (!style) continue;
|
||||
const display = String(style.display || '').toLowerCase();
|
||||
const visibility = String(style.visibility || '').toLowerCase();
|
||||
const contentVisibility = String(style.contentVisibility || '').toLowerCase();
|
||||
if (display === 'none' || visibility === 'hidden' || visibility === 'collapse' || contentVisibility === 'hidden') return false;
|
||||
const opacity = parseFloat(style.opacity);
|
||||
if (Number.isFinite(opacity) && opacity <= 0.01) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function dominantTypeRoleSize(samples) {
|
||||
const counts = new Map();
|
||||
for (const sample of samples) {
|
||||
counts.set(sample.size, (counts.get(sample.size) || 0) + 1);
|
||||
}
|
||||
const ranked = [...counts.entries()].sort((a, b) => b[1] - a[1] || a[0] - b[0]);
|
||||
if (ranked.length > 1 && ranked[0][1] === ranked[1][1]) return null;
|
||||
return ranked[0]?.[0] ?? null;
|
||||
}
|
||||
|
||||
function checkFlatTypeHierarchySamples(samples) {
|
||||
const byRole = new Map();
|
||||
for (const sample of samples || []) {
|
||||
const role = String(sample?.role || '');
|
||||
const size = Math.round(Number(sample?.size) * 10) / 10;
|
||||
if (!role || !Number.isFinite(size) || size < 8 || size >= 200) continue;
|
||||
if (!byRole.has(role)) byRole.set(role, []);
|
||||
byRole.get(role).push({ role, size });
|
||||
}
|
||||
|
||||
const roles = [...byRole.entries()].map(([role, roleSamples]) => ({
|
||||
role,
|
||||
size: dominantTypeRoleSize(roleSamples),
|
||||
})).filter(item => item.size !== null);
|
||||
|
||||
if (roles.length < TYPE_HIERARCHY_MIN_ROLES) return [];
|
||||
|
||||
const sorted = roles.slice().sort((a, b) => a.size - b.size || a.role.localeCompare(b.role));
|
||||
let largestStep = 1;
|
||||
for (let i = 1; i < sorted.length; i++) {
|
||||
largestStep = Math.max(largestStep, sorted[i].size / sorted[i - 1].size);
|
||||
}
|
||||
if (largestStep >= TYPE_HIERARCHY_MIN_STEP_RATIO) return [];
|
||||
|
||||
const roleSizes = sorted.map(item => `${item.role} ${item.size}px`).join(', ');
|
||||
return [{
|
||||
id: 'flat-type-hierarchy',
|
||||
snippet: `Role sizes: ${roleSizes} (largest adjacent step ${largestStep.toFixed(2)}:1; target ${TYPE_HIERARCHY_MIN_STEP_RATIO}:1)`,
|
||||
}];
|
||||
}
|
||||
|
||||
function checkFlatTypeHierarchyFromDoc(root, getStyle, options = {}) {
|
||||
const samples = [];
|
||||
for (const el of root.querySelectorAll(TYPE_HIERARCHY_SELECTOR)) {
|
||||
if (options.skipElement?.(el)) continue;
|
||||
if (!hasTextContent(el) || !isRenderedTypeElement(el, getStyle)) continue;
|
||||
const fontSize = parseFloat(getStyle(el)?.fontSize);
|
||||
if (!Number.isFinite(fontSize) || fontSize < 8 || fontSize >= 200) continue;
|
||||
samples.push({ role: typeHierarchyRole(el), size: fontSize });
|
||||
}
|
||||
return checkFlatTypeHierarchySamples(samples);
|
||||
}
|
||||
|
||||
// Browser page-level checks — use document/getComputedStyle globals
|
||||
|
||||
function checkTypography() {
|
||||
@@ -5222,17 +5304,10 @@ function checkTypography() {
|
||||
}
|
||||
}
|
||||
|
||||
const sizes = new Set();
|
||||
for (const el of document.querySelectorAll('h1,h2,h3,h4,h5,h6,p,span,a,li,td,th,label,button,div')) {
|
||||
const fs = parseFloat(getComputedStyle(el).fontSize);
|
||||
if (fs > 0 && fs < 200) sizes.add(Math.round(fs * 10) / 10);
|
||||
}
|
||||
if (sizes.size >= 3) {
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio < 2.0) {
|
||||
findings.push({ type: 'flat-type-hierarchy', detail: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` });
|
||||
}
|
||||
for (const finding of checkFlatTypeHierarchyFromDoc(document, getComputedStyle, {
|
||||
skipElement: el => el.closest?.('.impeccable-overlay, .impeccable-label, .impeccable-banner, .impeccable-tooltip, [id^="impeccable-live-"]'),
|
||||
})) {
|
||||
findings.push({ type: finding.id, detail: finding.snippet });
|
||||
}
|
||||
|
||||
return findings;
|
||||
@@ -5479,21 +5554,7 @@ function checkPageTypography(doc, win) {
|
||||
findings.push({ id: 'overused-font', snippet: `Primary font: ${font}` });
|
||||
}
|
||||
|
||||
// Flat type hierarchy
|
||||
const sizes = new Set();
|
||||
const textEls = doc.querySelectorAll('h1, h2, h3, h4, h5, h6, p, span, a, li, td, th, label, button, div');
|
||||
for (const el of textEls) {
|
||||
const fontSize = parseFloat(win.getComputedStyle(el).fontSize);
|
||||
// Filter out sub-8px values (jsdom doesn't resolve relative units properly)
|
||||
if (fontSize >= 8 && fontSize < 200) sizes.add(Math.round(fontSize * 10) / 10);
|
||||
}
|
||||
if (sizes.size >= 3) {
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio < 2.0) {
|
||||
findings.push({ id: 'flat-type-hierarchy', snippet: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` });
|
||||
}
|
||||
}
|
||||
findings.push(...checkFlatTypeHierarchyFromDoc(doc, el => win.getComputedStyle(el)));
|
||||
|
||||
return findings;
|
||||
}
|
||||
|
||||
@@ -607,32 +607,6 @@ const REGEX_MATCHERS = [
|
||||
];
|
||||
|
||||
const REGEX_ANALYZERS = [
|
||||
// Flat type hierarchy
|
||||
(content, filePath) => {
|
||||
const sizes = new Set();
|
||||
const REM = 16;
|
||||
let m;
|
||||
const sizeRe = /font-size\s*:\s*([\d.]+)(px|rem|em)\b/gi;
|
||||
while ((m = sizeRe.exec(content)) !== null) {
|
||||
const px = m[2] === 'px' ? +m[1] : +m[1] * REM;
|
||||
if (px > 0 && px < 200) sizes.add(Math.round(px * 10) / 10);
|
||||
}
|
||||
const clampRe = /font-size\s*:\s*clamp\(\s*([\d.]+)(px|rem|em)\s*,\s*[^,]+,\s*([\d.]+)(px|rem|em)\s*\)/gi;
|
||||
while ((m = clampRe.exec(content)) !== null) {
|
||||
sizes.add(Math.round((m[2] === 'px' ? +m[1] : +m[1] * REM) * 10) / 10);
|
||||
sizes.add(Math.round((m[4] === 'px' ? +m[3] : +m[3] * REM) * 10) / 10);
|
||||
}
|
||||
const TW = { 'text-xs': 12, 'text-sm': 14, 'text-base': 16, 'text-lg': 18, 'text-xl': 20, 'text-2xl': 24, 'text-3xl': 30, 'text-4xl': 36, 'text-5xl': 48, 'text-6xl': 60, 'text-7xl': 72, 'text-8xl': 96, 'text-9xl': 128 };
|
||||
for (const [cls, px] of Object.entries(TW)) { if (new RegExp(`\\b${cls}\\b`).test(content)) sizes.add(px); }
|
||||
if (sizes.size < 3) return [];
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio >= 2.0) return [];
|
||||
const lines = content.split('\n');
|
||||
let line = 1;
|
||||
for (let i = 0; i < lines.length; i++) { if (/font-size/i.test(lines[i]) || /\btext-(?:xs|sm|base|lg|xl|\d)/i.test(lines[i])) { line = i + 1; break; } }
|
||||
return [finding('flat-type-hierarchy', filePath, `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)`, line)];
|
||||
},
|
||||
// Monotonous spacing (regex)
|
||||
(content, filePath) => {
|
||||
const vals = [];
|
||||
@@ -1154,11 +1128,12 @@ const TEXT_CONTENT_ANALYZER_IDS = [
|
||||
function runTextContentAnalyzers(content, filePath, options = {}) {
|
||||
const profile = options?.profile;
|
||||
if (!shouldRunPageAnalyzers(content, filePath)) return [];
|
||||
// The 3 text-content analyzers are at indices 2-4 in REGEX_ANALYZERS
|
||||
// (single-font's removal on 2026-07-29 shifted every index down one).
|
||||
// The 3 text-content analyzers are at indices 1-3 in REGEX_ANALYZERS.
|
||||
// flat-type-hierarchy left this source-only path in issue #619 because it
|
||||
// needs rendered role and usage evidence.
|
||||
const findings = [];
|
||||
for (let i = 0; i < TEXT_CONTENT_ANALYZER_IDS.length; i++) {
|
||||
const analyzer = REGEX_ANALYZERS[2 + i];
|
||||
const analyzer = REGEX_ANALYZERS[1 + i];
|
||||
const ruleId = TEXT_CONTENT_ANALYZER_IDS[i];
|
||||
findings.push(...profileFindings(profile, {
|
||||
engine: 'regex',
|
||||
@@ -1284,7 +1259,6 @@ function detectText(content, filePath, options = {}) {
|
||||
// Page-level analyzers only run on full pages
|
||||
if (shouldRunPageAnalyzers(content, filePath)) {
|
||||
const analyzerIds = [
|
||||
'flat-type-hierarchy',
|
||||
'monotonous-spacing',
|
||||
'em-dash-overuse',
|
||||
'marketing-buzzword',
|
||||
|
||||
@@ -282,6 +282,7 @@ const STATIC_DEFAULT_STYLE = {
|
||||
marginLeft: '0px',
|
||||
position: 'static',
|
||||
visibility: 'visible',
|
||||
contentVisibility: 'visible',
|
||||
opacity: '1',
|
||||
top: 'auto',
|
||||
right: 'auto',
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
checkElementOversizedH1,
|
||||
checkElementQuality,
|
||||
checkElementRadialSpotlight,
|
||||
checkFlatTypeHierarchyFromDoc,
|
||||
checkCreamPalette,
|
||||
checkHtmlPatterns,
|
||||
checkKickerAboveHeadingFromDoc,
|
||||
@@ -59,18 +60,7 @@ function checkStaticPageTypography(document, window) {
|
||||
for (const font of overusedFound) {
|
||||
findings.push({ id: 'overused-font', snippet: `Primary font: ${font}` });
|
||||
}
|
||||
const sizes = new Set();
|
||||
for (const el of document.querySelectorAll('h1, h2, h3, h4, h5, h6, p, span, a, li, td, th, label, button, div')) {
|
||||
const fontSize = parseFloat(window.getComputedStyle(el).fontSize);
|
||||
if (fontSize >= 8 && fontSize < 200) sizes.add(Math.round(fontSize * 10) / 10);
|
||||
}
|
||||
if (sizes.size >= 3) {
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio < 2.0) {
|
||||
findings.push({ id: 'flat-type-hierarchy', snippet: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` });
|
||||
}
|
||||
}
|
||||
findings.push(...checkFlatTypeHierarchyFromDoc(document, el => window.getComputedStyle(el)));
|
||||
return findings;
|
||||
}
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ const ANTIPATTERNS = [
|
||||
scopes: ['type'],
|
||||
name: 'Flat type hierarchy',
|
||||
description:
|
||||
'Font sizes are too close together — no clear visual hierarchy. Use fewer sizes with more contrast (aim for at least a 1.25 ratio between steps).',
|
||||
'Dominant heading and body roles are separated by less than 1.25× at every step, leaving the size hierarchy flat. Add at least one stronger size step.',
|
||||
skillSection: 'Typography',
|
||||
skillGuideline: 'flat type hierarchy',
|
||||
},
|
||||
|
||||
+89
-26
@@ -3911,6 +3911,88 @@ function checkElementGlow(tag, style, effectiveBg) {
|
||||
|
||||
// ─── Section 6: Page-Level Checks ───────────────────────────────────────────
|
||||
|
||||
const TYPE_HIERARCHY_SELECTOR = 'h1,h2,h3,h4,h5,h6,p,li,td,th,dd,blockquote,figcaption';
|
||||
const TYPE_HIERARCHY_MIN_ROLES = 3;
|
||||
const TYPE_HIERARCHY_MIN_STEP_RATIO = 1.25;
|
||||
|
||||
function typeHierarchyRole(el) {
|
||||
const tag = String(el?.tagName || el?.nodeName || '').toLowerCase();
|
||||
return /^h[1-6]$/.test(tag) ? tag : 'body';
|
||||
}
|
||||
|
||||
function hasTextContent(el) {
|
||||
return String(el?.textContent || '').trim().length > 0;
|
||||
}
|
||||
|
||||
function isRenderedTypeElement(el, getStyle) {
|
||||
for (let current = el; current; current = current.parentElement) {
|
||||
const hiddenAttr = typeof current.getAttribute === 'function' && current.getAttribute('hidden') !== null;
|
||||
if (current.hidden || hiddenAttr) return false;
|
||||
const style = getStyle(current);
|
||||
if (!style) continue;
|
||||
const display = String(style.display || '').toLowerCase();
|
||||
const visibility = String(style.visibility || '').toLowerCase();
|
||||
const contentVisibility = String(style.contentVisibility || '').toLowerCase();
|
||||
if (display === 'none' || visibility === 'hidden' || visibility === 'collapse' || contentVisibility === 'hidden') return false;
|
||||
const opacity = parseFloat(style.opacity);
|
||||
if (Number.isFinite(opacity) && opacity <= 0.01) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function dominantTypeRoleSize(samples) {
|
||||
const counts = new Map();
|
||||
for (const sample of samples) {
|
||||
counts.set(sample.size, (counts.get(sample.size) || 0) + 1);
|
||||
}
|
||||
const ranked = [...counts.entries()].sort((a, b) => b[1] - a[1] || a[0] - b[0]);
|
||||
if (ranked.length > 1 && ranked[0][1] === ranked[1][1]) return null;
|
||||
return ranked[0]?.[0] ?? null;
|
||||
}
|
||||
|
||||
function checkFlatTypeHierarchySamples(samples) {
|
||||
const byRole = new Map();
|
||||
for (const sample of samples || []) {
|
||||
const role = String(sample?.role || '');
|
||||
const size = Math.round(Number(sample?.size) * 10) / 10;
|
||||
if (!role || !Number.isFinite(size) || size < 8 || size >= 200) continue;
|
||||
if (!byRole.has(role)) byRole.set(role, []);
|
||||
byRole.get(role).push({ role, size });
|
||||
}
|
||||
|
||||
const roles = [...byRole.entries()].map(([role, roleSamples]) => ({
|
||||
role,
|
||||
size: dominantTypeRoleSize(roleSamples),
|
||||
})).filter(item => item.size !== null);
|
||||
|
||||
if (roles.length < TYPE_HIERARCHY_MIN_ROLES) return [];
|
||||
|
||||
const sorted = roles.slice().sort((a, b) => a.size - b.size || a.role.localeCompare(b.role));
|
||||
let largestStep = 1;
|
||||
for (let i = 1; i < sorted.length; i++) {
|
||||
largestStep = Math.max(largestStep, sorted[i].size / sorted[i - 1].size);
|
||||
}
|
||||
if (largestStep >= TYPE_HIERARCHY_MIN_STEP_RATIO) return [];
|
||||
|
||||
const roleSizes = sorted.map(item => `${item.role} ${item.size}px`).join(', ');
|
||||
return [{
|
||||
id: 'flat-type-hierarchy',
|
||||
snippet: `Role sizes: ${roleSizes} (largest adjacent step ${largestStep.toFixed(2)}:1; target ${TYPE_HIERARCHY_MIN_STEP_RATIO}:1)`,
|
||||
}];
|
||||
}
|
||||
|
||||
function checkFlatTypeHierarchyFromDoc(root, getStyle, options = {}) {
|
||||
const samples = [];
|
||||
for (const el of root.querySelectorAll(TYPE_HIERARCHY_SELECTOR)) {
|
||||
if (options.skipElement?.(el)) continue;
|
||||
if (!hasTextContent(el) || !isRenderedTypeElement(el, getStyle)) continue;
|
||||
const fontSize = parseFloat(getStyle(el)?.fontSize);
|
||||
if (!Number.isFinite(fontSize) || fontSize < 8 || fontSize >= 200) continue;
|
||||
samples.push({ role: typeHierarchyRole(el), size: fontSize });
|
||||
}
|
||||
return checkFlatTypeHierarchySamples(samples);
|
||||
}
|
||||
|
||||
// Browser page-level checks — use document/getComputedStyle globals
|
||||
|
||||
function checkTypography() {
|
||||
@@ -3949,17 +4031,10 @@ function checkTypography() {
|
||||
}
|
||||
}
|
||||
|
||||
const sizes = new Set();
|
||||
for (const el of document.querySelectorAll('h1,h2,h3,h4,h5,h6,p,span,a,li,td,th,label,button,div')) {
|
||||
const fs = parseFloat(getComputedStyle(el).fontSize);
|
||||
if (fs > 0 && fs < 200) sizes.add(Math.round(fs * 10) / 10);
|
||||
}
|
||||
if (sizes.size >= 3) {
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio < 2.0) {
|
||||
findings.push({ type: 'flat-type-hierarchy', detail: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` });
|
||||
}
|
||||
for (const finding of checkFlatTypeHierarchyFromDoc(document, getComputedStyle, {
|
||||
skipElement: el => el.closest?.('.impeccable-overlay, .impeccable-label, .impeccable-banner, .impeccable-tooltip, [id^="impeccable-live-"]'),
|
||||
})) {
|
||||
findings.push({ type: finding.id, detail: finding.snippet });
|
||||
}
|
||||
|
||||
return findings;
|
||||
@@ -4206,21 +4281,7 @@ function checkPageTypography(doc, win) {
|
||||
findings.push({ id: 'overused-font', snippet: `Primary font: ${font}` });
|
||||
}
|
||||
|
||||
// Flat type hierarchy
|
||||
const sizes = new Set();
|
||||
const textEls = doc.querySelectorAll('h1, h2, h3, h4, h5, h6, p, span, a, li, td, th, label, button, div');
|
||||
for (const el of textEls) {
|
||||
const fontSize = parseFloat(win.getComputedStyle(el).fontSize);
|
||||
// Filter out sub-8px values (jsdom doesn't resolve relative units properly)
|
||||
if (fontSize >= 8 && fontSize < 200) sizes.add(Math.round(fontSize * 10) / 10);
|
||||
}
|
||||
if (sizes.size >= 3) {
|
||||
const sorted = [...sizes].sort((a, b) => a - b);
|
||||
const ratio = sorted[sorted.length - 1] / sorted[0];
|
||||
if (ratio < 2.0) {
|
||||
findings.push({ id: 'flat-type-hierarchy', snippet: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` });
|
||||
}
|
||||
}
|
||||
findings.push(...checkFlatTypeHierarchyFromDoc(doc, el => win.getComputedStyle(el)));
|
||||
|
||||
return findings;
|
||||
}
|
||||
@@ -5649,6 +5710,8 @@ export {
|
||||
checkKickerAboveHeadingFromDoc,
|
||||
checkElementMotion,
|
||||
checkElementGlow,
|
||||
checkFlatTypeHierarchySamples,
|
||||
checkFlatTypeHierarchyFromDoc,
|
||||
checkTypography,
|
||||
isCardLikeDOM,
|
||||
checkLayout,
|
||||
|
||||
Reference in New Issue
Block a user