From 80a2855bb38fb875efe4bdb46775ba851a09f4e2 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Tue, 24 Mar 2026 18:15:40 +0300 Subject: [PATCH] Fix review issues in undo and Vue SDK --- packages/core/src/editor/clipboard.ts | 51 +++++++++++--- packages/core/src/editor/undo.ts | 26 ++++--- .../vue/src/PropertyList/PropertyListRoot.vue | 14 +--- .../vue/src/commands/useEditorCommands.ts | 5 +- packages/vue/src/controls/useAppearance.ts | 31 +-------- packages/vue/src/controls/useNodeProps.ts | 67 ++++++++++++++++++- packages/vue/src/internal/useSceneComputed.ts | 17 +---- .../src/selection/useSelectionCapabilities.ts | 7 +- .../vue/src/selection/useSelectionState.ts | 2 +- packages/vue/src/shared/useCanvas.ts | 1 + src/components/CodePanel.vue | 14 ++-- .../properties/VariablesSection.vue | 16 ++--- 12 files changed, 153 insertions(+), 98 deletions(-) diff --git a/packages/core/src/editor/clipboard.ts b/packages/core/src/editor/clipboard.ts index ebde0d1b3..989fa802c 100644 --- a/packages/core/src/editor/clipboard.ts +++ b/packages/core/src/editor/clipboard.ts @@ -51,6 +51,7 @@ export function createClipboardActions(ctx: EditorContext) { const topLevel = selectedNodes.filter((n) => !n.parentId || !selectedSet.has(n.parentId)) const newRootIds: string[] = [] + const rootSnapshots: SceneNode[] = [] for (const node of topLevel) { const parentId = node.parentId ?? ctx.state.currentPageId @@ -59,26 +60,56 @@ export function createClipboardActions(ctx: EditorContext) { x: node.x + 20, y: node.y + 20 }) - if (clone) newRootIds.push(clone.id) + if (!clone) continue + newRootIds.push(clone.id) + const snapshot = ctx.graph.getNode(clone.id) + if (snapshot) rootSnapshots.push(structuredClone(snapshot)) + } + + const cloneSnapshotTree = (snapshot: SceneNode): SceneNode => { + const cloned = structuredClone(snapshot) + cloned.childIds = snapshot.childIds + .map((childId) => ctx.graph.getNode(childId)) + .filter((child): child is SceneNode => child != null) + .map((child) => cloneSnapshotTree(child)) + .map((child) => child.id) + return cloned + } + + for (let i = 0; i < rootSnapshots.length; i++) { + rootSnapshots[i] = cloneSnapshotTree(rootSnapshots[i]) + } + + const snapshotIndex = new Map() + for (const snapshot of rootSnapshots) { + snapshotIndex.set(snapshot.id, snapshot) + } + + const restoreTree = (snapshot: SceneNode, parentId: string) => { + const { id: _snapshotId, parentId: _snapshotParentId, childIds, ...rest } = snapshot + const created = ctx.graph.createNode(snapshot.type, parentId, rest) + for (const childId of childIds) { + const child = snapshotIndex.get(childId) + if (!child) continue + restoreTree(child, created.id) + } + return created.id } 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 of allCloned) { - ctx.graph.createNode(snapshot.type, snapshot.parentId ?? pageId, { - ...snapshot, - childIds: [] - }) + const restoredRootIds: string[] = [] + for (const snapshot of rootSnapshots) { + const parentId = snapshot.parentId ?? ctx.state.currentPageId + restoredRootIds.push(restoreTree(snapshot, parentId)) } - ctx.state.selectedIds = new Set(newRootIds) + ctx.state.selectedIds = new Set(restoredRootIds) }, inverse: () => { - for (const id of newRootIds) ctx.graph.deleteNode(id) + for (const id of [...ctx.state.selectedIds].reverse()) ctx.graph.deleteNode(id) ctx.state.selectedIds = prevSelection } }) diff --git a/packages/core/src/editor/undo.ts b/packages/core/src/editor/undo.ts index b88854825..269b33149 100644 --- a/packages/core/src/editor/undo.ts +++ b/packages/core/src/editor/undo.ts @@ -131,26 +131,32 @@ export function createUndoActions(ctx: EditorContext) { } function restorePageFromSnapshot(snapshot: Map) { - const page = ctx.graph.getNode(ctx.state.currentPageId) - if (!page) return + const pageId = ctx.state.currentPageId + const page = ctx.graph.getNode(pageId) + const pageSnap = snapshot.get(pageId) + if (!page || !pageSnap) return for (const childId of page.childIds.slice()) { ctx.graph.deleteNode(childId) } - const pageSnap = snapshot.get(ctx.state.currentPageId) - if (pageSnap) page.childIds = [...pageSnap.childIds] - - for (const [id, snap] of snapshot) { - if (id === ctx.state.currentPageId) continue - ctx.graph.nodes.set(id, structuredClone(snap)) + const restoreChildren = (parentId: string, childIds: string[]) => { + for (const childId of childIds) { + const snap = snapshot.get(childId) + if (!snap) continue + const { id: _snapId, parentId: _snapParentId, childIds: snapChildIds, ...rest } = snap + const restored = ctx.graph.createNode(snap.type, parentId, rest) + ctx.graph.reorderChild(restored.id, parentId, childIds.indexOf(childId)) + restoreChildren(restored.id, snapChildIds) + } } + restoreChildren(pageId, pageSnap.childIds) + ctx.graph.clearAbsPosCache() - computeAllLayouts(ctx.graph, ctx.state.currentPageId) + computeAllLayouts(ctx.graph, pageId) ctx.state.selectedIds = new Set() ctx.state.hoveredNodeId = null - ctx.graph.emitter.emit('node:reordered', ctx.state.currentPageId, ctx.graph.rootId, 0) ctx.requestRender() } diff --git a/packages/vue/src/PropertyList/PropertyListRoot.vue b/packages/vue/src/PropertyList/PropertyListRoot.vue index 828f1d379..157bd1f33 100644 --- a/packages/vue/src/PropertyList/PropertyListRoot.vue +++ b/packages/vue/src/PropertyList/PropertyListRoot.vue @@ -2,6 +2,7 @@ import { computed } from 'vue' import { useEditor } from '@open-pencil/vue/context/editorContext' +import { useNodeProps } from '@open-pencil/vue/controls/useNodeProps' import { providePropertyList } from './context' import type { Fill, Stroke, Effect, SceneNode } from '@open-pencil/core' @@ -23,6 +24,7 @@ const emit = defineEmits<{ }>() const editor = useEditor() +const { isArrayMixed } = useNodeProps() const selectedNodes = computed(() => editor.getSelectedNodes()) const activeNode = computed( @@ -31,17 +33,7 @@ const activeNode = computed( const isMulti = computed(() => selectedNodes.value.length > 1) const active = computed(() => selectedNodes.value.length > 0) -const isMixed = computed(() => { - const all = selectedNodes.value - if (all.length <= 1) return false - 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) -}) +const isMixed = computed(() => isArrayMixed(propKey)) const items = computed(() => { if (isMixed.value) return [] diff --git a/packages/vue/src/commands/useEditorCommands.ts b/packages/vue/src/commands/useEditorCommands.ts index c521aabd0..eae680c8e 100644 --- a/packages/vue/src/commands/useEditorCommands.ts +++ b/packages/vue/src/commands/useEditorCommands.ts @@ -169,7 +169,10 @@ export function useEditorCommands() { id: 'selection.moveToPage', label: 'Move to page', enabled: capabilities.canMoveToPage, - run: () => {} + run: () => { + const targetPage = otherPages.value[0] + if (targetPage) moveSelectionToPage(targetPage.id) + } }, 'view.zoom100': { id: 'view.zoom100', diff --git a/packages/vue/src/controls/useAppearance.ts b/packages/vue/src/controls/useAppearance.ts index 72a4e4b3c..384df6b14 100644 --- a/packages/vue/src/controls/useAppearance.ts +++ b/packages/vue/src/controls/useAppearance.ts @@ -1,12 +1,9 @@ import { computed } from 'vue' import { useEditor } from '@open-pencil/vue/context/editorContext' -import { MIXED } from '@open-pencil/vue/controls/useNodeProps' -import { usePropScrub } from '@open-pencil/vue/controls/usePropScrub' -import { useSceneComputed } from '@open-pencil/vue/internal/useSceneComputed' +import { MIXED, useNodeProps } from '@open-pencil/vue/controls/useNodeProps' import type { SceneNode } from '@open-pencil/core' -import type { MixedValue } from '@open-pencil/vue/controls/useNodeProps' const CORNER_RADIUS_TYPES = new Set([ 'RECTANGLE', @@ -18,21 +15,7 @@ const CORNER_RADIUS_TYPES = new Set([ export function useAppearance() { const editor = useEditor() - - const nodes = useSceneComputed(() => editor.getSelectedNodes()) - const node = useSceneComputed(() => editor.getSelectedNode() ?? null) - const active = computed(() => nodes.value.length > 0) - const isMulti = computed(() => nodes.value.length > 1) - - function merged(key: K): MixedValue { - const all = nodes.value - if (all.length === 0) return MIXED - const first = all[0][key] - for (let i = 1; i < all.length; i++) { - if (all[i][key] !== first) return MIXED - } - return first - } + const { nodes, node, active, isMulti, merged, updateProp, commitProp } = useNodeProps() const hasCornerRadius = computed(() => { if (isMulti.value) return nodes.value.every((n) => CORNER_RADIUS_TYPES.has(n.type)) @@ -60,16 +43,6 @@ export function useAppearance() { return v ? 'visible' : 'hidden' }) - const { updateProp: _updateProp, commitProp: _commitProp } = usePropScrub(editor) - - function updateProp(key: string, value: number | string) { - _updateProp(nodes.value, key, value) - } - - function commitProp(key: string, value: number | string, previous: number | string) { - _commitProp(nodes.value, key, value, previous) - } - function toggleVisibility() { if (isMulti.value) { const allVisible = nodes.value.every((n) => n.visible) diff --git a/packages/vue/src/controls/useNodeProps.ts b/packages/vue/src/controls/useNodeProps.ts index 2ca2e5770..04382ccdb 100644 --- a/packages/vue/src/controls/useNodeProps.ts +++ b/packages/vue/src/controls/useNodeProps.ts @@ -3,11 +3,13 @@ import { computed } from 'vue' import { useEditor } from '@open-pencil/vue/context/editorContext' import { useSceneComputed } from '@open-pencil/vue/internal/useSceneComputed' -import type { Fill, SceneNode, Stroke } from '@open-pencil/core' +import type { Effect, Fill, SceneNode, Stroke } from '@open-pencil/core' export const MIXED = Symbol('mixed') export type MixedValue = T | typeof MIXED +type ArrayItem = Fill | Stroke | Effect | Record + export function useNodeProps() { const store = useEditor() const node = useSceneComputed(() => store.getSelectedNode() ?? null) @@ -26,6 +28,46 @@ export function useNodeProps() { return first } + function areArrayItemsEqual(a: ArrayItem, b: ArrayItem): boolean { + if (a === b) return true + const aKeys = Object.keys(a) + const bKeys = Object.keys(b) + if (aKeys.length !== bKeys.length) return false + for (const key of aKeys) { + const aValue = a[key as keyof typeof a] + const bValue = b[key as keyof typeof b] + if (Array.isArray(aValue) && Array.isArray(bValue)) { + if (aValue.length !== bValue.length) return false + for (let i = 0; i < aValue.length; i++) { + const left = aValue[i] + const right = bValue[i] + if ( + typeof left === 'object' && + left != null && + typeof right === 'object' && + right != null + ) { + if (!areArrayItemsEqual(left as ArrayItem, right as ArrayItem)) return false + } else if (left !== right) { + return false + } + } + continue + } + if ( + typeof aValue === 'object' && + aValue != null && + typeof bValue === 'object' && + bValue != null + ) { + if (!areArrayItemsEqual(aValue as ArrayItem, bValue as ArrayItem)) return false + continue + } + if (aValue !== bValue) return false + } + return true + } + function prop(key: K) { return computed(() => merged(key)) } @@ -39,8 +81,27 @@ export function useNodeProps() { function isArrayMixed(key: keyof SceneNode): boolean { const all = nodes.value if (all.length <= 1) return false - const first = JSON.stringify(all[0][key]) - return all.some((n) => JSON.stringify(n[key]) !== first) + const first = all[0][key] + if (!Array.isArray(first)) return all.some((n) => n[key] !== first) + for (let i = 1; i < all.length; i++) { + const current = all[i][key] + if (!Array.isArray(current) || current.length !== first.length) return true + for (let j = 0; j < first.length; j++) { + const left = first[j] + const right = current[j] + if ( + typeof left === 'object' && + left != null && + typeof right === 'object' && + right != null + ) { + if (!areArrayItemsEqual(left as ArrayItem, right as ArrayItem)) return true + } else if (left !== right) { + return true + } + } + } + return false } type ArrayPropKey = 'fills' | 'strokes' | 'effects' diff --git a/packages/vue/src/internal/useSceneComputed.ts b/packages/vue/src/internal/useSceneComputed.ts index 5847868c1..977f99518 100644 --- a/packages/vue/src/internal/useSceneComputed.ts +++ b/packages/vue/src/internal/useSceneComputed.ts @@ -1,18 +1,5 @@ import { computed, type ComputedRef } from 'vue' -/** - * Creates a computed ref that re-evaluates when the scene graph changes. - * - * Tracks `state.sceneVersion` — a counter on a shallowReactive object - * that the core editor increments synchronously on every mutation via - * requestRender(). Vue tracks the read automatically, so the computed - * re-evaluates in the same tick as the change. Zero latency. - */ -export function useSceneComputed(fn: () => T, sceneVersion?: () => number): ComputedRef { - return computed(() => { - if (sceneVersion) { - void sceneVersion() - } - return fn() - }) +export function useSceneComputed(fn: () => T): ComputedRef { + return computed(fn) } diff --git a/packages/vue/src/selection/useSelectionCapabilities.ts b/packages/vue/src/selection/useSelectionCapabilities.ts index af3b3eaf2..17cdca199 100644 --- a/packages/vue/src/selection/useSelectionCapabilities.ts +++ b/packages/vue/src/selection/useSelectionCapabilities.ts @@ -1,5 +1,6 @@ import { computed } from 'vue' +import { useSceneComputed } from '@open-pencil/vue/internal/useSceneComputed' import { useSelectionState } from '@open-pencil/vue/selection/useSelectionState' export function useSelectionCapabilities() { @@ -30,9 +31,11 @@ export function useSelectionCapabilities() { const canToggleLock = computed(() => hasSelection.value) const canGoToMainComponent = computed(() => isInstance.value) const canCreateInstance = computed(() => selectedNode.value?.type === 'COMPONENT') - const canMoveToPage = computed(() => hasSelection.value && editor.graph.getPages().length > 1) + const canMoveToPage = useSceneComputed( + () => hasSelection.value && editor.graph.getPages().length > 1 + ) const canPaste = computed(() => true) - const canSelectAll = computed( + const canSelectAll = useSceneComputed( () => editor.graph.getChildren(editor.state.currentPageId).length > 0 ) const canUndo = computed(() => editor.undo.canUndo) diff --git a/packages/vue/src/selection/useSelectionState.ts b/packages/vue/src/selection/useSelectionState.ts index d91711562..be7987482 100644 --- a/packages/vue/src/selection/useSelectionState.ts +++ b/packages/vue/src/selection/useSelectionState.ts @@ -22,7 +22,7 @@ export function useSelectionState() { const isComponent = computed(() => selectedNodeType.value === 'COMPONENT') const isGroup = computed(() => selectedNodeType.value === 'GROUP') - const canCreateComponentSet = computed(() => { + const canCreateComponentSet = useSceneComputed(() => { if (selectedIds.value.size < 2) return false for (const id of selectedIds.value) { if (editor.graph.getNode(id)?.type !== 'COMPONENT') return false diff --git a/packages/vue/src/shared/useCanvas.ts b/packages/vue/src/shared/useCanvas.ts index 8930114dd..3a23cad78 100644 --- a/packages/vue/src/shared/useCanvas.ts +++ b/packages/vue/src/shared/useCanvas.ts @@ -201,6 +201,7 @@ export function useCanvas( const surface = makeGLSurface(canvas) if (!surface) { + console.warn('Falling back to full surface recreation after resize') createSurface(canvas) return } diff --git a/src/components/CodePanel.vue b/src/components/CodePanel.vue index c8994d055..fa169fc4d 100644 --- a/src/components/CodePanel.vue +++ b/src/components/CodePanel.vue @@ -20,14 +20,12 @@ function toggleFormat() { jsxFormat.value = jsxFormat.value === 'openpencil' ? 'tailwind' : 'openpencil' } -const jsxCode = useSceneComputed( - () => { - const ids = [...store.state.selectedIds] - if (ids.length === 0) return '' - return selectionToJSX(ids, store.graph, jsxFormat.value) - }, - () => store.state.sceneVersion -) +const jsxCode = useSceneComputed(() => { + void store.state.sceneVersion + const ids = [...store.state.selectedIds] + if (ids.length === 0) return '' + return selectionToJSX(ids, store.graph, jsxFormat.value) +}) const highlightedLines = computed(() => { if (!jsxCode.value) return [] diff --git a/src/components/properties/VariablesSection.vue b/src/components/properties/VariablesSection.vue index dc9d4c525..7241c9cca 100644 --- a/src/components/properties/VariablesSection.vue +++ b/src/components/properties/VariablesSection.vue @@ -10,14 +10,14 @@ import { useEditorStore } from '@/stores/editor' const emit = defineEmits<{ openDialog: [] }>() const editor = useEditorStore() -const collectionCount = useSceneComputed( - () => editor.getCollectionCount(), - () => editor.state.sceneVersion -) -const variableCount = useSceneComputed( - () => editor.getVariableCount(), - () => editor.state.sceneVersion -) +const collectionCount = useSceneComputed(() => { + void editor.state.sceneVersion + return editor.getCollectionCount() +}) +const variableCount = useSceneComputed(() => { + void editor.state.sceneVersion + return editor.getVariableCount() +}) const hasVariables = computed(() => variableCount.value > 0)