From 3ea63ad09f039a7a3a5598ad36bdbf5ddfbcad69 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Mon, 14 Sep 2026 00:25:36 +0300 Subject: [PATCH] fix: refresh Undo and Redo command availability Publish committed history changes independently of scene mutations so menus observe history recorded after the final draw. Align the assets regression with the documented top-left default. --- CHANGELOG.md | 1 + packages/core/src/editor/create.ts | 2 +- packages/core/src/editor/types.ts | 1 + packages/scene-graph/src/undo.ts | 8 +++ .../src/editor/selection-capabilities/use.ts | 9 ++-- tests/e2e/app/menu.spec.ts | 8 ++- tests/e2e/components/assets-panel.spec.ts | 15 +++--- .../engine/editor/undo/history-events.test.ts | 54 +++++++++++++++++++ 8 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 tests/engine/editor/undo/history-events.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c0f763a0..7d8187847 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ ### Fixed +- Keep Undo and Redo commands available as edit history changes, without requiring another scene edit. - Avoid recursive desktop HTTP proxy requests when font downloads intercept Tauri IPC traffic. - Keep FIT image fills proportional, centered, and fully visible without stretching or cropped edges. - Preserve edited instance text, including cleared labels, when saving and reopening `.fig` files. diff --git a/packages/core/src/editor/create.ts b/packages/core/src/editor/create.ts index 909562ed7..e86b0c82c 100644 --- a/packages/core/src/editor/create.ts +++ b/packages/core/src/editor/create.ts @@ -54,7 +54,7 @@ export { createDefaultEditorState } from './state' export function createEditor(options?: EditorOptions) { let _graph = options?.graph ?? new SceneGraph() const skipInitialGraphSetup = options?.skipInitialGraphSetup ?? false - const undo = new UndoManager() + const undo = new UndoManager({ onChange: () => emitEditorEvent('history:changed') }) const _loadFont = options?.loadFont ?? fontManager.loadFont.bind(fontManager) const _getViewportSize = options?.getViewportSize ?? diff --git a/packages/core/src/editor/types.ts b/packages/core/src/editor/types.ts index 60417b9d0..5ca931480 100644 --- a/packages/core/src/editor/types.ts +++ b/packages/core/src/editor/types.ts @@ -122,6 +122,7 @@ export interface EditorEvents extends SceneGraphEvents { 'render:requested': (versions: { renderVersion: number; sceneVersion: number }) => void 'repaint:requested': (versions: { renderVersion: number; sceneVersion: number }) => void 'graph:replaced': (graph: SceneGraph) => void + 'history:changed': () => void 'selection:changed': (selectedIds: string[], previousIds: string[]) => void 'tool:changed': (tool: Tool, previousTool: Tool) => void 'page:changed': (pageId: string, previousPageId: string) => void diff --git a/packages/scene-graph/src/undo.ts b/packages/scene-graph/src/undo.ts index 268aa556f..7b930d2e2 100644 --- a/packages/scene-graph/src/undo.ts +++ b/packages/scene-graph/src/undo.ts @@ -7,6 +7,8 @@ export interface UndoEntry { export interface UndoManagerOptions { limit?: number + /** Called after recording, undoing, redoing, or clearing committed history. */ + onChange?: () => void } interface UndoBatch { @@ -22,9 +24,11 @@ export class UndoManager { private redoStack: UndoEntry[] = [] private batches: UndoBatch[] = [] private readonly limit: number + private readonly onChange: (() => void) | undefined constructor(options: UndoManagerOptions = {}) { this.limit = options.limit ?? DEFAULT_HISTORY_LIMIT + this.onChange = options.onChange } apply(entry: UndoEntry): void { @@ -54,6 +58,7 @@ export class UndoManager { if (!entry) return null entry.inverse() this.redoStack.push(entry) + this.onChange?.() return entry.label } @@ -62,6 +67,7 @@ export class UndoManager { if (!entry) return null entry.forward() this.undoStack.push(entry) + this.onChange?.() return entry.label } @@ -101,6 +107,7 @@ export class UndoManager { this.undoStack = [] this.redoStack = [] this.batches = [] + this.onChange?.() } get isBatching(): boolean { @@ -148,6 +155,7 @@ export class UndoManager { } this.redoStack = [] this.trimUndoStack() + this.onChange?.() } private trimUndoStack(): void { diff --git a/packages/vue/src/editor/selection-capabilities/use.ts b/packages/vue/src/editor/selection-capabilities/use.ts index 3b28f51dc..e8df44799 100644 --- a/packages/vue/src/editor/selection-capabilities/use.ts +++ b/packages/vue/src/editor/selection-capabilities/use.ts @@ -1,7 +1,8 @@ -import { computed } from 'vue' +import { computed, shallowRef, triggerRef } from 'vue' import { canMakeBooleanSourceNode, hasVisibleStrokeSourceNode } from '@open-pencil/core/canvas' +import { useEditorEvent } from '#vue/editor/events/use' import { useSelectionState } from '#vue/editor/selection-state/use' import { useSceneComputed } from '#vue/internal/scene-computed/use' @@ -15,6 +16,8 @@ import { useSceneComputed } from '#vue/internal/scene-computed/use' export function useSelectionCapabilities() { const selection = useSelectionState() const { editor, selectedIds, selectedNode, selectedCount, hasSelection } = selection + const history = shallowRef(editor.undo) + useEditorEvent('history:changed', () => triggerRef(history)) const selectedNodesCanFlatten = useSceneComputed(() => { const nodes = editor.getSelectedNodes() @@ -73,8 +76,8 @@ export function useSelectionCapabilities() { ), // In vector edit mode, undo/redo route to the session-local history — // keep the commands enabled so the shortcut reaches them. - canUndo: useSceneComputed(() => editor.state.nodeEditState != null || editor.undo.canUndo), - canRedo: useSceneComputed(() => editor.state.nodeEditState != null || editor.undo.canRedo), + canUndo: useSceneComputed(() => editor.state.nodeEditState != null || history.value.canUndo), + canRedo: useSceneComputed(() => editor.state.nodeEditState != null || history.value.canRedo), canZoomToSelection: computed(() => hasSelection.value) } } diff --git a/tests/e2e/app/menu.spec.ts b/tests/e2e/app/menu.spec.ts index 74b12d3e2..470c92115 100644 --- a/tests/e2e/app/menu.spec.ts +++ b/tests/e2e/app/menu.spec.ts @@ -112,11 +112,17 @@ test('Undo via Edit menu works', async () => { expect(beforeUndo).toBe(1) await editor.page.locator('[role="menubar"] [role="menuitem"]', { hasText: 'Edit' }).click() - await editor.page.locator('[role="menu"] [role="menuitem"]', { hasText: 'Undo' }).click() + const undoItem = editor.page.getByRole('menuitem', { name: /^Undo\b/ }) + await expect(undoItem).toBeEnabled() + await undoItem.click() await editor.canvas.waitForRender() const afterUndo = await getStoreStateNumber('selectedIds') expect(afterUndo).toBe(0) + + await editor.page.getByRole('menuitem', { name: 'Edit', exact: true }).click() + await expect(editor.page.getByRole('menuitem', { name: /^Redo\b/ })).toBeEnabled() + await editor.page.keyboard.press('Escape') }) test('Duplicate via Edit menu works', async () => { diff --git a/tests/e2e/components/assets-panel.spec.ts b/tests/e2e/components/assets-panel.spec.ts index 06be45367..a2f7a1552 100644 --- a/tests/e2e/components/assets-panel.spec.ts +++ b/tests/e2e/components/assets-panel.spec.ts @@ -169,22 +169,23 @@ test('assets panel groups component sets and inserts the default variant', async const inserted = await selectedNodeSnapshot(page) expect(inserted?.type).toBe('INSTANCE') - expect(inserted?.componentId).toBe(ids.secondaryId) + // The spatially first variant is the documented default, regardless of property defaults. + expect(inserted?.componentId).toBe(ids.primaryId) expect(inserted?.parentId).toBe(inserted?.pageId) - expect(inserted?.width).toBe(132) - expect(inserted?.childTexts).toEqual(['Secondary']) + expect(inserted?.width).toBe(96) + expect(inserted?.childTexts).toEqual(['Primary']) const variantSection = page.getByRole('region', { name: 'Variants' }) await expect(variantSection).toBeVisible() await variantSection.getByRole('combobox', { name: 'Type' }).click() - await page.getByRole('option', { name: 'Primary' }).click() + await page.getByRole('option', { name: 'Secondary' }).click() 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']) + expect(switched?.componentId).toBe(ids.secondaryId) + expect(switched?.width).toBe(132) + expect(switched?.childTexts).toEqual(['Secondary']) canvas.assertNoErrors() }) diff --git a/tests/engine/editor/undo/history-events.test.ts b/tests/engine/editor/undo/history-events.test.ts new file mode 100644 index 000000000..e8892a1f9 --- /dev/null +++ b/tests/engine/editor/undo/history-events.test.ts @@ -0,0 +1,54 @@ +import { expect, test } from 'bun:test' + +import { createEditor } from '@open-pencil/core/editor' + +const entry = { label: 'Edit', forward: () => undefined, inverse: () => undefined } + +test('publishes history changes independently of scene mutations', () => { + const editor = createEditor() + const observed: Array<[boolean, boolean]> = [] + const off = editor.onEditorEvent('history:changed', () => { + observed.push([editor.undo.canUndo, editor.undo.canRedo]) + }) + const sceneVersion = editor.state.sceneVersion + try { + editor.undo.record(entry) + editor.undo.undo() + editor.undo.redo() + editor.undo.clear() + expect(observed).toEqual([ + [true, false], + [false, true], + [true, false], + [false, false] + ]) + expect(editor.state.sceneVersion).toBe(sceneVersion) + } finally { + off() + editor.dispose() + } +}) + +test('publishes committed batches and coalescing but not pending or rolled-back batches', () => { + const editor = createEditor() + const labels: Array = [] + const off = editor.onEditorEvent('history:changed', () => labels.push(editor.undo.undoLabel)) + try { + editor.undo.beginBatch('Cancelled') + editor.undo.record(entry) + editor.undo.rollbackBatch() + expect(labels).toEqual([]) + editor.undo.beginBatch('Batch', 'same-edit') + editor.undo.record(entry) + editor.undo.beginBatch('Nested') + editor.undo.record(entry) + editor.undo.commitBatch() + expect(labels).toEqual([]) + editor.undo.commitBatch() + editor.undo.record({ ...entry, label: 'Coalesced', coalesceKey: 'same-edit' }) + expect(labels).toEqual(['Batch', 'Coalesced']) + } finally { + off() + editor.dispose() + } +})