skills/pbakaus_impeccable
1
0
Fork 0
mirror of https://github.com/pbakaus/impeccable.git synced 2026-09-11 21:57:14 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix nested card detection: innermost-only, lower threshold, raw style fallback

Browse Source 
Three fixes:
- Only flag innermost nested cards: if L1>L2>L3, only L3 gets flagged
  (not L2). Uses ancestor-filtering after collection pass.
- Lower text threshold from 20 to 10 chars to catch short card content
  like "Inner card via CSS."
- isCardLike now also checks raw inline style attribute for box-shadow
  and border-radius (jsdom doesn't resolve CSS shorthands). Tightened
  heuristic: shadow or border is mandatory (not optional).

Fixes false positive on layout-should-pass where a tinted subsection
(rounded + bg, no shadow) inside a card was incorrectly flagged.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-03-17 16:34:04 -07:00
co-authored by Claude Opus 4.6
parent ae47758366
commit add8c1958d
4 changed files with 73 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.claude/skills/critique/scripts/detect-antipatterns.mjs
+30 -13
View File
@@ -566,9 +566,11 @@ function isCardLike(el, window) {
const radius = parseFloat(style.borderRadius) || 0;
const hasRadius = radius > 0;
// Check background: card-like if it has an opaque bg different from transparent
const rawBg = el.getAttribute?.('style')?.match(/background(?:-color)?\s*:\s*([^;]+)/i);
const hasBg = rawBg && !/transparent/i.test(rawBg[1]);
// Also check raw inline style (jsdom doesn't resolve shorthand properties reliably)
const rawStyle = el.getAttribute?.('style') || '';
const rawShadow = /box-shadow/i.test(rawStyle);
const rawRadius = /border-radius/i.test(rawStyle);
const rawBg = /background(?:-color)?\s*:\s*(?!transparent)/i.test(rawStyle);
// Also check Tailwind classes for card indicators
const cls = el.getAttribute?.('class') || '';
@@ -577,14 +579,16 @@ function isCardLike(el, window) {
const twBg = /\bbg-(?:white|gray-\d+|slate-\d+)\b/.test(cls);
const twBorder = /\bborder\b/.test(cls);
// A "card" needs at least 2 of: shadow, rounded, bg/border
const signals = [
hasShadow || twShadow,
hasRadius || twRounded,
hasBg || twBg || twBorder,
].filter(Boolean).length;
// A "card" needs shadow (or border) AND at least one of: rounded, bg
const hasShadowAny = hasShadow || twShadow || rawShadow;
const hasBorderAny = twBorder;
const hasRadiusAny = hasRadius || twRounded || rawRadius;
const hasBgAny = rawBg || twBg;
return signals >= 2;
// Must have shadow or border (the key card indicator)
if (!hasShadowAny && !hasBorderAny) return false;
// Plus at least one of: rounded, background
return hasRadiusAny || hasBgAny;
}
/**
@@ -596,7 +600,7 @@ function checkPageLayout(document, window) {
// --- Nested cards ---
const allEls = document.querySelectorAll('*');
const flaggedEls = new WeakSet();
const flaggedEls = new Set();
for (const el of allEls) {
if (!isCardLike(el, window)) continue;
if (flaggedEls.has(el)) continue;
@@ -607,7 +611,7 @@ function checkPageLayout(document, window) {
if (['pre', 'code'].includes(tag)) continue;
if (/\b(?:absolute|fixed)\b/.test(cls) || /position\s*:\s*(?:absolute|fixed)/i.test(rawStyle)) continue;
if ((el.textContent?.trim().length || 0) < 20) continue;
if ((el.textContent?.trim().length || 0) < 10) continue;
if (/\b(?:dropdown|popover|tooltip|menu|modal|dialog)\b/i.test(cls)) continue;
// Walk up to find card-like ancestor
@@ -615,13 +619,26 @@ function checkPageLayout(document, window) {
while (parent) {
if (isCardLike(parent, window)) {
flaggedEls.add(el);
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
break;
}
parent = parent.parentElement;
}
}
// Only report innermost nested cards — remove any flagged el that is an ancestor of another
for (const el of flaggedEls) {
let isAncestorOfFlagged = false;
for (const other of flaggedEls) {
if (other !== el && el.contains(other)) {
isAncestorOfFlagged = true;
break;
}
}
if (!isAncestorOfFlagged) {
findings.push({ id: 'nested-cards', snippet: `Card inside card (${el.tagName.toLowerCase()})` });
}
}
// --- Monotonous spacing ---
// Regex on raw HTML — jsdom doesn't compute inline px spacing reliably
public/js/detect-antipatterns-browser.js
+12 -2
View File
@@ -298,7 +298,7 @@
const style = getComputedStyle(el);
if (style.position === 'absolute' || style.position === 'fixed') continue;
if (/\b(?:dropdown|popover|tooltip|menu|modal|dialog)\b/i.test(cls)) continue;
if ((el.textContent?.trim().length || 0) < 20) continue;
if ((el.textContent?.trim().length || 0) < 10) continue;
const rect = el.getBoundingClientRect();
if (rect.width < 50 || rect.height < 30) continue;
@@ -306,13 +306,23 @@
while (parent) {
if (isCardLike(parent)) {
flaggedEls.add(el);
findings.push({ type: 'nested-cards', detail: `Card inside card`, el });
break;
}
parent = parent.parentElement;
}
}
// Only report innermost nested cards — skip any that are ancestors of other flagged cards
for (const el of flaggedEls) {
let isAncestor = false;
for (const other of flaggedEls) {
if (other !== el && el.contains(other)) { isAncestor = true; break; }
}
if (!isAncestor) {
findings.push({ type: 'nested-cards', detail: 'Card inside card', el });
}
}
return findings;
}
source/skills/critique/scripts/detect-antipatterns.mjs
+30 -13
View File
@@ -566,9 +566,11 @@ function isCardLike(el, window) {
const radius = parseFloat(style.borderRadius) || 0;
const hasRadius = radius > 0;
// Check background: card-like if it has an opaque bg different from transparent
const rawBg = el.getAttribute?.('style')?.match(/background(?:-color)?\s*:\s*([^;]+)/i);
const hasBg = rawBg && !/transparent/i.test(rawBg[1]);
// Also check raw inline style (jsdom doesn't resolve shorthand properties reliably)
const rawStyle = el.getAttribute?.('style') || '';
const rawShadow = /box-shadow/i.test(rawStyle);
const rawRadius = /border-radius/i.test(rawStyle);
const rawBg = /background(?:-color)?\s*:\s*(?!transparent)/i.test(rawStyle);
// Also check Tailwind classes for card indicators
const cls = el.getAttribute?.('class') || '';
@@ -577,14 +579,16 @@ function isCardLike(el, window) {
const twBg = /\bbg-(?:white|gray-\d+|slate-\d+)\b/.test(cls);
const twBorder = /\bborder\b/.test(cls);
// A "card" needs at least 2 of: shadow, rounded, bg/border
const signals = [
hasShadow || twShadow,
hasRadius || twRounded,
hasBg || twBg || twBorder,
].filter(Boolean).length;
// A "card" needs shadow (or border) AND at least one of: rounded, bg
const hasShadowAny = hasShadow || twShadow || rawShadow;
const hasBorderAny = twBorder;
const hasRadiusAny = hasRadius || twRounded || rawRadius;
const hasBgAny = rawBg || twBg;
return signals >= 2;
// Must have shadow or border (the key card indicator)
if (!hasShadowAny && !hasBorderAny) return false;
// Plus at least one of: rounded, background
return hasRadiusAny || hasBgAny;
}
/**
@@ -596,7 +600,7 @@ function checkPageLayout(document, window) {
// --- Nested cards ---
const allEls = document.querySelectorAll('*');
const flaggedEls = new WeakSet();
const flaggedEls = new Set();
for (const el of allEls) {
if (!isCardLike(el, window)) continue;
if (flaggedEls.has(el)) continue;
@@ -607,7 +611,7 @@ function checkPageLayout(document, window) {
if (['pre', 'code'].includes(tag)) continue;
if (/\b(?:absolute|fixed)\b/.test(cls) || /position\s*:\s*(?:absolute|fixed)/i.test(rawStyle)) continue;
if ((el.textContent?.trim().length || 0) < 20) continue;
if ((el.textContent?.trim().length || 0) < 10) continue;
if (/\b(?:dropdown|popover|tooltip|menu|modal|dialog)\b/i.test(cls)) continue;
// Walk up to find card-like ancestor
@@ -615,13 +619,26 @@ function checkPageLayout(document, window) {
while (parent) {
if (isCardLike(parent, window)) {
flaggedEls.add(el);
findings.push({ id: 'nested-cards', snippet: `Card inside card (${tag} in ${parent.tagName.toLowerCase()})` });
break;
}
parent = parent.parentElement;
}
}
// Only report innermost nested cards — remove any flagged el that is an ancestor of another
for (const el of flaggedEls) {
let isAncestorOfFlagged = false;
for (const other of flaggedEls) {
if (other !== el && el.contains(other)) {
isAncestorOfFlagged = true;
break;
}
}
if (!isAncestorOfFlagged) {
findings.push({ id: 'nested-cards', snippet: `Card inside card (${el.tagName.toLowerCase()})` });
}
}
// --- Monotonous spacing ---
// Regex on raw HTML — jsdom doesn't compute inline px spacing reliably
tests/detect-antipatterns.test.js
+1 -1
View File
@@ -285,7 +285,7 @@ describe('detectHtml — layout', () => {
test('layout-should-flag: detects all nested cards', async () => {
const f = await detectHtml(path.join(FIXTURES, 'layout-should-flag.html'));
const nested = f.filter(r => r.antipattern === 'nested-cards');
// Classic, 3-level (2 inner cards), CSS, shadcn = at least 5 nested card findings
// Classic, level 3, CSS inner, shadcn inner + any other innermost nested cards
expect(nested.length).toBeGreaterThanOrEqual(4);
});