From 2b534c7cf33ba1e001e808cd63d35da34c845981 Mon Sep 17 00:00:00 2001 From: Fini Date: Sat, 9 May 2026 20:59:58 +0800 Subject: [PATCH] feat(ai): detect mobile section glued to screen edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add detectEdgeSectionPadding (12th pre-validation detector). Flags a mobile-shaped page root (width 320–480) when its horizontal padding is 0 AND a child content section also has 0 left padding AND that section contains visible text or icon descendants — the chain that produced the "Categories" no-padding bug. Suggested fix sets root.padding to [top, 16, bottom, 16] preserving vertical padding. False-positive guards skip top-nav / bottom-nav / hero / banner roles and image-only sections that are intentionally full-bleed. The new detector lives in its own detectors-spacing.ts since detectors.ts is already over the 800-line file limit. --- .../src/__tests__/detectors-spacing.test.ts | 180 ++++++++++++++++++ .../src/diagnostics/detectors-spacing.ts | 149 +++++++++++++++ .../src/diagnostics/detectors.ts | 4 +- .../pen-ai-skills/src/diagnostics/index.ts | 1 + .../pen-ai-skills/src/diagnostics/types.ts | 3 +- packages/pen-mcp/src/routes/debug-routes.ts | 1 + 6 files changed, 336 insertions(+), 2 deletions(-) create mode 100644 packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts create mode 100644 packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts diff --git a/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts b/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts new file mode 100644 index 000000000..a52a848f2 --- /dev/null +++ b/packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts @@ -0,0 +1,180 @@ +import { describe, it, expect } from 'vitest'; +import type { PenNode } from '@zseven-w/pen-types'; +import { detectEdgeSectionPadding } from '../diagnostics/detectors-spacing'; + +// 2026-05-09 user report — image 6: "Categories" mobile section had its +// chip text glued to the screen edge because both the page root and the +// section frame had 0 left padding. The detector flags the page root +// (single fix point) and suggests a 16px horizontal gutter. + +const mobileRoot = (children: PenNode[], padding?: unknown): PenNode => + ({ + id: 'page', + type: 'frame', + width: 393, + height: 852, + layout: 'vertical', + padding, + children, + }) as unknown as PenNode; + +const section = ( + id: string, + children: PenNode[], + opts: { role?: string; padding?: unknown } = {}, +): PenNode => + ({ + id, + type: 'frame', + layout: 'vertical', + role: opts.role, + padding: opts.padding, + children, + }) as unknown as PenNode; + +const text = (id: string, content = 'Hello'): PenNode => + ({ id, type: 'text', content }) as unknown as PenNode; + +const icon = (id: string): PenNode => + ({ id, type: 'icon_font', iconName: 'star' }) as unknown as PenNode; + +const image = (id: string): PenNode => ({ id, type: 'image' }) as unknown as PenNode; + +describe('detectEdgeSectionPadding', () => { + it('flags mobile root with padding=0 and a content section also at padding=0', () => { + const root = mobileRoot([section('categories', [text('t1', 'Tacos'), text('t2', 'Pizza')])]); + // Need >= 2 children on root to qualify as a real page; add a second. + (root as unknown as { children: PenNode[] }).children.push(section('news', [text('t3')])); + + const issues = detectEdgeSectionPadding(root); + + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('page'); + expect(issues[0].category).toBe('edge-section-padding'); + expect(issues[0].suggestedValue).toEqual([0, 16, 0, 16]); + }); + + it('flags when offending section contains an icon (not just text)', () => { + const root = mobileRoot([ + section('iconbar', [icon('i1'), icon('i2')]), + section('body', [text('t')]), + ]); + + const issues = detectEdgeSectionPadding(root); + + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('page'); + }); + + it('does NOT flag when root already has horizontal padding', () => { + const root = mobileRoot( + [section('cat', [text('t1')]), section('news', [text('t2')])], + [0, 16, 0, 16], + ); + + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('does NOT flag desktop-shaped roots (width > 480)', () => { + const root = { + id: 'page', + type: 'frame', + width: 1280, + height: 800, + layout: 'vertical', + children: [section('cat', [text('t1')]), section('news', [text('t2')])], + } as unknown as PenNode; + + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('does NOT flag tiny component-shaped frames (width < 320)', () => { + const root = { + id: 'card', + type: 'frame', + width: 240, + height: 100, + layout: 'vertical', + children: [section('a', [text('t1')]), section('b', [text('t2')])], + } as unknown as PenNode; + + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('does NOT flag full-bleed chrome roles (top-nav, bottom-nav, status-bar)', () => { + const root = mobileRoot([ + section('topnav', [text('back'), icon('back-icon')], { role: 'top-nav' }), + section('botnav', [icon('home'), icon('search')], { role: 'bottom-nav' }), + ]); + + // Both children are full-bleed chrome — no offending content sections. + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('does NOT flag image-only sections (intentional full-bleed media)', () => { + const root = mobileRoot([section('hero', [image('img1')]), section('banner', [image('img2')])]); + + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('does NOT flag sections that have their own left padding > 0', () => { + const root = mobileRoot([ + section('a', [text('t1')], { padding: [16, 16, 16, 16] }), + section('b', [text('t2')], { padding: 16 }), + ]); + + // Both sections take care of gutters themselves — root staying at 0 + // is intentional. + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); + + it('preserves vertical padding when suggesting the fix', () => { + const root = mobileRoot( + [section('a', [text('t1')]), section('b', [text('t2')])], + [24, 0, 32, 0], + ); + + const issues = detectEdgeSectionPadding(root); + + expect(issues).toHaveLength(1); + expect(issues[0].suggestedValue).toEqual([24, 16, 32, 16]); + }); + + it('expands shorthand number padding (e.g. padding: 0) to 4-tuple keeping top/bottom 0', () => { + const root = mobileRoot([section('a', [text('t1')]), section('b', [text('t2')])], 0); + + const issues = detectEdgeSectionPadding(root); + + expect(issues).toHaveLength(1); + expect(issues[0].suggestedValue).toEqual([0, 16, 0, 16]); + }); + + it('walks descendants — nested mobile page mockup in a desktop wrapper', () => { + const innerRoot = mobileRoot([ + section('cat', [text('t1'), text('t2')]), + section('news', [text('t3')]), + ]); + const wrapper = { + id: 'wrap', + type: 'frame', + width: 1440, + height: 900, + layout: 'horizontal', + children: [innerRoot], + } as unknown as PenNode; + + const issues = detectEdgeSectionPadding(wrapper); + + expect(issues).toHaveLength(1); + expect(issues[0].nodeId).toBe('page'); + }); + + it('does NOT flag mobile root with only a single chrome child (no content sections)', () => { + const root = mobileRoot([ + section('topnav', [text('Title')], { role: 'top-nav' }), + section('botnav', [icon('home')], { role: 'bottom-nav' }), + ]); + + expect(detectEdgeSectionPadding(root)).toHaveLength(0); + }); +}); diff --git a/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts b/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts new file mode 100644 index 000000000..ccabf18ee --- /dev/null +++ b/packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts @@ -0,0 +1,149 @@ +import type { PenNode } from '@zseven-w/pen-types'; +import type { Issue } from './types'; + +/** + * Roles that legitimately span the full mobile viewport width and SHOULD + * have 0 horizontal padding from the page root. Sectioning detectors must + * skip these — flagging them produces false positives that the autofix + * would damage (forcing 16px gutters on a top-nav inset breaks chrome). + */ +const FULL_BLEED_ROLES = new Set([ + 'hero', + 'banner', + 'cover', + 'header', + 'top-nav', + 'bottom-nav', + 'status-bar', + 'tab-bar', + 'tabbar', + 'navbar', +]); + +function getPaddingLeft(node: PenNode): number { + const p = (node as unknown as { padding?: unknown }).padding; + if (typeof p === 'number') return p; + if (Array.isArray(p)) { + if (p.length === 4) return Number(p[3] ?? 0); + if (p.length === 2) return Number(p[1] ?? 0); + if (p.length === 1) return Number(p[0] ?? 0); + } + return 0; +} + +function hasTextOrIconDescendant(node: PenNode): boolean { + if (node.type === 'text') return true; + if ((node.type as string) === 'icon_font') return true; + if ('children' in node && Array.isArray(node.children)) { + for (const c of node.children) { + if (hasTextOrIconDescendant(c)) return true; + } + } + return false; +} + +/** + * A child whose only descendants are images / image-placeholders is treated + * as a deliberately full-bleed media tile — flagging its 0-padding would + * force a gutter on banner-shaped cells that the user explicitly designed + * edge-to-edge. + */ +function isImageOnlySection(node: PenNode): boolean { + if (!('children' in node) || !Array.isArray(node.children)) return false; + if (node.children.length === 0) return false; + for (const c of node.children) { + if (c.type === 'image') continue; + const role = ((c as { role?: string }).role ?? '').toLowerCase(); + if (role === 'image-placeholder') continue; + return false; + } + return true; +} + +/** + * Aesthetic detector: mobile-page content section glued to the screen edge. + * + * Detects the "Categories no padding" bug from the 2026-05-09 user report — + * a mobile page where the root has 0 left padding AND a content section + * also has 0 left padding AND that section contains visible text or icon + * descendants. The combined chain means content visually butts against the + * viewport edge, which is unintentional in nearly every mobile UI. + * + * Why root-level fix instead of per-section: + * The user's mental model is "the page has gutters" — a single root + * padding update affects every section at once and matches how mobile + * pages are typically expressed in production codebases (`safe-area` + * inset on the page container, not on every child). Per-section padding + * would compound when a section also wants its own internal padding. + * + * False-positive guards: + * - Mobile-shaped root only (width 320–480 + layout + ≥2 children) + * - Skip child sections with chrome roles (top-nav / bottom-nav / status-bar) + * - Skip child sections with full-bleed roles (hero / banner / cover / header) + * - Skip image-only sections (intentional full-bleed media) + * - Only flag when the offending section contains text or icon_font + * + * Suggested fix: set root.padding to [top, 16, bottom, 16] preserving + * vertical padding. 16 is the iOS HIG / Material default mobile gutter. + */ +export function detectEdgeSectionPadding(root: PenNode): Issue[] { + const issues: Issue[] = []; + walk(root); + return issues; + + function walk(node: PenNode): void { + const width = (node as unknown as { width?: unknown }).width; + const isMobileRoot = + node.type === 'frame' && + typeof width === 'number' && + width >= 320 && + width <= 480 && + 'layout' in node && + (node as { layout?: unknown }).layout != null && + 'children' in node && + Array.isArray(node.children) && + node.children.length >= 2; + + if (isMobileRoot) { + const rootPadL = getPaddingLeft(node); + if (rootPadL === 0) { + const offendingChildren: PenNode[] = []; + for (const child of (node as { children: PenNode[] }).children) { + if (child.type !== 'frame') continue; + const role = ((child as { role?: string }).role ?? '').toLowerCase(); + if (FULL_BLEED_ROLES.has(role)) continue; + if (isImageOnlySection(child)) continue; + if (getPaddingLeft(child) > 0) continue; + if (!hasTextOrIconDescendant(child)) continue; + offendingChildren.push(child); + } + if (offendingChildren.length > 0) { + const currentPad = (node as unknown as { padding?: unknown }).padding; + let suggested: number[]; + if (Array.isArray(currentPad) && currentPad.length === 4) { + suggested = [Number(currentPad[0]) || 0, 16, Number(currentPad[2]) || 0, 16]; + } else if (Array.isArray(currentPad) && currentPad.length === 2) { + suggested = [Number(currentPad[0]) || 0, 16, Number(currentPad[0]) || 0, 16]; + } else if (typeof currentPad === 'number') { + suggested = [currentPad, 16, currentPad, 16]; + } else { + suggested = [0, 16, 0, 16]; + } + issues.push({ + nodeId: node.id, + category: 'edge-section-padding', + severity: 'warning', + property: 'padding', + currentValue: currentPad ?? null, + suggestedValue: suggested, + reason: `mobile root has 0 horizontal padding while ${offendingChildren.length} content section(s) glue text/icon to screen edge`, + }); + } + } + } + + if ('children' in node && Array.isArray(node.children)) { + for (const c of node.children) walk(c); + } + } +} diff --git a/packages/pen-ai-skills/src/diagnostics/detectors.ts b/packages/pen-ai-skills/src/diagnostics/detectors.ts index 49be36fac..0570b25ac 100644 --- a/packages/pen-ai-skills/src/diagnostics/detectors.ts +++ b/packages/pen-ai-skills/src/diagnostics/detectors.ts @@ -1,5 +1,6 @@ import type { PenNode, PenDocument } from '@zseven-w/pen-types'; import type { Issue } from './types'; +import { detectEdgeSectionPadding } from './detectors-spacing'; /** Extract the first fill color from a node (raw, including variable refs) */ function getFirstFillColor(node: PenNode): string | null { @@ -750,7 +751,7 @@ export function detectExcessiveFrameEffects(root: PenNode): Issue[] { } /** - * Run all 11 detectors and return the deduplicated combined issue list. + * Run all 12 detectors and return the deduplicated combined issue list. * Dedup key: `${nodeId}:${property}` (matches runPreValidationFixes). * On collision, the first issue wins (detector execution order below). */ @@ -767,6 +768,7 @@ export function detectAllIssues(root: PenNode, doc: PenDocument): Issue[] { ...detectTextStroke(root), ...detectMixedSiblingPadding(root), ...detectExcessiveFrameEffects(root), + ...detectEdgeSectionPadding(root), ]; const seen = new Set(); const unique: Issue[] = []; diff --git a/packages/pen-ai-skills/src/diagnostics/index.ts b/packages/pen-ai-skills/src/diagnostics/index.ts index 12b30032d..bbfafbfe1 100644 --- a/packages/pen-ai-skills/src/diagnostics/index.ts +++ b/packages/pen-ai-skills/src/diagnostics/index.ts @@ -14,3 +14,4 @@ export { detectExcessiveFrameEffects, detectAllIssues, } from './detectors'; +export { detectEdgeSectionPadding } from './detectors-spacing'; diff --git a/packages/pen-ai-skills/src/diagnostics/types.ts b/packages/pen-ai-skills/src/diagnostics/types.ts index 6057a4a6e..41dea235e 100644 --- a/packages/pen-ai-skills/src/diagnostics/types.ts +++ b/packages/pen-ai-skills/src/diagnostics/types.ts @@ -11,7 +11,8 @@ export type IssueCategory = | 'text-effect' | 'text-stroke' | 'mixed-sibling-padding' - | 'excessive-frame-effects'; + | 'excessive-frame-effects' + | 'edge-section-padding'; export interface Issue { /** Node id where the issue was detected */ diff --git a/packages/pen-mcp/src/routes/debug-routes.ts b/packages/pen-mcp/src/routes/debug-routes.ts index d97ad82b7..320f04b4e 100644 --- a/packages/pen-mcp/src/routes/debug-routes.ts +++ b/packages/pen-mcp/src/routes/debug-routes.ts @@ -46,6 +46,7 @@ export const DEBUG_TOOL_DEFINITIONS = [ 'text-stroke', 'mixed-sibling-padding', 'excessive-frame-effects', + 'edge-section-padding', ], }, description: 'Filter to specific detector categories.',