From 4bf161b983293784cb8c5dae8045abf397bf7ab9 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Tue, 12 May 2026 18:43:11 +0300 Subject: [PATCH] fix(components): sync instances when switching variants - Swap instance contents when changing variants instead of only updating componentId - Recreate instance redo state without stale child IDs - Insert assets into entered containers using local coordinates - Fall back to Windows paths when URL parsing fails --- desktop/src/lib.rs | 4 +- .../core/src/editor/components/instances.ts | 10 +- .../core/src/editor/components/variants.ts | 6 +- packages/core/src/scene-graph/index.ts | 4 + packages/core/src/scene-graph/instances.ts | 23 ++++ src/components/AssetsPanel.vue | 20 ++-- tests/e2e/components/assets-panel.spec.ts | 106 +++++++++++++++--- 7 files changed, 145 insertions(+), 28 deletions(-) diff --git a/desktop/src/lib.rs b/desktop/src/lib.rs index 31f4fc4f0..f832f2f96 100644 --- a/desktop/src/lib.rs +++ b/desktop/src/lib.rs @@ -47,7 +47,9 @@ fn path_from_arg(arg: String, cwd: &Path) -> Option { } if let Ok(url) = tauri::Url::parse(&arg) { - return url.to_file_path().ok(); + if let Ok(path) = url.to_file_path() { + return Some(path); + } } let path = PathBuf::from(arg); diff --git a/packages/core/src/editor/components/instances.ts b/packages/core/src/editor/components/instances.ts index 4cc60b6a2..23f699c03 100644 --- a/packages/core/src/editor/components/instances.ts +++ b/packages/core/src/editor/components/instances.ts @@ -1,6 +1,13 @@ import type { EditorContext } from '#core/editor/types' import type { SceneNode } from '#core/scene-graph' +type InstanceCreateSnapshot = Partial & { id: string } + +function createInstanceSnapshot(instance: SceneNode): InstanceCreateSnapshot { + const { childIds: _childIds, parentId: _parentId, type: _type, ...snapshot } = instance + return snapshot +} + export function createComponentInstanceActions(ctx: EditorContext) { function createInstanceFromComponent( componentId: string, @@ -18,12 +25,13 @@ export function createComponentInstanceActions(ctx: EditorContext) { if (!instance) return null const instanceId = instance.id + const snapshot = createInstanceSnapshot(instance) ctx.setSelectedIds(new Set([instanceId])) ctx.undo.push({ label: 'Create instance', forward: () => { - ctx.graph.createInstance(componentId, parentId, { ...instance }) + ctx.graph.createInstance(componentId, parentId, { ...snapshot }) ctx.setSelectedIds(new Set([instanceId])) }, inverse: () => { diff --git a/packages/core/src/editor/components/variants.ts b/packages/core/src/editor/components/variants.ts index 39da7f208..1a66f7af4 100644 --- a/packages/core/src/editor/components/variants.ts +++ b/packages/core/src/editor/components/variants.ts @@ -266,15 +266,15 @@ export function createVariantActions(ctx: EditorContext) { if (!target || target.id === instance.componentId) return const prevComponentId = instance.componentId - ctx.graph.updateNode(instanceId, { componentId: target.id }) + ctx.graph.swapInstanceComponent(instanceId, target.id) ctx.undo.push({ label: 'Switch variant', forward: () => { - ctx.graph.updateNode(instanceId, { componentId: target.id }) + ctx.graph.swapInstanceComponent(instanceId, target.id) ctx.requestRender() }, inverse: () => { - ctx.graph.updateNode(instanceId, { componentId: prevComponentId }) + ctx.graph.swapInstanceComponent(instanceId, prevComponentId) ctx.requestRender() } }) diff --git a/packages/core/src/scene-graph/index.ts b/packages/core/src/scene-graph/index.ts index 25af14d37..d9df54aac 100644 --- a/packages/core/src/scene-graph/index.ts +++ b/packages/core/src/scene-graph/index.ts @@ -538,6 +538,10 @@ export class SceneGraph { Instances.populateInstanceChildren(this, instanceId, componentId) } + swapInstanceComponent(instanceId: string, componentId: string): void { + Instances.swapInstanceComponent(this, instanceId, componentId) + } + syncInstances(componentId: string): void { Instances.syncInstances(this, componentId) } diff --git a/packages/core/src/scene-graph/instances.ts b/packages/core/src/scene-graph/instances.ts index c78feef5e..afe283732 100644 --- a/packages/core/src/scene-graph/instances.ts +++ b/packages/core/src/scene-graph/instances.ts @@ -186,6 +186,29 @@ export function populateInstanceChildren( cloneChildrenWithMapping(graph, componentId, instanceId) } +export function swapInstanceComponent( + graph: SceneGraph, + instanceId: string, + componentId: string +): void { + const instance = graph.nodes.get(instanceId) + const component = graph.nodes.get(componentId) + if (!instance || component?.type !== 'COMPONENT' || instance.type !== 'INSTANCE') return + + const previousComponent = instance.componentId ? graph.nodes.get(instance.componentId) : undefined + const updates: Partial = { componentId } + for (const key of INSTANCE_SYNC_PROPS) { + if (key in instance.overrides) continue + copyProp(updates, component, key) + } + if (!previousComponent || instance.name === previousComponent.name) updates.name = component.name + + const childIds = Array.from(instance.childIds) + for (const childId of childIds) graph.deleteNode(childId) + graph.updateNode(instanceId, updates) + cloneChildrenWithMapping(graph, componentId, instanceId) +} + export function syncInstances(graph: SceneGraph, componentId: string): void { const component = graph.nodes.get(componentId) if (component?.type !== 'COMPONENT') return diff --git a/src/components/AssetsPanel.vue b/src/components/AssetsPanel.vue index c38e2eec1..8bb14780d 100644 --- a/src/components/AssetsPanel.vue +++ b/src/components/AssetsPanel.vue @@ -71,16 +71,20 @@ const filteredAssets = computed(() => { return assets.value.filter((asset) => asset.name.toLowerCase().includes(normalized)) }) -function insertionPoint(component: SceneNode) { +function insertionPoint(component: SceneNode, parentId: string) { const canvas = document.querySelector('[data-test-id="canvas-area"]') const rect = canvas?.getBoundingClientRect() const center = editor.screenToCanvas( (rect?.width ?? window.innerWidth) / 2, (rect?.height ?? window.innerHeight) / 2 ) + const parentOffset = + parentId === editor.state.currentPageId + ? { x: 0, y: 0 } + : editor.graph.getAbsolutePosition(parentId) return { - x: center.x - component.width / 2, - y: center.y - component.height / 2 + x: center.x - parentOffset.x - component.width / 2, + y: center.y - parentOffset.y - component.height / 2 } } @@ -88,13 +92,9 @@ function insertAsset(asset: LocalAsset) { if (!asset.componentId) return const component = editor.graph.getNode(asset.componentId) if (!component) return - const point = insertionPoint(component) - editor.createInstanceFromComponent( - asset.componentId, - point.x, - point.y, - editor.state.enteredContainerId ?? editor.state.currentPageId - ) + const parentId = editor.state.enteredContainerId ?? editor.state.currentPageId + const point = insertionPoint(component, parentId) + editor.createInstanceFromComponent(asset.componentId, point.x, point.y, parentId) editor.requestRender() } diff --git a/tests/e2e/components/assets-panel.spec.ts b/tests/e2e/components/assets-panel.spec.ts index 62cfc0952..f5f419a2b 100644 --- a/tests/e2e/components/assets-panel.spec.ts +++ b/tests/e2e/components/assets-panel.spec.ts @@ -15,7 +15,12 @@ async function selectedNodeSnapshot(page: Page) { type: selected.type, parentId: selected.parentId, componentId: selected.componentId, - pageId: store.state.currentPageId + pageId: store.state.currentPageId, + width: selected.width, + childTexts: store.graph + .getChildren(selected.id) + .filter((child) => child.type === 'TEXT') + .map((child) => child.text) } : null }) @@ -57,19 +62,31 @@ test('assets panel groups component sets and inserts the default variant', async height: 40, componentPropertyValues: { Type: 'Primary' } }) + store.graph.createNode('TEXT', primary.id, { + name: 'Label', + text: 'Primary', + width: 72, + height: 20 + }) const secondary = store.graph.createNode('COMPONENT', set.id, { name: 'Type=Secondary', x: 120, y: 0, - width: 96, + width: 132, height: 40, componentPropertyValues: { Type: 'Secondary' } }) + store.graph.createNode('TEXT', secondary.id, { + name: 'Label', + text: 'Secondary', + width: 96, + height: 20 + }) const duplicateSecondary = store.graph.createNode('COMPONENT', set.id, { name: 'Type=Secondary duplicate', - x: 240, + x: 280, y: 0, - width: 96, + width: 132, height: 40, componentPropertyValues: { Type: 'Secondary' } }) @@ -131,25 +148,88 @@ test('assets panel groups component sets and inserts the default variant', async expect(inserted?.type).toBe('INSTANCE') expect(inserted?.componentId).toBe(ids.secondaryId) expect(inserted?.parentId).toBe(inserted?.pageId) + expect(inserted?.width).toBe(132) + expect(inserted?.childTexts).toEqual(['Secondary']) await expect(page.locator('[data-test-id="variant-section"]')).toBeVisible() await page.locator('[data-test-id="variant-section"] [data-test-id="app-select-trigger"]').click() await page.getByRole('option', { name: 'Primary' }).click() - const switchedComponentId = await page.evaluate( - (instanceId) => { - const store = window.openPencil?.getStore?.() - if (!store) throw new Error('OpenPencil store not initialized') - return store.graph.getNode(instanceId)?.componentId - }, - expectDefined(inserted?.id, 'inserted instance id') - ) - expect(switchedComponentId).toBe(ids.primaryId) + expectDefined(inserted?.id, 'inserted instance id') + const switched = await selectedNodeSnapshot(page) + expect(switched?.componentId).toBe(ids.primaryId) + expect(switched?.width).toBe(96) + expect(switched?.childTexts).toEqual(['Primary']) canvas.assertNoErrors() }) +test('assets insertion accounts for entered container coordinates', async ({ page }) => { + const canvas = new CanvasHelper(page) + await page.goto('/?test') + await canvas.waitForInit() + + const setup = await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + store.state.panX = 0 + store.state.panY = 0 + store.state.zoom = 1 + const pageNode = store.graph.getNode(store.state.currentPageId) + if (!pageNode) throw new Error('Current page not found') + const frame = store.graph.createNode('FRAME', pageNode.id, { + name: 'Target Frame', + x: 200, + y: 150, + width: 300, + height: 240 + }) + const component = store.graph.createNode('COMPONENT', pageNode.id, { + name: 'Panel Card', + x: 40, + y: 40, + width: 120, + height: 60 + }) + store.state.enteredContainerId = frame.id + store.requestRender() + return { frameId: frame.id, componentId: component.id } + }) + await canvas.waitForRender() + + await page.locator('[data-test-id="left-panel-assets-tab"]').click() + await page.locator(`[data-asset-id="${setup.componentId}"] [data-test-id="asset-insert"]`).click() + await canvas.waitForRender() + + const inserted = await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const selectedId = [...store.state.selectedIds][0] + const selected = selectedId ? store.graph.getNode(selectedId) : null + if (!selected) return null + const abs = store.graph.getAbsolutePosition(selected.id) + const canvasEl = document.querySelector('[data-test-id="canvas-area"]') + const rect = canvasEl?.getBoundingClientRect() + const center = store.screenToCanvas( + (rect?.width ?? window.innerWidth) / 2, + (rect?.height ?? window.innerHeight) / 2 + ) + return { + parentId: selected.parentId, + centerX: abs.x + selected.width / 2, + centerY: abs.y + selected.height / 2, + expectedCenterX: center.x, + expectedCenterY: center.y + } + }) + + expect(inserted?.parentId).toBe(setup.frameId) + expect(inserted?.centerX).toBeCloseTo(inserted?.expectedCenterX ?? 0, 1) + expect(inserted?.centerY).toBeCloseTo(inserted?.expectedCenterY ?? 0, 1) + canvas.assertNoErrors() +}) + test('demo exposes component set assets', async ({ page }) => { const canvas = new CanvasHelper(page) await page.goto('/demo')