fix(pen): honor layout and sizing defaults (#652)

Fixes #564.

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.

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'
 }
 

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') {

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 }
+  })
+})

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' })
+    }
+  )
+})

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',

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"
+            }
+          ]
+        }
+      ]
+    }
+  ]
+}