From 260706c42f0d55146084eb9db663aab5c4fdb7ac Mon Sep 17 00:00:00 2001 From: Marcel Mraz Date: Tue, 21 Nov 2023 22:27:18 +0100 Subject: [PATCH] First step towards delta based history Introducing independent change detection for appState and elements Generalizing object change, cleanup, refactoring, comments, solving typing issues Shaping increment, change, delta hierarchy Structural clone of elements Introducing store and incremental API Disabling buttons for canvas actions, smaller store and changes improvements Update history entry based on latest changes, iterate through the stack for visible changes to limit empty commands Solving concurrency issues, solving (partly) linear element issues, introducing commitToStore breaking change Fixing existing tests, updating snapshots Trying to be smarter on the appstate change detection Extending collab test, refactoring action / updateScene params, bugfixes Resetting snapshots Resetting snapshots UI / API tests for history - WIP Changing actions related to the observed appstate to at least update the store snapshot - WIP Adding skipping of snapshot update flag for most no-breaking changes compatible solution Ignoring uncomitted elements from local async actions, updating store directly in updateScene Bound element issues - WIP --- .../excalidraw/api/props/excalidraw-api.mdx | 5 +- excalidraw-app/collab/Collab.tsx | 21 +- excalidraw-app/collab/reconciliation.ts | 2 +- excalidraw-app/data/index.ts | 2 +- excalidraw-app/index.tsx | 3 +- excalidraw-app/tests/collab.test.tsx | 25 +- src/actions/actionAddToLibrary.ts | 7 +- src/actions/actionAlign.tsx | 13 +- src/actions/actionBoundText.tsx | 7 +- src/actions/actionCanvas.tsx | 39 +- src/actions/actionClipboard.tsx | 27 +- src/actions/actionDeleteSelected.tsx | 11 +- src/actions/actionDistribute.tsx | 5 +- src/actions/actionDuplicateSelection.tsx | 6 +- src/actions/actionElementLock.ts | 5 +- src/actions/actionExport.tsx | 26 +- src/actions/actionFinalize.tsx | 8 +- src/actions/actionFlip.ts | 5 +- src/actions/actionFrame.ts | 13 +- src/actions/actionGroup.tsx | 11 +- src/actions/actionHistory.tsx | 66 +- src/actions/actionLinearEditor.ts | 3 +- src/actions/actionMenu.tsx | 7 +- src/actions/actionNavigate.tsx | 5 +- src/actions/actionProperties.tsx | 31 +- src/actions/actionSelectAll.ts | 3 +- src/actions/actionStyles.ts | 7 +- src/actions/actionToggleGridMode.tsx | 3 +- src/actions/actionToggleObjectsSnapMode.tsx | 3 +- src/actions/actionToggleStats.tsx | 3 +- src/actions/actionToggleViewMode.tsx | 3 +- src/actions/actionToggleZenMode.tsx | 3 +- src/actions/actionZindex.tsx | 9 +- src/actions/types.ts | 9 +- src/change.ts | 567 +++++++++++++++ src/components/Actions.scss | 1 + src/components/App.tsx | 177 +++-- src/components/ToolButton.tsx | 9 +- src/components/ToolIcon.scss | 20 +- src/constants.ts | 1 + src/css/theme.scss | 2 + src/css/variables.module.scss | 9 + src/element/Hyperlink.tsx | 3 +- src/element/embeddable.ts | 4 +- src/element/linearElementEditor.ts | 6 +- src/element/mutateElement.ts | 31 +- src/history.ts | 334 ++++----- src/packages/excalidraw/CHANGELOG.md | 11 + src/scene/Scene.ts | 4 + src/store.ts | 307 ++++++++ src/tests/contextmenu.test.tsx | 2 +- src/tests/helpers/api.ts | 9 +- src/tests/helpers/ui.ts | 12 + src/tests/history.test.tsx | 677 +++++++++++++++++- src/tests/regressionTests.test.tsx | 18 +- src/types.ts | 29 +- 56 files changed, 2137 insertions(+), 492 deletions(-) create mode 100644 src/change.ts create mode 100644 src/store.ts diff --git a/dev-docs/docs/@excalidraw/excalidraw/api/props/excalidraw-api.mdx b/dev-docs/docs/@excalidraw/excalidraw/api/props/excalidraw-api.mdx index 22e9034fdb..12a6982d84 100644 --- a/dev-docs/docs/@excalidraw/excalidraw/api/props/excalidraw-api.mdx +++ b/dev-docs/docs/@excalidraw/excalidraw/api/props/excalidraw-api.mdx @@ -22,7 +22,7 @@ You can use this prop when you want to access some [Excalidraw APIs](https://git | API | Signature | Usage | | --- | --- | --- | | [updateScene](#updatescene) | `function` | updates the scene with the sceneData | -| [updateLibrary](#updatelibrary) | `function` | updates the scene with the sceneData | +| [updateLibrary](#updatelibrary) | `function` | updates the library | | [addFiles](#addfiles) | `function` | add files data to the appState | | [resetScene](#resetscene) | `function` | Resets the scene. If `resetLoadingState` is passed as true then it will also force set the loading state to false. | | [getSceneElementsIncludingDeleted](#getsceneelementsincludingdeleted) | `function` | Returns all the elements including the deleted in the scene | @@ -65,7 +65,8 @@ You can use this function to update the scene with the sceneData. It accepts the | `elements` | [`ImportedDataState["elements"]`](https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L38) | The `elements` to be updated in the scene | | `appState` | [`ImportedDataState["appState"]`](https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L39) | The `appState` to be updated in the scene. | | `collaborators` | MapCollaborator> | The list of collaborators to be updated in the scene. | -| `commitToHistory` | `boolean` | Implies if the `history (undo/redo)` should be recorded. Defaults to `false`. | +| `commitToStore` | `boolean` | Implies if the `store` should update it's snapshot, capture the update and calculates the diff. Captured changes are emmitted and listened to by other components, such as `History` for undo / redo purposes. Defaults to `false`. | +| `skipSnapshotUpdate` | `boolean` | Implies whether the `store` should skip update of its snapshot, which is necessary for correct diff calculation. Relevant only when `elements` or `appState` are passed in. When `true`, `commitToStore` value will be ignored. Defaults to `false`. | ```jsx live function App() { diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index 0d57a89063..0ecedaa927 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -302,7 +302,6 @@ class Collab extends PureComponent { this.excalidrawAPI.updateScene({ elements, - commitToHistory: false, }); } }; @@ -449,14 +448,12 @@ class Collab extends PureComponent { } return element; }); - // remove deleted elements from elements array & history to ensure we don't + // remove deleted elements from elements array to ensure we don't // expose potentially sensitive user data in case user manually deletes // existing elements (or clears scene), which would otherwise be persisted // to database even if deleted before creating the room. - this.excalidrawAPI.history.clear(); this.excalidrawAPI.updateScene({ elements, - commitToHistory: true, }); this.saveCollabRoomToFirebase(getSyncableElements(elements)); @@ -491,9 +488,7 @@ class Collab extends PureComponent { this.initializeRoom({ fetchScene: false }); const remoteElements = decryptedData.payload.elements; const reconciledElements = this.reconcileElements(remoteElements); - this.handleRemoteSceneUpdate(reconciledElements, { - init: true, - }); + this.handleRemoteSceneUpdate(reconciledElements); // noop if already resolved via init from firebase scenePromise.resolve({ elements: reconciledElements, @@ -649,21 +644,11 @@ class Collab extends PureComponent { }); }, LOAD_IMAGES_TIMEOUT); - private handleRemoteSceneUpdate = ( - elements: ReconciledElements, - { init = false }: { init?: boolean } = {}, - ) => { + private handleRemoteSceneUpdate = (elements: ReconciledElements) => { this.excalidrawAPI.updateScene({ elements, - commitToHistory: !!init, }); - // We haven't yet implemented multiplayer undo functionality, so we clear the undo stack - // when we receive any messages from another peer. This UX can be pretty rough -- if you - // undo, a user makes a change, and then try to redo, your element(s) will be lost. However, - // right now we think this is the right tradeoff. - this.excalidrawAPI.history.clear(); - this.loadImageFiles(); }; diff --git a/excalidraw-app/collab/reconciliation.ts b/excalidraw-app/collab/reconciliation.ts index fdff13c549..d6bb338694 100644 --- a/excalidraw-app/collab/reconciliation.ts +++ b/excalidraw-app/collab/reconciliation.ts @@ -19,7 +19,7 @@ const shouldDiscardRemoteElement = ( // local element is being edited (local.id === localAppState.editingElement?.id || local.id === localAppState.resizingElement?.id || - local.id === localAppState.draggingElement?.id || + local.id === localAppState.draggingElement?.id || // Is this still valid? As draggingElement is selection element, which is never part of the elements array // local element is newer local.version > remote.version || // resolve conflicting edits deterministically by taking the one with diff --git a/excalidraw-app/data/index.ts b/excalidraw-app/data/index.ts index 4dfb780179..db92e33aef 100644 --- a/excalidraw-app/data/index.ts +++ b/excalidraw-app/data/index.ts @@ -278,7 +278,7 @@ export const loadScene = async ( // in the scene database/localStorage, and instead fetch them async // from a different database files: data.files, - commitToHistory: false, + commitToStore: false, }; }; diff --git a/excalidraw-app/index.tsx b/excalidraw-app/index.tsx index 7708fc1901..ed9b038386 100644 --- a/excalidraw-app/index.tsx +++ b/excalidraw-app/index.tsx @@ -409,7 +409,7 @@ const ExcalidrawWrapper = () => { excalidrawAPI.updateScene({ ...data.scene, ...restore(data.scene, null, null, { repairBindings: true }), - commitToHistory: true, + commitToStore: true, }); } }); @@ -590,6 +590,7 @@ const ExcalidrawWrapper = () => { if (didChange) { excalidrawAPI.updateScene({ elements, + skipSnapshotUpdate: true, }); } } diff --git a/excalidraw-app/tests/collab.test.tsx b/excalidraw-app/tests/collab.test.tsx index 343a67ac46..bcccb05a3c 100644 --- a/excalidraw-app/tests/collab.test.tsx +++ b/excalidraw-app/tests/collab.test.tsx @@ -64,7 +64,7 @@ vi.mock("socket.io-client", () => { }); describe("collaboration", () => { - it("creating room should reset deleted elements", async () => { + it("creating room should reset deleted elements while keeping store snapshot in sync", async () => { await render(); // To update the scene with deleted elements before starting collab updateSceneData({ @@ -76,26 +76,43 @@ describe("collaboration", () => { isDeleted: true, }), ], + commitToStore: true, }); await waitFor(() => { + expect(API.getUndoStack().length).toBe(1); expect(h.elements).toEqual([ expect.objectContaining({ id: "A" }), expect.objectContaining({ id: "B", isDeleted: true }), ]); - expect(API.getStateHistory().length).toBe(1); + expect(Array.from(h.store.snapshot.elements.values())).toEqual([ + expect.objectContaining({ id: "A" }), + expect.objectContaining({ id: "B", isDeleted: true }), + ]); }); window.collab.startCollaboration(null); await waitFor(() => { + expect(API.getUndoStack().length).toBe(1); expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]); - expect(API.getStateHistory().length).toBe(1); + // We never delete from the local store as it is used for correct diff calculation + expect(Array.from(h.store.snapshot.elements.values())).toEqual([ + expect.objectContaining({ id: "A" }), + expect.objectContaining({ id: "B", isDeleted: true }), + ]); }); const undoAction = createUndoAction(h.history); // noop h.app.actionManager.executeAction(undoAction); + + // As it was introduced #2270, undo is a noop here, but we might want to re-enable it, + // since inability to undo your own deletions could be a bigger upsetting factor here await waitFor(() => { + expect(h.history.isUndoStackEmpty).toBeTruthy(); expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]); - expect(API.getStateHistory().length).toBe(1); + expect(Array.from(h.store.snapshot.elements.values())).toEqual([ + expect.objectContaining({ id: "A" }), + expect.objectContaining({ id: "B", isDeleted: true }), + ]); }); }); }); diff --git a/src/actions/actionAddToLibrary.ts b/src/actions/actionAddToLibrary.ts index 1686554e4f..051d59f330 100644 --- a/src/actions/actionAddToLibrary.ts +++ b/src/actions/actionAddToLibrary.ts @@ -3,6 +3,7 @@ import { deepCopyElement } from "../element/newElement"; import { randomId } from "../random"; import { t } from "../i18n"; import { LIBRARY_DISABLED_TYPES } from "../constants"; +import { StoreAction } from "./types"; export const actionAddToLibrary = register({ name: "addToLibrary", @@ -17,7 +18,7 @@ export const actionAddToLibrary = register({ for (const type of LIBRARY_DISABLED_TYPES) { if (selectedElements.some((element) => element.type === type)) { return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: t(`errors.libraryElementTypeError.${type}`), @@ -41,7 +42,7 @@ export const actionAddToLibrary = register({ }) .then(() => { return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, toast: { message: t("toast.addedToLibrary") }, @@ -50,7 +51,7 @@ export const actionAddToLibrary = register({ }) .catch((error) => { return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: error.message, diff --git a/src/actions/actionAlign.tsx b/src/actions/actionAlign.tsx index 137f68ae9f..5f84642661 100644 --- a/src/actions/actionAlign.tsx +++ b/src/actions/actionAlign.tsx @@ -18,6 +18,7 @@ import { isSomeElementSelected } from "../scene"; import { AppClassProperties, AppState } from "../types"; import { arrayToMap, getShortcutKey } from "../utils"; import { register } from "./register"; +import { StoreAction } from "./types"; const alignActionsPredicate = ( elements: readonly ExcalidrawElement[], @@ -63,7 +64,7 @@ export const actionAlignTop = register({ position: "start", axis: "y", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => @@ -94,7 +95,7 @@ export const actionAlignBottom = register({ position: "end", axis: "y", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => @@ -125,7 +126,7 @@ export const actionAlignLeft = register({ position: "start", axis: "x", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => @@ -156,7 +157,7 @@ export const actionAlignRight = register({ position: "end", axis: "x", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => @@ -187,7 +188,7 @@ export const actionAlignVerticallyCentered = register({ position: "center", axis: "y", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData, app }) => ( @@ -214,7 +215,7 @@ export const actionAlignHorizontallyCentered = register({ position: "center", axis: "x", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData, app }) => ( diff --git a/src/actions/actionBoundText.tsx b/src/actions/actionBoundText.tsx index b421695446..90a6dd2c7f 100644 --- a/src/actions/actionBoundText.tsx +++ b/src/actions/actionBoundText.tsx @@ -33,6 +33,7 @@ import { AppState } from "../types"; import { Mutable } from "../utility-types"; import { getFontString } from "../utils"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionUnbindText = register({ name: "unbindText", @@ -80,7 +81,7 @@ export const actionUnbindText = register({ return { elements, appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, }); @@ -149,7 +150,7 @@ export const actionBindText = register({ return { elements: pushTextAboveContainer(elements, container, textElement), appState: { ...appState, selectedElementIds: { [container.id]: true } }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, }); @@ -299,7 +300,7 @@ export const actionWrapTextInContainer = register({ ...appState, selectedElementIds: containerIds, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, }); diff --git a/src/actions/actionCanvas.tsx b/src/actions/actionCanvas.tsx index f61f57dbd3..7842014aa5 100644 --- a/src/actions/actionCanvas.tsx +++ b/src/actions/actionCanvas.tsx @@ -1,7 +1,13 @@ import { ColorPicker } from "../components/ColorPicker/ColorPicker"; import { ZoomInIcon, ZoomOutIcon } from "../components/icons"; import { ToolButton } from "../components/ToolButton"; -import { CURSOR_TYPE, MIN_ZOOM, THEME, ZOOM_STEP } from "../constants"; +import { + CURSOR_TYPE, + MAX_ZOOM, + MIN_ZOOM, + THEME, + ZOOM_STEP, +} from "../constants"; import { getCommonBounds, getNonDeletedElements } from "../element"; import { ExcalidrawElement } from "../element/types"; import { t } from "../i18n"; @@ -22,6 +28,7 @@ import { import { DEFAULT_CANVAS_BACKGROUND_PICKS } from "../colors"; import { Bounds } from "../element/bounds"; import { setCursor } from "../cursor"; +import { StoreAction } from "./types"; export const actionChangeViewBackgroundColor = register({ name: "changeViewBackgroundColor", @@ -35,7 +42,9 @@ export const actionChangeViewBackgroundColor = register({ perform: (_, appState, value) => { return { appState: { ...appState, ...value }, - commitToHistory: !!value.viewBackgroundColor, + storeAction: !!value.viewBackgroundColor + ? StoreAction.CAPTURE + : StoreAction.NONE, }; }, PanelComponent: ({ elements, appState, updateData, appProps }) => { @@ -88,7 +97,7 @@ export const actionClearCanvas = register({ ? { ...appState.activeTool, type: "selection" } : appState.activeTool, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, }); @@ -110,16 +119,17 @@ export const actionZoomIn = register({ appState, ), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, - PanelComponent: ({ updateData }) => ( + PanelComponent: ({ updateData, appState }) => ( = MAX_ZOOM} onClick={() => { updateData(null); }} @@ -147,16 +157,17 @@ export const actionZoomOut = register({ appState, ), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, - PanelComponent: ({ updateData }) => ( + PanelComponent: ({ updateData, appState }) => ( { updateData(null); }} @@ -184,7 +195,7 @@ export const actionResetZoom = register({ appState, ), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ updateData, appState }) => ( @@ -261,8 +272,8 @@ export const zoomToFit = ({ // Apply clamping to newZoomValue to be between 10% and 3000% newZoomValue = Math.min( - Math.max(newZoomValue, 0.1), - 30.0, + Math.max(newZoomValue, MIN_ZOOM), + MAX_ZOOM, ) as NormalizedZoomValue; let appStateWidth = appState.width; @@ -307,7 +318,7 @@ export const zoomToFit = ({ scrollY, zoom: { value: newZoomValue }, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }; @@ -377,7 +388,7 @@ export const actionToggleTheme = register({ theme: value || (appState.theme === THEME.LIGHT ? THEME.DARK : THEME.LIGHT), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, keyTest: (event) => event.altKey && event.shiftKey && event.code === CODES.D, @@ -414,7 +425,7 @@ export const actionToggleEraserTool = register({ activeEmbeddable: null, activeTool, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => event.key === KEYS.E, @@ -449,7 +460,7 @@ export const actionToggleHandTool = register({ activeEmbeddable: null, activeTool, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => diff --git a/src/actions/actionClipboard.tsx b/src/actions/actionClipboard.tsx index dadc610136..eaea7f276c 100644 --- a/src/actions/actionClipboard.tsx +++ b/src/actions/actionClipboard.tsx @@ -13,6 +13,7 @@ import { exportCanvas, prepareElementsForExport } from "../data/index"; import { isTextElement } from "../element"; import { t } from "../i18n"; import { isFirefox } from "../constants"; +import { StoreAction } from "./types"; export const actionCopy = register({ name: "copy", @@ -28,7 +29,7 @@ export const actionCopy = register({ await copyToClipboard(elementsToCopy, app.files, event); } catch (error: any) { return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: error.message, @@ -37,7 +38,7 @@ export const actionCopy = register({ } return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.copy", @@ -63,7 +64,7 @@ export const actionPaste = register({ if (isFirefox) { return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: t("hints.firefox_clipboard_write"), @@ -72,7 +73,7 @@ export const actionPaste = register({ } return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: t("errors.asyncPasteFailedOnRead"), @@ -85,7 +86,7 @@ export const actionPaste = register({ } catch (error: any) { console.error(error); return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, errorMessage: t("errors.asyncPasteFailedOnParse"), @@ -94,7 +95,7 @@ export const actionPaste = register({ } return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.paste", @@ -119,7 +120,7 @@ export const actionCopyAsSvg = register({ perform: async (elements, appState, _data, app) => { if (!app.canvas) { return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; } @@ -141,7 +142,7 @@ export const actionCopyAsSvg = register({ }, ); return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; } catch (error: any) { console.error(error); @@ -150,7 +151,7 @@ export const actionCopyAsSvg = register({ ...appState, errorMessage: error.message, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; } }, @@ -166,7 +167,7 @@ export const actionCopyAsPng = register({ perform: async (elements, appState, _data, app) => { if (!app.canvas) { return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; } const selectedElements = app.scene.getSelectedElements({ @@ -199,7 +200,7 @@ export const actionCopyAsPng = register({ }), }, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; } catch (error: any) { console.error(error); @@ -208,7 +209,7 @@ export const actionCopyAsPng = register({ ...appState, errorMessage: error.message, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; } }, @@ -238,7 +239,7 @@ export const copyText = register({ .join("\n\n"); copyTextToSystemClipboard(text); return { - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, predicate: (elements, appState, _, app) => { diff --git a/src/actions/actionDeleteSelected.tsx b/src/actions/actionDeleteSelected.tsx index de25ed8986..0349f7a19a 100644 --- a/src/actions/actionDeleteSelected.tsx +++ b/src/actions/actionDeleteSelected.tsx @@ -13,6 +13,7 @@ import { fixBindingsAfterDeletion } from "../element/binding"; import { isBoundToContainer, isFrameLikeElement } from "../element/typeChecks"; import { updateActiveTool } from "../utils"; import { TrashIcon } from "../components/icons"; +import { StoreAction } from "./types"; const deleteSelectedElements = ( elements: readonly ExcalidrawElement[], @@ -109,7 +110,7 @@ export const actionDeleteSelected = register({ ...nextAppState, editingLinearElement: null, }, - commitToHistory: false, + storeAction: StoreAction.UPDATE, }; } @@ -141,7 +142,7 @@ export const actionDeleteSelected = register({ : [0], }, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; } let { elements: nextElements, appState: nextAppState } = @@ -161,10 +162,12 @@ export const actionDeleteSelected = register({ multiElement: null, activeEmbeddable: null, }, - commitToHistory: isSomeElementSelected( + storeAction: isSomeElementSelected( getNonDeletedElements(elements), appState, - ), + ) + ? StoreAction.CAPTURE + : StoreAction.NONE, }; }, contextItemLabel: "labels.delete", diff --git a/src/actions/actionDistribute.tsx b/src/actions/actionDistribute.tsx index bf51bedf4b..13a3a74ad8 100644 --- a/src/actions/actionDistribute.tsx +++ b/src/actions/actionDistribute.tsx @@ -14,6 +14,7 @@ import { isSomeElementSelected } from "../scene"; import { AppClassProperties, AppState } from "../types"; import { arrayToMap, getShortcutKey } from "../utils"; import { register } from "./register"; +import { StoreAction } from "./types"; const enableActionGroup = (appState: AppState, app: AppClassProperties) => { const selectedElements = app.scene.getSelectedElements(appState); @@ -53,7 +54,7 @@ export const distributeHorizontally = register({ space: "between", axis: "x", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => @@ -83,7 +84,7 @@ export const distributeVertically = register({ space: "between", axis: "y", }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => diff --git a/src/actions/actionDuplicateSelection.tsx b/src/actions/actionDuplicateSelection.tsx index 916c146cf1..2ba64271c5 100644 --- a/src/actions/actionDuplicateSelection.tsx +++ b/src/actions/actionDuplicateSelection.tsx @@ -14,7 +14,7 @@ import { } from "../groups"; import { AppState } from "../types"; import { fixBindingsAfterDuplication } from "../element/binding"; -import { ActionResult } from "./types"; +import { ActionResult, StoreAction } from "./types"; import { GRID_SIZE } from "../constants"; import { bindTextToShapeAfterDuplication, @@ -48,13 +48,13 @@ export const actionDuplicateSelection = register({ return { elements, appState: ret.appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; } return { ...duplicateElements(elements, appState), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.duplicateSelection", diff --git a/src/actions/actionElementLock.ts b/src/actions/actionElementLock.ts index 164240b290..c1b5224927 100644 --- a/src/actions/actionElementLock.ts +++ b/src/actions/actionElementLock.ts @@ -4,6 +4,7 @@ import { ExcalidrawElement } from "../element/types"; import { KEYS } from "../keys"; import { arrayToMap } from "../utils"; import { register } from "./register"; +import { StoreAction } from "./types"; const shouldLock = (elements: readonly ExcalidrawElement[]) => elements.every((el) => !el.locked); @@ -44,7 +45,7 @@ export const actionToggleElementLock = register({ ? null : appState.selectedLinearElement, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: (elements, appState, app) => { @@ -98,7 +99,7 @@ export const actionUnlockAllElements = register({ lockedElements.map((el) => [el.id, true]), ), }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.elementLock.unlockAll", diff --git a/src/actions/actionExport.tsx b/src/actions/actionExport.tsx index 74dff34c85..d7d68a5900 100644 --- a/src/actions/actionExport.tsx +++ b/src/actions/actionExport.tsx @@ -19,12 +19,16 @@ import { nativeFileSystemSupported } from "../data/filesystem"; import { Theme } from "../element/types"; import "../components/ToolIcon.scss"; +import { StoreAction } from "./types"; export const actionChangeProjectName = register({ name: "changeProjectName", trackEvent: false, perform: (_elements, appState, value) => { - return { appState: { ...appState, name: value }, commitToHistory: false }; + return { + appState: { ...appState, name: value }, + storeAction: StoreAction.UPDATE, + }; }, PanelComponent: ({ appState, updateData, appProps, data }) => ( { return { appState: { ...appState, exportScale: value }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ elements: allElements, appState, updateData }) => { @@ -94,7 +98,7 @@ export const actionChangeExportBackground = register({ perform: (_elements, appState, value) => { return { appState: { ...appState, exportBackground: value }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ appState, updateData }) => ( @@ -113,7 +117,7 @@ export const actionChangeExportEmbedScene = register({ perform: (_elements, appState, value) => { return { appState: { ...appState, exportEmbedScene: value }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ appState, updateData }) => ( @@ -148,7 +152,7 @@ export const actionSaveToActiveFile = register({ : await saveAsJSON(elements, appState, app.files); return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, fileHandle, @@ -170,7 +174,7 @@ export const actionSaveToActiveFile = register({ } else { console.warn(error); } - return { commitToHistory: false }; + return { storeAction: StoreAction.NONE }; } }, keyTest: (event) => @@ -192,7 +196,7 @@ export const actionSaveFileToDisk = register({ app.files, ); return { - commitToHistory: false, + storeAction: StoreAction.NONE, appState: { ...appState, openDialog: null, @@ -206,7 +210,7 @@ export const actionSaveFileToDisk = register({ } else { console.warn(error); } - return { commitToHistory: false }; + return { storeAction: StoreAction.NONE }; } }, keyTest: (event) => @@ -244,7 +248,7 @@ export const actionLoadScene = register({ elements: loadedElements, appState: loadedAppState, files, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; } catch (error: any) { if (error?.name === "AbortError") { @@ -255,7 +259,7 @@ export const actionLoadScene = register({ elements, appState: { ...appState, errorMessage: error.message }, files: app.files, - commitToHistory: false, + storeAction: StoreAction.NONE, }; } }, @@ -268,7 +272,7 @@ export const actionExportWithDarkMode = register({ perform: (_elements, appState, value) => { return { appState: { ...appState, exportWithDarkMode: value }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ appState, updateData }) => ( diff --git a/src/actions/actionFinalize.tsx b/src/actions/actionFinalize.tsx index a7c34c5ac5..0d052b24fc 100644 --- a/src/actions/actionFinalize.tsx +++ b/src/actions/actionFinalize.tsx @@ -16,6 +16,7 @@ import { import { isBindingElement, isLinearElement } from "../element/typeChecks"; import { AppState } from "../types"; import { resetCursor } from "../cursor"; +import { StoreAction } from "./types"; export const actionFinalize = register({ name: "finalize", @@ -49,7 +50,7 @@ export const actionFinalize = register({ cursorButton: "up", editingLinearElement: null, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; } } @@ -190,7 +191,10 @@ export const actionFinalize = register({ : appState.selectedLinearElement, pendingImageElementId: null, }, - commitToHistory: appState.activeTool.type === "freedraw", + storeAction: + appState.activeTool.type === "freedraw" + ? StoreAction.CAPTURE + : StoreAction.UPDATE, }; }, keyTest: (event, appState) => diff --git a/src/actions/actionFlip.ts b/src/actions/actionFlip.ts index 12d5e2e48e..9b8874550f 100644 --- a/src/actions/actionFlip.ts +++ b/src/actions/actionFlip.ts @@ -13,6 +13,7 @@ import { unbindLinearElements, } from "../element/binding"; import { updateFrameMembershipOfSelectedElements } from "../frame"; +import { StoreAction } from "./types"; export const actionFlipHorizontal = register({ name: "flipHorizontal", @@ -25,7 +26,7 @@ export const actionFlipHorizontal = register({ app, ), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => event.shiftKey && event.code === CODES.H, @@ -43,7 +44,7 @@ export const actionFlipVertical = register({ app, ), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => diff --git a/src/actions/actionFrame.ts b/src/actions/actionFrame.ts index 4cddb2ac0f..2acdb44d2b 100644 --- a/src/actions/actionFrame.ts +++ b/src/actions/actionFrame.ts @@ -8,6 +8,7 @@ import { updateActiveTool } from "../utils"; import { setCursorForShape } from "../cursor"; import { register } from "./register"; import { isFrameLikeElement } from "../element/typeChecks"; +import { StoreAction } from "./types"; const isSingleFrameSelected = (appState: AppState, app: AppClassProperties) => { const selectedElements = app.scene.getSelectedElements(appState); @@ -39,14 +40,14 @@ export const actionSelectAllElementsInFrame = register({ return acc; }, {} as Record), }, - commitToHistory: false, + storeAction: StoreAction.CAPTURE, }; } return { elements, appState, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.selectAllElementsInFrame", @@ -74,14 +75,14 @@ export const actionRemoveAllElementsFromFrame = register({ [selectedElement.id]: true, }, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; } return { elements, appState, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.removeAllElementsFromFrame", @@ -103,7 +104,7 @@ export const actionupdateFrameRendering = register({ enabled: !appState.frameRendering.enabled, }, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.updateFrameRendering", @@ -131,7 +132,7 @@ export const actionSetFrameAsActiveTool = register({ type: "frame", }), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, keyTest: (event) => diff --git a/src/actions/actionGroup.tsx b/src/actions/actionGroup.tsx index e6cb058401..6c9d12864c 100644 --- a/src/actions/actionGroup.tsx +++ b/src/actions/actionGroup.tsx @@ -27,6 +27,7 @@ import { removeElementsFromFrame, replaceAllElementsInFrame, } from "../frame"; +import { StoreAction } from "./types"; const allElementsInSameGroup = (elements: readonly ExcalidrawElement[]) => { if (elements.length >= 2) { @@ -69,7 +70,7 @@ export const actionGroup = register({ }); if (selectedElements.length < 2) { // nothing to group - return { appState, elements, commitToHistory: false }; + return { appState, elements, storeAction: StoreAction.NONE }; } // if everything is already grouped into 1 group, there is nothing to do const selectedGroupIds = getSelectedGroupIds(appState); @@ -89,7 +90,7 @@ export const actionGroup = register({ ]); if (combinedSet.size === elementIdsInGroup.size) { // no incremental ids in the selected ids - return { appState, elements, commitToHistory: false }; + return { appState, elements, storeAction: StoreAction.NONE }; } } @@ -155,7 +156,7 @@ export const actionGroup = register({ ), }, elements: nextElements, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.group", @@ -182,7 +183,7 @@ export const actionUngroup = register({ perform: (elements, appState, _, app) => { const groupIds = getSelectedGroupIds(appState); if (groupIds.length === 0) { - return { appState, elements, commitToHistory: false }; + return { appState, elements, storeAction: StoreAction.NONE, }; } let nextElements = [...elements]; @@ -250,7 +251,7 @@ export const actionUngroup = register({ return { appState: { ...appState, ...updateAppState }, elements: nextElements, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, keyTest: (event) => diff --git a/src/actions/actionHistory.tsx b/src/actions/actionHistory.tsx index 2e0f4c0916..f860e4c3e8 100644 --- a/src/actions/actionHistory.tsx +++ b/src/actions/actionHistory.tsx @@ -1,62 +1,48 @@ -import { Action, ActionResult } from "./types"; +import { Action, ActionResult, StoreAction } from "./types"; import { UndoIcon, RedoIcon } from "../components/icons"; import { ToolButton } from "../components/ToolButton"; import { t } from "../i18n"; -import History, { HistoryEntry } from "../history"; -import { ExcalidrawElement } from "../element/types"; +import { History } from "../history"; import { AppState } from "../types"; import { KEYS } from "../keys"; -import { newElementWith } from "../element/mutateElement"; -import { fixBindingsAfterDeletion } from "../element/binding"; import { arrayToMap } from "../utils"; import { isWindows } from "../constants"; +import { ExcalidrawElement } from "../element/types"; +import { fixBindingsAfterDeletion } from "../element/binding"; const writeData = ( - prevElements: readonly ExcalidrawElement[], - appState: AppState, - updater: () => HistoryEntry | null, + appState: Readonly, + updater: () => [Map, AppState] | void, ): ActionResult => { - const commitToHistory = false; if ( !appState.multiElement && !appState.resizingElement && !appState.editingElement && !appState.draggingElement ) { - const data = updater(); - if (data === null) { - return { commitToHistory }; + const result = updater(); + + if (!result) { + return { storeAction: StoreAction.NONE }; } - const prevElementMap = arrayToMap(prevElements); - const nextElements = data.elements; - const nextElementMap = arrayToMap(nextElements); + // TODO_UNDO: worth detecting z-index deltas or do we just order based on fractional indices? + const [nextElementsMap, nextAppState] = result; + const nextElements = Array.from(nextElementsMap.values()); - const deletedElements = prevElements.filter( - (prevElement) => !nextElementMap.has(prevElement.id), - ); - const elements = nextElements - .map((nextElement) => - newElementWith( - prevElementMap.get(nextElement.id) || nextElement, - nextElement, - ), - ) - .concat( - deletedElements.map((prevElement) => - newElementWith(prevElement, { isDeleted: true }), - ), - ); - fixBindingsAfterDeletion(elements, deletedElements); + // TODO_UNDO: these are all deleted elements, but ideally we should get just those that were delted at this moment + const deletedElements = nextElements.filter((element) => element.isDeleted); + // TODO_UNDO: this doesn't really work for bound text + fixBindingsAfterDeletion(nextElements, deletedElements); return { - elements, - appState: { ...appState, ...data.appState }, - commitToHistory, - syncHistory: true, + appState: nextAppState, + elements: Array.from(nextElementsMap.values()), + storeAction: StoreAction.UPDATE, }; } - return { commitToHistory }; + + return { storeAction: StoreAction.NONE }; }; type ActionCreator = (history: History) => Action; @@ -65,7 +51,7 @@ export const createUndoAction: ActionCreator = (history) => ({ name: "undo", trackEvent: { category: "history" }, perform: (elements, appState) => - writeData(elements, appState, () => history.undoOnce()), + writeData(appState, () => history.undo(arrayToMap(elements), appState)), keyTest: (event) => event[KEYS.CTRL_OR_CMD] && event.key.toLowerCase() === KEYS.Z && @@ -77,16 +63,16 @@ export const createUndoAction: ActionCreator = (history) => ({ aria-label={t("buttons.undo")} onClick={updateData} size={data?.size || "medium"} + disabled={history.isUndoStackEmpty} /> ), - commitToHistory: () => false, }); export const createRedoAction: ActionCreator = (history) => ({ name: "redo", trackEvent: { category: "history" }, perform: (elements, appState) => - writeData(elements, appState, () => history.redoOnce()), + writeData(appState, () => history.redo(arrayToMap(elements), appState)), keyTest: (event) => (event[KEYS.CTRL_OR_CMD] && event.shiftKey && @@ -99,7 +85,7 @@ export const createRedoAction: ActionCreator = (history) => ({ aria-label={t("buttons.redo")} onClick={updateData} size={data?.size || "medium"} + disabled={history.isRedoStackEmpty} /> ), - commitToHistory: () => false, }); diff --git a/src/actions/actionLinearEditor.ts b/src/actions/actionLinearEditor.ts index 83611b027f..a75d5c6325 100644 --- a/src/actions/actionLinearEditor.ts +++ b/src/actions/actionLinearEditor.ts @@ -2,6 +2,7 @@ import { LinearElementEditor } from "../element/linearElementEditor"; import { isLinearElement } from "../element/typeChecks"; import { ExcalidrawLinearElement } from "../element/types"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionToggleLinearEditor = register({ name: "toggleLinearEditor", @@ -30,7 +31,7 @@ export const actionToggleLinearEditor = register({ ...appState, editingLinearElement, }, - commitToHistory: false, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: (elements, appState, app) => { diff --git a/src/actions/actionMenu.tsx b/src/actions/actionMenu.tsx index b259d72675..330bdc1918 100644 --- a/src/actions/actionMenu.tsx +++ b/src/actions/actionMenu.tsx @@ -4,6 +4,7 @@ import { t } from "../i18n"; import { showSelectedShapeActions, getNonDeletedElements } from "../element"; import { register } from "./register"; import { KEYS } from "../keys"; +import { StoreAction } from "./types"; export const actionToggleCanvasMenu = register({ name: "toggleCanvasMenu", @@ -13,7 +14,7 @@ export const actionToggleCanvasMenu = register({ ...appState, openMenu: appState.openMenu === "canvas" ? null : "canvas", }, - commitToHistory: false, + storeAction: StoreAction.NONE, }), PanelComponent: ({ appState, updateData }) => ( ( event.key === KEYS.QUESTION_MARK, diff --git a/src/actions/actionNavigate.tsx b/src/actions/actionNavigate.tsx index 126e547ae2..30c28f883b 100644 --- a/src/actions/actionNavigate.tsx +++ b/src/actions/actionNavigate.tsx @@ -3,6 +3,7 @@ import { Avatar } from "../components/Avatar"; import { centerScrollOn } from "../scene/scroll"; import { Collaborator } from "../types"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionGoToCollaborator = register({ name: "goToCollaborator", @@ -11,7 +12,7 @@ export const actionGoToCollaborator = register({ perform: (_elements, appState, value) => { const point = value as Collaborator["pointer"]; if (!point) { - return { appState, commitToHistory: false }; + return { appState, storeAction: StoreAction.NONE }; } return { @@ -28,7 +29,7 @@ export const actionGoToCollaborator = register({ // Close mobile menu openMenu: appState.openMenu === "canvas" ? null : appState.openMenu, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, PanelComponent: ({ updateData, data }) => { diff --git a/src/actions/actionProperties.tsx b/src/actions/actionProperties.tsx index d17a87e36f..0645fe6d36 100644 --- a/src/actions/actionProperties.tsx +++ b/src/actions/actionProperties.tsx @@ -92,6 +92,7 @@ import { import { hasStrokeColor } from "../scene/comparisons"; import { arrayToMap, getShortcutKey } from "../utils"; import { register } from "./register"; +import { StoreAction } from "./types"; const FONT_SIZE_RELATIVE_INCREASE_STEP = 0.1; @@ -222,7 +223,7 @@ const changeFontSize = ( ? [...newFontSizes][0] : fallbackValue ?? appState.currentItemFontSize, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }; @@ -251,7 +252,9 @@ export const actionChangeStrokeColor = register({ ...appState, ...value, }, - commitToHistory: !!value.currentItemStrokeColor, + storeAction: !!value.currentItemStrokeColor + ? StoreAction.CAPTURE + : StoreAction.NONE, }; }, PanelComponent: ({ elements, appState, updateData, appProps }) => ( @@ -294,7 +297,9 @@ export const actionChangeBackgroundColor = register({ ...appState, ...value, }, - commitToHistory: !!value.currentItemBackgroundColor, + storeAction: !!value.currentItemBackgroundColor + ? StoreAction.CAPTURE + : StoreAction.NONE, }; }, PanelComponent: ({ elements, appState, updateData, appProps }) => ( @@ -337,7 +342,7 @@ export const actionChangeFillStyle = register({ }), ), appState: { ...appState, currentItemFillStyle: value }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { @@ -409,7 +414,7 @@ export const actionChangeStrokeWidth = register({ }), ), appState: { ...appState, currentItemStrokeWidth: value }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => ( @@ -463,7 +468,7 @@ export const actionChangeSloppiness = register({ }), ), appState: { ...appState, currentItemRoughness: value }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => ( @@ -513,7 +518,7 @@ export const actionChangeStrokeStyle = register({ }), ), appState: { ...appState, currentItemStrokeStyle: value }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => ( @@ -567,7 +572,7 @@ export const actionChangeOpacity = register({ true, ), appState: { ...appState, currentItemOpacity: value }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => ( @@ -725,7 +730,7 @@ export const actionChangeFontFamily = register({ ...appState, currentItemFontFamily: value, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { @@ -814,7 +819,7 @@ export const actionChangeTextAlign = register({ ...appState, currentItemTextAlign: value, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { @@ -894,7 +899,7 @@ export const actionChangeVerticalAlign = register({ appState: { ...appState, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { @@ -967,7 +972,7 @@ export const actionChangeRoundness = register({ ...appState, currentItemRoundness: value, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { @@ -1047,7 +1052,7 @@ export const actionChangeArrowhead = register({ ? "currentItemStartArrowhead" : "currentItemEndArrowhead"]: value.type, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, PanelComponent: ({ elements, appState, updateData }) => { diff --git a/src/actions/actionSelectAll.ts b/src/actions/actionSelectAll.ts index 49f5072ce3..532f49e4a3 100644 --- a/src/actions/actionSelectAll.ts +++ b/src/actions/actionSelectAll.ts @@ -6,6 +6,7 @@ import { ExcalidrawElement } from "../element/types"; import { isLinearElement } from "../element/typeChecks"; import { LinearElementEditor } from "../element/linearElementEditor"; import { excludeElementsInFramesFromSelection } from "../scene/selection"; +import { StoreAction } from "./types"; export const actionSelectAll = register({ name: "selectAll", @@ -46,7 +47,7 @@ export const actionSelectAll = register({ ? new LinearElementEditor(elements[0], app.scene) : null, }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.selectAll", diff --git a/src/actions/actionStyles.ts b/src/actions/actionStyles.ts index 9c6589bbc7..e9d4e3fbf2 100644 --- a/src/actions/actionStyles.ts +++ b/src/actions/actionStyles.ts @@ -25,6 +25,7 @@ import { } from "../element/typeChecks"; import { getSelectedElements } from "../scene"; import { ExcalidrawTextElement } from "../element/types"; +import { StoreAction } from "./types"; // `copiedStyles` is exported only for tests. export let copiedStyles: string = "{}"; @@ -48,7 +49,7 @@ export const actionCopyStyles = register({ ...appState, toast: { message: t("toast.copyStyles") }, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, contextItemLabel: "labels.copyStyles", @@ -64,7 +65,7 @@ export const actionPasteStyles = register({ const pastedElement = elementsCopied[0]; const boundTextElement = elementsCopied[1]; if (!isExcalidrawElement(pastedElement)) { - return { elements, commitToHistory: false }; + return { elements, storeAction: StoreAction.NONE }; } const selectedElements = getSelectedElements(elements, appState, { @@ -149,7 +150,7 @@ export const actionPasteStyles = register({ } return element; }), - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.pasteStyles", diff --git a/src/actions/actionToggleGridMode.tsx b/src/actions/actionToggleGridMode.tsx index e4f930bff1..a6a838a19a 100644 --- a/src/actions/actionToggleGridMode.tsx +++ b/src/actions/actionToggleGridMode.tsx @@ -2,6 +2,7 @@ import { CODES, KEYS } from "../keys"; import { register } from "./register"; import { GRID_SIZE } from "../constants"; import { AppState } from "../types"; +import { StoreAction } from "./types"; export const actionToggleGridMode = register({ name: "gridMode", @@ -17,7 +18,7 @@ export const actionToggleGridMode = register({ gridSize: this.checked!(appState) ? null : GRID_SIZE, objectsSnapModeEnabled: false, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, checked: (appState: AppState) => appState.gridSize !== null, diff --git a/src/actions/actionToggleObjectsSnapMode.tsx b/src/actions/actionToggleObjectsSnapMode.tsx index 60986137b2..4b3e0c03c2 100644 --- a/src/actions/actionToggleObjectsSnapMode.tsx +++ b/src/actions/actionToggleObjectsSnapMode.tsx @@ -1,5 +1,6 @@ import { CODES, KEYS } from "../keys"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionToggleObjectsSnapMode = register({ name: "objectsSnapMode", @@ -15,7 +16,7 @@ export const actionToggleObjectsSnapMode = register({ objectsSnapModeEnabled: !this.checked!(appState), gridSize: null, }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, checked: (appState) => appState.objectsSnapModeEnabled, diff --git a/src/actions/actionToggleStats.tsx b/src/actions/actionToggleStats.tsx index 71ba6bef16..dd28d2b6a8 100644 --- a/src/actions/actionToggleStats.tsx +++ b/src/actions/actionToggleStats.tsx @@ -1,5 +1,6 @@ import { register } from "./register"; import { CODES, KEYS } from "../keys"; +import { StoreAction } from "./types"; export const actionToggleStats = register({ name: "stats", @@ -11,7 +12,7 @@ export const actionToggleStats = register({ ...appState, showStats: !this.checked!(appState), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, checked: (appState) => appState.showStats, diff --git a/src/actions/actionToggleViewMode.tsx b/src/actions/actionToggleViewMode.tsx index dc9db0c373..d7dc52c647 100644 --- a/src/actions/actionToggleViewMode.tsx +++ b/src/actions/actionToggleViewMode.tsx @@ -1,5 +1,6 @@ import { CODES, KEYS } from "../keys"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionToggleViewMode = register({ name: "viewMode", @@ -14,7 +15,7 @@ export const actionToggleViewMode = register({ ...appState, viewModeEnabled: !this.checked!(appState), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, checked: (appState) => appState.viewModeEnabled, diff --git a/src/actions/actionToggleZenMode.tsx b/src/actions/actionToggleZenMode.tsx index 28956640c2..ed9b196611 100644 --- a/src/actions/actionToggleZenMode.tsx +++ b/src/actions/actionToggleZenMode.tsx @@ -1,5 +1,6 @@ import { CODES, KEYS } from "../keys"; import { register } from "./register"; +import { StoreAction } from "./types"; export const actionToggleZenMode = register({ name: "zenMode", @@ -14,7 +15,7 @@ export const actionToggleZenMode = register({ ...appState, zenModeEnabled: !this.checked!(appState), }, - commitToHistory: false, + storeAction: StoreAction.NONE, }; }, checked: (appState) => appState.zenModeEnabled, diff --git a/src/actions/actionZindex.tsx b/src/actions/actionZindex.tsx index 0ecfdcb2e6..73b38a3c67 100644 --- a/src/actions/actionZindex.tsx +++ b/src/actions/actionZindex.tsx @@ -15,6 +15,7 @@ import { SendToBackIcon, } from "../components/icons"; import { isDarwin } from "../constants"; +import { StoreAction } from "./types"; export const actionSendBackward = register({ name: "sendBackward", @@ -23,7 +24,7 @@ export const actionSendBackward = register({ return { elements: moveOneLeft(elements, appState), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.sendBackward", @@ -51,7 +52,7 @@ export const actionBringForward = register({ return { elements: moveOneRight(elements, appState), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.bringForward", @@ -79,7 +80,7 @@ export const actionSendToBack = register({ return { elements: moveAllLeft(elements, appState), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.sendToBack", @@ -115,7 +116,7 @@ export const actionBringToFront = register({ return { elements: moveAllRight(elements, appState), appState, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }; }, contextItemLabel: "labels.bringToFront", diff --git a/src/actions/types.ts b/src/actions/types.ts index c74e19552c..5c3a99138a 100644 --- a/src/actions/types.ts +++ b/src/actions/types.ts @@ -10,6 +10,12 @@ import { MarkOptional } from "../utility-types"; export type ActionSource = "ui" | "keyboard" | "contextMenu" | "api"; +export enum StoreAction { + NONE = "none", + UPDATE = "update", // TODO_UNDO: think about better naming as this one is confusing + CAPTURE = "capture", +} + /** if false, the action should be prevented */ export type ActionResult = | { @@ -19,8 +25,7 @@ export type ActionResult = "offsetTop" | "offsetLeft" | "width" | "height" > | null; files?: BinaryFiles | null; - commitToHistory: boolean; - syncHistory?: boolean; + storeAction: StoreAction; replaceFiles?: boolean; } | false; diff --git a/src/change.ts b/src/change.ts new file mode 100644 index 0000000000..dc2985869b --- /dev/null +++ b/src/change.ts @@ -0,0 +1,567 @@ +import { newElementWith } from "./element/mutateElement"; +import { ExcalidrawElement } from "./element/types"; +import { + AppState, + ObservedAppState, + ObservedElementsAppState, + ObservedStandaloneAppState, +} from "./types"; +import { SubtypeOf } from "./utility-types"; +import { isShallowEqual } from "./utils"; + +/** + * Represents the difference between two `T` objects. + * + * Keeping it as pure object (without transient state, side-effects, etc.), so we don't have to instantiate it on load. + */ +class Delta { + private constructor( + public readonly from: Partial, + public readonly to: Partial, + ) {} + + public static create( + from: Partial, + to: Partial, + modifier?: (delta: Partial) => Partial, + modifierOptions?: "from" | "to", + ) { + const modifiedFrom = + modifier && modifierOptions !== "to" ? modifier(from) : from; + const modifiedTo = + modifier && modifierOptions !== "from" ? modifier(to) : to; + + return new Delta(modifiedFrom, modifiedTo); + } + + /** + * Calculates the delta between two objects. + * + * @param prevObject - The previous state of the object. + * @param nextObject - The next state of the object. + * + * @returns new Delta instance. + */ + public static calculate( + prevObject: T, + nextObject: T, + modifier?: (delta: Partial) => Partial, + ): Delta { + if (prevObject === nextObject) { + return Delta.empty(); + } + + const from = {} as Partial; + const to = {} as Partial; + + const unionOfKeys = new Set([ + ...Object.keys(prevObject), + ...Object.keys(nextObject), + ]); + + for (const key of unionOfKeys) { + const prevValue = prevObject[key as keyof T]; + const nextValue = nextObject[key as keyof T]; + + if (prevValue !== nextValue) { + from[key as keyof T] = prevValue; + to[key as keyof T] = nextValue; + } + } + return Delta.create(from, to, modifier); + } + + public static empty() { + return new Delta({}, {}); + } + + public static isEmpty(delta: Delta): boolean { + return !Object.keys(delta.from).length && !Object.keys(delta.to).length; + } + + /** + * Compares if the delta contains any different values compared to the object. + * + * WARN: it's based on shallow compare performed only on the first level, won't work for objects with deeper props. + */ + public static containsDifference(delta: Partial, object: T): boolean { + const anyDistinctKey = this.distinctKeysIterator(delta, object).next() + .value; + return !!anyDistinctKey; + } + + /** + * Returns all the keys that have distinct values. + * + * WARN: it's based on shallow compare performed only on the first level, won't work for objects with deeper props. + */ + public static gatherDifferences(delta: Partial, object: T) { + const distinctKeys = new Set(); + + for (const key of this.distinctKeysIterator(delta, object)) { + distinctKeys.add(key); + } + + return Array.from(distinctKeys); + } + + private static *distinctKeysIterator(delta: Partial, object: T) { + for (const [key, deltaValue] of Object.entries(delta)) { + const objectValue = object[key as keyof T]; + + if (deltaValue !== objectValue) { + // TODO_UNDO: staticly fail (typecheck) on deeper objects? + if ( + typeof deltaValue === "object" && + typeof objectValue === "object" && + deltaValue !== null && + objectValue !== null && + isShallowEqual( + deltaValue as Record, + objectValue as Record, + ) + ) { + continue; + } + + yield key; + } + } + } +} + +/** + * Encapsulates the modifications captured as `Delta`/s. + */ +interface Change { + /** + * Inverses the `Delta`s inside while creating a new `Change`. + */ + inverse(): Change; + + /** + * Applies the `Change` to the previous object. + * + * @returns new object instance and boolean, indicating if there was any visible change made. + */ + applyTo(previous: Readonly, ...options: unknown[]): [T, boolean]; + + /** + * Checks whether there are actually `Delta`s. + */ + isEmpty(): boolean; +} + +export class AppStateChange implements Change { + private constructor(private readonly delta: Delta) {} + + public static calculate>( + prevAppState: T, + nextAppState: T, + ): AppStateChange { + const delta = Delta.calculate(prevAppState, nextAppState); + return new AppStateChange(delta); + } + + public static empty() { + return new AppStateChange(Delta.create({}, {})); + } + + public inverse(): AppStateChange { + const inversedDelta = Delta.create(this.delta.to, this.delta.from); + return new AppStateChange(inversedDelta); + } + + public applyTo( + appState: Readonly, + elements: Readonly>, + ): [AppState, boolean] { + const constainsVisibleChanges = this.checkForVisibleChanges( + appState, + elements, + ); + + const newAppState = { + ...appState, + ...this.delta.to, // TODO_UNDO: probably shouldn't apply element related changes + }; + + return [newAppState, constainsVisibleChanges]; + } + + public isEmpty(): boolean { + return Delta.isEmpty(this.delta); + } + + private checkForVisibleChanges( + appState: ObservedAppState, + elements: Map, + ): boolean { + const containsStandaloneDifference = Delta.containsDifference( + AppStateChange.stripElementsProps(this.delta.to), + appState, + ); + + if (containsStandaloneDifference) { + // We detected a a difference which is unrelated to the elements + return true; + } + + const containsElementsDifference = Delta.containsDifference( + AppStateChange.stripStandaloneProps(this.delta.to), + appState, + ); + + if (!containsStandaloneDifference && !containsElementsDifference) { + // There is no difference detected at all + return false; + } + + // We need to handle elements differences separately, + // as they could be related to deleted elements and/or they could on their own result in no visible action + const changedDeltaKeys = Delta.gatherDifferences( + AppStateChange.stripStandaloneProps(this.delta.to), + appState, + ) as Array; + + // Check whether delta properties are related to the existing non-deleted elements + for (const key of changedDeltaKeys) { + switch (key) { + case "selectedElementIds": + if ( + AppStateChange.checkForSelectedElementsDifferences( + this.delta.to[key], + appState, + elements, + ) + ) { + return true; + } + break; + case "selectedLinearElement": + case "editingLinearElement": + if ( + AppStateChange.checkForLinearElementDifferences( + this.delta.to[key], + elements, + ) + ) { + return true; + } + break; + case "editingGroupId": + case "selectedGroupIds": + return AppStateChange.checkForGroupsDifferences(); + default: { + // WARN: this exhaustive check in the switch statement is here to catch unexpected future changes + // TODO_UNDO: use assertNever + const exhaustiveCheck: never = key; + throw new Error( + `Unknown ObservedElementsAppState key '${exhaustiveCheck}'.`, + ); + } + } + } + + return false; + } + + private static checkForSelectedElementsDifferences( + deltaIds: ObservedElementsAppState["selectedElementIds"] | undefined, + appState: Pick, + elements: Map, + ) { + if (!deltaIds) { + // There are no selectedElementIds in the delta + return; + } + + // TODO_UNDO: it could have been visible before (and now it's not) + // TODO_UNDO: it could have been selected + for (const id of Object.keys(deltaIds)) { + const element = elements.get(id); + + if (element && !element.isDeleted) { + // // TODO_UNDO: breaks multi selection + // if (appState.selectedElementIds[id]) { + // // Element is already selected + // return; + // } + + // Found related visible element! + return true; + } + } + } + + private static checkForLinearElementDifferences( + linearElement: + | ObservedElementsAppState["editingLinearElement"] + | ObservedAppState["selectedLinearElement"] + | undefined, + elements: Map, + ) { + if (!linearElement) { + return; + } + + const element = elements.get(linearElement.elementId); + + if (element && !element.isDeleted) { + // Found related visible element! + return true; + } + } + + // Currently we don't have an index of elements by groupIds, which means + // the calculation for getting the visible elements based on the groupIds stored in delta + // is not worth performing - due to perf. and dev. complexity. + // + // Therefore we are accepting in these cases empty undos / redos, which should be pretty rare: + // - only when one of these (or both) are in delta and the are no non deleted elements containing these group ids + private static checkForGroupsDifferences() { + return true; + } + + private static stripElementsProps( + delta: Partial, + ): Partial { + // WARN: Do not remove the type-casts as they here for exhaustive type checks + const { + editingGroupId, + selectedGroupIds, + selectedElementIds, + editingLinearElement, + selectedLinearElement, + ...standaloneProps + } = delta as ObservedAppState; + + return standaloneProps as SubtypeOf< + typeof standaloneProps, + ObservedStandaloneAppState + >; + } + + private static stripStandaloneProps( + delta: Partial, + ): Partial { + // WARN: Do not remove the type-casts as they here for exhaustive type checks + const { name, viewBackgroundColor, ...elementsProps } = + delta as ObservedAppState; + + return elementsProps as SubtypeOf< + typeof elementsProps, + ObservedElementsAppState + >; + } +} + +/** + * Elements change is a low level primitive to capture a change between two sets of elements. + * It does so by encapsulating forward and backward `Delta`s, which allow to travel in both directions. + * + * We could be smarter about the change in the future, ideas for improvements are: + * - for memory, share the same delta instances between different deltas (flyweight-like) + * - for serialization, compress the deltas into a tree-like structures with custom pointers or let one delta instance contain multiple element ids + * - for performance, emit the changes directly by the user actions, then apply them in from store into the state (no diffing!) + * - for performance, add operations in addition to deltas, which increment (decrement) properties by given value (could be used i.e. for presence-like move) + */ +export class ElementsChange implements Change> { + private constructor( + // TODO_UNDO: re-think the possible need for added/ remove/ updated deltas (possibly for handling edge cases with deletion, fixing bindings for deletion, showing changes added/modified/updated for version end etc.) + private readonly deltas: Map>, + ) {} + + public static create(deltas: Map>) { + return new ElementsChange(deltas); + } + + /** + * Calculates the `Delta`s between the previous and next set of elements. + * + * @param prevElements - Map representing the previous state of elements. + * @param nextElements - Map representing the next state of elements. + * + * @returns `ElementsChange` instance representing the `Delta` changes between the two sets of elements. + */ + public static calculate( + prevElements: Map, + nextElements: Map, + ): ElementsChange { + if (prevElements === nextElements) { + return ElementsChange.empty(); + } + + const deltas = new Map>(); + + // This might be needed only in same edge cases, like during collab, when `isDeleted` elements get removed + for (const prevElement of prevElements.values()) { + const nextElement = nextElements.get(prevElement.id); + + // Element got removed + if (!nextElement) { + const from = { ...prevElement, isDeleted: false } as T; + const to = { isDeleted: true } as T; + + const delta = Delta.create( + from, + to, + ElementsChange.stripIrrelevantProps, + ); + + deltas.set(prevElement.id, delta as Delta); + } + } + + for (const nextElement of nextElements.values()) { + const prevElement = prevElements.get(nextElement.id); + + // Element got added + if (!prevElement) { + if (nextElement.isDeleted) { + // Special case when an element is added as deleted (i.e. through the API). + // Creating a delta for it wouldn't make sense, as it would go from isDeleted `true` into `true` again. + // We are going to skip it for now, later we could be have separate `added` & `removed` entries in the elements change, + // so that we would distinguish between actual addition, removal and "soft" (un)deletion. + continue; + } + + const from = { isDeleted: true } as T; + const to = { ...nextElement, isDeleted: false } as T; + + const delta = Delta.create( + from, + to, + ElementsChange.stripIrrelevantProps, + ); + + deltas.set(nextElement.id, delta as Delta); + + continue; + } + + // Element got updated + if (prevElement.versionNonce !== nextElement.versionNonce) { + // O(n^2) here, but it's not as bad as it looks: + // - we do this only on history recordings, not on every frame + // - we do this only on changed elements + // - # of element's properties is reasonably small + // - otherwise we would have to emit deltas on user actions & apply them on every frame + const delta = Delta.calculate( + prevElement, + nextElement, + ElementsChange.stripIrrelevantProps, + ); + + // Make sure there are at least some changes (except changes to irrelevant data) + if (!Delta.isEmpty(delta)) { + deltas.set(nextElement.id, delta as Delta); + } + } + } + + return new ElementsChange(deltas); + } + + public static empty() { + return new ElementsChange(new Map()); + } + + public inverse(): ElementsChange { + const deltas = new Map>(); + + for (const [id, delta] of this.deltas.entries()) { + deltas.set(id, Delta.create(delta.to, delta.from)); + } + + return new ElementsChange(deltas); + } + + public applyTo( + elements: Readonly>, + ): [Map, boolean] { + let containsVisibleDifference = false; + + for (const [id, delta] of this.deltas.entries()) { + const existingElement = elements.get(id); + + if (existingElement) { + // Check if there was actually any visible change before applying + if (!containsVisibleDifference) { + // Special case, when delta deletes element, it results in a visible change + if (existingElement.isDeleted && delta.to.isDeleted === false) { + containsVisibleDifference = true; + } else if (!existingElement.isDeleted) { + // Check for any difference on a visible element + containsVisibleDifference = Delta.containsDifference( + delta.to, + existingElement, + ); + } + } + + elements.set(id, newElementWith(existingElement, delta.to, true)); + } + } + + return [elements, containsVisibleDifference]; + } + + public isEmpty(): boolean { + // TODO_UNDO: might need to go through all deltas and check for emptiness + return this.deltas.size === 0; + } + + /** + * Update the delta/s based on the existing elements. + * + * @param elements current elements + * @param modifierOptions defines which of the delta (`from` or `to`) will be updated + * @returns new instance with modified delta/s + */ + public applyLatestChanges( + elements: Map, + modifierOptions: "from" | "to", + ): ElementsChange { + const modifier = + (element: ExcalidrawElement) => (partial: Partial) => { + const modifiedPartial: { [key: string]: unknown } = {}; + + for (const key of Object.keys(partial)) { + modifiedPartial[key] = element[key as keyof ExcalidrawElement]; + } + + return modifiedPartial; + }; + + const deltas = new Map>(); + + for (const [id, delta] of this.deltas.entries()) { + const existingElement = elements.get(id); + + if (existingElement) { + const modifiedDelta = Delta.create( + delta.from, + delta.to, + modifier(existingElement), + modifierOptions, + ); + + deltas.set(id, modifiedDelta); + } else { + // Keep whatever we had + deltas.set(id, delta); + } + } + + return ElementsChange.create(deltas); + } + + private static stripIrrelevantProps(delta: Partial) { + // TODO_UNDO: is seed correctly stripped? + const { id, updated, version, versionNonce, seed, ...strippedDelta } = + delta; + + return strippedDelta; + } +} diff --git a/src/components/Actions.scss b/src/components/Actions.scss index df0d73755d..5826628de1 100644 --- a/src/components/Actions.scss +++ b/src/components/Actions.scss @@ -12,6 +12,7 @@ font-size: 0.875rem !important; width: var(--lg-button-size); height: var(--lg-button-size); + svg { width: var(--lg-icon-size) !important; height: var(--lg-icon-size) !important; diff --git a/src/components/App.tsx b/src/components/App.tsx index abdd484f1c..7d094dc9ac 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -41,6 +41,7 @@ import { createRedoAction, createUndoAction } from "../actions/actionHistory"; import { ActionManager } from "../actions/manager"; import { actions } from "../actions/register"; import { Action, ActionResult } from "../actions/types"; +import { ActionResult, StoreAction } from "../actions/types"; import { trackEvent } from "../analytics"; import { getDefaultAppState, @@ -194,7 +195,7 @@ import { isSelectedViaGroup, selectGroupsForSelectedElements, } from "../groups"; -import History from "../history"; +import { History } from "../history"; import { defaultLang, getLanguage, languages, setLanguage, t } from "../i18n"; import { CODES, @@ -268,6 +269,7 @@ import { muteFSAbortError, isTestEnv, easeOut, + isShallowEqual, arrayToMap, } from "../utils"; import { @@ -400,6 +402,7 @@ import { ElementCanvasButton } from "./MagicButton"; import { MagicIcon, copyIcon, fullscreenIcon } from "./icons"; import { EditorLocalStorage } from "../data/EditorLocalStorage"; import { fixFractionalIndices } from "../fractionalIndex"; +import { Store } from "../store"; const AppContext = React.createContext(null!); const AppPropsContext = React.createContext(null!); @@ -514,6 +517,7 @@ class App extends React.Component { public library: AppClassProperties["library"]; public libraryItemsFromStorage: LibraryItems | undefined; public id: string; + private store: Store; private history: History; private excalidrawContainerValue: { container: HTMLDivElement | null; @@ -597,6 +601,9 @@ class App extends React.Component { this.rc = rough.canvas(this.canvas); this.renderer = new Renderer(this.scene); + this.store = new Store(); + this.history = new History(); + if (excalidrawAPI) { const api: ExcalidrawImperativeAPI = { updateScene: this.updateScene, @@ -604,6 +611,10 @@ class App extends React.Component { addFiles: this.addFiles, resetScene: this.resetScene, getSceneElementsIncludingDeleted: this.getSceneElementsIncludingDeleted, + store: { + clear: this.store.clear, + listen: this.store.listen, + }, history: { clear: this.resetHistory, }, @@ -643,8 +654,14 @@ class App extends React.Component { onSceneUpdated: this.onSceneUpdated, }); this.history = new History(); - this.actionManager.registerAll(actions); + this.actionManager = new ActionManager( + this.syncActionResult, + () => this.state, + () => this.scene.getElementsIncludingDeleted(), + this, + ); + this.actionManager.registerAll(actions); this.actionManager.registerAction(createUndoAction(this.history)); this.actionManager.registerAction(createRedoAction(this.history)); } @@ -1915,15 +1932,16 @@ class App extends React.Component { if (shouldUpdateStrokeColor) { this.syncActionResult({ appState: { ...this.state, currentItemStrokeColor: color }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }); } else { this.syncActionResult({ appState: { ...this.state, currentItemBackgroundColor: color }, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }); } } else { + // TODO_UNDO: test if we didn't regress here - shouldn't this commit to store? this.updateScene({ elements: this.scene.getElementsIncludingDeleted().map((el) => { if (this.state.selectedElementIds[el.id]) { @@ -1959,8 +1977,11 @@ class App extends React.Component { } }); this.scene.replaceAllElements(actionResult.elements); - if (actionResult.commitToHistory) { - this.history.resumeRecording(); + + if (actionResult.storeAction === StoreAction.UPDATE) { + this.store.scheduleSnapshotUpdate(); + } else if (actionResult.storeAction === StoreAction.CAPTURE) { + this.store.resumeCapturing(); } } @@ -1972,8 +1993,10 @@ class App extends React.Component { } if (actionResult.appState || editingElement || this.state.contextMenu) { - if (actionResult.commitToHistory) { - this.history.resumeRecording(); + if (actionResult.storeAction === StoreAction.UPDATE) { + this.store.scheduleSnapshotUpdate(); + } else if (actionResult.storeAction === StoreAction.CAPTURE) { + this.store.resumeCapturing(); } let viewModeEnabled = actionResult?.appState?.viewModeEnabled || false; @@ -2007,34 +2030,24 @@ class App extends React.Component { editingElement = null; } - this.setState( - (state) => { - // using Object.assign instead of spread to fool TS 4.2.2+ into - // regarding the resulting type as not containing undefined - // (which the following expression will never contain) - return Object.assign(actionResult.appState || {}, { - // NOTE this will prevent opening context menu using an action - // or programmatically from the host, so it will need to be - // rewritten later - contextMenu: null, - editingElement, - viewModeEnabled, - zenModeEnabled, - gridSize, - theme, - name, - errorMessage, - }); - }, - () => { - if (actionResult.syncHistory) { - this.history.setCurrentState( - this.state, - this.scene.getElementsIncludingDeleted(), - ); - } - }, - ); + this.setState((state) => { + // using Object.assign instead of spread to fool TS 4.2.2+ into + // regarding the resulting type as not containing undefined + // (which the following expression will never contain) + return Object.assign(actionResult.appState || {}, { + // NOTE this will prevent opening context menu using an action + // or programmatically from the host, so it will need to be + // rewritten later + contextMenu: null, + editingElement, + viewModeEnabled, + zenModeEnabled, + gridSize, + theme, + name, + errorMessage, + }); + }); } }, ); @@ -2058,6 +2071,10 @@ class App extends React.Component { this.history.clear(); }; + private resetStore = () => { + this.store.clear(); + }; + /** * Resets scene & history. * ! Do not use to clear scene user action ! @@ -2070,6 +2087,7 @@ class App extends React.Component { isLoading: opts?.resetLoadingState ? false : state.isLoading, theme: this.state.theme, })); + this.resetStore(); this.resetHistory(); }, ); @@ -2154,10 +2172,11 @@ class App extends React.Component { // seems faster even in browsers that do fire the loadingdone event. this.fonts.loadFontsForElements(scene.elements); + this.resetStore(); this.resetHistory(); this.syncActionResult({ ...scene, - commitToHistory: true, + storeAction: StoreAction.UPDATE, // TODO_UNDO: double-check for regression }); }; @@ -2247,9 +2266,17 @@ class App extends React.Component { configurable: true, value: this.history, }, + store: { + configurable: true, + value: this.store, + }, }); } + this.store.listen((...args) => { + this.history.record(...args); + }); + this.scene.addCallback(this.onSceneUpdated); this.addEventListeners(); @@ -2305,6 +2332,7 @@ class App extends React.Component { this.library.destroy(); this.laserPathManager.destroy(); this.onChangeEmitter.destroy(); + this.store.destroy(); ShapeCache.destroy(); SnapCache.destroy(); clearTimeout(touchTimeout); @@ -2554,7 +2582,7 @@ class App extends React.Component { this.state.editingLinearElement && !this.state.selectedElementIds[this.state.editingLinearElement.elementId] ) { - // defer so that the commitToHistory flag isn't reset via current update + // defer so that the storeAction flag isn't reset via current update setTimeout(() => { // execute only if the condition still holds when the deferred callback // executes (it can be scheduled multiple times depending on how @@ -2598,7 +2626,11 @@ class App extends React.Component { ), ); } - this.history.record(this.state, this.scene.getElementsIncludingDeleted()); + + this.store.capture( + arrayToMap(this.scene.getElementsIncludingDeleted()), + this.state, + ); // Do not notify consumers if we're still loading the scene. Among other // potential issues, this fixes a case where the tab isn't focused during @@ -2936,7 +2968,7 @@ class App extends React.Component { this.files = { ...this.files, ...opts.files }; } - this.history.resumeRecording(); + this.store.resumeCapturing(); const nextElementsToSelect = excludeElementsInFramesFromSelection(newElements); @@ -3177,7 +3209,7 @@ class App extends React.Component { PLAIN_PASTE_TOAST_SHOWN = true; } - this.history.resumeRecording(); + this.store.resumeCapturing(); } setAppState: React.Component["setState"] = ( @@ -3438,10 +3470,33 @@ class App extends React.Component { elements?: SceneData["elements"]; appState?: Pick | null; collaborators?: SceneData["collaborators"]; - commitToHistory?: SceneData["commitToHistory"]; + commitToStore?: SceneData["commitToStore"]; + skipSnapshotUpdate?: SceneData["skipSnapshotUpdate"]; }) => { - if (sceneData.commitToHistory) { - this.history.resumeRecording(); + if ( + !sceneData.skipSnapshotUpdate && + (sceneData.elements || sceneData.appState) + ) { + this.store.scheduleSnapshotUpdate(); + + if (sceneData.commitToStore) { + this.store.resumeCapturing(); + } + + // We need to filter out yet uncomitted local elements + // Once we will be exchanging just store increments and updating changes this won't be necessary + const localElements = this.scene.getElementsIncludingDeleted(); + const nextElements = this.store.ignoreUncomittedElements( + arrayToMap(localElements), + arrayToMap(sceneData.elements || localElements), // Here we expect all next elements + ); + + const nextAppState: AppState = { + ...this.state, + ...(sceneData.appState || {}), // Here we expect just partial appState + }; + + this.store.capture(nextElements, nextAppState); } if (sceneData.appState) { @@ -3650,7 +3705,7 @@ class App extends React.Component { this.state.editingLinearElement.elementId !== selectedElements[0].id ) { - this.history.resumeRecording(); + this.store.resumeCapturing(); this.setState({ editingLinearElement: new LinearElementEditor( selectedElement, @@ -4042,7 +4097,7 @@ class App extends React.Component { ]); } if (!isDeleted || isExistingElement) { - this.history.resumeRecording(); + this.store.resumeCapturing(); } this.setState({ @@ -4334,7 +4389,7 @@ class App extends React.Component { (!this.state.editingLinearElement || this.state.editingLinearElement.elementId !== selectedElements[0].id) ) { - this.history.resumeRecording(); + this.store.resumeCapturing(); this.setState({ editingLinearElement: new LinearElementEditor( selectedElements[0], @@ -5154,6 +5209,7 @@ class App extends React.Component { this.state, ), }, + skipSnapshotUpdate: true, // TODO_UNDO: test if we didn't regress here }); return; } @@ -5754,7 +5810,7 @@ class App extends React.Component { const ret = LinearElementEditor.handlePointerDown( event, this.state, - this.history, + this.store, pointerDownState.origin, linearElementEditor, ); @@ -7307,7 +7363,7 @@ class App extends React.Component { if (isLinearElement(draggingElement)) { if (draggingElement!.points.length > 1) { - this.history.resumeRecording(); + this.store.resumeCapturing(); } const pointerCoords = viewportCoordsToSceneCoords( childEvent, @@ -7542,7 +7598,7 @@ class App extends React.Component { } if (resizingElement) { - this.history.resumeRecording(); + this.store.resumeCapturing(); } if (resizingElement && isInvisiblySmallElement(resizingElement)) { @@ -7843,9 +7899,13 @@ class App extends React.Component { if ( activeTool.type !== "selection" || - isSomeElementSelected(this.scene.getNonDeletedElements(), this.state) + isSomeElementSelected(this.scene.getNonDeletedElements(), this.state) || + !isShallowEqual( + this.state.previousSelectedElementIds, + this.state.selectedElementIds, + ) ) { - this.history.resumeRecording(); + this.store.resumeCapturing(); } if (pointerDownState.drag.hasOccurred || isResizing || isRotating) { @@ -7951,7 +8011,7 @@ class App extends React.Component { return ele; }); - this.history.resumeRecording(); + this.store.resumeCapturing(); this.scene.replaceAllElements(elements); }; @@ -8499,7 +8559,7 @@ class App extends React.Component { isLoading: false, }, replaceFiles: true, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }); return; } catch (error: any) { @@ -8585,15 +8645,21 @@ class App extends React.Component { fileHandle, ); if (ret.type === MIME_TYPES.excalidraw) { + // First we need to delete existing elements, so they get recorded in the undo stack + const deletedExistingElements = this.scene + .getNonDeletedElements() + .map((element) => newElementWith(element, { isDeleted: true })); + this.setState({ isLoading: true }); this.syncActionResult({ ...ret.data, + elements: deletedExistingElements.concat(ret.data.elements), appState: { ...(ret.data.appState || this.state), isLoading: false, }, replaceFiles: true, - commitToHistory: true, + storeAction: StoreAction.CAPTURE, }); } else if (ret.type === MIME_TYPES.excalidrawlib) { await this.library @@ -9209,6 +9275,7 @@ declare global { setState: React.Component["setState"]; app: InstanceType; history: History; + store: Store; }; } } diff --git a/src/components/ToolButton.tsx b/src/components/ToolButton.tsx index ffe9a382cf..52e8975d35 100644 --- a/src/components/ToolButton.tsx +++ b/src/components/ToolButton.tsx @@ -24,6 +24,7 @@ type ToolButtonBaseProps = { hidden?: boolean; visible?: boolean; selected?: boolean; + disabled?: boolean; className?: string; style?: CSSProperties; isLoading?: boolean; @@ -123,10 +124,14 @@ export const ToolButton = React.forwardRef((props: ToolButtonProps, ref) => { type={type} onClick={onClick} ref={innerRef} - disabled={isLoading || props.isLoading} + disabled={isLoading || props.isLoading || !!props.disabled} > {(props.icon || props.label) && ( -