mirror of
https://github.com/ZSeven-W/openpencil.git
synced 2026-05-31 19:04:29 +07:00
fix(canvas): require explicit role:'overlay' for layout-flow escape hatch
isBadgeOverlayNode matched role:'badge'|'pill'|'tag' and pulled those children out of their parent's auto-layout, rendering them at (0,0) of the parent and stacking them on top of siblings. But in this repo badge/pill/tag are inline-component roles (see role-resolver NAME_EXACT_MAP and strip-redundant-section-fills PROTECTED_ROLES) — they're meant to flow in layout like any other child. Rename to isOverlayNode and narrow to role:'overlay'. Add matching "Layout-escape roles" guidance in role-definitions.md so generation prompts can reach the new opt-in. Inline roles now flow correctly; true floating decorations (notification dots, corner ribbons) still have a dedicated marker.
This commit is contained in:
parent
572f71a1a8
commit
1c0cb8c7ef
11 changed files with 77 additions and 56 deletions
|
|
@ -1,2 +1,2 @@
|
|||
// Re-export from @zseven-w/pen-core — the canonical source
|
||||
export { isBadgeOverlayNode } from '@zseven-w/pen-core';
|
||||
export { isOverlayNode } from '@zseven-w/pen-core';
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ import {
|
|||
sanitizeLayoutChildPositions,
|
||||
sanitizeScreenFrameBounds,
|
||||
hasActiveLayout,
|
||||
isBadgeOverlayNode,
|
||||
isOverlayNode,
|
||||
} from './design-node-sanitization';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -224,7 +224,7 @@ export function insertStreamingNode(node: PenNode, parentId: string | null): voi
|
|||
|
||||
const parentNode = resolvedParent ? getNodeById(resolvedParent) : null;
|
||||
|
||||
if (parentNode && hasActiveLayout(parentNode) && !isBadgeOverlayNode(node)) {
|
||||
if (parentNode && hasActiveLayout(parentNode) && !isOverlayNode(node)) {
|
||||
if ('x' in node) delete (node as { x?: number }).x;
|
||||
if ('y' in node) delete (node as { y?: number }).y;
|
||||
// Text defaults inside layout frames:
|
||||
|
|
@ -362,7 +362,7 @@ export function insertStreamingNode(node: PenNode, parentId: string | null): voi
|
|||
|
||||
// Badge/overlay nodes prepend (index 0) so they render on top (earlier = higher z-order).
|
||||
// All other nodes append to preserve auto-layout generation order.
|
||||
addNode(insertParent, node, isBadgeOverlayNode(node) ? 0 : Infinity);
|
||||
addNode(insertParent, node, isOverlayNode(node) ? 0 : Infinity);
|
||||
|
||||
// When a frame is inserted into a horizontal layout, equalize sibling card widths
|
||||
// to prevent overflow when multiple cards are placed in the same row.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import type { PenNode } from '@/types/pen';
|
||||
import { clamp } from './generation-utils';
|
||||
export { isBadgeOverlayNode } from '@/canvas/node-helpers';
|
||||
import { isBadgeOverlayNode } from '@/canvas/node-helpers';
|
||||
export { isOverlayNode } from '@/canvas/node-helpers';
|
||||
import { isOverlayNode } from '@/canvas/node-helpers';
|
||||
export { deepCloneNode } from '@/stores/document-tree-utils';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -59,11 +59,11 @@ export function hasActiveLayout(node: PenNode): boolean {
|
|||
return node.layout === 'vertical' || node.layout === 'horizontal';
|
||||
}
|
||||
|
||||
// isBadgeOverlayNode moved to @/canvas/node-helpers — re-exported above
|
||||
// isOverlayNode moved to @/canvas/node-helpers — re-exported above
|
||||
|
||||
export function sanitizeLayoutChildPositions(node: PenNode, parentHasLayout: boolean): void {
|
||||
// Badge/overlay nodes retain their x/y for absolute positioning
|
||||
if (parentHasLayout && !isBadgeOverlayNode(node)) {
|
||||
// Overlay nodes (role: 'overlay') retain their x/y for absolute positioning
|
||||
if (parentHasLayout && !isOverlayNode(node)) {
|
||||
if ('x' in node) delete (node as { x?: number }).x;
|
||||
if ('y' in node) delete (node as { y?: number }).y;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -77,6 +77,10 @@ Media roles:
|
|||
- avatar: width/height=48, cornerRadius=24, clipContent=true (size adapts to explicit width)
|
||||
- icon: width=24, height=24
|
||||
|
||||
Layout-escape roles:
|
||||
|
||||
- overlay: the ONLY way to place a child at absolute x/y inside a parent that uses `layout: vertical|horizontal`. Use for notification dots on an icon, corner ribbons on a card, floating status indicators. The child keeps its explicit `x`/`y` while siblings flow normally. Do NOT use `role: 'overlay'` for inline components — `badge`, `pill`, `tag` are inline (they flow in layout like any other child, NOT floating). Do NOT use `role: 'overlay'` as a substitute for `layout: 'none'` on the parent.
|
||||
|
||||
Typography roles:
|
||||
|
||||
- heading: lineHeight=1.2 (CJK: 1.35), letterSpacing=-0.5 (CJK: 0). In vertical layout: textGrowth=fixed-width, width=fill_container
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ Pure document tree operations, layout engine, variables, normalization, boolean
|
|||
- `src/arc-path.ts` — `buildEllipseArcPath`, `isArcEllipse`: SVG arc path generation for partial ellipses
|
||||
- `src/path-anchors.ts` — `anchorsToPathData`, `pathDataToAnchors`, `getPathBoundsFromAnchors`, `inferPathAnchorPointType`
|
||||
- `src/font-utils.ts` — `cssFontFamily`: CSS font-family string builder
|
||||
- `src/node-helpers.ts` — `isBadgeOverlayNode`, `sanitizeName` (PascalCase conversion)
|
||||
- `src/node-helpers.ts` — `isOverlayNode` (detects `role: 'overlay'`), `sanitizeName` (PascalCase conversion)
|
||||
- `src/design-md-parser.ts` — `parseDesignMd`, `generateDesignMd`, `designMdColorsToVariables`, `extractDesignMdFromDocument`
|
||||
- `src/constants.ts` — Canvas rendering constants (zoom limits, colors, snap thresholds, pen tool sizes, guide styling)
|
||||
- `src/id.ts` — `generateId` (nanoid wrapper)
|
||||
|
|
|
|||
|
|
@ -1,30 +1,40 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import type { PenNode } from '@zseven-w/pen-types';
|
||||
import { isBadgeOverlayNode } from '../node-helpers';
|
||||
import { isOverlayNode } from '../node-helpers';
|
||||
|
||||
describe('isBadgeOverlayNode', () => {
|
||||
it('returns true for badge role', () => {
|
||||
describe('isOverlayNode', () => {
|
||||
it('returns true for overlay role', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', role: 'overlay' };
|
||||
expect(isOverlayNode(node)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for badge role (inline component, not overlay)', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', role: 'badge' };
|
||||
expect(isBadgeOverlayNode(node)).toBe(true);
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true for pill role', () => {
|
||||
it('returns false for pill role (inline component, not overlay)', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', role: 'pill' };
|
||||
expect(isBadgeOverlayNode(node)).toBe(true);
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true for name containing "badge"', () => {
|
||||
it('returns false for tag role (inline component, not overlay)', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', role: 'tag' };
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for name containing "badge" without explicit role', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', name: 'Notification Badge' };
|
||||
expect(isBadgeOverlayNode(node)).toBe(true);
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true for name containing "overlay"', () => {
|
||||
it('returns false for name containing "overlay" without explicit role', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', name: 'Image Overlay' };
|
||||
expect(isBadgeOverlayNode(node)).toBe(true);
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for regular nodes', () => {
|
||||
const node: PenNode = { id: '1', type: 'rectangle', name: 'Button' };
|
||||
expect(isBadgeOverlayNode(node)).toBe(false);
|
||||
expect(isOverlayNode(node)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -126,11 +126,11 @@ describe('normalizeTreeLayout', () => {
|
|||
expect('y' in kids[1]).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps x and y on badge overlay children', () => {
|
||||
const badge: PenNode = {
|
||||
id: 'badge1',
|
||||
it('keeps x and y on overlay children', () => {
|
||||
const overlay: PenNode = {
|
||||
id: 'overlay1',
|
||||
type: 'rectangle',
|
||||
role: 'badge',
|
||||
role: 'overlay',
|
||||
width: 8,
|
||||
height: 8,
|
||||
x: 40,
|
||||
|
|
@ -138,7 +138,7 @@ describe('normalizeTreeLayout', () => {
|
|||
} as PenNode;
|
||||
const node = frame({
|
||||
layout: 'horizontal',
|
||||
children: [rect('a', { x: 5, y: 5 }), badge],
|
||||
children: [rect('a', { x: 5, y: 5 }), overlay],
|
||||
});
|
||||
normalizeTreeLayout(node);
|
||||
const kids = (node as PenNode & { children: PenNode[] }).children;
|
||||
|
|
@ -178,25 +178,25 @@ describe('normalizeTreeLayout', () => {
|
|||
});
|
||||
|
||||
it('overlay-only children do not block the vertical fallback', () => {
|
||||
// A container whose only positioned children are overlays (badges) is
|
||||
// still considered "model forgot layout" — the overlays retain their
|
||||
// A container whose only positioned children are overlays is still
|
||||
// considered "model forgot layout" — the overlays retain their
|
||||
// coordinates while the base frame gets a vertical layout for the rest.
|
||||
const badge: PenNode = {
|
||||
id: 'badge1',
|
||||
const overlay: PenNode = {
|
||||
id: 'overlay1',
|
||||
type: 'rectangle',
|
||||
role: 'badge',
|
||||
role: 'overlay',
|
||||
width: 8,
|
||||
height: 8,
|
||||
x: 40,
|
||||
y: 0,
|
||||
} as PenNode;
|
||||
const node = frame({
|
||||
children: [rect('a'), rect('b'), badge],
|
||||
children: [rect('a'), rect('b'), overlay],
|
||||
});
|
||||
normalizeTreeLayout(node);
|
||||
expect((node as PenNode & { layout?: string }).layout).toBe('vertical');
|
||||
const kids = (node as PenNode & { children: PenNode[] }).children;
|
||||
// badge still carries its absolute coordinates
|
||||
// overlay still carries its absolute coordinates
|
||||
expect(kids[2].x).toBe(40);
|
||||
expect(kids[2].y).toBe(0);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -143,7 +143,7 @@ export { type BooleanOpType, canBooleanOp, executeBooleanOp } from './boolean-op
|
|||
export { cssFontFamily } from './font-utils.js';
|
||||
|
||||
// Node helpers
|
||||
export { isBadgeOverlayNode, sanitizeName } from './node-helpers.js';
|
||||
export { isOverlayNode, sanitizeName } from './node-helpers.js';
|
||||
|
||||
// Design-MD parser
|
||||
export {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import type { PenNode, ContainerProps, SizingBehavior, Padding } from '@zseven-w/pen-types';
|
||||
export type { Padding } from '@zseven-w/pen-types';
|
||||
import { isBadgeOverlayNode } from '../node-helpers.js';
|
||||
import { isOverlayNode } from '../node-helpers.js';
|
||||
import {
|
||||
parseSizing,
|
||||
estimateTextWidth,
|
||||
|
|
@ -119,7 +119,7 @@ export function inferLayout(node: PenNode): 'horizontal' | undefined {
|
|||
export function fitContentWidth(node: PenNode, parentAvail?: number): number {
|
||||
if (!('children' in node) || !node.children?.length) return 0;
|
||||
const visibleChildren = node.children.filter(
|
||||
(child) => isNodeVisible(child) && !isBadgeOverlayNode(child),
|
||||
(child) => isNodeVisible(child) && !isOverlayNode(child),
|
||||
);
|
||||
if (visibleChildren.length === 0) return 0;
|
||||
const c = node as PenNode & ContainerProps;
|
||||
|
|
@ -147,7 +147,7 @@ export function fitContentWidth(node: PenNode, parentAvail?: number): number {
|
|||
export function fitContentHeight(node: PenNode, parentAvailW?: number): number {
|
||||
if (!('children' in node) || !node.children?.length) return 0;
|
||||
const visibleChildren = node.children.filter(
|
||||
(child) => isNodeVisible(child) && !isBadgeOverlayNode(child),
|
||||
(child) => isNodeVisible(child) && !isOverlayNode(child),
|
||||
);
|
||||
if (visibleChildren.length === 0) return 0;
|
||||
const c = node as PenNode & ContainerProps;
|
||||
|
|
@ -254,8 +254,8 @@ export function computeLayoutPositions(parent: PenNode, children: PenNode[]): Pe
|
|||
const layout = c.layout || inferLayout(parent);
|
||||
if (!layout || layout === 'none') return visibleChildren;
|
||||
|
||||
const badgeNodes = visibleChildren.filter(isBadgeOverlayNode);
|
||||
const layoutChildren = visibleChildren.filter((ch) => !isBadgeOverlayNode(ch));
|
||||
const overlayNodes = visibleChildren.filter(isOverlayNode);
|
||||
const layoutChildren = visibleChildren.filter((ch) => !isOverlayNode(ch));
|
||||
if (layoutChildren.length === 0) return visibleChildren;
|
||||
|
||||
const pW = parseSizing(c.width);
|
||||
|
|
@ -399,8 +399,8 @@ export function computeLayoutPositions(parent: PenNode, children: PenNode[]): Pe
|
|||
return out as unknown as PenNode;
|
||||
});
|
||||
|
||||
if (badgeNodes.length > 0) {
|
||||
return [...badgeNodes, ...positioned];
|
||||
if (overlayNodes.length > 0) {
|
||||
return [...overlayNodes, ...positioned];
|
||||
}
|
||||
return positioned;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { PenNode, ContainerProps } from '@zseven-w/pen-types';
|
||||
import { isBadgeOverlayNode } from '../node-helpers.js';
|
||||
import { isOverlayNode } from '../node-helpers.js';
|
||||
import { inferLayout } from './engine.js';
|
||||
|
||||
/**
|
||||
|
|
@ -17,8 +17,8 @@ import { inferLayout } from './engine.js';
|
|||
* images with floating overlays, etc.) and we leave it alone.
|
||||
*
|
||||
* 2. When a frame has an active layout (`vertical` or `horizontal`), strip
|
||||
* `x`/`y` from every non-overlay child. Overlay children (badges, pills,
|
||||
* tags, floating indicators) keep their absolute coordinates.
|
||||
* `x`/`y` from every non-overlay child. Overlay children (opt-in via
|
||||
* `role: 'overlay'`) keep their absolute coordinates.
|
||||
*
|
||||
* Used as a post-generation pass after an AI model produces a subtree. It
|
||||
* corrects two common model mistakes:
|
||||
|
|
@ -58,7 +58,7 @@ export function normalizeTreeLayout(node: PenNode): void {
|
|||
// (2) Strip x/y from non-overlay children of active-layout frames.
|
||||
if (c.layout === 'vertical' || c.layout === 'horizontal') {
|
||||
for (const child of children) {
|
||||
if (!isBadgeOverlayNode(child)) {
|
||||
if (!isOverlayNode(child)) {
|
||||
if ('x' in child) delete (child as { x?: number }).x;
|
||||
if ('y' in child) delete (child as { y?: number }).y;
|
||||
}
|
||||
|
|
@ -105,9 +105,9 @@ export function normalizeTreeLayout(node: PenNode): void {
|
|||
* still get the vertical fallback, because those are typically
|
||||
* content stacks where verticalization is the right call.
|
||||
*
|
||||
* Overlay nodes (badges/pills/tags via `isBadgeOverlayNode`) are excluded
|
||||
* from the count — they legitimately carry x/y inside auto-layout frames
|
||||
* and shouldn't tip the heuristic.
|
||||
* Overlay nodes (opt-in via `role: 'overlay'`, detected by `isOverlayNode`)
|
||||
* are excluded from the count — they legitimately carry x/y inside
|
||||
* auto-layout frames and shouldn't tip the heuristic.
|
||||
*
|
||||
* This is intentionally conservative: it accepts a few false negatives
|
||||
* (some genuinely vertical all-shape stacks will be left un-normalized,
|
||||
|
|
@ -120,7 +120,7 @@ const COMPOSITION_PRIMITIVE_TYPES = new Set(['frame', 'ellipse', 'path']);
|
|||
function hasAbsolutePositionedChild(children: PenNode[]): boolean {
|
||||
// Signal 1: explicit numeric x/y on any non-overlay child.
|
||||
for (const child of children) {
|
||||
if (isBadgeOverlayNode(child)) continue;
|
||||
if (isOverlayNode(child)) continue;
|
||||
const c = child as PenNode & { x?: number; y?: number };
|
||||
if (typeof c.x === 'number' || typeof c.y === 'number') return true;
|
||||
}
|
||||
|
|
@ -130,7 +130,7 @@ function hasAbsolutePositionedChild(children: PenNode[]): boolean {
|
|||
let nonOverlayCount = 0;
|
||||
let primitiveCount = 0;
|
||||
for (const child of children) {
|
||||
if (isBadgeOverlayNode(child)) continue;
|
||||
if (isOverlayNode(child)) continue;
|
||||
nonOverlayCount++;
|
||||
if (COMPOSITION_PRIMITIVE_TYPES.has(child.type)) primitiveCount++;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,16 +1,23 @@
|
|||
import type { PenNode } from '@zseven-w/pen-types';
|
||||
|
||||
/**
|
||||
* Check if a node is a badge/overlay that uses absolute positioning
|
||||
* and should not participate in layout flow.
|
||||
* Check if a node is an overlay that uses absolute positioning and should
|
||||
* not participate in layout flow.
|
||||
*
|
||||
* Requires explicit `role: 'overlay'`. Earlier versions matched on
|
||||
* `role: 'badge' | 'pill' | 'tag'` plus name regexes, but those are
|
||||
* inline-component markers in this repo (see `role-resolver.ts` and
|
||||
* `strip-redundant-section-fills.ts` PROTECTED_ROLES) — pulling them out
|
||||
* of layout flow collapsed them to (0,0) of their parent and stacked
|
||||
* them on top of siblings. `role: 'overlay'` is the dedicated opt-in for
|
||||
* notification dots and true floating decorations.
|
||||
*/
|
||||
export function isBadgeOverlayNode(node: PenNode): boolean {
|
||||
export function isOverlayNode(node: PenNode): boolean {
|
||||
if ('role' in node) {
|
||||
const role = (node as { role?: string }).role;
|
||||
if (role === 'badge' || role === 'pill' || role === 'tag') return true;
|
||||
if (role === 'overlay') return true;
|
||||
}
|
||||
const name = (node.name ?? '').toLowerCase();
|
||||
return /badge|indicator|notification[-_\s]?dot|overlay|floating/i.test(name);
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Reference in a new issue