Bladeren bron

rewrite wysiwyg property updating (#1387)

* rewrite wysiwyg property updating

* reuse existing class

* fix case of focus being stolen by other UIs

* revert mistake csp removal

* ensure we don't run cleanup twice

* fix opacity updating

* add shape actions menu class to constants
David Luzar 5 jaren geleden
bovenliggende
commit
6771b505ad

+ 0 - 1
src/appState.ts

@@ -8,7 +8,6 @@ export const DEFAULT_TEXT_ALIGN = "left";
 
 export function getDefaultAppState(): AppState {
   return {
-    wysiwygElement: null,
     isLoading: false,
     errorMessage: null,
     draggingElement: null,

+ 3 - 6
src/components/App.tsx

@@ -1228,7 +1228,8 @@ class App extends React.Component<any, AppState> {
       ]);
     };
 
-    const wysiwygElement = textWysiwyg({
+    textWysiwyg({
+      id: element.id,
       x,
       y,
       initText: element.text,
@@ -1248,7 +1249,6 @@ class App extends React.Component<any, AppState> {
       onSubmit: withBatchedUpdates((text) => {
         updateElement(text);
         this.setState((prevState) => ({
-          wysiwygElement: null,
           selectedElementIds: {
             ...prevState.selectedElementIds,
             [element.id]: true,
@@ -1269,7 +1269,7 @@ class App extends React.Component<any, AppState> {
       }),
     });
     // deselect all other elements when inserting text
-    this.setState({ selectedElementIds: {}, wysiwygElement });
+    this.setState({ selectedElementIds: {} });
 
     // do an initial update to re-initialize element position since we were
     //  modifying element's x/y for sake of editor (case: syncing to remote)
@@ -1579,9 +1579,6 @@ class App extends React.Component<any, AppState> {
   private handleCanvasPointerDown = (
     event: React.PointerEvent<HTMLCanvasElement>,
   ) => {
-    if (this.state.wysiwygElement && this.state.wysiwygElement.submit) {
-      this.state.wysiwygElement.submit();
-    }
     if (lastPointerUp !== null) {
       // Unfortunately, sometimes we don't get a pointerup after a pointerdown,
       // this can happen when a contextual menu or alert is triggered. In order to avoid

+ 2 - 1
src/components/LayerUI.tsx

@@ -26,6 +26,7 @@ import { ErrorDialog } from "./ErrorDialog";
 import { ShortcutsDialog } from "./ShortcutsDialog";
 import { LoadingMessage } from "./LoadingMessage";
 import { GitHubCorner } from "./GitHubCorner";
+import { CLASSES } from "../constants";
 
 interface LayerUIProps {
   actionManager: ActionManager;
@@ -146,7 +147,7 @@ export const LayerUI = React.memo(
               </Section>
               {showSelectedShapeActions(appState, elements) && (
                 <Section heading="selectedShapeActions">
-                  <Island className="App-menu__left" padding={4}>
+                  <Island className={CLASSES.SHAPE_ACTIONS_MENU} padding={4}>
                     <SelectedShapeActions
                       appState={appState}
                       elements={elements}

+ 4 - 0
src/constants.ts

@@ -53,3 +53,7 @@ export const BROADCAST = {
   SERVER_VOLATILE: "server-volatile-broadcast",
   SERVER: "server-broadcast",
 };
+
+export const CLASSES = {
+  SHAPE_ACTIONS_MENU: "App-menu__left",
+};

+ 64 - 11
src/element/textWysiwyg.tsx

@@ -1,6 +1,8 @@
 import { KEYS } from "../keys";
-import { selectNode } from "../utils";
-import { WysiwigElement } from "./types";
+import { selectNode, isWritableElement } from "../utils";
+import { globalSceneState } from "../scene";
+import { isTextElement } from "./typeChecks";
+import { CLASSES } from "../constants";
 
 function trimText(text: string) {
   // whitespace only → trim all because we'd end up inserting invisible element
@@ -14,6 +16,7 @@ function trimText(text: string) {
 }
 
 type TextWysiwygParams = {
+  id: string;
   initText: string;
   x: number;
   y: number;
@@ -29,6 +32,7 @@ type TextWysiwygParams = {
 };
 
 export function textWysiwyg({
+  id,
   initText,
   x,
   y,
@@ -41,7 +45,7 @@ export function textWysiwyg({
   textAlign,
   onSubmit,
   onCancel,
-}: TextWysiwygParams): WysiwigElement {
+}: TextWysiwygParams) {
   const editable = document.createElement("div");
   try {
     editable.contentEditable = "plaintext-only";
@@ -136,25 +140,74 @@ export function textWysiwyg({
   }
 
   function cleanup() {
+    if (isDestroyed) {
+      return;
+    }
+    isDestroyed = true;
     // remove events to ensure they don't late-fire
+    editable.onblur = null;
     editable.onpaste = null;
     editable.oninput = null;
     editable.onkeydown = null;
 
     window.removeEventListener("wheel", stopEvent, true);
+    window.removeEventListener("pointerdown", onPointerDown);
+    window.removeEventListener("pointerup", rebindBlur);
+    window.removeEventListener("blur", handleSubmit);
+
+    unbindUpdate();
+
     document.body.removeChild(editable);
   }
 
+  const rebindBlur = () => {
+    window.removeEventListener("pointerup", rebindBlur);
+    // deferred to guard against focus traps on various UIs that steal focus
+    //  upon pointerUp
+    setTimeout(() => {
+      editable.onblur = handleSubmit;
+      // case: clicking on the same property → no change → no update → no focus
+      editable.focus();
+    });
+  };
+
+  // prevent blur when changing properties from the menu
+  const onPointerDown = (event: MouseEvent) => {
+    if (
+      event.target instanceof HTMLElement &&
+      event.target.closest(CLASSES.SHAPE_ACTIONS_MENU) &&
+      !isWritableElement(event.target)
+    ) {
+      editable.onblur = null;
+      window.addEventListener("pointerup", rebindBlur);
+      // handle edge-case where pointerup doesn't fire e.g. due to user
+      //  alt-tabbing away
+      window.addEventListener("blur", handleSubmit);
+    }
+  };
+
+  // handle updates of textElement properties of editing element
+  const unbindUpdate = globalSceneState.addCallback(() => {
+    const editingElement = globalSceneState
+      .getElementsIncludingDeleted()
+      .find((element) => element.id === id);
+    if (editingElement && isTextElement(editingElement)) {
+      Object.assign(editable.style, {
+        font: editingElement.font,
+        textAlign: editingElement.textAlign,
+        color: editingElement.strokeColor,
+        opacity: editingElement.opacity / 100,
+      });
+    }
+    editable.focus();
+  });
+
+  let isDestroyed = false;
+
+  editable.onblur = handleSubmit;
+  window.addEventListener("pointerdown", onPointerDown);
   window.addEventListener("wheel", stopEvent, true);
   document.body.appendChild(editable);
   editable.focus();
   selectNode(editable);
-
-  return {
-    submit: handleSubmit,
-    changeStyle: (style: any) => {
-      Object.assign(editable.style, style);
-      editable.focus();
-    },
-  };
 }

+ 0 - 5
src/element/types.ts

@@ -68,8 +68,3 @@ export type ResizeArrowFnType = (
   pointerY: number,
   perfect: boolean,
 ) => void;
-
-export type WysiwigElement = {
-  submit: () => void;
-  changeStyle: (style: Record<string, any>) => void;
-};

+ 0 - 13
src/renderer/renderScene.ts

@@ -14,7 +14,6 @@ import {
   handlerRectangles,
   getCommonBounds,
   canResizeMutlipleElements,
-  isTextElement,
 } from "../element";
 
 import { roundRect } from "./roundRect";
@@ -104,18 +103,6 @@ export function renderScene(
     return { atLeastOneVisibleElement: false };
   }
 
-  if (
-    appState.wysiwygElement?.changeStyle &&
-    isTextElement(appState.editingElement)
-  ) {
-    appState.wysiwygElement.changeStyle({
-      font: appState.editingElement.font,
-      textAlign: appState.editingElement.textAlign,
-      color: appState.editingElement.strokeColor,
-      opacity: appState.editingElement.opacity,
-    });
-  }
-
   const context = canvas.getContext("2d")!;
   context.scale(scale, scale);
 

+ 0 - 41
src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -41,7 +41,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -241,7 +240,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -360,7 +358,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -636,7 +633,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -797,7 +793,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -998,7 +993,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -1258,7 +1252,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -1658,7 +1651,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2283,7 +2275,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2402,7 +2393,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2521,7 +2511,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2640,7 +2629,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2781,7 +2769,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -2922,7 +2909,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3063,7 +3049,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3182,7 +3167,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3301,7 +3285,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3442,7 +3425,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3561,7 +3543,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -3634,7 +3615,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -4520,7 +4500,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -4945,7 +4924,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -5277,7 +5255,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -5520,7 +5497,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -5694,7 +5670,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -6531,7 +6506,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -7259,7 +7233,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -7882,7 +7855,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -8405,7 +8377,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -8878,7 +8849,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -9256,7 +9226,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -9543,7 +9512,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -9759,7 +9727,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -10652,7 +10619,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -11434,7 +11400,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -12109,7 +12074,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -12677,7 +12641,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -13056,7 +13019,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -13113,7 +13075,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -13170,7 +13131,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;
@@ -13467,7 +13427,6 @@ Object {
   "showShortcutsDialog": false,
   "username": "",
   "viewBackgroundColor": "#ffffff",
-  "wysiwygElement": null,
   "zoom": 1,
 }
 `;

+ 0 - 2
src/types.ts

@@ -4,7 +4,6 @@ import {
   NonDeletedExcalidrawElement,
   NonDeleted,
   TextAlign,
-  WysiwigElement,
 } from "./element/types";
 import { SHAPES } from "./shapes";
 import { Point as RoughPoint } from "roughjs/bin/geometry";
@@ -14,7 +13,6 @@ export type FlooredNumber = number & { _brand: "FlooredNumber" };
 export type Point = Readonly<RoughPoint>;
 
 export type AppState = {
-  wysiwygElement: WysiwigElement | null;
   isLoading: boolean;
   errorMessage: string | null;
   draggingElement: NonDeletedExcalidrawElement | null;