mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 00:26:41 +03:00
Make cramped-padding rule asymmetric and proportional to font-size
The old rule used a fixed 8px floor on minPad, which produced false positives on small inline pills (like the homepage's .detection-cmd at 6px vertical / 14px horizontal on 13px font) and false negatives on large text (a 24px heading with 8px padding all around passed the floor but is genuinely too tight for the text size). The new rule uses two independent axis thresholds that scale with font-size: vertical: max(4px, fontSize × 0.3) horizontal: max(8px, fontSize × 0.5) The asymmetry reflects typographic reality: line-height already provides built-in vertical breathing room (the line box is taller than the cap height), so vertical padding can be tighter than horizontal. Both thresholds scale with font-size — bigger text demands proportionally more padding. Behavior changes - Small inline pills with line-height-aware padding now pass (.detection-cmd: V 6 ≥ 4, H 14 ≥ 8). The homepage CSS is unchanged. - Cramped large text now flags (24px heading with 8px padding fails H 8 < 12). The old rule missed this entirely. - All original 8px-floor flag cases still flag — 4px on 14px text is still 4 < 4.2 vertical, 2px is still cramped, etc. - Snippet now indicates which axis failed and the specific threshold for the font-size: "6px vertical padding (need ≥4.8px for 16px text)" instead of the old "6px padding (need >=8px)". Fixture - tests/fixtures/antipatterns/cramped-padding.html is a new comprehensive side-by-side fixture with 8 flag cases and 12 pass cases spanning small pills, cards, code blocks, interactive elements, and big text. Replaces the prior 3-case version. Test - tests/detect-antipatterns-browser.test.mjs asserts exactly 8 cramped-padding findings with detailed comments listing each expected case and which axis fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
092e4a9e4f
commit
3569085cea
@@ -1052,6 +1052,12 @@ function checkQuality(opts) {
|
||||
}
|
||||
|
||||
// --- Cramped padding --- (browser-only: needs rect to skip small badges/labels)
|
||||
// Vertical and horizontal thresholds are independent because line-height
|
||||
// already provides built-in vertical breathing room (the line box is taller
|
||||
// than the cap height), but horizontal has no equivalent. Both scale with
|
||||
// font-size — bigger text demands proportionally more padding.
|
||||
// vertical: max(4px, fontSize × 0.3)
|
||||
// horizontal: max(8px, fontSize × 0.5)
|
||||
if (rect && hasDirectText && textLen > 20 && rect.width > 100 && rect.height > 30) {
|
||||
const borders = {
|
||||
top: parseFloat(style.borderTopWidth) || 0,
|
||||
@@ -1062,16 +1068,22 @@ function checkQuality(opts) {
|
||||
const borderCount = Object.values(borders).filter(w => w > 0).length;
|
||||
const hasBg = style.backgroundColor && style.backgroundColor !== 'rgba(0, 0, 0, 0)';
|
||||
if (borderCount >= 2 || hasBg) {
|
||||
const paddings = [];
|
||||
if (hasBg || borders.top > 0) paddings.push(parseFloat(style.paddingTop) || 0);
|
||||
if (hasBg || borders.right > 0) paddings.push(parseFloat(style.paddingRight) || 0);
|
||||
if (hasBg || borders.bottom > 0) paddings.push(parseFloat(style.paddingBottom) || 0);
|
||||
if (hasBg || borders.left > 0) paddings.push(parseFloat(style.paddingLeft) || 0);
|
||||
if (paddings.length > 0) {
|
||||
const minPad = Math.min(...paddings);
|
||||
if (minPad < 8) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${minPad}px padding (need >=8px)` });
|
||||
}
|
||||
const vPads = [], hPads = [];
|
||||
if (hasBg || borders.top > 0) vPads.push(parseFloat(style.paddingTop) || 0);
|
||||
if (hasBg || borders.bottom > 0) vPads.push(parseFloat(style.paddingBottom) || 0);
|
||||
if (hasBg || borders.left > 0) hPads.push(parseFloat(style.paddingLeft) || 0);
|
||||
if (hasBg || borders.right > 0) hPads.push(parseFloat(style.paddingRight) || 0);
|
||||
|
||||
const vMin = vPads.length ? Math.min(...vPads) : Infinity;
|
||||
const hMin = hPads.length ? Math.min(...hPads) : Infinity;
|
||||
const vThresh = Math.max(4, fontSize * 0.3);
|
||||
const hThresh = Math.max(8, fontSize * 0.5);
|
||||
|
||||
// Emit at most one finding per element — pick whichever axis is worse.
|
||||
if (vMin < vThresh) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${vMin}px vertical padding (need ≥${vThresh.toFixed(1)}px for ${fontSize}px text)` });
|
||||
} else if (hMin < hThresh) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${hMin}px horizontal padding (need ≥${hThresh.toFixed(1)}px for ${fontSize}px text)` });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+22
-10
@@ -1047,6 +1047,12 @@ function checkQuality(opts) {
|
||||
}
|
||||
|
||||
// --- Cramped padding --- (browser-only: needs rect to skip small badges/labels)
|
||||
// Vertical and horizontal thresholds are independent because line-height
|
||||
// already provides built-in vertical breathing room (the line box is taller
|
||||
// than the cap height), but horizontal has no equivalent. Both scale with
|
||||
// font-size — bigger text demands proportionally more padding.
|
||||
// vertical: max(4px, fontSize × 0.3)
|
||||
// horizontal: max(8px, fontSize × 0.5)
|
||||
if (rect && hasDirectText && textLen > 20 && rect.width > 100 && rect.height > 30) {
|
||||
const borders = {
|
||||
top: parseFloat(style.borderTopWidth) || 0,
|
||||
@@ -1057,16 +1063,22 @@ function checkQuality(opts) {
|
||||
const borderCount = Object.values(borders).filter(w => w > 0).length;
|
||||
const hasBg = style.backgroundColor && style.backgroundColor !== 'rgba(0, 0, 0, 0)';
|
||||
if (borderCount >= 2 || hasBg) {
|
||||
const paddings = [];
|
||||
if (hasBg || borders.top > 0) paddings.push(parseFloat(style.paddingTop) || 0);
|
||||
if (hasBg || borders.right > 0) paddings.push(parseFloat(style.paddingRight) || 0);
|
||||
if (hasBg || borders.bottom > 0) paddings.push(parseFloat(style.paddingBottom) || 0);
|
||||
if (hasBg || borders.left > 0) paddings.push(parseFloat(style.paddingLeft) || 0);
|
||||
if (paddings.length > 0) {
|
||||
const minPad = Math.min(...paddings);
|
||||
if (minPad < 8) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${minPad}px padding (need >=8px)` });
|
||||
}
|
||||
const vPads = [], hPads = [];
|
||||
if (hasBg || borders.top > 0) vPads.push(parseFloat(style.paddingTop) || 0);
|
||||
if (hasBg || borders.bottom > 0) vPads.push(parseFloat(style.paddingBottom) || 0);
|
||||
if (hasBg || borders.left > 0) hPads.push(parseFloat(style.paddingLeft) || 0);
|
||||
if (hasBg || borders.right > 0) hPads.push(parseFloat(style.paddingRight) || 0);
|
||||
|
||||
const vMin = vPads.length ? Math.min(...vPads) : Infinity;
|
||||
const hMin = hPads.length ? Math.min(...hPads) : Infinity;
|
||||
const vThresh = Math.max(4, fontSize * 0.3);
|
||||
const hThresh = Math.max(8, fontSize * 0.5);
|
||||
|
||||
// Emit at most one finding per element — pick whichever axis is worse.
|
||||
if (vMin < vThresh) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${vMin}px vertical padding (need ≥${vThresh.toFixed(1)}px for ${fontSize}px text)` });
|
||||
} else if (hMin < hThresh) {
|
||||
findings.push({ id: 'cramped-padding', snippet: `${hMin}px horizontal padding (need ≥${hThresh.toFixed(1)}px for ${fontSize}px text)` });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user