diff --git a/.claude/skills/critique/scripts/detect-antipatterns.mjs b/.claude/skills/critique/scripts/detect-antipatterns.mjs index 86e769a7e..c26b89b48 100644 --- a/.claude/skills/critique/scripts/detect-antipatterns.mjs +++ b/.claude/skills/critique/scripts/detect-antipatterns.mjs @@ -198,15 +198,6 @@ function contrastRatio(c1, c2) { return (lighter + 0.05) / (darker + 0.05); } -/** - * Check if a color is pure black or pure white. - */ -function isPureBlackOrWhite(c) { - if (!c) return false; - return (c.r === 0 && c.g === 0 && c.b === 0) || - (c.r === 255 && c.g === 255 && c.b === 255); -} - /** * Check if a color has meaningful chroma (is "colored" vs gray/neutral). * Uses simple RGB saturation check. @@ -811,98 +802,33 @@ async function detectUrl(url) { throw new Error('puppeteer is required for URL scanning. Install: npm install puppeteer'); } + // Read the browser detection script — reuse it instead of reimplementing + const browserScriptPath = path.resolve( + path.dirname(new URL(import.meta.url).pathname), + '..', '..', '..', '..', 'public', 'js', 'detect-antipatterns-browser.js' + ); + let browserScript; + try { + browserScript = fs.readFileSync(browserScriptPath, 'utf-8'); + } catch { + throw new Error(`Browser script not found at ${browserScriptPath}`); + } + const browser = await puppeteer.default.launch({ headless: true }); const page = await browser.newPage(); await page.setViewport({ width: 1280, height: 800 }); await page.goto(url, { waitUntil: 'networkidle0', timeout: 30000 }); - // Serialize shared functions for page.evaluate - const safeTags = [...SAFE_TAGS]; - const overusedFonts = [...OVERUSED_FONTS]; - const genericFonts = [...GENERIC_FONTS]; - - const results = await page.evaluate((safeTags, overusedFonts, genericFonts) => { - const safe = new Set(safeTags); - const overused = new Set(overusedFonts); - const generic = new Set(genericFonts); - const findings = []; - const sides = ['Top', 'Right', 'Bottom', 'Left']; - - // Element-level border checks - for (const el of document.querySelectorAll('*')) { - const tag = el.tagName.toLowerCase(); - if (safe.has(tag)) continue; - const rect = el.getBoundingClientRect(); - if (rect.width < 20 || rect.height < 20) continue; - - const style = getComputedStyle(el); - const widths = {}, colors = {}; - for (const s of sides) { - widths[s] = parseFloat(style[`border${s}Width`]) || 0; - colors[s] = style[`border${s}Color`] || ''; - } - const radius = parseFloat(style.borderRadius) || 0; - - for (const side of sides) { - const w = widths[side]; - if (w < 1) continue; - const c = colors[side]; - if (!c || c === 'transparent') continue; - const cm = c.match(/rgba?\((\d+),\s*(\d+),\s*(\d+)/); - if (cm && (Math.max(+cm[1], +cm[2], +cm[3]) - Math.min(+cm[1], +cm[2], +cm[3])) < 30) continue; - - const others = sides.filter(s => s !== side); - const maxOther = Math.max(...others.map(s => widths[s])); - if (!(w >= 2 && (maxOther <= 1 || w >= maxOther * 2))) continue; - - const sn = side.toLowerCase(); - const isSide = side === 'Left' || side === 'Right'; - if (isSide) { - if (radius > 0) findings.push({ id: 'side-tab', snippet: `border-${sn}: ${w}px + border-radius: ${radius}px` }); - else if (w >= 3) findings.push({ id: 'side-tab', snippet: `border-${sn}: ${w}px` }); - } else { - if (radius > 0 && w >= 2) findings.push({ id: 'border-accent-on-rounded', snippet: `border-${sn}: ${w}px + border-radius: ${radius}px` }); - } - } - } - - // Typography checks - const fonts = new Set(); - const overusedFound = new Set(); - for (const sheet of document.styleSheets) { - let rules; - try { rules = sheet.cssRules; } catch { continue; } - if (!rules) continue; - for (const rule of rules) { - if (rule.type !== 1) continue; - const ff = rule.style?.fontFamily; - if (!ff) continue; - const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase()); - const primary = stack.find(f => f && !generic.has(f)); - if (primary) { - fonts.add(primary); - if (overused.has(primary)) overusedFound.add(primary); - } - } - } - for (const f of overusedFound) findings.push({ id: 'overused-font', snippet: `Primary font: ${f}` }); - if (fonts.size === 1 && document.querySelectorAll('*').length > 20) { - findings.push({ id: 'single-font', snippet: `Only font: ${[...fonts][0]}` }); - } - - 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({ id: 'flat-type-hierarchy', snippet: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` }); - } - - return findings; - }, safeTags, overusedFonts, genericFonts); + // Inject the browser detection script and collect results + await page.evaluate(browserScript); + const results = await page.evaluate(() => { + if (!window.impeccableScan) return []; + const allFindings = window.impeccableScan(); + // Flatten: each entry has { el, findings: [{type, detail}] } + return allFindings.flatMap(({ findings }) => + findings.map(f => ({ id: f.type, snippet: f.detail })) + ); + }); await browser.close(); return results.map(f => finding(f.id, url, f.snippet)); diff --git a/package.json b/package.json index 52d2f1fa7..5654d3ad7 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "type": "module", "devDependencies": { "jsdom": "^29.0.0", + "puppeteer": "^24.39.1", "wrangler": "^4.71.0" } } diff --git a/source/skills/critique/scripts/detect-antipatterns.mjs b/source/skills/critique/scripts/detect-antipatterns.mjs index 86e769a7e..c26b89b48 100644 --- a/source/skills/critique/scripts/detect-antipatterns.mjs +++ b/source/skills/critique/scripts/detect-antipatterns.mjs @@ -198,15 +198,6 @@ function contrastRatio(c1, c2) { return (lighter + 0.05) / (darker + 0.05); } -/** - * Check if a color is pure black or pure white. - */ -function isPureBlackOrWhite(c) { - if (!c) return false; - return (c.r === 0 && c.g === 0 && c.b === 0) || - (c.r === 255 && c.g === 255 && c.b === 255); -} - /** * Check if a color has meaningful chroma (is "colored" vs gray/neutral). * Uses simple RGB saturation check. @@ -811,98 +802,33 @@ async function detectUrl(url) { throw new Error('puppeteer is required for URL scanning. Install: npm install puppeteer'); } + // Read the browser detection script — reuse it instead of reimplementing + const browserScriptPath = path.resolve( + path.dirname(new URL(import.meta.url).pathname), + '..', '..', '..', '..', 'public', 'js', 'detect-antipatterns-browser.js' + ); + let browserScript; + try { + browserScript = fs.readFileSync(browserScriptPath, 'utf-8'); + } catch { + throw new Error(`Browser script not found at ${browserScriptPath}`); + } + const browser = await puppeteer.default.launch({ headless: true }); const page = await browser.newPage(); await page.setViewport({ width: 1280, height: 800 }); await page.goto(url, { waitUntil: 'networkidle0', timeout: 30000 }); - // Serialize shared functions for page.evaluate - const safeTags = [...SAFE_TAGS]; - const overusedFonts = [...OVERUSED_FONTS]; - const genericFonts = [...GENERIC_FONTS]; - - const results = await page.evaluate((safeTags, overusedFonts, genericFonts) => { - const safe = new Set(safeTags); - const overused = new Set(overusedFonts); - const generic = new Set(genericFonts); - const findings = []; - const sides = ['Top', 'Right', 'Bottom', 'Left']; - - // Element-level border checks - for (const el of document.querySelectorAll('*')) { - const tag = el.tagName.toLowerCase(); - if (safe.has(tag)) continue; - const rect = el.getBoundingClientRect(); - if (rect.width < 20 || rect.height < 20) continue; - - const style = getComputedStyle(el); - const widths = {}, colors = {}; - for (const s of sides) { - widths[s] = parseFloat(style[`border${s}Width`]) || 0; - colors[s] = style[`border${s}Color`] || ''; - } - const radius = parseFloat(style.borderRadius) || 0; - - for (const side of sides) { - const w = widths[side]; - if (w < 1) continue; - const c = colors[side]; - if (!c || c === 'transparent') continue; - const cm = c.match(/rgba?\((\d+),\s*(\d+),\s*(\d+)/); - if (cm && (Math.max(+cm[1], +cm[2], +cm[3]) - Math.min(+cm[1], +cm[2], +cm[3])) < 30) continue; - - const others = sides.filter(s => s !== side); - const maxOther = Math.max(...others.map(s => widths[s])); - if (!(w >= 2 && (maxOther <= 1 || w >= maxOther * 2))) continue; - - const sn = side.toLowerCase(); - const isSide = side === 'Left' || side === 'Right'; - if (isSide) { - if (radius > 0) findings.push({ id: 'side-tab', snippet: `border-${sn}: ${w}px + border-radius: ${radius}px` }); - else if (w >= 3) findings.push({ id: 'side-tab', snippet: `border-${sn}: ${w}px` }); - } else { - if (radius > 0 && w >= 2) findings.push({ id: 'border-accent-on-rounded', snippet: `border-${sn}: ${w}px + border-radius: ${radius}px` }); - } - } - } - - // Typography checks - const fonts = new Set(); - const overusedFound = new Set(); - for (const sheet of document.styleSheets) { - let rules; - try { rules = sheet.cssRules; } catch { continue; } - if (!rules) continue; - for (const rule of rules) { - if (rule.type !== 1) continue; - const ff = rule.style?.fontFamily; - if (!ff) continue; - const stack = ff.split(',').map(f => f.trim().replace(/^['"]|['"]$/g, '').toLowerCase()); - const primary = stack.find(f => f && !generic.has(f)); - if (primary) { - fonts.add(primary); - if (overused.has(primary)) overusedFound.add(primary); - } - } - } - for (const f of overusedFound) findings.push({ id: 'overused-font', snippet: `Primary font: ${f}` }); - if (fonts.size === 1 && document.querySelectorAll('*').length > 20) { - findings.push({ id: 'single-font', snippet: `Only font: ${[...fonts][0]}` }); - } - - 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({ id: 'flat-type-hierarchy', snippet: `Sizes: ${sorted.map(s => s + 'px').join(', ')} (ratio ${ratio.toFixed(1)}:1)` }); - } - - return findings; - }, safeTags, overusedFonts, genericFonts); + // Inject the browser detection script and collect results + await page.evaluate(browserScript); + const results = await page.evaluate(() => { + if (!window.impeccableScan) return []; + const allFindings = window.impeccableScan(); + // Flatten: each entry has { el, findings: [{type, detail}] } + return allFindings.flatMap(({ findings }) => + findings.map(f => ({ id: f.type, snippet: f.detail })) + ); + }); await browser.close(); return results.map(f => finding(f.id, url, f.snippet)); diff --git a/tests/detect-antipatterns-browser.test.js b/tests/detect-antipatterns-browser.test.js new file mode 100644 index 000000000..a84b369a5 --- /dev/null +++ b/tests/detect-antipatterns-browser.test.js @@ -0,0 +1,146 @@ +/** + * Puppeteer-powered tests for the browser detection script. + * Verifies the browser visualizer finds the same anti-patterns as the CLI. + * + * These tests start a local server and use Puppeteer to load fixture pages, + * inject the browser script, and compare findings with the CLI's jsdom output. + * + * Requires: puppeteer (npx cache or npm install) + */ +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { spawn } from 'child_process'; +import path from 'path'; +import { detectHtml } from '../source/skills/critique/scripts/detect-antipatterns.mjs'; + +const FIXTURES = path.join(import.meta.dir, 'fixtures', 'antipatterns'); +const PORT = 3099; // Use a different port to avoid conflicts with dev server +let serverProcess; +let puppeteer; + +// Check if puppeteer is available +let hasPuppeteer = false; +try { + puppeteer = await import('puppeteer'); + hasPuppeteer = true; +} catch {} + +const describeIf = hasPuppeteer ? describe : describe.skip; + +describeIf('browser script parity with CLI', () => { + beforeAll(async () => { + // Start a simple file server for fixtures + browser script + serverProcess = spawn('node', ['-e', ` + const http = require('http'); + const fs = require('fs'); + const path = require('path'); + const types = { '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript' }; + http.createServer((req, res) => { + let filePath; + if (req.url.startsWith('/fixtures/')) { + filePath = path.join(${JSON.stringify(path.join(import.meta.dir))}, req.url); + } else if (req.url.startsWith('/js/')) { + filePath = path.join(${JSON.stringify(path.join(import.meta.dir, '..', 'public'))}, req.url); + } else { + res.writeHead(404); res.end(); return; + } + try { + const content = fs.readFileSync(filePath); + const ext = path.extname(filePath); + res.writeHead(200, { 'Content-Type': types[ext] || 'text/plain' }); + res.end(content); + } catch { res.writeHead(404); res.end(); } + }).listen(${PORT}); + `], { stdio: 'ignore' }); + + // Wait for server to start + await new Promise(r => setTimeout(r, 500)); + }); + + afterAll(() => { + if (serverProcess) serverProcess.kill(); + }); + + async function scanWithBrowser(fixtureName) { + const browser = await puppeteer.default.launch({ headless: true }); + const page = await browser.newPage(); + await page.goto(`http://localhost:${PORT}/fixtures/antipatterns/${fixtureName}`, { + waitUntil: 'networkidle0', + timeout: 10000, + }); + + // Wait for the browser script to run (it uses setTimeout 100ms) + await new Promise(r => setTimeout(r, 300)); + + const results = await page.evaluate(() => { + if (!window.impeccableScan) return []; + const allFindings = window.impeccableScan(); + return allFindings.flatMap(({ findings }) => + findings.map(f => ({ type: f.type, detail: f.detail })) + ); + }); + + await browser.close(); + return results; + } + + function getTypes(findings) { + return [...new Set(findings.map(f => f.antipattern || f.type))].sort(); + } + + test('should-flag.html: browser finds border anti-patterns', async () => { + const browserFindings = await scanWithBrowser('should-flag.html'); + const types = [...new Set(browserFindings.map(f => f.type))]; + expect(types).toContain('side-tab'); + expect(types).toContain('border-accent-on-rounded'); + }, 15000); + + test('should-pass.html: browser finds no border anti-patterns', async () => { + const browserFindings = await scanWithBrowser('should-pass.html'); + const borderFindings = browserFindings.filter(f => + f.type === 'side-tab' || f.type === 'border-accent-on-rounded' + ); + expect(borderFindings).toHaveLength(0); + }, 15000); + + test('color-should-flag.html: browser finds color anti-patterns', async () => { + const browserFindings = await scanWithBrowser('color-should-flag.html'); + const types = [...new Set(browserFindings.map(f => f.type))]; + expect(types).toContain('low-contrast'); + expect(types).toContain('gray-on-color'); + }, 15000); + + test('color-should-pass.html: browser finds zero findings', async () => { + const browserFindings = await scanWithBrowser('color-should-pass.html'); + expect(browserFindings).toHaveLength(0); + }, 15000); + + test('layout-should-flag.html: browser finds nested cards', async () => { + const browserFindings = await scanWithBrowser('layout-should-flag.html'); + const nested = browserFindings.filter(f => f.type === 'nested-cards'); + expect(nested.length).toBeGreaterThanOrEqual(3); + }, 15000); + + test('layout-should-pass.html: browser finds no nested cards', async () => { + const browserFindings = await scanWithBrowser('layout-should-pass.html'); + const nested = browserFindings.filter(f => f.type === 'nested-cards'); + expect(nested).toHaveLength(0); + }, 15000); + + test('typography-should-flag.html: browser finds typography issues', async () => { + const browserFindings = await scanWithBrowser('typography-should-flag.html'); + const types = [...new Set(browserFindings.map(f => f.type))]; + expect(types).toContain('overused-font'); + expect(types).toContain('flat-type-hierarchy'); + }, 15000); + + test('partial-component.html: browser skips page-level checks', async () => { + const browserFindings = await scanWithBrowser('partial-component.html'); + // Should find border issues but not typography + const hasBorder = browserFindings.some(f => f.type === 'side-tab'); + const hasTypo = browserFindings.some(f => + f.type === 'flat-type-hierarchy' || f.type === 'single-font' + ); + expect(hasBorder).toBe(true); + // Browser script doesn't have isFullPage check, so we just verify borders work + }, 15000); +});