sambruk/excalidraw
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: stale appState.pendingImageElement (#5322)

Browse Source 
* fix: stale `appState.pendingImageElement`

* unrelated fix for devTools race conditions

* snap fix
This commit is contained in:
David Luzar 2022-06-19 14:13:43 +02:00 committed by GitHub
parent fd48c2cf79
commit 4712393b62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 107 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/actions/actionFinalize.tsx
View File

@ -19,7 +19,7 @@ import { AppState } from "../types";
export const actionFinalize = register({
name: "finalize",
trackEvent: false,
perform: (elements, appState, _, { canvas, focusContainer }) => {
perform: (elements, appState, _, { canvas, focusContainer, scene }) => {
if (appState.editingLinearElement) {
const { elementId, startBindingElement, endBindingElement } =
appState.editingLinearElement;
@ -50,8 +50,12 @@ export const actionFinalize = register({
let newElements = elements;
if (appState.pendingImageElement) {
mutateElement(appState.pendingImageElement, { isDeleted: true }, false);
const pendingImageElement =
appState.pendingImageElementId &&
scene.getElement(appState.pendingImageElementId);
if (pendingImageElement) {
mutateElement(pendingImageElement, { isDeleted: true }, false);
}
if (window.document.activeElement instanceof HTMLElement) {
@ -177,7 +181,7 @@ export const actionFinalize = register({
[multiPointElement.id]: true,
}
: appState.selectedElementIds,
pendingImageElement: null,
pendingImageElementId: null,
},
commitToHistory: appState.activeTool.type === "freedraw",
};

4
src/appState.ts
View File

@ -87,7 +87,7 @@ export const getDefaultAppState = (): Omit<
value: 1 as NormalizedZoomValue,
},
viewModeEnabled: false,
pendingImageElement: null,
pendingImageElementId: null,
showHyperlinkPopup: false,
};
};
@ -177,7 +177,7 @@ const APP_STATE_STORAGE_CONF = (<
zenModeEnabled: { browser: true, export: false, server: false },
zoom: { browser: true, export: false, server: false },
viewModeEnabled: { browser: false, export: false, server: false },
pendingImageElement: { browser: false, export: false, server: false },
pendingImageElementId: { browser: false, export: false, server: false },
showHyperlinkPopup: { browser: false, export: false, server: false },
});

36
src/components/App.tsx
View File

@ -309,7 +309,7 @@ class App extends React.Component<AppProps, AppState> {
UIOptions: DEFAULT_UI_OPTIONS,
};
private scene: Scene;
public scene: Scene;
private resizeObserver: ResizeObserver | undefined;
private nearestScrollableContainer: HTMLElement | Document | undefined;
public library: AppClassProperties["library"];
@ -1141,8 +1141,7 @@ class App extends React.Component<AppProps, AppState> {
if (isImageElement(element)) {
if (
// not placed on canvas yet (but in elements array)
this.state.pendingImageElement &&
element.id === this.state.pendingImageElement.id
this.state.pendingImageElementId === element.id
) {
return false;
}
@ -3002,19 +3001,24 @@ class App extends React.Component<AppProps, AppState> {
// reset image preview on pointerdown
setCursor(this.canvas, CURSOR_TYPE.CROSSHAIR);
if (!this.state.pendingImageElement) {
// retrieve the latest element as the state may be stale
const pendingImageElement =
this.state.pendingImageElementId &&
this.scene.getElement(this.state.pendingImageElementId);
if (!pendingImageElement) {
return;
}
this.setState({
draggingElement: this.state.pendingImageElement,
editingElement: this.state.pendingImageElement,
pendingImageElement: null,
draggingElement: pendingImageElement,
editingElement: pendingImageElement,
pendingImageElementId: null,
multiElement: null,
});
const { x, y } = viewportCoordsToSceneCoords(event, this.state);
mutateElement(this.state.pendingImageElement, {
mutateElement(pendingImageElement, {
x,
y,
});
@ -4330,8 +4334,8 @@ class App extends React.Component<AppProps, AppState> {
pointerDownState.eventListeners.onKeyUp!,
);
if (this.state.pendingImageElement) {
this.setState({ pendingImageElement: null });
if (this.state.pendingImageElementId) {
this.setState({ pendingImageElementId: null });
}
if (draggingElement?.type === "freedraw") {
@ -4819,7 +4823,7 @@ class App extends React.Component<AppProps, AppState> {
await cachedImageData.image;
}
if (
this.state.pendingImageElement?.id !== imageElement.id &&
this.state.pendingImageElementId !== imageElement.id &&
this.state.draggingElement?.id !== imageElement.id
) {
this.initializeImageDimensions(imageElement, true);
@ -4901,7 +4905,7 @@ class App extends React.Component<AppProps, AppState> {
previewDataURL = canvas.toDataURL(MIME_TYPES.svg) as DataURL;
}
if (this.state.pendingImageElement) {
if (this.state.pendingImageElementId) {
setCursor(this.canvas, `url(${previewDataURL}) 4 4, auto`);
}
};
@ -4942,7 +4946,7 @@ class App extends React.Component<AppProps, AppState> {
} else {
this.setState(
{
pendingImageElement: imageElement,
pendingImageElementId: imageElement.id,
},
() => {
this.insertImageElement(
@ -4961,7 +4965,7 @@ class App extends React.Component<AppProps, AppState> {
}
this.setState(
{
pendingImageElement: null,
pendingImageElementId: null,
editingElement: null,
activeTool: updateActiveTool(this.state, { type: "selection" }),
},
@ -5881,10 +5885,10 @@ if (
elements: {
configurable: true,
get() {
return this.app.scene.getElementsIncludingDeleted();
return this.app?.scene.getElementsIncludingDeleted();
},
set(elements: ExcalidrawElement[]) {
return this.app.scene.replaceAllElements(elements);
return this.app?.scene.replaceAllElements(elements);
},
},
});

2
src/components/HintViewer.tsx
View File

@ -45,7 +45,7 @@ const getHints = ({ appState, elements, isMobile }: HintViewerProps) => {
return t("hints.text");
}
if (appState.activeTool.type === "image" && appState.pendingImageElement) {
if (appState.activeTool.type === "image" && appState.pendingImageElementId) {
return t("hints.placeImage");
}

11
src/excalidraw-app/index.tsx
View File

@ -479,19 +479,17 @@ const ExcalidrawWrapper = () => {
if (excalidrawAPI) {
let didChange = false;
let pendingImageElement = appState.pendingImageElement;
const elements = excalidrawAPI
.getSceneElementsIncludingDeleted()
.map((element) => {
if (
LocalData.fileStorage.shouldUpdateImageElementStatus(element)
) {
const newElement = newElementWith(element, { status: "saved" });
if (newElement !== element) {
didChange = true;
const newEl = newElementWith(element, { status: "saved" });
if (pendingImageElement === element) {
pendingImageElement = newEl;
}
return newEl;
return newElement;
}
return element;
});
@ -499,9 +497,6 @@ const ExcalidrawWrapper = () => {
if (didChange) {
excalidrawAPI.updateScene({
elements,
appState: {
pendingImageElement,
},
});
}
}

34
src/tests/__snapshots__/contextmenu.test.tsx.snap
View File

@ -55,7 +55,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -228,7 +228,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -405,7 +405,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -743,7 +743,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -1081,7 +1081,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -1258,7 +1258,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -1471,7 +1471,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -1743,7 +1743,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id1": true,
},
@ -2099,7 +2099,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -2899,7 +2899,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -3237,7 +3237,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -3575,7 +3575,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id1": true,
},
@ -3993,7 +3993,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -4271,7 +4271,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -4630,7 +4630,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -4736,7 +4736,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -4820,7 +4820,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,

104
src/tests/__snapshots__/regressionTests.test.tsx.snap
View File

@ -55,7 +55,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -564,7 +564,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -1079,7 +1079,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -1939,7 +1939,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -2160,7 +2160,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id3": true,
@ -2666,7 +2666,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -2942,7 +2942,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -3119,7 +3119,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id2": true,
},
@ -3608,7 +3608,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -3865,7 +3865,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -4086,7 +4086,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -4351,7 +4351,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id1": true,
},
@ -4626,7 +4626,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id2": true,
},
@ -5048,7 +5048,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -5372,7 +5372,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -5671,7 +5671,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -5898,7 +5898,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -6075,7 +6075,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -6576,7 +6576,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -6924,7 +6924,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -9163,7 +9163,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -9561,7 +9561,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -9837,7 +9837,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -10074,7 +10074,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id2": true,
@ -10380,7 +10380,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -10557,7 +10557,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -10734,7 +10734,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -10911,7 +10911,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -11118,7 +11118,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -11325,7 +11325,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -11550,7 +11550,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -11757,7 +11757,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -11934,7 +11934,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -12141,7 +12141,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -12318,7 +12318,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -12495,7 +12495,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -12720,7 +12720,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -13514,7 +13514,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id3": true,
@ -13790,7 +13790,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": -2.916666666666668,
@ -13898,7 +13898,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -14004,7 +14004,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -14184,7 +14184,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -14535,7 +14535,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -14752,7 +14752,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
"id1": true,
@ -15664,7 +15664,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 60,
@ -15770,7 +15770,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id0": true,
},
@ -16609,7 +16609,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id1": true,
"id2": true,
@ -17056,7 +17056,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {
"id1": true,
},
@ -17355,7 +17355,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 10,
@ -17463,7 +17463,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -18007,7 +18007,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,
@ -18113,7 +18113,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,

2
src/tests/packages/__snapshots__/utils.test.ts.snap
View File

@ -53,7 +53,7 @@ Object {
},
"penDetected": false,
"penMode": false,
"pendingImageElement": null,
"pendingImageElementId": null,
"previousSelectedElementIds": Object {},
"resizingElement": null,
"scrollX": 0,

3
src/types.ts
View File

@ -176,7 +176,7 @@ export type AppState = {
data: Spreadsheet;
};
/** imageElement waiting to be placed on canvas */
pendingImageElement: NonDeleted<ExcalidrawImageElement> | null;
pendingImageElementId: ExcalidrawImageElement["id"] | null;
showHyperlinkPopup: false | "info" | "editor";
};
@ -378,6 +378,7 @@ export type AppClassProperties = {
>;
files: BinaryFiles;
deviceType: App["deviceType"];
scene: App["scene"];
};
export type PointerDownState = Readonly<{