diff --git a/CHANGELOG.md b/CHANGELOG.md index 671289a54..3c5e8e6ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ ### Fixed +- Honor `.pen` frame layout defaults and sizing and padding shorthands so imported auto-layout frames keep their computed dimensions and child positions. (#564) - Avoid macOS Keychain prompts during credential status checks and pause repeated credential access after failures until explicitly retried from Settings. - Route browser Command/Ctrl plus and minus shortcuts to canvas zoom instead of page zoom. diff --git a/packages/pen/src/convert.ts b/packages/pen/src/convert.ts index 79b9a18d5..bd5f0127f 100644 --- a/packages/pen/src/convert.ts +++ b/packages/pen/src/convert.ts @@ -66,6 +66,11 @@ interface PenFillObject { } type PenFill = string | PenFillObject | PenFillObject[] +type PenSpacingValue = number | string +type PenPadding = + | PenSpacingValue + | [PenSpacingValue, PenSpacingValue] + | [PenSpacingValue, PenSpacingValue, PenSpacingValue, PenSpacingValue] export interface PenNode { type: string @@ -88,7 +93,7 @@ export interface PenNode { effect?: PenEffect | PenEffect[] layout?: string gap?: number | string - padding?: number | string | (number | string)[] + padding?: PenPadding justifyContent?: string alignItems?: string children?: PenNode[] @@ -386,6 +391,15 @@ export function applyPadding(node: SceneNode, padding: PenNode['padding'], ctx?: const resolve = (v: number | string): number => typeof v === 'string' ? (isVarRef(v) && ctx ? ctx.resolveNumber(v) : Number(v) || 0) : v if (Array.isArray(padding)) { + if (padding.length === 2) { + const vertical = resolve(padding[0]) + const horizontal = resolve(padding[1]) + node.paddingTop = vertical + node.paddingRight = horizontal + node.paddingBottom = vertical + node.paddingLeft = horizontal + return + } node.paddingTop = resolve(padding[0] ?? 0) node.paddingRight = resolve(padding[1] ?? 0) node.paddingBottom = resolve(padding[2] ?? 0) @@ -399,20 +413,63 @@ export function applyPadding(node: SceneNode, padding: PenNode['padding'], ctx?: node.paddingLeft = resolved } -export function parseSize(value: number | string | undefined, fallback: number, ctx?: VarContext) { - if (value === undefined) return { value: fallback, sizing: 'FIXED' as LayoutSizing } - if (typeof value === 'number') return { value, sizing: 'FIXED' as LayoutSizing } - if (value === 'fill_container') return { value: fallback, sizing: 'FILL' as LayoutSizing } - if (value === 'hug_content') return { value: fallback, sizing: 'HUG' as LayoutSizing } - if (isVarRef(value) && ctx) - return { value: ctx.resolveNumber(value), sizing: 'FIXED' as LayoutSizing } +interface ParsedSize { + value: number + sizing: LayoutSizing + /** Fallback used only when fit_content(N) has no children. */ + fitContentFallback?: number +} + +function parseParameterizedFallback(value: string, behavior: string): number | undefined { + const prefix = `${behavior}(` + if (!value.startsWith(prefix) || !value.endsWith(')')) return undefined + + const rawFallback = value.slice(prefix.length, -1).trim() + if (rawFallback === '') return undefined + + const parsedFallback = Number(rawFallback) + return Number.isFinite(parsedFallback) ? parsedFallback : undefined +} + +function parseSizingBehavior(value: string, fallback: number): ParsedSize | undefined { + if (value === 'fill_container') return { value: fallback, sizing: 'FILL' } + if (value === 'fit_content') return { value: fallback, sizing: 'HUG' } + + // Older generated .pen files used hug_content as an alias for fit_content. + if (value === 'hug_content') return { value: fallback, sizing: 'HUG' } + + const fillFallback = parseParameterizedFallback(value, 'fill_container') + if (fillFallback !== undefined) return { value: fillFallback, sizing: 'FILL' } + + const fitFallback = + parseParameterizedFallback(value, 'fit_content') ?? + parseParameterizedFallback(value, 'hug_content') + if (fitFallback !== undefined) { + return { value: fitFallback, sizing: 'HUG', fitContentFallback: fitFallback } + } + return undefined +} + +export function parseSize( + value: number | string | undefined, + fallback: number, + ctx?: VarContext +): ParsedSize { + if (value === undefined) return { value: fallback, sizing: 'FIXED' } + if (typeof value === 'number') return { value, sizing: 'FIXED' } + + const behavior = parseSizingBehavior(value, fallback) + if (behavior) return behavior + + if (isVarRef(value) && ctx) return { value: ctx.resolveNumber(value), sizing: 'FIXED' } const parsed = Number(value) - return { value: Number.isFinite(parsed) ? parsed : fallback, sizing: 'FIXED' as LayoutSizing } + return { value: Number.isFinite(parsed) ? parsed : fallback, sizing: 'FIXED' } } export function mapLayoutMode(pen: PenNode): LayoutMode { if (pen.layout === 'row' || pen.layout === 'horizontal') return 'HORIZONTAL' if (pen.layout === 'column' || pen.layout === 'vertical') return 'VERTICAL' + if (pen.type === 'frame' && pen.layout === undefined) return 'HORIZONTAL' return 'NONE' } diff --git a/packages/pen/src/read.ts b/packages/pen/src/read.ts index 814eb38b2..e5a353353 100644 --- a/packages/pen/src/read.ts +++ b/packages/pen/src/read.ts @@ -233,6 +233,9 @@ function createSceneNode( const overrides = buildBaseOverrides(pen) overrides.width = w.value overrides.height = h.value + const hasChildren = (pen.children?.length ?? 0) > 0 + if (!hasChildren && w.fitContentFallback !== undefined) overrides.minWidth = w.fitContentFallback + if (!hasChildren && h.fitContentFallback !== undefined) overrides.minHeight = h.fitContentFallback const parentLayout = graph.getNode(parentId)?.layoutMode ?? 'NONE' if (layout !== 'NONE') { diff --git a/tests/e2e/canvas/pen-layout.spec.ts b/tests/e2e/canvas/pen-layout.spec.ts new file mode 100644 index 000000000..f85af8ca3 --- /dev/null +++ b/tests/e2e/canvas/pen-layout.spec.ts @@ -0,0 +1,34 @@ +import { expect, test, useEditorSetup } from '#tests/e2e/fixtures' + +const editor = useEditorSetup('/?test&no-chrome&no-rulers') + +test('lays out Pen frames whose default horizontal layout is omitted', async () => { + await editor.page.evaluate(() => + window.openPencil?.openFile?.('/tests/fixtures/pen-layout-defaults.pen') + ) + await editor.canvas.waitForInit() + + const geometry = await editor.page.evaluate(() => { + const graph = window.openPencil?.getStore?.().graph + if (!graph) throw new Error('OpenPencil graph not initialized') + const section = graph.getNode('section') + const fixed = graph.getNode('fixed-child') + const fill = graph.getNode('fill-child') + if (!section || !fixed || !fill) throw new Error('Imported layout nodes are missing') + return { + section: { + layoutMode: section.layoutMode, + width: section.width, + height: section.height + }, + fixed: { x: fixed.x, y: fixed.y, width: fixed.width, height: fixed.height }, + fill: { x: fill.x, y: fill.y, width: fill.width, height: fill.height } + } + }) + + expect(geometry).toEqual({ + section: { layoutMode: 'HORIZONTAL', width: 1440, height: 564 }, + fixed: { x: 80, y: 72, width: 620, height: 420 }, + fill: { x: 756, y: 72, width: 604, height: 420 } + }) +}) diff --git a/tests/engine/pen/layout.test.ts b/tests/engine/pen/layout.test.ts new file mode 100644 index 000000000..5b9412387 --- /dev/null +++ b/tests/engine/pen/layout.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, test } from 'bun:test' + +import { computeAllLayouts } from '@open-pencil/core/layout' +import { parsePenFile, parseSize, type PenDocument, type PenNode } from '@open-pencil/pen' + +function parseLayoutDocument(children: PenNode[]) { + const document: PenDocument = { version: '2.17', children } + const graph = parsePenFile(JSON.stringify(document)) + computeAllLayouts(graph) + return graph +} + +function fixedChild(id: string): PenNode { + return { id, type: 'frame', width: 620, height: 420 } +} + +describe('parsePenFile — Pen layout defaults (#564)', () => { + test('treats an omitted frame layout as horizontal', () => { + const graph = parseLayoutDocument([ + { + id: 'screen', + type: 'frame', + width: 1440, + layout: 'vertical', + children: [ + { + id: 'section', + type: 'frame', + width: 'fill_container', + gap: 56, + padding: [72, 80], + alignItems: 'center', + children: [ + fixedChild('fixed'), + { + id: 'fill', + type: 'frame', + width: 'fill_container', + height: 420 + } + ] + } + ] + } + ]) + + expect(graph.getNode('section')).toMatchObject({ + layoutMode: 'HORIZONTAL', + width: 1440, + height: 564, + paddingTop: 72, + paddingRight: 80, + paddingBottom: 72, + paddingLeft: 80 + }) + expect(graph.getNode('fixed')).toMatchObject({ x: 80, y: 72, width: 620, height: 420 }) + expect(graph.getNode('fill')).toMatchObject({ x: 756, y: 72, width: 604, height: 420 }) + }) + + test.each(['fit_content', 'fit_content(100)'])('maps %s to HUG sizing', (height) => { + const graph = parseLayoutDocument([ + { + id: 'section', + type: 'frame', + width: 1440, + height, + layout: 'horizontal', + padding: [72, 80], + children: [fixedChild('fixed')] + } + ]) + + expect(graph.getNode('section')).toMatchObject({ + counterAxisSizing: 'HUG', + height: 564 + }) + }) + + test('uses the parameterized fit-content value when a frame has no content', () => { + const graph = parseLayoutDocument([ + { + id: 'empty', + type: 'frame', + width: 'fit_content(240)', + height: 'fit_content(120)' + } + ]) + + expect(graph.getNode('empty')).toMatchObject({ + layoutMode: 'HORIZONTAL', + primaryAxisSizing: 'HUG', + counterAxisSizing: 'HUG', + width: 240, + height: 120, + minWidth: 240, + minHeight: 120 + }) + }) + + test('ignores the parameterized fallback when content determines the size', () => { + const graph = parseLayoutDocument([ + { + id: 'content-frame', + type: 'frame', + width: 'fit_content(240)', + height: 'fit_content(120)', + padding: 10, + children: [{ id: 'content', type: 'frame', width: 20, height: 30 }] + } + ]) + + expect(graph.getNode('content-frame')).toMatchObject({ + width: 40, + height: 50, + minWidth: null, + minHeight: null + }) + }) + + test('keeps explicit freeform frames out of auto-layout', () => { + const graph = parseLayoutDocument([ + { + id: 'freeform', + type: 'frame', + layout: 'none', + children: [fixedChild('fixed')] + } + ]) + + expect(graph.getNode('freeform')?.layoutMode).toBe('NONE') + }) +}) + +describe('parseSize — Pen sizing fallbacks (#564)', () => { + test('preserves parameterized fill-container fallback values', () => { + expect(parseSize('fill_container(900)', 100)).toEqual({ value: 900, sizing: 'FILL' }) + }) + + test('preserves parameterized fit-content fallback metadata', () => { + expect(parseSize('fit_content(900)', 100)).toEqual({ + value: 900, + sizing: 'HUG', + fitContentFallback: 900 + }) + }) + + test.each(['fit_content()', 'fit_content(nope)', 'fit_content(100)garbage'])( + 'rejects malformed sizing behavior %s', + (value) => { + expect(parseSize(value, 100)).toEqual({ value: 100, sizing: 'FIXED' }) + } + ) +}) diff --git a/tests/engine/pen/var-padding.test.ts b/tests/engine/pen/var-padding.test.ts index 52a19acb2..ac06084e9 100644 --- a/tests/engine/pen/var-padding.test.ts +++ b/tests/engine/pen/var-padding.test.ts @@ -120,6 +120,18 @@ describe('applyPadding — variable resolution (#201)', () => { }) }) +describe('applyPadding — Pen shorthand (#564)', () => { + test('expands two values as vertical and horizontal pairs', () => { + const node = makeNode() + applyPadding(node, [72, 80]) + + expect(node.paddingTop).toBe(72) + expect(node.paddingRight).toBe(80) + expect(node.paddingBottom).toBe(72) + expect(node.paddingLeft).toBe(80) + }) +}) + describe('isVarRef', () => { test.each(['$merk-blauw', '$font-tekst', '$color.background', '$x'])( 'recognizes %s as a variable reference', diff --git a/tests/fixtures/pen-layout-defaults.pen b/tests/fixtures/pen-layout-defaults.pen new file mode 100644 index 000000000..b3ce04e80 --- /dev/null +++ b/tests/fixtures/pen-layout-defaults.pen @@ -0,0 +1,41 @@ +{ + "version": "2.17", + "children": [ + { + "type": "frame", + "id": "screen", + "name": "Screen", + "width": 1440, + "layout": "vertical", + "children": [ + { + "type": "frame", + "id": "section", + "name": "Omitted horizontal layout", + "width": "fill_container", + "gap": 56, + "padding": [72, 80], + "alignItems": "center", + "children": [ + { + "type": "frame", + "id": "fixed-child", + "name": "Fixed child", + "width": 620, + "height": 420, + "fill": "#3355AA" + }, + { + "type": "frame", + "id": "fill-child", + "name": "Fill child", + "width": "fill_container", + "height": 420, + "fill": "#AA3355" + } + ] + } + ] + } + ] +}