mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
Fix nested card detection, drop identical-card-grid
- Nested cards: fixed dedup to use WeakSet on actual elements instead of tag-name key, so all nested card instances are found (not just the first div-in-div pair). Now catches all 4+ nesting examples. - Dropped identical-card-grid: too many legitimate uses (data displays, pricing cards, navigation tiles) make false positives unavoidable. - Removed from CLI, browser script, tests, and ANTIPATTERNS registry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
94a20d811b
commit
f90e640892
@@ -119,12 +119,6 @@ const ANTIPATTERNS = [
|
||||
description:
|
||||
'Cards inside cards create visual noise and excessive depth. Flatten the hierarchy — use spacing, typography, and dividers instead of nesting containers.',
|
||||
},
|
||||
{
|
||||
id: 'identical-card-grid',
|
||||
name: 'Identical card grid',
|
||||
description:
|
||||
'Same-sized cards with identical icon + heading + text structure, repeated endlessly. Vary card sizes, layouts, or content structure to create visual interest.',
|
||||
},
|
||||
{
|
||||
id: 'monotonous-spacing',
|
||||
name: 'Monotonous spacing',
|
||||
@@ -602,71 +596,32 @@ function checkPageLayout(document, window) {
|
||||
|
||||
// --- Nested cards ---
|
||||
const allEls = document.querySelectorAll('*');
|
||||
const flaggedNested = new Set();
|
||||
const flaggedEls = new WeakSet();
|
||||
for (const el of allEls) {
|
||||
if (!isCardLike(el, window)) continue;
|
||||
if (flaggedEls.has(el)) continue;
|
||||
|
||||
const tag = el.tagName.toLowerCase();
|
||||
const cls = el.getAttribute?.('class') || '';
|
||||
const rawStyle = el.getAttribute?.('style') || '';
|
||||
|
||||
// Exclude elements that look like non-card components
|
||||
if (['pre', 'code'].includes(tag)) continue;
|
||||
// Exclude absolutely/fixed positioned elements (dropdowns, modals, tooltips)
|
||||
if (/\b(?:absolute|fixed)\b/.test(cls) || /position\s*:\s*(?:absolute|fixed)/i.test(rawStyle)) continue;
|
||||
// Exclude small elements (badges, chips, icons) — text < 20 chars
|
||||
if ((el.textContent?.trim().length || 0) < 20) continue;
|
||||
// Exclude form elements that happen to match card heuristics
|
||||
if (/\b(?:dropdown|popover|tooltip|menu|modal|dialog)\b/i.test(cls)) continue;
|
||||
|
||||
// Walk up to find card-like ancestor
|
||||
let parent = el.parentElement;
|
||||
while (parent) {
|
||||
if (isCardLike(parent, window)) {
|
||||
const key = `${parent.tagName}:${el.tagName}`;
|
||||
if (!flaggedNested.has(key)) {
|
||||
flaggedNested.add(key);
|
||||
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
|
||||
}
|
||||
flaggedEls.add(el);
|
||||
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
|
||||
break;
|
||||
}
|
||||
parent = parent.parentElement;
|
||||
}
|
||||
}
|
||||
|
||||
// --- Identical card grid ---
|
||||
const gridParents = document.querySelectorAll('[class*="grid"], [style*="display: grid"], [style*="display: flex"]');
|
||||
for (const grid of gridParents) {
|
||||
const children = [...grid.children].filter(c => {
|
||||
const tag = c.tagName.toLowerCase();
|
||||
return tag !== 'script' && tag !== 'style';
|
||||
});
|
||||
if (children.length < 3) continue;
|
||||
|
||||
// Compare structural fingerprint of each child
|
||||
function fingerprint(el) {
|
||||
const childTags = [...el.children].map(c => c.tagName.toLowerCase());
|
||||
// Check for icon-like element (svg, img, or div with fixed size classes)
|
||||
const hasIcon = childTags.includes('svg') || childTags.includes('img') ||
|
||||
[...el.children].some(c => {
|
||||
const cls = c.getAttribute?.('class') || '';
|
||||
return /\bw-\d+\b.*\bh-\d+\b/.test(cls) && /\brounded/.test(cls);
|
||||
});
|
||||
const hasHeading = childTags.some(t => /^h[1-6]$/.test(t));
|
||||
const hasParagraph = childTags.includes('p');
|
||||
return `icon:${hasIcon}|h:${hasHeading}|p:${hasParagraph}|children:${childTags.length}`;
|
||||
}
|
||||
|
||||
const fps = children.map(fingerprint);
|
||||
const allSame = fps.every(f => f === fps[0]);
|
||||
// Only flag if structure includes icon + heading + paragraph (the template pattern)
|
||||
if (allSame && fps[0].includes('icon:true') && fps[0].includes('h:true') && fps[0].includes('p:true')) {
|
||||
findings.push({
|
||||
id: 'identical-card-grid',
|
||||
snippet: `${children.length} identical cards (icon + heading + text)`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// --- Monotonous spacing ---
|
||||
// Regex on raw HTML — jsdom doesn't compute inline px spacing reliably
|
||||
|
||||
@@ -319,24 +319,6 @@
|
||||
}
|
||||
}
|
||||
|
||||
// --- Identical card grid ---
|
||||
for (const grid of document.querySelectorAll('[class*="grid"], [style*="display: grid"], [style*="display: flex"]')) {
|
||||
const children = [...grid.children].filter(c => !['script','style'].includes(c.tagName.toLowerCase()));
|
||||
if (children.length < 3) continue;
|
||||
|
||||
function fingerprint(el) {
|
||||
const tags = [...el.children].map(c => c.tagName.toLowerCase());
|
||||
const hasIcon = tags.includes('svg') || tags.includes('img') ||
|
||||
[...el.children].some(c => /\bw-\d+\b.*\bh-\d+\b/.test(c.getAttribute('class') || '') && /\brounded/.test(c.getAttribute('class') || ''));
|
||||
return `icon:${hasIcon}|h:${tags.some(t => /^h[1-6]$/.test(t))}|p:${tags.includes('p')}|n:${tags.length}`;
|
||||
}
|
||||
|
||||
const fps = children.map(fingerprint);
|
||||
if (fps.every(f => f === fps[0]) && fps[0].includes('icon:true') && fps[0].includes('h:true') && fps[0].includes('p:true')) {
|
||||
findings.push({ type: 'identical-card-grid', detail: `${children.length} identical cards`, el: grid });
|
||||
}
|
||||
}
|
||||
|
||||
return findings;
|
||||
}
|
||||
|
||||
@@ -357,7 +339,6 @@
|
||||
'gradient-text': 'gradient text',
|
||||
'ai-color-palette': 'AI palette',
|
||||
'nested-cards': 'nested cards',
|
||||
'identical-card-grid': 'identical grid',
|
||||
};
|
||||
|
||||
function highlight(el, findings) {
|
||||
|
||||
@@ -119,12 +119,6 @@ const ANTIPATTERNS = [
|
||||
description:
|
||||
'Cards inside cards create visual noise and excessive depth. Flatten the hierarchy — use spacing, typography, and dividers instead of nesting containers.',
|
||||
},
|
||||
{
|
||||
id: 'identical-card-grid',
|
||||
name: 'Identical card grid',
|
||||
description:
|
||||
'Same-sized cards with identical icon + heading + text structure, repeated endlessly. Vary card sizes, layouts, or content structure to create visual interest.',
|
||||
},
|
||||
{
|
||||
id: 'monotonous-spacing',
|
||||
name: 'Monotonous spacing',
|
||||
@@ -602,71 +596,32 @@ function checkPageLayout(document, window) {
|
||||
|
||||
// --- Nested cards ---
|
||||
const allEls = document.querySelectorAll('*');
|
||||
const flaggedNested = new Set();
|
||||
const flaggedEls = new WeakSet();
|
||||
for (const el of allEls) {
|
||||
if (!isCardLike(el, window)) continue;
|
||||
if (flaggedEls.has(el)) continue;
|
||||
|
||||
const tag = el.tagName.toLowerCase();
|
||||
const cls = el.getAttribute?.('class') || '';
|
||||
const rawStyle = el.getAttribute?.('style') || '';
|
||||
|
||||
// Exclude elements that look like non-card components
|
||||
if (['pre', 'code'].includes(tag)) continue;
|
||||
// Exclude absolutely/fixed positioned elements (dropdowns, modals, tooltips)
|
||||
if (/\b(?:absolute|fixed)\b/.test(cls) || /position\s*:\s*(?:absolute|fixed)/i.test(rawStyle)) continue;
|
||||
// Exclude small elements (badges, chips, icons) — text < 20 chars
|
||||
if ((el.textContent?.trim().length || 0) < 20) continue;
|
||||
// Exclude form elements that happen to match card heuristics
|
||||
if (/\b(?:dropdown|popover|tooltip|menu|modal|dialog)\b/i.test(cls)) continue;
|
||||
|
||||
// Walk up to find card-like ancestor
|
||||
let parent = el.parentElement;
|
||||
while (parent) {
|
||||
if (isCardLike(parent, window)) {
|
||||
const key = `${parent.tagName}:${el.tagName}`;
|
||||
if (!flaggedNested.has(key)) {
|
||||
flaggedNested.add(key);
|
||||
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
|
||||
}
|
||||
flaggedEls.add(el);
|
||||
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
|
||||
break;
|
||||
}
|
||||
parent = parent.parentElement;
|
||||
}
|
||||
}
|
||||
|
||||
// --- Identical card grid ---
|
||||
const gridParents = document.querySelectorAll('[class*="grid"], [style*="display: grid"], [style*="display: flex"]');
|
||||
for (const grid of gridParents) {
|
||||
const children = [...grid.children].filter(c => {
|
||||
const tag = c.tagName.toLowerCase();
|
||||
return tag !== 'script' && tag !== 'style';
|
||||
});
|
||||
if (children.length < 3) continue;
|
||||
|
||||
// Compare structural fingerprint of each child
|
||||
function fingerprint(el) {
|
||||
const childTags = [...el.children].map(c => c.tagName.toLowerCase());
|
||||
// Check for icon-like element (svg, img, or div with fixed size classes)
|
||||
const hasIcon = childTags.includes('svg') || childTags.includes('img') ||
|
||||
[...el.children].some(c => {
|
||||
const cls = c.getAttribute?.('class') || '';
|
||||
return /\bw-\d+\b.*\bh-\d+\b/.test(cls) && /\brounded/.test(cls);
|
||||
});
|
||||
const hasHeading = childTags.some(t => /^h[1-6]$/.test(t));
|
||||
const hasParagraph = childTags.includes('p');
|
||||
return `icon:${hasIcon}|h:${hasHeading}|p:${hasParagraph}|children:${childTags.length}`;
|
||||
}
|
||||
|
||||
const fps = children.map(fingerprint);
|
||||
const allSame = fps.every(f => f === fps[0]);
|
||||
// Only flag if structure includes icon + heading + paragraph (the template pattern)
|
||||
if (allSame && fps[0].includes('icon:true') && fps[0].includes('h:true') && fps[0].includes('p:true')) {
|
||||
findings.push({
|
||||
id: 'identical-card-grid',
|
||||
snippet: `${children.length} identical cards (icon + heading + text)`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// --- Monotonous spacing ---
|
||||
// Regex on raw HTML — jsdom doesn't compute inline px spacing reliably
|
||||
|
||||
@@ -282,14 +282,11 @@ describe('partials skip page-level checks', () => {
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('detectHtml — layout', () => {
|
||||
test('layout-should-flag: detects nested cards', async () => {
|
||||
test('layout-should-flag: detects all nested cards', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'layout-should-flag.html'));
|
||||
expect(f.some(r => r.antipattern === 'nested-cards')).toBe(true);
|
||||
});
|
||||
|
||||
test('layout-should-flag: detects identical card grid', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'layout-should-flag.html'));
|
||||
expect(f.some(r => r.antipattern === 'identical-card-grid')).toBe(true);
|
||||
const nested = f.filter(r => r.antipattern === 'nested-cards');
|
||||
// Classic, 3-level (2 inner cards), CSS, shadcn = at least 5 nested card findings
|
||||
expect(nested.length).toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
|
||||
test('detects monotonous spacing via regex', () => {
|
||||
@@ -320,11 +317,6 @@ describe('detectHtml — layout', () => {
|
||||
expect(f.filter(r => r.antipattern === 'nested-cards')).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('layout-should-pass: no identical-card-grid false positives', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
||||
expect(f.filter(r => r.antipattern === 'identical-card-grid')).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('layout-should-pass: no monotonous-spacing false positives', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
||||
expect(f.filter(r => r.antipattern === 'monotonous-spacing')).toHaveLength(0);
|
||||
|
||||
Reference in New Issue
Block a user