From 8ba46fef49fcd456b1e9bcd4e4f90d22f1419220 Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 30 Apr 2024 12:01:39 +0100 Subject: [PATCH] fix undo/redo issues (#3658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix some issues with the new undo/redo system - there were a few things that were undoable that shouldn't be, and a few things that weren't but should ### Change Type - [x] `sdk` — Changes the tldraw SDK - [x] `bugfix` — Bug fix --- apps/dotcom/src/hooks/useUrlState.ts | 4 ++- packages/editor/src/lib/editor/Editor.ts | 21 +++++++++------ .../src/test/commands/centerOnPoint.test.ts | 7 +++++ packages/tldraw/src/test/commands/pan.test.ts | 7 +++++ .../src/test/commands/resetZoom.test.ts | 14 ++++++++++ .../src/test/commands/setSelectedIds.test.ts | 26 +++++++++++++++++++ .../tldraw/src/test/commands/zoomIn.test.ts | 7 +++++ .../tldraw/src/test/commands/zoomOut.test.ts | 7 +++++ .../src/test/commands/zoomToBounds.test.ts | 7 +++++ .../src/test/commands/zoomToFit.test.ts | 8 ++++++ .../src/test/commands/zoomToSelection.test.ts | 9 +++++++ 11 files changed, 108 insertions(+), 9 deletions(-) diff --git a/apps/dotcom/src/hooks/useUrlState.ts b/apps/dotcom/src/hooks/useUrlState.ts index 451408566..ed27fa12d 100644 --- a/apps/dotcom/src/hooks/useUrlState.ts +++ b/apps/dotcom/src/hooks/useUrlState.ts @@ -56,7 +56,9 @@ export function useUrlState(onChangeUrl: (params: UrlStateParams) => void) { url.searchParams.get(PARAMS.page) ?? 'page:' + url.searchParams.get(PARAMS.p) if (newPageId) { if (editor.store.has(newPageId as TLPageId)) { - editor.setCurrentPage(newPageId as TLPageId) + editor.history.ignore(() => { + editor.setCurrentPage(newPageId as TLPageId) + }) } } } diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index bb0a53808..d205a368d 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -1450,15 +1450,18 @@ export class Editor extends EventEmitter { * @public */ setSelectedShapes(shapes: TLShapeId[] | TLShape[]): this { - return this.batch(() => { - const ids = shapes.map((shape) => (typeof shape === 'string' ? shape : shape.id)) - const { selectedShapeIds: prevSelectedShapeIds } = this.getCurrentPageState() - const prevSet = new Set(prevSelectedShapeIds) + return this.batch( + () => { + const ids = shapes.map((shape) => (typeof shape === 'string' ? shape : shape.id)) + const { selectedShapeIds: prevSelectedShapeIds } = this.getCurrentPageState() + const prevSet = new Set(prevSelectedShapeIds) - if (ids.length === prevSet.size && ids.every((id) => prevSet.has(id))) return null + if (ids.length === prevSet.size && ids.every((id) => prevSet.has(id))) return null - this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds: ids }]) - }) + this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds: ids }]) + }, + { history: 'record-preserveRedoStack' } + ) } /** @@ -2030,7 +2033,9 @@ export class Editor extends EventEmitter { this.batch(() => { const camera = { ...currentCamera, ...point } - this.store.put([camera]) // include id and meta here + this.history.ignore(() => { + this.store.put([camera]) // include id and meta here + }) // Dispatch a new pointer move because the pointer's page will have changed // (its screen position will compute to a new page position given the new camera position) diff --git a/packages/tldraw/src/test/commands/centerOnPoint.test.ts b/packages/tldraw/src/test/commands/centerOnPoint.test.ts index b3720e28f..76a5e82a0 100644 --- a/packages/tldraw/src/test/commands/centerOnPoint.test.ts +++ b/packages/tldraw/src/test/commands/centerOnPoint.test.ts @@ -19,3 +19,10 @@ it('centers on the point with animation', () => { jest.advanceTimersByTime(200) expect(editor.getViewportPageCenter()).toMatchObject({ x: 400, y: 400 }) }) + +it('is not undoable', () => { + editor.mark() + editor.centerOnPoint({ x: 400, y: 400 }) + editor.undo() + expect(editor.getViewportPageCenter()).toMatchObject({ x: 400, y: 400 }) +}) diff --git a/packages/tldraw/src/test/commands/pan.test.ts b/packages/tldraw/src/test/commands/pan.test.ts index f3bad0be1..487001b6f 100644 --- a/packages/tldraw/src/test/commands/pan.test.ts +++ b/packages/tldraw/src/test/commands/pan.test.ts @@ -14,6 +14,13 @@ describe('When panning', () => { editor.expectCameraToBe(200, 200, 1) }) + it('Is not undoable', () => { + editor.mark() + editor.pan({ x: 200, y: 200 }) + editor.undo() + editor.expectCameraToBe(200, 200, 1) + }) + it('Updates the pageBounds', () => { const screenBounds = editor.getViewportScreenBounds() const beforeScreenBounds = new Box( diff --git a/packages/tldraw/src/test/commands/resetZoom.test.ts b/packages/tldraw/src/test/commands/resetZoom.test.ts index 8e21f8c8f..0c8a7d8e8 100644 --- a/packages/tldraw/src/test/commands/resetZoom.test.ts +++ b/packages/tldraw/src/test/commands/resetZoom.test.ts @@ -27,4 +27,18 @@ describe('When resetting zoom', () => { editor.resetZoom() expect(editor.getViewportScreenBounds().center.clone()).toMatchObject(center) }) + + it('is not undoable', () => { + editor.zoomOut() + editor.mark() + editor.resetZoom() + editor.undo() + expect(editor.getZoomLevel()).toBe(1) + + editor.mark() + editor.zoomIn() + editor.resetZoom() + editor.undo() + expect(editor.getZoomLevel()).toBe(1) + }) }) diff --git a/packages/tldraw/src/test/commands/setSelectedIds.test.ts b/packages/tldraw/src/test/commands/setSelectedIds.test.ts index 3fcffe74b..79bcf3592 100644 --- a/packages/tldraw/src/test/commands/setSelectedIds.test.ts +++ b/packages/tldraw/src/test/commands/setSelectedIds.test.ts @@ -55,3 +55,29 @@ it('Deleting the parent also deletes descendants', () => { expect(editor.getShape(ids.box2)).toBeUndefined() expect(editor.getShape(ids.ellipse1)).toBeUndefined() }) + +it('preserves the redo stack', () => { + editor.mark() + editor.select(ids.box1) + editor.translateSelection(10, 10) + expect(editor.getShape(ids.box1)).toMatchObject({ x: 110, y: 110 }) + + editor.mark() + editor.translateSelection(10, 10) + expect(editor.getShape(ids.box1)).toMatchObject({ x: 120, y: 120 }) + + editor.undo() + editor.undo() + expect(editor.getShape(ids.box1)).toMatchObject({ x: 100, y: 100 }) + + editor.deselect() + editor.redo() + expect(editor.getShape(ids.box1)).toMatchObject({ x: 110, y: 110 }) + + editor.select(ids.box2) + editor.redo() + expect(editor.getShape(ids.box1)).toMatchObject({ x: 120, y: 120 }) + + editor.undo() + expect(editor.getShape(ids.box1)).toMatchObject({ x: 110, y: 110 }) +}) diff --git a/packages/tldraw/src/test/commands/zoomIn.test.ts b/packages/tldraw/src/test/commands/zoomIn.test.ts index f3cca8314..d9bd335fb 100644 --- a/packages/tldraw/src/test/commands/zoomIn.test.ts +++ b/packages/tldraw/src/test/commands/zoomIn.test.ts @@ -25,6 +25,13 @@ it('zooms by increments', () => { expect(editor.getZoomLevel()).toBe(ZOOMS[6]) }) +it('is ignored by undo/redo', () => { + editor.mark() + editor.zoomIn() + editor.undo() + expect(editor.getZoomLevel()).toBe(ZOOMS[4]) +}) + it('preserves the screen center', () => { const viewportCenter = editor.getViewportPageCenter().toJson() const screenCenter = editor.getViewportScreenCenter().toJson() diff --git a/packages/tldraw/src/test/commands/zoomOut.test.ts b/packages/tldraw/src/test/commands/zoomOut.test.ts index aec509e5f..1f0ea2fff 100644 --- a/packages/tldraw/src/test/commands/zoomOut.test.ts +++ b/packages/tldraw/src/test/commands/zoomOut.test.ts @@ -22,6 +22,13 @@ it('zooms by increments', () => { expect(editor.getZoomLevel()).toBe(ZOOMS[0]) }) +it('is ignored by undo/redo', () => { + editor.mark() + editor.zoomOut() + editor.undo() + expect(editor.getZoomLevel()).toBe(ZOOMS[2]) +}) + it('does not zoom out when camera is frozen', () => { editor.setCamera({ x: 0, y: 0, z: 1 }) expect(editor.getCamera()).toMatchObject({ x: 0, y: 0, z: 1 }) diff --git a/packages/tldraw/src/test/commands/zoomToBounds.test.ts b/packages/tldraw/src/test/commands/zoomToBounds.test.ts index 42b6f1555..3f3152ed6 100644 --- a/packages/tldraw/src/test/commands/zoomToBounds.test.ts +++ b/packages/tldraw/src/test/commands/zoomToBounds.test.ts @@ -48,3 +48,10 @@ it('does not zoom to bounds when camera is frozen', () => { editor.zoomToBounds(new Box(200, 300, 300, 300)) expect(editor.getViewportPageCenter().toJson()).toCloselyMatchObject({ x: 500, y: 500 }) }) + +it('is ignored by undo/redo', () => { + editor.mark() + editor.zoomToBounds(new Box(200, 300, 300, 300)) + editor.undo() + expect(editor.getViewportPageCenter().toJson()).toCloselyMatchObject({ x: 350, y: 450 }) +}) diff --git a/packages/tldraw/src/test/commands/zoomToFit.test.ts b/packages/tldraw/src/test/commands/zoomToFit.test.ts index b5fecd252..d40a5b59f 100644 --- a/packages/tldraw/src/test/commands/zoomToFit.test.ts +++ b/packages/tldraw/src/test/commands/zoomToFit.test.ts @@ -18,3 +18,11 @@ it('does not zoom to bounds when camera is frozen', () => { editor.zoomToFit() expect(editor.getCamera()).toMatchObject(cameraBefore) }) + +it('is ignored by undo/redo', () => { + editor.mark() + editor.zoomToFit() + const camera = editor.getCamera() + editor.undo() + expect(editor.getCamera()).toBe(camera) +}) diff --git a/packages/tldraw/src/test/commands/zoomToSelection.test.ts b/packages/tldraw/src/test/commands/zoomToSelection.test.ts index b49215ecd..e880a95c9 100644 --- a/packages/tldraw/src/test/commands/zoomToSelection.test.ts +++ b/packages/tldraw/src/test/commands/zoomToSelection.test.ts @@ -40,3 +40,12 @@ it('does not zoom to selection when camera is frozen', () => { editor.zoomToSelection() expect(editor.getCamera()).toMatchObject(cameraBefore) }) + +it('is ignored by undo/redo', () => { + editor.mark() + editor.setSelectedShapes([ids.box1, ids.box2]) + editor.zoomToSelection() + const camera = editor.getCamera() + editor.undo() + expect(editor.getCamera()).toBe(camera) +})