diff --git a/CHANGELOG.md b/CHANGELOG.md index c85d93118..b70f44f42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ - 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. +- Honor explicit Design JSX instance dimensions and preserve authored overrides through component synchronization. - Route browser Command/Ctrl plus and minus shortcuts to canvas zoom instead of page zoom. - Resolve `$name` references in imported `.pen` fills, stroke fills, font families, dimensions, and spacing without requiring a `--` prefix. (#563) - Resolve bound fields in each layer’s mode, keep variable edits scoped and undoable, and make broken bindings visible and recoverable. diff --git a/packages/core/src/design-jsx/reference/authoring.md b/packages/core/src/design-jsx/reference/authoring.md index 89bf846a2..3b421fe26 100644 --- a/packages/core/src/design-jsx/reference/authoring.md +++ b/packages/core/src/design-jsx/reference/authoring.md @@ -35,7 +35,7 @@ This reference describes scene creation, not React DOM output. Use the `render` - Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. - Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. - Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. -- Distinguish a main component's default size from its instance's placement constraints. Verify the actual instance bounds in narrower parents; a requested Fill dimension alone is not proof that inherited sizing changed. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. +- Explicit instance `w` / `h` replace the inherited sizing mode on that axis; omitted dimensions retain the main component's sizing. Authored overrides survive component synchronization. Distinguish those placement constraints from the main component's default size, and verify actual bounds in narrower parents. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. ## Verification diff --git a/packages/core/src/design-jsx/renderer.ts b/packages/core/src/design-jsx/renderer.ts index 927e51365..95bc566e0 100644 --- a/packages/core/src/design-jsx/renderer.ts +++ b/packages/core/src/design-jsx/renderer.ts @@ -430,13 +430,34 @@ async function renderInstanceNode( const label = typeof ref === 'string' || typeof ref === 'number' ? String(ref) : '' throw new Error(` component not found: ${label}`) } - const overrides = { + const overrides: Partial = { ...propsToOverrides(props, false, parentLayout), ...componentMetadata(props, 'INSTANCE', componentPropertyScope(graph, parentId)) } + // Instances inherit their container layout, but explicitly authored dimensions + // must also replace the inherited sizing mode on that axis. + const layout = overrides.layoutMode ?? component.layoutMode + if (layout !== 'NONE') { + const axes = + layout === 'HORIZONTAL' + ? (['primaryAxisSizing', 'counterAxisSizing'] as const) + : (['counterAxisSizing', 'primaryAxisSizing'] as const) + for (const [dimension, field] of [ + ['w', axes[0]], + ['h', axes[1]] + ] as const) { + const value = props[dimension] + if (typeof value === 'number') overrides[field] = 'FIXED' + else if (value === 'hug' || value === 'fill') overrides[field] = 'HUG' + } + } const instance = graph.createInstance(component.id, parentId, overrides) ?? graph.createNode('FRAME', parentId) try { + for (const [field, value] of Object.entries(overrides)) { + setInstanceOverride(instance.instanceOverrides, instance.id, instance.id, field, value) + } + graph.updateNode(instance.id, { instanceOverrides: instance.instanceOverrides }) applyBindings(graph, instance.id, bindings) applyInstanceOverrides(graph, instance, tree.props.overrides) assignComponentProperties(graph, instance, props.properties) diff --git a/packages/docs/reference/design-authoring.md b/packages/docs/reference/design-authoring.md index ca61b2d0e..36395a9f6 100644 --- a/packages/docs/reference/design-authoring.md +++ b/packages/docs/reference/design-authoring.md @@ -37,7 +37,7 @@ This reference describes scene creation, not React DOM output. Use the `render` - Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. - Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. - Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. -- Distinguish a main component's default size from its instance's placement constraints. Verify the actual instance bounds in narrower parents; a requested Fill dimension alone is not proof that inherited sizing changed. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. +- Explicit instance `w` / `h` replace the inherited sizing mode on that axis; omitted dimensions retain the main component's sizing. Authored overrides survive component synchronization. Distinguish those placement constraints from the main component's default size, and verify actual bounds in narrower parents. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. ## Verification diff --git a/skills/open-pencil/references/design-authoring.md b/skills/open-pencil/references/design-authoring.md index ca61b2d0e..36395a9f6 100644 --- a/skills/open-pencil/references/design-authoring.md +++ b/skills/open-pencil/references/design-authoring.md @@ -37,7 +37,7 @@ This reference describes scene creation, not React DOM output. Use the `render` - Child `propertyRefs` connect fields to stable property IDs, for example `[{ propertyId: 'message', field: 'TEXT' }]`. Supported fields are `TEXT`, `VISIBLE`, and `INSTANCE_SWAP`; text and swap references require text and instance nodes respectively. References do not depend on layer names. - Instance assignments use the native string values (including `'true'` / `'false'` for BOOLEAN properties and component IDs for swaps). For example `Instance({ of: noteId, properties: { message: 'Updated review' } })`. Assignments persist through component synchronization; unknown IDs and invalid values fail rather than silently creating inert overrides. Select variants through component-set variant props, not through instance property assignments. - Reuse existing local or library components before recreating them. Keep meaningful text, visibility, and swap properties exposed rather than hand-editing cloned child nodes. -- Distinguish a main component's default size from its instance's placement constraints. Verify the actual instance bounds in narrower parents; a requested Fill dimension alone is not proof that inherited sizing changed. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. +- Explicit instance `w` / `h` replace the inherited sizing mode on that axis; omitted dimensions retain the main component's sizing. Authored overrides survive component synchronization. Distinguish those placement constraints from the main component's default size, and verify actual bounds in narrower parents. Do not compensate for a sizing mismatch with guessed heights, clipping, or manually positioned siblings. ## Verification diff --git a/tests/e2e/canvas/instance-sizing.spec.ts b/tests/e2e/canvas/instance-sizing.spec.ts new file mode 100644 index 000000000..b7780762d --- /dev/null +++ b/tests/e2e/canvas/instance-sizing.spec.ts @@ -0,0 +1,26 @@ +import { expect, test, useEditorSetupWithClear } from '#tests/e2e/fixtures' +import type * as SizingFixture from '#tests/helpers/canvas/instance-sizing' + +test.use({ viewport: { width: 900, height: 700 } }) + +const editor = useEditorSetupWithClear('/?test&no-chrome&no-rulers') + +test('Fill instances wrap inside narrower parents after component synchronization', async () => { + await editor.page.evaluate(async () => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const fixtureURL = '/tests/helpers/canvas/instance-sizing.ts' + const { createInstanceSizingScene }: typeof SizingFixture = await import(fixtureURL) + const { boardId, componentId } = await createInstanceSizingScene( + store.graph, + store.state.currentPageId + ) + await store.loadFontsForNodes([boardId, componentId]) + store.clearSelection() + store.zoomToFit() + store.requestRender() + }) + await editor.canvas.waitForRender() + editor.canvas.assertNoErrors() + expect(await editor.canvas.screenshotCanvasRegion()).toMatchSnapshot('instance-fill-sizing.png') +}) diff --git a/tests/e2e/canvas/instance-sizing.spec.ts-snapshots/instance-fill-sizing-openpencil-darwin.png b/tests/e2e/canvas/instance-sizing.spec.ts-snapshots/instance-fill-sizing-openpencil-darwin.png new file mode 100644 index 000000000..7ad08de0e Binary files /dev/null and b/tests/e2e/canvas/instance-sizing.spec.ts-snapshots/instance-fill-sizing-openpencil-darwin.png differ diff --git a/tests/engine/render/jsx/instance-sizing.test.ts b/tests/engine/render/jsx/instance-sizing.test.ts new file mode 100644 index 000000000..336aad266 --- /dev/null +++ b/tests/engine/render/jsx/instance-sizing.test.ts @@ -0,0 +1,62 @@ +import { expect, test } from 'bun:test' + +import { Component, Frame, Instance, Rectangle, renderTree } from '@open-pencil/core/design-jsx' +import { computeAllLayouts } from '@open-pencil/core/layout' + +import { getNodeOrThrow } from '#tests/helpers/assert' +import { makeSceneGraph } from '#tests/helpers/scene' + +test('overriding one instance dimension preserves the other inherited dimension', async () => { + const graph = makeSceneGraph() + const component = await renderTree(graph, Component({ flex: 'col', w: 280, h: 100 })) + const result = await renderTree(graph, Instance({ of: component.id, w: 120 })) + graph.syncInstances(component.id) + computeAllLayouts(graph) + const instance = getNodeOrThrow(graph, result.id) + expect(instance.width).toBe(120) + expect(instance.height).toBe(100) + expect(instance.counterAxisSizing).toBe('FIXED') + expect(instance.primaryAxisSizing).toBe('FIXED') +}) + +for (const flex of ['row', 'col'] as const) { + test(`instances fill their parent across inherited fixed ${flex} dimensions`, async () => { + const graph = makeSceneGraph() + const component = await renderTree( + graph, + Component({ flex, w: 280, h: 100, children: Rectangle({ w: 20, h: 20 }) }) + ) + const parent = await renderTree( + graph, + Frame({ + flex: 'col', + w: 220, + h: 180, + children: Instance({ of: component.id, w: 'fill', h: 'fill' }) + }) + ) + const instance = graph.getChildren(parent.id)[0] + expect(instance.width).toBe(220) + expect(instance.height).toBe(180) + graph.syncInstances(component.id) + computeAllLayouts(graph) + expect(instance.width).toBe(220) + expect(instance.height).toBe(180) + }) + + test(`explicit instance dimensions override inherited Hug ${flex} sizing`, async () => { + const graph = makeSceneGraph() + const component = await renderTree( + graph, + Component({ flex, w: 'hug', h: 'hug', children: Rectangle({ w: 20, h: 20 }) }) + ) + const result = await renderTree(graph, Instance({ of: component.id, w: 120, h: 80 })) + const instance = getNodeOrThrow(graph, result.id) + expect(instance.width).toBe(120) + expect(instance.height).toBe(80) + graph.syncInstances(component.id) + computeAllLayouts(graph) + expect(instance.width).toBe(120) + expect(instance.height).toBe(80) + }) +} diff --git a/tests/helpers/canvas/instance-sizing.ts b/tests/helpers/canvas/instance-sizing.ts new file mode 100644 index 000000000..60801e93c --- /dev/null +++ b/tests/helpers/canvas/instance-sizing.ts @@ -0,0 +1,65 @@ +import { Component, Frame, Instance, Text, renderTree } from '@open-pencil/core/design-jsx' +import type { SceneGraph } from '@open-pencil/scene-graph' + +export async function createInstanceSizingScene(graph: SceneGraph, parentId: string) { + const library = graph.addPage('Sizing component') + const component = await renderTree( + graph, + Component({ + name: 'Note', + flex: 'col', + w: 280, + h: 'hug', + p: 16, + bg: '#FFFFFF', + rounded: 10, + properties: [{ id: 'message', name: 'Message', type: 'TEXT', defaultValue: 'A short note.' }], + children: Text({ + name: 'Message', + w: 'fill', + font: 'Inter', + size: 14, + lineHeight: 20, + color: '#252A31', + propertyRefs: [{ propertyId: 'message', field: 'TEXT' }], + children: 'A short note.' + }) + }), + { parentId: library.id } + ) + const board = await renderTree( + graph, + Frame({ + name: 'Instance sizing', + flex: 'row', + w: 'hug', + h: 'hug', + gap: 32, + p: 32, + bg: '#E9E7E2', + children: [280, 220].map((width) => + Frame({ + name: `Container ${width}`, + flex: 'col', + w: width, + h: 'hug', + gap: 12, + children: [ + Text({ children: `${width}px container`, font: 'Inter', size: 12, color: '#626975' }), + Instance({ + of: component.id, + w: 'fill', + properties: { + message: + 'The blue feels right. Give the date a little more room at the bottom, and keep the quieter version for the print edition.' + } + }) + ] + }) + ) + }), + { parentId } + ) + graph.syncInstances(component.id) + return { boardId: board.id, componentId: component.id } +}