diff --git a/BUILD-PLAN-issue-14.md b/BUILD-PLAN-issue-14.md new file mode 100644 index 0000000..707997e --- /dev/null +++ b/BUILD-PLAN-issue-14.md @@ -0,0 +1,12 @@ +# BUILD-PLAN-issue-14.md — CSS layout anti-pattern detection + +**Card:** https://github.com/nujovich/mint-radar/issues/14 + +**Decision:** PLAN defined 4 concrete code milestones (no decision pending). + +## Milestones + +- [ ] Milestone 1 — Extend audit prompt in lib/prompts.mjs with a layout health step (STEP 14): detect nested grid containers without subgrid and flex items at overflow risk (fixed flex-basis). Visual reorder vs DOM order is already reported by STEP 9, so it is not duplicated here. +- [ ] Milestone 2 — Add layoutWarnings field to AuditReport type in lib/types.ts (array of {pattern, element, severity, suggestion}) +- [ ] Milestone 3 — Wire layout warnings in playground UI as layout health alerts +- [ ] Milestone 4 — Add test fixture with problematic layout patterns and tests in lib/**tests**/ diff --git a/components/AuditView.tsx b/components/AuditView.tsx index 2086793..063b893 100644 --- a/components/AuditView.tsx +++ b/components/AuditView.tsx @@ -11,9 +11,11 @@ interface Props { interface LintIssue { selector?: string + element?: string rule?: string severity: string - reason: string + reason?: string + suggestion?: string } function nearestScaleValue( @@ -860,7 +862,7 @@ export default function AuditView({ audit, onResolve }: Props) { color: 'var(--text)', }} > - {issue.selector || issue.rule || '—'} + {issue.selector || issue.element || issue.rule || '—'}
- {issue.reason} + {issue.reason || issue.suggestion}
diff --git a/lib/__fixtures__/layout-patterns.css b/lib/__fixtures__/layout-patterns.css new file mode 100644 index 0000000..1a89b64 --- /dev/null +++ b/lib/__fixtures__/layout-patterns.css @@ -0,0 +1,89 @@ +/* Layout patterns test fixture + * Exercises the two layout anti-patterns detected by STEP 14 (LAYOUT HEALTH): + * 1. nested-grid-without-subgrid — a grid container inside another grid that + * re-declares its own tracks instead of using subgrid + * 2. flex-overflow-risk — a flex item with a fixed flex-basis inside a nowrap + * flex container + * + * Also includes clean cases that should NOT trigger either pattern. + */ + +/* --- NESTED GRID WITHOUT SUBGRID (should trigger) --- */ + +.page-grid { + display: grid; + grid-template-columns: 1fr 3fr 1fr; + gap: 1rem; +} + +/* .card-grid is a child of .page-grid and re-declares its own explicit tracks + * instead of using subgrid — columns and gutters will not align */ +.page-grid .card-grid { + display: grid; + grid-template-columns: repeat(3, 1fr); + gap: 0.5rem; +} + +/* .card-grid .inner is nested yet another level and also uses explicit tracks */ +.card-grid .inner { + display: grid; + grid-template-columns: 1fr 1fr; +} + +/* --- FLEX OVERFLOW RISK (should trigger) --- */ + +.nav { + display: flex; + /* no flex-wrap — items will not wrap */ + gap: 8px; + padding: 0 16px; +} + +/* .nav-item has a fixed flex-basis of 300px inside a nowrap flex container, + * risking overflow when the container is narrower than the sum of all items */ +.nav-item { + flex-basis: 300px; + min-width: 200px; +} + +/* Another flex item with a large fixed width inside a nowrap container */ +.toolbar { + display: flex; +} + +.toolbar-button { + width: 250px; + flex-shrink: 0; +} + +/* --- CLEAN CASES (should NOT trigger) --- */ + +/* Grid with subgrid — correct usage, no warning expected */ +.page-grid .subgrid-section { + display: grid; + grid-template-columns: subgrid; + grid-column: 1 / -1; +} + +/* Flex container WITH flex-wrap — safe, no overflow risk */ +.wrap-nav { + display: flex; + flex-wrap: wrap; + gap: 8px; +} + +.wrap-nav .wrap-item { + flex-basis: 300px; +} + +/* Standalone grid (not nested) — no warning expected */ +.standalone-grid { + display: grid; + grid-template-columns: repeat(4, 1fr); + gap: 1rem; +} + +/* Flex item with flexible sizing — no overflow risk */ +.flex-auto { + flex: 1; +} diff --git a/lib/__tests__/audit-summary.test.mjs b/lib/__tests__/audit-summary.test.mjs index e0d43d4..65a38b3 100644 --- a/lib/__tests__/audit-summary.test.mjs +++ b/lib/__tests__/audit-summary.test.mjs @@ -52,6 +52,13 @@ describe('formatLintSummary', () => { '1 layout a11y · 1 modern-practice · 1 adoption · 1 overflow · 1 property types' ) }) + + it('includes the layout health count when warnings are present', () => { + const audit = { + layoutWarnings: [{ pattern: 'flex-overflow-risk', severity: 'warning' }], + } + expect(formatLintSummary(audit)).toBe('1 layout health') + }) }) describe('collectLintGroups', () => { @@ -124,4 +131,20 @@ describe('collectLintGroups', () => { }, ]) }) + + it('labels the layout health category and carries element + suggestion', () => { + const issue = { + pattern: 'nested-grid-without-subgrid', + element: '.card-grid .inner', + severity: 'suggestion', + suggestion: 'Nested grid re-declares its own tracks; consider subgrid', + } + expect(collectLintGroups({ layoutWarnings: [issue] })).toEqual([ + { + key: 'layoutWarnings', + label: 'Layout health', + issues: [issue], + }, + ]) + }) }) diff --git a/lib/__tests__/css-auditor.test.mjs b/lib/__tests__/css-auditor.test.mjs index 6f1c35b..08f6ea4 100644 --- a/lib/__tests__/css-auditor.test.mjs +++ b/lib/__tests__/css-auditor.test.mjs @@ -659,3 +659,95 @@ describe('CssAuditor propertyTypeIssues integration', () => { expect(result.propertyTypeIssues).toBeUndefined() }) }) + +describe('buildAuditPrompt layout health', () => { + it('includes STEP 14 for layout health detection', () => { + const css = 'body { color: red; }' + const prompt = buildAuditPrompt(css) + expect(prompt.content).toContain('STEP 14') + expect(prompt.content).toContain('LAYOUT HEALTH') + }) + + it('instructs LLM to detect nested grid without subgrid', () => { + const css = + '.outer { display: grid; } .outer .inner { display: grid; grid-template-columns: 1fr 1fr; }' + const prompt = buildAuditPrompt(css) + expect(prompt.content).toContain('NESTED GRID WITHOUT SUBGRID') + expect(prompt.content).toContain('nested-grid-without-subgrid') + expect(prompt.content).toContain('grid-template-columns: subgrid') + }) + + it('instructs LLM to detect flex overflow risk', () => { + const css = + '.nav { display: flex; } .nav-item { flex-basis: 300px; }' + const prompt = buildAuditPrompt(css) + expect(prompt.content).toContain('FLEX OVERFLOW RISK') + expect(prompt.content).toContain('flex-overflow-risk') + expect(prompt.content).toContain('flex-wrap: wrap') + }) + + it('instructs LLM to report at most 10 layoutWarnings', () => { + const css = 'body { color: red; }' + const prompt = buildAuditPrompt(css) + expect(prompt.content).toContain('up to 10 layoutWarnings') + }) + + it('includes layoutWarnings in the output example', () => { + const css = 'body { color: red; }' + const prompt = buildAuditPrompt(css) + expect(prompt.content).toContain('"layoutWarnings"') + expect(prompt.content).toContain('nested-grid-without-subgrid') + expect(prompt.content).toContain('flex-overflow-risk') + }) +}) + +describe('CssAuditor layoutWarnings integration', () => { + const baseReport = { + brand: 'test', + chaosScore: 3, + summary: '', + colorClusters: [], + fonts: [], + spacing: { found: [], suggestedScale: {}, nonScaleValues: [] }, + lineHeights: { found: [], suggestedScale: {}, unitlessMix: false }, + layoutA11yIssues: [], + modernPracticeIssues: [], + adoptionSuggestions: [], + overflowSafetyIssues: [], + propertyTypeIssues: [], + } + + const auditWith = async (report) => { + const sendPromptSpy = vi.fn().mockResolvedValue(JSON.stringify(report)) + const auditor = new CssAuditor( + { sendPrompt: sendPromptSpy }, + { maxTokens: { audit: 3000, parse: 4000, export: 6000 } } + ) + return auditor.audit({ content: 'test', system: 'sys' }) + } + + it('audit parses response with layoutWarnings', async () => { + const warning = { + pattern: 'nested-grid-without-subgrid', + element: '.card-grid .inner', + severity: 'suggestion', + suggestion: + 'Nested grid re-declares its own tracks; consider grid-template-columns: subgrid to align with the parent grid', + } + const result = await auditWith({ + ...baseReport, + layoutWarnings: [warning], + }) + expect(result.layoutWarnings).toEqual([warning]) + }) + + it('audit handles empty layoutWarnings array', async () => { + const result = await auditWith({ ...baseReport, layoutWarnings: [] }) + expect(result.layoutWarnings).toEqual([]) + }) + + it('audit handles missing layoutWarnings gracefully', async () => { + const result = await auditWith(baseReport) + expect(result.layoutWarnings).toBeUndefined() + }) +}) diff --git a/lib/__tests__/prompts.test.mjs b/lib/__tests__/prompts.test.mjs index 0edee3b..810b1ab 100644 --- a/lib/__tests__/prompts.test.mjs +++ b/lib/__tests__/prompts.test.mjs @@ -153,7 +153,7 @@ describe('buildAuditPrompt', () => { expect(content).toContain('') }) - it('includes all thirteen analysis steps', () => { + it('includes all fourteen analysis steps', () => { const content = buildAuditPrompt(css).content expect(content).toContain('STEP 1') expect(content).toContain('STEP 2') @@ -168,6 +168,7 @@ describe('buildAuditPrompt', () => { expect(content).toContain('STEP 11') expect(content).toContain('STEP 12') expect(content).toContain('STEP 13') + expect(content).toContain('STEP 14') }) it('covers all required JSON output fields in the example', () => { @@ -181,6 +182,7 @@ describe('buildAuditPrompt', () => { expect(content).toContain('"lineHeights"') expect(content).toContain('"motion"') expect(content).toContain('"propertyTypeIssues"') + expect(content).toContain('"layoutWarnings"') }) it('includes all allowed semantic color names', () => { @@ -610,3 +612,41 @@ describe('buildAuditPrompt lint issue contract', () => { } ) }) + +describe('buildAuditPrompt with the layout patterns fixture', () => { + const fixture = readFileSync( + new URL('../__fixtures__/layout-patterns.css', import.meta.url), + 'utf8' + ) + + it('embeds the fixture in the prompt', () => { + const content = buildAuditPrompt(fixture).content + expect(content).toContain('.page-grid') + expect(content).toContain('.nav-item') + expect(content).toContain('.subgrid-section') + }) + + it('preserves the nested grid without subgrid pattern through preprocessing', () => { + const processed = preprocessCss(fixture) + expect(processed).toContain('.page-grid .card-grid') + expect(processed).toContain('grid-template-columns: repeat(3, 1fr)') + expect(processed).toContain('.card-grid .inner') + }) + + it('preserves the flex overflow risk pattern through preprocessing', () => { + const processed = preprocessCss(fixture) + expect(processed).toContain('.nav') + expect(processed).toContain('flex-basis: 300px') + expect(processed).toContain('min-width: 200px') + }) + + it('preserves the clean subgrid case through preprocessing', () => { + const processed = preprocessCss(fixture) + expect(processed).toContain('grid-template-columns: subgrid') + }) + + it('strips comments from the fixture', () => { + const processed = preprocessCss(fixture) + expect(processed).not.toContain('/*') + }) +}) diff --git a/lib/audit-summary.mjs b/lib/audit-summary.mjs index e7bc6fc..2d29452 100644 --- a/lib/audit-summary.mjs +++ b/lib/audit-summary.mjs @@ -27,6 +27,11 @@ const LINT_CATEGORIES = [ shortLabel: 'property types', label: '@property type safety', }, + { + key: 'layoutWarnings', + shortLabel: 'layout health', + label: 'Layout health', + }, ] /** diff --git a/lib/prompts.mjs b/lib/prompts.mjs index e90d3ee..d8bff14 100644 --- a/lib/prompts.mjs +++ b/lib/prompts.mjs @@ -242,7 +242,26 @@ Scan every var() usage of a registered property for a fallback whose type contra 13c. FOREIGN CONTEXT USAGE A registered property assigned to a CSS property its declared syntax cannot satisfy — a property used in \`color\`, or a property used in \`width\`. Report only when the mismatch is unambiguous from this stylesheet alone; skip any var() whose value may come from another stylesheet or from JavaScript. Record these with "rule": "property-type-mismatch" and "severity": "suggestion". -Only report up to 8 propertyTypeIssues total. Prioritize invalid-initial-value, then fallback-type-mismatch, then property-type-mismatch. If the CSS contains no @property registrations, record an empty array. +Only report up to 8 propertyTypeIssues total. Prioritize invalid-initial-value, then fallback-type-mismatch, then property-type-mismatch. If the CSS contains no @property registrations, record an empty array. + +STEP 14 — LAYOUT HEALTH +Scan the CSS for layout anti-patterns that hurt maintainability and responsive robustness. Visual reorder and tab order issues are already reported in layoutA11yIssues (STEP 9); do not duplicate them here. Focus on two checks: + +14a. NESTED GRID WITHOUT SUBGRID +Look for a grid container (display: grid or display: inline-grid) that is itself a child of another grid container and does NOT declare "grid-template-columns: subgrid" or "grid-template-rows: subgrid". A nested grid that re-declares its own explicit tracks cannot align its columns or rows to the parent grid, producing misaligned gutters and layout drift. For each such case, record: +- "pattern": "nested-grid-without-subgrid" +- "element": the selector of the nested grid container +- "severity": "suggestion" +- "suggestion": "Nested grid re-declares its own tracks; consider grid-template-columns: subgrid to align with the parent grid" + +14b. FLEX OVERFLOW RISK +Look for a flex item (an element whose parent has display: flex or display: inline-flex) that declares a fixed "flex-basis" (or "width"/"min-width") large enough that, combined with sibling items and gap/padding, it can overflow the container. Strong signals: a flex-basis larger than the item's typical content size, or a fixed flex-basis on an item inside a flex container that has no flex-wrap. For each such case, record: +- "pattern": "flex-overflow-risk" +- "element": the selector of the flex item +- "severity": "warning" +- "suggestion": a specific message identifying the risk and a concrete fix (for example "Flex item with a large fixed flex-basis inside a nowrap container; consider flex-wrap: wrap on the parent, or min-width: 0 with flex-shrink: 1 on the item") + +Only report up to 10 layoutWarnings total. Prioritize flex-overflow-risk (warning) first, then nested-grid-without-subgrid (suggestion). Return ONLY a valid JSON object matching the structure in the example below. No markdown fences, no backticks, no text before or after the JSON. @@ -316,6 +335,10 @@ Return ONLY a valid JSON object matching the structure in the example below. No { "selector": "@property --spacing-unit", "property": "initial-value", "rule": "invalid-initial-value", "severity": "warning", "reason": "Registered as \`\` but \`initial-value: red\` is a \`\` — the browser rejects this registration entirely", "propertyName": "--spacing-unit", "declaredSyntax": "" }, { "selector": ".button", "property": "background-color", "rule": "fallback-type-mismatch", "severity": "warning", "reason": "\`--my-color\` is registered as \`\` but falls back to \`14px\` — the fallback can never apply", "propertyName": "--my-color", "declaredSyntax": "" }, { "selector": ".card", "property": "color", "rule": "property-type-mismatch", "severity": "suggestion", "reason": "\`--spacing-unit\` is registered as \`\` but is used as a color value", "propertyName": "--spacing-unit", "declaredSyntax": "" } + ], + "layoutWarnings": [ + { "pattern": "nested-grid-without-subgrid", "element": ".card-grid .inner", "severity": "suggestion", "suggestion": "Nested grid re-declares its own tracks; consider grid-template-columns: subgrid to align with the parent grid" }, + { "pattern": "flex-overflow-risk", "element": ".nav-item", "severity": "warning", "suggestion": "Flex item with a large fixed flex-basis inside a nowrap container; consider flex-wrap: wrap on the parent, or min-width: 0 with flex-shrink: 1" } ] } diff --git a/lib/types.ts b/lib/types.ts index 7171834..c9912fa 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -169,6 +169,13 @@ export interface PropertyTypeIssue { declaredSyntax: string // the @property syntax descriptor, e.g. '' } +export interface LayoutWarning { + pattern: string // 'nested-grid-without-subgrid' | 'flex-overflow-risk' + element: string // the selector of the offending container or item + severity: 'warning' | 'suggestion' + suggestion: string // human-readable explanation and concrete fix +} + export interface AuditReport { brand: string chaosScore: number @@ -183,6 +190,7 @@ export interface AuditReport { adoptionSuggestions?: AdoptionSuggestion[] overflowSafetyIssues?: OverflowSafetyIssue[] propertyTypeIssues?: PropertyTypeIssue[] + layoutWarnings?: LayoutWarning[] } export interface ColorDecision {