diff --git a/packages/element/src/delta.ts b/packages/element/src/delta.ts index a86b5b523..87a23d0c2 100644 --- a/packages/element/src/delta.ts +++ b/packages/element/src/delta.ts @@ -2,7 +2,6 @@ import { arrayToMap, arrayToObject, assertNever, - invariant, isDevEnv, isShallowEqual, isTestEnv, @@ -561,70 +560,50 @@ export class AppStateDelta implements DeltaContainer { ): [AppState, boolean] { try { const { - selectedElementIds: removedSelectedElementIds = {}, - selectedGroupIds: removedSelectedGroupIds = {}, + selectedElementIds: deletedSelectedElementIds = {}, + selectedGroupIds: deletedSelectedGroupIds = {}, } = this.delta.deleted; const { - selectedElementIds: addedSelectedElementIds = {}, - selectedGroupIds: addedSelectedGroupIds = {}, - selectedLinearElementId, - selectedLinearElementIsEditing, + selectedElementIds: insertedSelectedElementIds = {}, + selectedGroupIds: insertedSelectedGroupIds = {}, + selectedLinearElement: insertedSelectedLinearElement, ...directlyApplicablePartial } = this.delta.inserted; const mergedSelectedElementIds = Delta.mergeObjects( appState.selectedElementIds, - addedSelectedElementIds, - removedSelectedElementIds, + insertedSelectedElementIds, + deletedSelectedElementIds, ); const mergedSelectedGroupIds = Delta.mergeObjects( appState.selectedGroupIds, - addedSelectedGroupIds, - removedSelectedGroupIds, + insertedSelectedGroupIds, + deletedSelectedGroupIds, ); - let selectedLinearElement = appState.selectedLinearElement; - - if (selectedLinearElementId === null) { - // Unselect linear element (visible change) - selectedLinearElement = null; - } else if ( - selectedLinearElementId && - nextElements.has(selectedLinearElementId) - ) { - selectedLinearElement = new LinearElementEditor( - nextElements.get( - selectedLinearElementId, - ) as NonDeleted, - nextElements, - selectedLinearElementIsEditing === true, // Can be unknown which is defaulted to false - ); - } - - if ( - // Value being 'null' is equivaluent to unknown in this case because it only gets set - // to null when 'selectedLinearElementId' is set to null - selectedLinearElementIsEditing != null - ) { - invariant( - selectedLinearElement, - `selectedLinearElement is null when selectedLinearElementIsEditing is set to ${selectedLinearElementIsEditing}`, - ); - - selectedLinearElement = { - ...selectedLinearElement, - isEditing: selectedLinearElementIsEditing, - }; - } + const selectedLinearElement = + insertedSelectedLinearElement && + nextElements.has(insertedSelectedLinearElement.elementId) + ? new LinearElementEditor( + nextElements.get( + insertedSelectedLinearElement.elementId, + ) as NonDeleted, + nextElements, + insertedSelectedLinearElement.isEditing, + ) + : null; const nextAppState = { ...appState, ...directlyApplicablePartial, selectedElementIds: mergedSelectedElementIds, selectedGroupIds: mergedSelectedGroupIds, - selectedLinearElement, + selectedLinearElement: + typeof insertedSelectedLinearElement !== "undefined" + ? selectedLinearElement + : appState.selectedLinearElement, }; const constainsVisibleChanges = this.filterInvisibleChanges( @@ -753,64 +732,53 @@ export class AppStateDelta implements DeltaContainer { } break; - case "selectedLinearElementId": { - const appStateKey = AppStateDelta.convertToAppStateKey(key); - const linearElement = nextAppState[appStateKey]; + case "selectedLinearElement": + const nextLinearElement = nextAppState[key]; - if (!linearElement) { + if (!nextLinearElement) { // previously there was a linear element (assuming visible), now there is none visibleDifferenceFlag.value = true; } else { - const element = nextElements.get(linearElement.elementId); + const element = nextElements.get(nextLinearElement.elementId); if (element && !element.isDeleted) { // previously there wasn't a linear element, now there is one which is visible visibleDifferenceFlag.value = true; } else { // there was assigned a linear element now, but it's deleted - nextAppState[appStateKey] = null; + nextAppState[key] = null; } } break; - } - case "selectedLinearElementIsEditing": { - // Changes in editing state are always visible - const prevIsEditing = - prevAppState.selectedLinearElement?.isEditing ?? false; - const nextIsEditing = - nextAppState.selectedLinearElement?.isEditing ?? false; - - if (prevIsEditing !== nextIsEditing) { - visibleDifferenceFlag.value = true; - } - break; - } - case "lockedMultiSelections": { + case "lockedMultiSelections": const prevLockedUnits = prevAppState[key] || {}; const nextLockedUnits = nextAppState[key] || {}; + // TODO: this seems wrong, we are already doing this comparison generically above, + // hence instead we should check whether elements are actually visible, + // so that once these changes are applied they actually result in a visible change to the user if (!isShallowEqual(prevLockedUnits, nextLockedUnits)) { visibleDifferenceFlag.value = true; } break; - } - case "activeLockedId": { + case "activeLockedId": const prevHitLockedId = prevAppState[key] || null; const nextHitLockedId = nextAppState[key] || null; + // TODO: this seems wrong, we are already doing this comparison generically above, + // hence instead we should check whether elements are actually visible, + // so that once these changes are applied they actually result in a visible change to the user if (prevHitLockedId !== nextHitLockedId) { visibleDifferenceFlag.value = true; } break; - } - default: { + default: assertNever( key, `Unknown ObservedElementsAppState's key "${key}"`, true, ); - } } } } @@ -818,15 +786,6 @@ export class AppStateDelta implements DeltaContainer { return visibleDifferenceFlag.value; } - private static convertToAppStateKey( - key: keyof Pick, - ): keyof Pick { - switch (key) { - case "selectedLinearElementId": - return "selectedLinearElement"; - } - } - private static filterSelectedElements( selectedElementIds: AppState["selectedElementIds"], elements: SceneElementsMap, @@ -891,8 +850,7 @@ export class AppStateDelta implements DeltaContainer { editingGroupId, selectedGroupIds, selectedElementIds, - selectedLinearElementId, - selectedLinearElementIsEditing, + selectedLinearElement, croppingElementId, lockedMultiSelections, activeLockedId, diff --git a/packages/element/src/store.ts b/packages/element/src/store.ts index 56316c5e3..e304486ff 100644 --- a/packages/element/src/store.ts +++ b/packages/element/src/store.ts @@ -978,8 +978,7 @@ const getDefaultObservedAppState = (): ObservedAppState => { viewBackgroundColor: COLOR_PALETTE.white, selectedElementIds: {}, selectedGroupIds: {}, - selectedLinearElementId: null, - selectedLinearElementIsEditing: null, + selectedLinearElement: null, croppingElementId: null, activeLockedId: null, lockedMultiSelections: {}, @@ -998,14 +997,12 @@ export const getObservedAppState = ( croppingElementId: appState.croppingElementId, activeLockedId: appState.activeLockedId, lockedMultiSelections: appState.lockedMultiSelections, - selectedLinearElementId: - (appState as AppState).selectedLinearElement?.elementId ?? - (appState as ObservedAppState).selectedLinearElementId ?? - null, - selectedLinearElementIsEditing: - (appState as AppState).selectedLinearElement?.isEditing ?? - (appState as ObservedAppState).selectedLinearElementIsEditing ?? - null, + selectedLinearElement: appState.selectedLinearElement + ? { + elementId: appState.selectedLinearElement.elementId, + isEditing: !!appState.selectedLinearElement.isEditing, + } + : null, }; Reflect.defineProperty(observedAppState, hiddenObservedAppStateProp, { diff --git a/packages/element/tests/delta.test.tsx b/packages/element/tests/delta.test.tsx index 4d56aac83..89501c866 100644 --- a/packages/element/tests/delta.test.tsx +++ b/packages/element/tests/delta.test.tsx @@ -7,7 +7,10 @@ describe("AppStateDelta", () => { describe("ensure stable delta properties order", () => { it("should maintain stable order for root properties", () => { const name = "untitled scene"; - const selectedLinearElementId = "id1" as LinearElementEditor["elementId"]; + const selectedLinearElement = { + elementId: "id1" as LinearElementEditor["elementId"], + isEditing: false, + }; const commonAppState = { viewBackgroundColor: "#ffffff", @@ -24,23 +27,23 @@ describe("AppStateDelta", () => { const prevAppState1: ObservedAppState = { ...commonAppState, name: "", - selectedLinearElementId: null, + selectedLinearElement: null, }; const nextAppState1: ObservedAppState = { ...commonAppState, name, - selectedLinearElementId, + selectedLinearElement, }; const prevAppState2: ObservedAppState = { - selectedLinearElementId: null, + selectedLinearElement: null, name: "", ...commonAppState, }; const nextAppState2: ObservedAppState = { - selectedLinearElementId, + selectedLinearElement, name, ...commonAppState, }; @@ -58,9 +61,7 @@ describe("AppStateDelta", () => { selectedGroupIds: {}, editingGroupId: null, croppingElementId: null, - selectedLinearElementId: null, - selectedLinearElementIsEditing: null, - editingLinearElementId: null, + selectedLinearElement: null, activeLockedId: null, lockedMultiSelections: {}, }; @@ -106,9 +107,7 @@ describe("AppStateDelta", () => { selectedElementIds: {}, editingGroupId: null, croppingElementId: null, - selectedLinearElementId: null, - selectedLinearElementIsEditing: null, - editingLinearElementId: null, + selectedLinearElement: null, activeLockedId: null, lockedMultiSelections: {}, }; diff --git a/packages/excalidraw/data/reconcile.ts b/packages/excalidraw/data/reconcile.ts index d758796a7..a93f95100 100644 --- a/packages/excalidraw/data/reconcile.ts +++ b/packages/excalidraw/data/reconcile.ts @@ -20,7 +20,7 @@ export type ReconciledExcalidrawElement = OrderedExcalidrawElement & export type RemoteExcalidrawElement = OrderedExcalidrawElement & MakeBrand<"RemoteExcalidrawElement">; -const shouldDiscardRemoteElement = ( +export const shouldDiscardRemoteElement = ( localAppState: AppState, local: OrderedExcalidrawElement | undefined, remote: RemoteExcalidrawElement, @@ -30,7 +30,7 @@ const shouldDiscardRemoteElement = ( // local element is being edited (local.id === localAppState.editingTextElement?.id || local.id === localAppState.resizingElement?.id || - local.id === localAppState.newElement?.id || // TODO: Is this still valid? As newElement is selection element, which is never part of the elements array + local.id === localAppState.newElement?.id || // local element is newer local.version > remote.version || // resolve conflicting edits deterministically by taking the one with diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index a609c0a0e..c7ec8d728 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -291,6 +291,7 @@ const restoreElement = ( // if empty text, mark as deleted. We keep in array // for data integrity purposes (collab etc.) if (!text && !element.isDeleted) { + // TODO: we should not do this since it breaks sync / versioning when we exchange / apply just deltas and restore the elements (deletion isn't recorded) element = { ...element, originalText: text, isDeleted: true }; element = bumpVersion(element); } diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 9d7be4e0d..50f08c4c2 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -543,13 +543,14 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "selectedElementIds": { "id4": true, }, - "selectedLinearElementId": "id4", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id4", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -1026,13 +1027,14 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "selectedElementIds": { "id4": true, }, - "selectedLinearElementId": "id4", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id4", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -2414,13 +2416,14 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "selectedElementIds": { "id4": true, }, - "selectedLinearElementId": "id4", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id4", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -7028,9 +7031,7 @@ exports[`history > multiplayer undo/redo > should iterate through the history wh "scrollX": 0, "scrollY": 0, "searchMatches": null, - "selectedElementIds": { - "id0": true, - }, + "selectedElementIds": {}, "selectedElementsAreBeingDragged": false, "selectedGroupIds": {}, "selectionElement": null, @@ -7131,74 +7132,70 @@ exports[`history > multiplayer undo/redo > should iterate through the history wh }, "elements": { "added": {}, - "removed": { - "id0": { - "deleted": { - "angle": 0, - "backgroundColor": "transparent", - "boundElements": null, - "customData": undefined, - "elbowed": false, - "endArrowhead": "arrow", - "endBinding": null, - "fillStyle": "solid", - "frameId": null, - "groupIds": [], - "height": 10, - "index": "a0", - "isDeleted": false, - "lastCommittedPoint": [ - 10, - 10, - ], - "link": null, - "locked": false, - "opacity": 100, - "points": [ - [ - 0, - 0, - ], - [ - 10, - 10, - ], - ], - "roughness": 1, - "roundness": { - "type": 2, - }, - "startArrowhead": null, - "startBinding": null, - "strokeColor": "#1e1e1e", - "strokeStyle": "solid", - "strokeWidth": 2, - "type": "arrow", - "version": 6, - "width": 10, - "x": 0, - "y": 0, + "removed": {}, + "updated": {}, + }, + "id": "id13", + }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": { + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, }, - "inserted": { - "isDeleted": true, - "version": 5, + }, + "inserted": { + "selectedLinearElement": null, + }, + }, + }, + "elements": { + "added": {}, + "removed": {}, + "updated": {}, + }, + "id": "id14", + }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": { + "selectedLinearElement": { + "elementId": "id0", + "isEditing": true, + }, + }, + "inserted": { + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, }, }, }, + }, + "elements": { + "added": {}, + "removed": {}, "updated": {}, }, - "id": "id2", + "id": "id15", }, { "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": true, + }, }, }, }, @@ -7207,43 +7204,7 @@ exports[`history > multiplayer undo/redo > should iterate through the history wh "removed": {}, "updated": {}, }, - "id": "id4", - }, - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": { - "selectedLinearElementIsEditing": true, - }, - "inserted": { - "selectedLinearElementIsEditing": false, - }, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": {}, - }, - "id": "id6", - }, - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": { - "selectedLinearElementIsEditing": false, - }, - "inserted": { - "selectedLinearElementIsEditing": true, - }, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": {}, - }, - "id": "id10", + "id": "id16", }, ] `; @@ -10210,12 +10171,13 @@ exports[`history > multiplayer undo/redo > should override remotely added points "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -15770,13 +15732,14 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -16064,15 +16027,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -16688,15 +16652,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -17320,15 +17285,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -18017,15 +17983,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -18132,15 +18099,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, }, }, @@ -18744,15 +18712,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -18859,15 +18828,16 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { "selectedElementIds": { "id13": true, }, - "selectedLinearElementId": "id13", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id13", + "isEditing": false, + }, }, }, }, @@ -20656,12 +20626,13 @@ exports[`history > singleplayer undo/redo > should support linear element creati "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -20676,10 +20647,16 @@ exports[`history > singleplayer undo/redo > should support linear element creati "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementIsEditing": true, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": true, + }, }, "inserted": { - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, }, }, @@ -20747,10 +20724,16 @@ exports[`history > singleplayer undo/redo > should support linear element creati "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementIsEditing": true, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": true, + }, }, }, }, diff --git a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap index f22cfd28c..a895eb636 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -6394,15 +6394,16 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "selectedElementIds": { "id9": true, }, - "selectedLinearElementId": "id9", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id9", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id6": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -6470,13 +6471,19 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "selectedElementIds": { "id12": true, }, - "selectedLinearElementId": "id12", + "selectedLinearElement": { + "elementId": "id12", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": { "id9": true, }, - "selectedLinearElementId": "id9", + "selectedLinearElement": { + "elementId": "id9", + "isEditing": false, + }, }, }, }, @@ -6542,15 +6549,16 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "selectedElementIds": { "id15": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { "selectedElementIds": { "id12": true, }, - "selectedLinearElementId": "id12", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id12", + "isEditing": false, + }, }, }, }, @@ -6677,12 +6685,13 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": "id15", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id15", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -6700,15 +6709,16 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "selectedElementIds": { "id22": true, }, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { "selectedElementIds": { "id15": true, }, - "selectedLinearElementId": "id15", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id15", + "isEditing": false, + }, }, }, }, @@ -6833,12 +6843,13 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": "id22", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id22", + "isEditing": false, + }, }, "inserted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -6873,12 +6884,13 @@ exports[`regression tests > draw every type of shape > [end of test] undo stack "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { - "selectedLinearElementId": "id22", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id22", + "isEditing": false, + }, }, }, }, @@ -8719,13 +8731,14 @@ exports[`regression tests > key 5 selects arrow tool > [end of test] undo stack "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -8946,13 +8959,14 @@ exports[`regression tests > key 6 selects line tool > [end of test] undo stack 1 "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -9365,13 +9379,14 @@ exports[`regression tests > key a selects arrow tool > [end of test] undo stack "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -9770,13 +9785,14 @@ exports[`regression tests > key l selects line tool > [end of test] undo stack 1 "selectedElementIds": { "id0": true, }, - "selectedLinearElementId": "id0", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id0", + "isEditing": false, + }, }, "inserted": { "selectedElementIds": {}, - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, }, }, @@ -14494,12 +14510,13 @@ exports[`regression tests > undo/redo drawing an element > [end of test] redo st "appState": AppStateDelta { "delta": Delta { "deleted": { - "selectedLinearElementId": null, - "selectedLinearElementIsEditing": null, + "selectedLinearElement": null, }, "inserted": { - "selectedLinearElementId": "id6", - "selectedLinearElementIsEditing": false, + "selectedLinearElement": { + "elementId": "id6", + "isEditing": false, + }, }, }, }, diff --git a/packages/excalidraw/tests/history.test.tsx b/packages/excalidraw/tests/history.test.tsx index a0953b8d4..707fe4e48 100644 --- a/packages/excalidraw/tests/history.test.tsx +++ b/packages/excalidraw/tests/history.test.tsx @@ -3043,15 +3043,14 @@ describe("history", () => { }); Keyboard.undo(); - expect(API.getUndoStack().length).toBe(3); - expect(API.getRedoStack().length).toBe(1); - expect(h.state.selectedLinearElement).not.toBeNull(); - expect(h.state.selectedLinearElement?.isEditing).toBe(true); + expect(API.getUndoStack().length).toBe(0); + expect(API.getRedoStack().length).toBe(4); + expect(h.state.selectedLinearElement).toBeNull(); Keyboard.redo(); expect(API.getUndoStack().length).toBe(4); expect(API.getRedoStack().length).toBe(0); - expect(h.state.selectedLinearElement).not.toBeNull(); + expect(h.state.selectedLinearElement).toBeNull(); expect(h.state.selectedLinearElement?.isEditing ?? false).toBe(false); }); diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index c265d7ca6..e321b34cb 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -248,8 +248,10 @@ export type ObservedElementsAppState = { editingGroupId: AppState["editingGroupId"]; selectedElementIds: AppState["selectedElementIds"]; selectedGroupIds: AppState["selectedGroupIds"]; - selectedLinearElementId: LinearElementEditor["elementId"] | null; - selectedLinearElementIsEditing: boolean | null; + selectedLinearElement: { + elementId: LinearElementEditor["elementId"]; + isEditing: boolean; + } | null; croppingElementId: AppState["croppingElementId"]; lockedMultiSelections: AppState["lockedMultiSelections"]; activeLockedId: AppState["activeLockedId"];