From c01e5ec1417405be79faff90eb02bdc44be0d9d2 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Tue, 17 Mar 2026 16:58:25 +0300 Subject: [PATCH] Fix review round 2 remaining: duplicate children, JSON.stringify perf - duplicateSelected: recursive subtree cloning via graph.cloneTree() Filters to top-level selection to avoid double-cloning. Children get new IDs wired to new parent. Single undo entry. - PropertyListRoot isMixed: compare array lengths first (fast path), only JSON.stringify when lengths match (rare). - glContext in use-canvas.ts: ownership verified correct, no change. --- packages/core/src/editor/clipboard.ts | 32 +++++++++++-------- .../vue/src/PropertyList/PropertyListRoot.vue | 7 +++- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/core/src/editor/clipboard.ts b/packages/core/src/editor/clipboard.ts index 9763ba07e..ba7cc27a0 100644 --- a/packages/core/src/editor/clipboard.ts +++ b/packages/core/src/editor/clipboard.ts @@ -51,34 +51,38 @@ export function createClipboardActions(ctx: EditorContext) { function duplicateSelected(selectedNodes: SceneNode[]) { const prevSelection = new Set(ctx.state.selectedIds) - const newIds: string[] = [] - const snapshots: Array<{ id: string; parentId: string; snapshot: SceneNode }> = [] + const selectedSet = new Set(selectedNodes.map((n) => n.id)) + const topLevel = selectedNodes.filter((n) => !n.parentId || !selectedSet.has(n.parentId)) - for (const node of selectedNodes) { + const newRootIds: string[] = [] + + for (const node of topLevel) { const parentId = node.parentId ?? ctx.state.currentPageId - const { id: _srcId, parentId: _srcParent, childIds: _srcChildren, ...srcRest } = node - const created = ctx.graph.createNode(node.type, parentId, { - ...srcRest, + const clone = ctx.graph.cloneTree(node.id, parentId, { name: node.name + ' copy', x: node.x + 20, y: node.y + 20 }) - newIds.push(created.id) - snapshots.push({ id: created.id, parentId, snapshot: { ...created } }) + if (clone) newRootIds.push(clone.id) } - if (newIds.length > 0) { - ctx.state.selectedIds = new Set(newIds) + if (newRootIds.length > 0) { + const allCloned = collectSubtrees(ctx.graph, newRootIds) + const pageId = ctx.state.currentPageId + ctx.state.selectedIds = new Set(newRootIds) ctx.undo.push({ label: 'Duplicate', forward: () => { - for (const { snapshot, parentId } of snapshots) { - ctx.graph.createNode(snapshot.type, parentId, snapshot) + for (const snapshot of allCloned) { + ctx.graph.createNode(snapshot.type, snapshot.parentId ?? pageId, { + ...snapshot, + childIds: [] + }) } - ctx.state.selectedIds = new Set(newIds) + ctx.state.selectedIds = new Set(newRootIds) }, inverse: () => { - for (const { id } of snapshots) ctx.graph.deleteNode(id) + for (const id of newRootIds) ctx.graph.deleteNode(id) ctx.state.selectedIds = prevSelection } }) diff --git a/packages/vue/src/PropertyList/PropertyListRoot.vue b/packages/vue/src/PropertyList/PropertyListRoot.vue index 62c754fc6..9807567da 100644 --- a/packages/vue/src/PropertyList/PropertyListRoot.vue +++ b/packages/vue/src/PropertyList/PropertyListRoot.vue @@ -24,7 +24,12 @@ const active = computed(() => selectedNodes.value.length > 0) const isMixed = computed(() => { const all = selectedNodes.value if (all.length <= 1) return false - const first = JSON.stringify(all[0][propKey]) + const firstArr = all[0][propKey] as unknown[] + for (let i = 1; i < all.length; i++) { + const arr = all[i][propKey] as unknown[] + if (arr.length !== firstArr.length) return true + } + const first = JSON.stringify(firstArr) return all.some((n) => JSON.stringify(n[propKey]) !== first) })