Sync panel props to editing element (#470)

* ensure panel props are sync to editing elem

* ensure we don't create empty-text elements (fixes #468)

* remove dead code

Co-authored-by: Christopher Chedeau <vjeuxx@gmail.com>

src/actions/actionProperties.tsx

@@ -1,10 +1,11 @@
 import React from "react";
 import { Action } from "./types";
 import { ExcalidrawElement, ExcalidrawTextElement } from "../element/types";
-import { getSelectedAttribute } from "../scene";
+import { getCommonAttributeOfSelectedElements } from "../scene";
 import { ButtonSelect } from "../components/ButtonSelect";
 import { isTextElement, redrawTextBoundingBox } from "../element";
 import { ColorPicker } from "../components/ColorPicker";
+import { AppState } from "../../src/types";
 
 const changeProperty = (
   elements: readonly ExcalidrawElement[],
@@ -18,6 +19,20 @@ const changeProperty = (
   });
 };
 
+const getFormValue = function<T>(
+  editingElement: AppState["editingElement"],
+  elements: readonly ExcalidrawElement[],
+  getAttribute: (element: ExcalidrawElement) => T,
+  defaultValue?: T
+): T | null {
+  return (
+    (editingElement && getAttribute(editingElement)) ||
+    getCommonAttributeOfSelectedElements(elements, getAttribute) ||
+    defaultValue ||
+    null
+  );
+};
+
 export const actionChangeStrokeColor: Action = {
   name: "changeStrokeColor",
   perform: (elements, appState, value) => {
@@ -30,21 +45,21 @@ export const actionChangeStrokeColor: Action = {
       appState: { ...appState, currentItemStrokeColor: value }
     };
   },
-  PanelComponent: ({ elements, appState, updateData, t }) => {
+  PanelComponent: ({ elements, appState, updateData, t }) => (
-    return (
     <>
       <h5>{t("labels.stroke")}</h5>
       <ColorPicker
         type="elementStroke"
-          color={
+        color={getFormValue(
-            getSelectedAttribute(elements, element => element.strokeColor) ||
+          appState.editingElement,
+          elements,
+          element => element.strokeColor,
           appState.currentItemStrokeColor
-          }
+        )}
         onChange={updateData}
       />
     </>
-    );
+  )
-  }
 };
 
 export const actionChangeBackgroundColor: Action = {
@@ -64,10 +79,12 @@ export const actionChangeBackgroundColor: Action = {
       <h5>{t("labels.background")}</h5>
       <ColorPicker
         type="elementBackground"
-        color={
+        color={getFormValue(
-          getSelectedAttribute(elements, element => element.backgroundColor) ||
+          appState.editingElement,
+          elements,
+          element => element.backgroundColor,
           appState.currentItemBackgroundColor
-        }
+        )}
         onChange={updateData}
       />
     </>
@@ -85,7 +102,7 @@ export const actionChangeFillStyle: Action = {
       }))
     };
   },
-  PanelComponent: ({ elements, updateData, t }) => (
+  PanelComponent: ({ elements, appState, updateData, t }) => (
     <>
       <h5>{t("labels.fill")}</h5>
       <ButtonSelect
@@ -94,7 +111,11 @@ export const actionChangeFillStyle: Action = {
           { value: "hachure", text: "Hachure" },
           { value: "cross-hatch", text: "Cross-hatch" }
         ]}
-        value={getSelectedAttribute(elements, element => element.fillStyle)}
+        value={getFormValue(
+          appState.editingElement,
+          elements,
+          element => element.fillStyle
+        )}
         onChange={value => {
           updateData(value);
         }}
@@ -123,7 +144,11 @@ export const actionChangeStrokeWidth: Action = {
           { value: 2, text: "Bold" },
           { value: 4, text: "Extra Bold" }
         ]}
-        value={getSelectedAttribute(elements, element => element.strokeWidth)}
+        value={getFormValue(
+          appState.editingElement,
+          elements,
+          element => element.strokeWidth
+        )}
         onChange={value => updateData(value)}
       />
     </>
@@ -150,7 +175,11 @@ export const actionChangeSloppiness: Action = {
           { value: 1, text: "Artist" },
           { value: 3, text: "Cartoonist" }
         ]}
-        value={getSelectedAttribute(elements, element => element.roughness)}
+        value={getFormValue(
+          appState.editingElement,
+          elements,
+          element => element.roughness
+        )}
         onChange={value => updateData(value)}
       />
     </>
@@ -168,7 +197,7 @@ export const actionChangeOpacity: Action = {
       }))
     };
   },
-  PanelComponent: ({ elements, updateData, t }) => (
+  PanelComponent: ({ elements, appState, updateData, t }) => (
     <>
       <h5>{t("labels.oppacity")}</h5>
       <input
@@ -177,8 +206,12 @@ export const actionChangeOpacity: Action = {
         max="100"
         onChange={e => updateData(+e.target.value)}
         value={
-          getSelectedAttribute(elements, element => element.opacity) ||
+          getFormValue(
-          0 /* Put the opacity at 0 if there are two conflicting ones */
+            appState.editingElement,
+            elements,
+            element => element.opacity,
+            100 /* default opacity */
+          ) || undefined
         }
       />
     </>
@@ -204,7 +237,7 @@ export const actionChangeFontSize: Action = {
       })
     };
   },
-  PanelComponent: ({ elements, updateData, t }) => (
+  PanelComponent: ({ elements, appState, updateData, t }) => (
     <>
       <h5>{t("labels.fontSize")}</h5>
       <ButtonSelect
@@ -214,7 +247,8 @@ export const actionChangeFontSize: Action = {
           { value: 28, text: "Large" },
           { value: 36, text: "Very Large" }
         ]}
-        value={getSelectedAttribute(
+        value={getFormValue(
+          appState.editingElement,
           elements,
           element => isTextElement(element) && +element.font.split("px ")[0]
         )}
@@ -243,7 +277,7 @@ export const actionChangeFontFamily: Action = {
       })
     };
   },
-  PanelComponent: ({ elements, updateData, t }) => (
+  PanelComponent: ({ elements, appState, updateData, t }) => (
     <>
       <h5>{t("labels.fontFamily")}</h5>
       <ButtonSelect
@@ -252,7 +286,8 @@ export const actionChangeFontFamily: Action = {
           { value: "Helvetica", text: t("labels.normal") },
           { value: "Cascadia", text: t("labels.code") }
         ]}
-        value={getSelectedAttribute(
+        value={getFormValue(
+          appState.editingElement,
           elements,
           element => isTextElement(element) && element.font.split("px ")[1]
         )}

src/components/ColorPicker.tsx

@@ -12,7 +12,7 @@ const Picker = function({
   onChange
 }: {
   colors: string[];
-  color: string | undefined;
+  color: string | null;
   onChange: (color: string) => void;
 }) {
   return (
@@ -55,7 +55,7 @@ function ColorInput({
   color,
   onChange
 }: {
-  color: string | undefined;
+  color: string | null;
   onChange: (color: string) => void;
 }) {
   const colorRegex = /^([0-9a-f]{3}|[0-9a-f]{6}|[0-9a-f]{8}|transparent)$/;
@@ -93,7 +93,7 @@ export function ColorPicker({
   onChange
 }: {
   type: "canvasBackground" | "elementBackground" | "elementStroke";
-  color: string | undefined;
+  color: string | null;
   onChange: (color: string) => void;
 }) {
   const [isActive, setActive] = React.useState(false);
@@ -119,7 +119,7 @@ export function ColorPicker({
           <Popover onCloseRequest={() => setActive(false)}>
             <Picker
               colors={colors[type]}
-              color={color || undefined}
+              color={color || null}
               onChange={changedColor => {
                 onChange(changedColor);
               }}

src/element/index.ts

@@ -1,4 +1,4 @@
-export { newElement, duplicateElement } from "./newElement";
+export { newElement, newTextElement, duplicateElement } from "./newElement";
 export {
   getElementAbsoluteCoords,
   getDiamondPoints,

src/element/newElement.ts

@@ -2,6 +2,9 @@ import { randomSeed } from "roughjs/bin/math";
 import nanoid from "nanoid";
 import { Drawable } from "roughjs/bin/core";
 
+import { ExcalidrawElement, ExcalidrawTextElement } from "../element/types";
+import { measureText } from "../utils";
+
 export function newElement(
   type: string,
   x: number,
@@ -35,6 +38,28 @@ export function newElement(
   return element;
 }
 
+export function newTextElement(
+  element: ExcalidrawElement,
+  text: string,
+  font: string
+) {
+  const metrics = measureText(text, font);
+  const textElement: ExcalidrawTextElement = {
+    ...element,
+    type: "text",
+    text: text,
+    font: font,
+    // Center the text
+    x: element.x - metrics.width / 2,
+    y: element.y - metrics.height / 2,
+    width: metrics.width,
+    height: metrics.height,
+    baseline: metrics.baseline
+  };
+
+  return textElement;
+}
+
 export function duplicateElement(element: ReturnType<typeof newElement>) {
   const copy = { ...element };
   delete copy.shape;

src/index.tsx

@@ -6,6 +6,7 @@ import { RoughCanvas } from "roughjs/bin/canvas";
 
 import {
   newElement,
+  newTextElement,
   duplicateElement,
   resizeTest,
   isInvisiblySmallElement,
@@ -33,9 +34,9 @@ import {
 
 import { renderScene } from "./renderer";
 import { AppState } from "./types";
-import { ExcalidrawElement, ExcalidrawTextElement } from "./element/types";
+import { ExcalidrawElement } from "./element/types";
 
-import { isInputLike, measureText, debounce, capitalizeString } from "./utils";
+import { isInputLike, debounce, capitalizeString } from "./utils";
 import { KEYS, isArrowKey } from "./keys";
 
 import { findShapeByKey, shapesShortcutKeys, SHAPES } from "./shapes";
@@ -90,29 +91,6 @@ function resetCursor() {
   document.documentElement.style.cursor = "";
 }
 
-function addTextElement(
-  element: ExcalidrawTextElement,
-  text: string,
-  font: string
-) {
-  resetCursor();
-  if (text === null || text === "") {
-    return false;
-  }
-
-  const metrics = measureText(text, font);
-  element.text = text;
-  element.font = font;
-  // Center the text
-  element.x -= metrics.width / 2;
-  element.y -= metrics.height / 2;
-  element.width = metrics.width;
-  element.height = metrics.height;
-  element.baseline = metrics.baseline;
-
-  return true;
-}
-
 const ELEMENT_SHIFT_TRANSLATE_AMOUNT = 5;
 const ELEMENT_TRANSLATE_AMOUNT = 1;
 const TEXT_TO_CENTER_SNAP_THRESHOLD = 30;
@@ -754,7 +732,7 @@ export class App extends React.Component<any, AppState> {
 
             const { x, y } = viewportCoordsToSceneCoords(e, this.state);
 
-            const element = newElement(
+            let element = newElement(
               this.state.elementType,
               x,
               y,
@@ -766,6 +744,10 @@ export class App extends React.Component<any, AppState> {
               100
             );
 
+            if (isTextElement(element)) {
+              element = newTextElement(element, "", this.state.currentItemFont);
+            }
+
             type ResizeTestType = ReturnType<typeof resizeTest>;
             let resizeHandle: ResizeTestType = false;
             let isResizingElements = false;
@@ -851,8 +833,19 @@ export class App extends React.Component<any, AppState> {
                 strokeColor: this.state.currentItemStrokeColor,
                 font: this.state.currentItemFont,
                 onSubmit: text => {
-                  addTextElement(element, text, this.state.currentItemFont);
+                  if (text) {
-                  elements = [...elements, { ...element, isSelected: true }];
+                    elements = [
+                      ...elements,
+                      {
+                        ...newTextElement(
+                          element,
+                          text,
+                          this.state.currentItemFont
+                        ),
+                        isSelected: true
+                      }
+                    ];
+                  }
                   this.setState({
                     draggingElement: null,
                     editingElement: null,
@@ -867,16 +860,8 @@ export class App extends React.Component<any, AppState> {
               return;
             }
 
-            if (this.state.elementType === "text") {
-              elements = [...elements, { ...element, isSelected: true }];
-              this.setState({
-                draggingElement: null,
-                elementType: "selection"
-              });
-            } else {
             elements = [...elements, element];
             this.setState({ draggingElement: element });
-            }
 
             let lastX = x;
             let lastY = y;
@@ -1142,7 +1127,11 @@ export class App extends React.Component<any, AppState> {
 
             const elementAtPosition = getElementAtPosition(elements, x, y);
 
-            const element = newElement(
+            const element =
+              elementAtPosition && isTextElement(elementAtPosition)
+                ? elementAtPosition
+                : newTextElement(
+                    newElement(
                       "text",
                       x,
                       y,
@@ -1152,11 +1141,13 @@ export class App extends React.Component<any, AppState> {
                       1,
                       1,
                       100
-            ) as ExcalidrawTextElement;
+                    ),
+                    "", // default text
+                    this.state.currentItemFont // default font
+                  );
 
             this.setState({ editingElement: element });
 
-            let initText = "";
             let textX = e.clientX;
             let textY = e.clientY;
 
@@ -1166,11 +1157,6 @@ export class App extends React.Component<any, AppState> {
               );
               this.forceUpdate();
 
-              Object.assign(element, elementAtPosition);
-              // x and y will change after calling addTextElement function
-              element.x = elementAtPosition.x + elementAtPosition.width / 2;
-              element.y = elementAtPosition.y + elementAtPosition.height / 2;
-              initText = elementAtPosition.text;
               textX =
                 this.state.scrollX +
                 elementAtPosition.x +
@@ -1181,6 +1167,10 @@ export class App extends React.Component<any, AppState> {
                 elementAtPosition.y +
                 CANVAS_WINDOW_OFFSET_TOP +
                 elementAtPosition.height / 2;
+
+              // x and y will change after calling newTextElement function
+              element.x = elementAtPosition.x + elementAtPosition.width / 2;
+              element.y = elementAtPosition.y + elementAtPosition.height / 2;
             } else if (!e.altKey) {
               const snappedToCenterPosition = this.getTextWysiwygSnappedToCenterPosition(
                 x,
@@ -1196,18 +1186,23 @@ export class App extends React.Component<any, AppState> {
             }
 
             textWysiwyg({
-              initText,
+              initText: element.text,
               x: textX,
               y: textY,
               strokeColor: element.strokeColor,
-              font: element.font || this.state.currentItemFont,
+              font: element.font,
               onSubmit: text => {
-                addTextElement(
+                if (text) {
-                  element,
+                  elements = [
-                  text,
+                    ...elements,
-                  element.font || this.state.currentItemFont
+                    {
-                );
+                      // we need to recreate the element to update dimensions &
-                elements = [...elements, { ...element, isSelected: true }];
+                      //  position
+                      ...newTextElement(element, text, element.font),
+                      isSelected: true
+                    }
+                  ];
+                }
                 this.setState({
                   draggingElement: null,
                   editingElement: null,

src/scene/index.ts

@@ -5,7 +5,7 @@ export {
   deleteSelectedElements,
   someElementIsSelected,
   getElementsWithinSelection,
-  getSelectedAttribute
+  getCommonAttributeOfSelectedElements
 } from "./selection";
 export {
   exportCanvas,

src/scene/selection.ts

@@ -56,7 +56,11 @@ export function getSelectedIndices(elements: readonly ExcalidrawElement[]) {
 export const someElementIsSelected = (elements: readonly ExcalidrawElement[]) =>
   elements.some(element => element.isSelected);
 
-export function getSelectedAttribute<T>(
+/**
+ * Returns common attribute (picked by `getAttribute` callback) of selected
+ *  elements. If elements don't share the same value, returns `null`.
+ */
+export function getCommonAttributeOfSelectedElements<T>(
   elements: readonly ExcalidrawElement[],
   getAttribute: (element: ExcalidrawElement) => T
 ): T | null {