feat(ai): detect mobile section glued to screen edge
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.
This commit is contained in:
parent
e5ac53017d
commit
2b534c7cf3
180
packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts
Normal file
180
packages/pen-ai-skills/src/__tests__/detectors-spacing.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
149
packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts
Normal file
149
packages/pen-ai-skills/src/diagnostics/detectors-spacing.ts
Normal file
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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<string>();
|
||||
const unique: Issue[] = [];
|
||||
|
|
|
|||
|
|
@ -14,3 +14,4 @@ export {
|
|||
detectExcessiveFrameEffects,
|
||||
detectAllIssues,
|
||||
} from './detectors';
|
||||
export { detectEdgeSectionPadding } from './detectors-spacing';
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
|
|
@ -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.',
|
||||
|
|
|
|||
Loading…
Reference in a new issue