From 865328457f3f085e4848bf39a20a110a7a153f5e Mon Sep 17 00:00:00 2001 From: dwelle <5153846+dwelle@users.noreply.github.com> Date: Wed, 30 Apr 2025 13:50:46 +0200 Subject: [PATCH] fix popup component lifecycle --- .../actions/actionToggleShapeSwitch.tsx | 2 - packages/excalidraw/components/App.tsx | 24 +++++++---- .../excalidraw/components/ShapeSwitch.tsx | 40 +++++++++---------- packages/excalidraw/editor-jotai.ts | 9 ++++- packages/excalidraw/types.ts | 1 + 5 files changed, 43 insertions(+), 33 deletions(-) diff --git a/packages/excalidraw/actions/actionToggleShapeSwitch.tsx b/packages/excalidraw/actions/actionToggleShapeSwitch.tsx index 13e99ca24..be27960d0 100644 --- a/packages/excalidraw/actions/actionToggleShapeSwitch.tsx +++ b/packages/excalidraw/actions/actionToggleShapeSwitch.tsx @@ -25,8 +25,6 @@ export const actionToggleShapeSwitch = register({ }); return { - appState, - commitToHistory: false, captureUpdate: CaptureUpdateAction.NEVER, }; }, diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 19b611f77..21c84b97e 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -431,7 +431,7 @@ import { } from "../components/hyperlink/Hyperlink"; import { Fonts } from "../fonts"; -import { editorJotaiStore } from "../editor-jotai"; +import { editorJotaiStore, type WritableAtom } from "../editor-jotai"; import { ImageSceneDataError } from "../errors"; import { getSnapLinesAtPointer, @@ -821,6 +821,15 @@ class App extends React.Component { ); } + updateEditorAtom = ( + atom: WritableAtom, + ...args: Args + ): Result => { + const result = editorJotaiStore.set(atom, ...args); + this.triggerRender(); + return result; + }; + private onWindowMessage(event: MessageEvent) { if ( event.origin !== "https://player.vimeo.com" && @@ -2148,7 +2157,7 @@ class App extends React.Component { }; private openEyeDropper = ({ type }: { type: "stroke" | "background" }) => { - editorJotaiStore.set(activeEyeDropperAtom, { + this.updateEditorAtom(activeEyeDropperAtom, { swapPreviewOnAlt: true, colorPickerType: type === "stroke" ? "elementStroke" : "elementBackground", @@ -4169,7 +4178,7 @@ class App extends React.Component { // Shape switching if (event.key === KEYS.ESCAPE) { - editorJotaiStore.set(shapeSwitchAtom, null); + this.updateEditorAtom(shapeSwitchAtom, null); } else if ( event.key === KEYS.TAB && (document.activeElement === this.excalidrawContainerRef?.current || @@ -4193,10 +4202,9 @@ class App extends React.Component { } } if (switchCategory) { - editorJotaiStore.set(shapeSwitchAtom, { + this.updateEditorAtom(shapeSwitchAtom, { type: "panel", }); - this.triggerRender(); } } @@ -4658,7 +4666,7 @@ class App extends React.Component { event[KEYS.CTRL_OR_CMD] && (event.key === KEYS.BACKSPACE || event.key === KEYS.DELETE) ) { - editorJotaiStore.set(activeConfirmDialogAtom, "clearCanvas"); + this.updateEditorAtom(activeConfirmDialogAtom, "clearCanvas"); } // eye dropper @@ -6407,11 +6415,11 @@ class App extends React.Component { focus: false, })), })); - editorJotaiStore.set(searchItemInFocusAtom, null); + this.updateEditorAtom(searchItemInFocusAtom, null); } if (editorJotaiStore.get(shapeSwitchAtom)) { - editorJotaiStore.set(shapeSwitchAtom, null); + this.updateEditorAtom(shapeSwitchAtom, null); } // since contextMenu options are potentially evaluated on each render, diff --git a/packages/excalidraw/components/ShapeSwitch.tsx b/packages/excalidraw/components/ShapeSwitch.tsx index 4ab65bc1d..344090bba 100644 --- a/packages/excalidraw/components/ShapeSwitch.tsx +++ b/packages/excalidraw/components/ShapeSwitch.tsx @@ -68,7 +68,7 @@ import { sceneCoordsToViewportCoords, } from ".."; import { trackEvent } from "../analytics"; -import { atom, editorJotaiStore, useAtom } from "../editor-jotai"; +import { atom, editorJotaiStore, useSetAtom } from "../editor-jotai"; import "./ShapeSwitch.scss"; import { ToolButton } from "./ToolButton"; @@ -94,6 +94,7 @@ export const shapeSwitchAtom = atom<{ type: "panel"; } | null>(null); +// NOTE doesn't need to be an atom. Review once we integrate with properties panel. export const shapeSwitchFontSizeAtom = atom<{ [id: string]: { fontSize: number; @@ -101,6 +102,7 @@ export const shapeSwitchFontSizeAtom = atom<{ }; } | null>(null); +// NOTE doesn't need to be an atom. Review once we integrate with properties panel. export const shapeSwitchLinearAtom = atom<{ [id: string]: { properties: @@ -111,18 +113,19 @@ export const shapeSwitchLinearAtom = atom<{ } | null>(null); const ShapeSwitch = ({ app }: { app: App }) => { - const [shapeSwitch, setShapeSwitch] = useAtom(shapeSwitchAtom); - const [, setShapeSwitchFontSize] = useAtom(shapeSwitchFontSizeAtom); - const [, setShapeSwitchLinear] = useAtom(shapeSwitchLinearAtom); + const setShapeSwitchFontSize = useSetAtom(shapeSwitchFontSizeAtom); + const setShapeSwitchLinear = useSetAtom(shapeSwitchLinearAtom); - const selectedElements = useMemo( - () => app.scene.getSelectedElements(app.state), - [app.scene, app.state], - ); + const selectedElements = app.scene.getSelectedElements(app.state); const elementsCategoryRef = useRef(null); // close shape switch panel if selecting different "types" of elements useEffect(() => { + if (selectedElements.length === 0) { + app.updateEditorAtom(shapeSwitchAtom, null); + return; + } + const switchCategory = getSwitchCategoryFromElements(selectedElements); if (switchCategory && !elementsCategoryRef.current) { @@ -132,22 +135,17 @@ const ShapeSwitch = ({ app }: { app: App }) => { (elementsCategoryRef.current && switchCategory !== elementsCategoryRef.current) ) { - setShapeSwitch(null); + app.updateEditorAtom(shapeSwitchAtom, null); elementsCategoryRef.current = null; } - }, [selectedElements, app.state.selectedElementIds, setShapeSwitch]); + }, [selectedElements, app]); - // clear if not active - if (!shapeSwitch) { - setShapeSwitchFontSize(null); - setShapeSwitchLinear(null); - return null; - } - - if (selectedElements.length === 0) { - setShapeSwitch(null); - return null; - } + useEffect(() => { + return () => { + setShapeSwitchFontSize(null); + setShapeSwitchLinear(null); + }; + }, [setShapeSwitchFontSize, setShapeSwitchLinear]); return ; }; diff --git a/packages/excalidraw/editor-jotai.ts b/packages/excalidraw/editor-jotai.ts index 28bc69306..464accfb3 100644 --- a/packages/excalidraw/editor-jotai.ts +++ b/packages/excalidraw/editor-jotai.ts @@ -1,10 +1,15 @@ // eslint-disable-next-line no-restricted-imports -import { atom, createStore, type PrimitiveAtom } from "jotai"; +import { + atom, + createStore, + type PrimitiveAtom, + type WritableAtom, +} from "jotai"; import { createIsolation } from "jotai-scope"; const jotai = createIsolation(); -export { atom, PrimitiveAtom }; +export { atom, PrimitiveAtom, WritableAtom }; export const { useAtom, useSetAtom, useAtomValue, useStore } = jotai; export const EditorJotaiProvider: ReturnType< typeof createIsolation diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index ebc31029c..bd6e21fdc 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -714,6 +714,7 @@ export type AppClassProperties = { excalidrawContainerValue: App["excalidrawContainerValue"]; onPointerUpEmitter: App["onPointerUpEmitter"]; + updateEditorAtom: App["updateEditorAtom"]; }; export type PointerDownState = Readonly<{