marcel/excalidraw
1
0
Fork 0
mirror of https://github.com/excalidraw/excalidraw.git synced 2025-09-23 17:30:44 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: editing linear element (#9839)

Browse Source
This commit is contained in:
Marcel Mraz
2025-08-08 09:30:11 +02:00
committed by GitHub
parent df25de7e68
commit 9036812b6d
9 changed files with 264 additions and 308 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
packages/excalidraw/data/reconcile.ts
View File

@@ -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

1
packages/excalidraw/data/restore.ts
View File

@@ -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);
}

285
packages/excalidraw/tests/__snapshots__/history.test.tsx.snap
View File

@@ -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,
},
},
},
},

109
packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap
View File

@@ -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,
},
},
},
},

9
packages/excalidraw/tests/history.test.tsx
View File

@@ -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);
});

6
packages/excalidraw/types.ts
View File

@@ -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"];