Merge remote-tracking branch 'origin/master' into aakansha-improve-text-wrap-ellipse

src/actions/actionFinalize.tsx

@@ -33,9 +33,6 @@ export const actionFinalize = register({
             endBindingElement,
           );
         }
-        const selectedLinearElement = appState.selectedLinearElement
-          ? new LinearElementEditor(element, scene, appState)
-          : null;
         return {
           elements:
             element.points.length < 2 || isInvisiblySmallElement(element)
@@ -45,7 +42,6 @@ export const actionFinalize = register({
             ...appState,
             cursorButton: "up",
             editingLinearElement: null,
-            selectedLinearElement,
           },
           commitToHistory: true,
         };
@@ -188,7 +184,7 @@ export const actionFinalize = register({
         // To select the linear element when user has finished mutipoint editing
         selectedLinearElement:
           multiPointElement && isLinearElement(multiPointElement)
-            ? new LinearElementEditor(multiPointElement, scene, appState)
+            ? new LinearElementEditor(multiPointElement, scene)
             : appState.selectedLinearElement,
         pendingImageElementId: null,
       },

src/actions/actionSelectAll.ts

@@ -35,7 +35,7 @@ export const actionSelectAll = register({
             // single linear element selected
             Object.keys(selectedElementIds).length === 1 &&
             isLinearElement(elements[0])
-              ? new LinearElementEditor(elements[0], app.scene, appState)
+              ? new LinearElementEditor(elements[0], app.scene)
               : null,
           editingGroupId: null,
           selectedElementIds,

src/components/App.tsx

@@ -1907,8 +1907,6 @@ class App extends React.Component<AppProps, AppState> {
                 editingLinearElement: new LinearElementEditor(
                   selectedElements[0],
                   this.scene,
-                  this.state,
-                  true,
                 ),
               });
             }
@@ -2401,7 +2399,6 @@ class App extends React.Component<AppProps, AppState> {
         );
       }
     }
-
     const element = existingTextElement
       ? existingTextElement
       : newTextElement({
@@ -2487,8 +2484,6 @@ class App extends React.Component<AppProps, AppState> {
           editingLinearElement: new LinearElementEditor(
             selectedElements[0],
             this.scene,
-            this.state,
-            true,
           ),
         });
       }
@@ -3075,7 +3070,7 @@ class App extends React.Component<AppProps, AppState> {
         ])
       ) {
         hoverPointIndex = LinearElementEditor.getPointIndexUnderCursor(
-          this.state.selectedLinearElement,
+          element,
           this.state.zoom,
           scenePointerX,
           scenePointerY,
@@ -4492,7 +4487,6 @@ class App extends React.Component<AppProps, AppState> {
                     ? new LinearElementEditor(
                         elementsWithinSelection[0],
                         this.scene,
-                        this.state,
                       )
                     : null,
               },
@@ -4757,7 +4751,6 @@ class App extends React.Component<AppProps, AppState> {
               selectedLinearElement: new LinearElementEditor(
                 draggingElement,
                 this.scene,
-                this.state,
               ),
             }));
           } else {
@@ -4825,7 +4818,6 @@ class App extends React.Component<AppProps, AppState> {
             selectedLinearElement: new LinearElementEditor(
               hitElement,
               this.scene,
-              this.state,
             ),
           });
         }
@@ -4928,7 +4920,6 @@ class App extends React.Component<AppProps, AppState> {
                         ? new LinearElementEditor(
                             newSelectedElements[0],
                             this.scene,
-                            this.state,
                           )
                         : prevState.selectedLinearElement,
                   },
@@ -4957,11 +4948,7 @@ class App extends React.Component<AppProps, AppState> {
                   // Don't set `selectedLinearElement` if its same as the hitElement, this is mainly to prevent resetting the `hoverPointIndex` to -1.
                   // Future we should update the API to take care of setting the correct `hoverPointIndex` when initialized
                   prevState.selectedLinearElement?.elementId !== hitElement.id
-                    ? new LinearElementEditor(
-                        hitElement,
-                        this.scene,
-                        this.state,
-                      )
+                    ? new LinearElementEditor(hitElement, this.scene)
                     : prevState.selectedLinearElement,
               },
               this.scene.getNonDeletedElements(),
@@ -5721,7 +5708,7 @@ class App extends React.Component<AppProps, AppState> {
             ...this.state,
             selectedElementIds: { [element.id]: true },
             selectedLinearElement: isLinearElement(element)
-              ? new LinearElementEditor(element, this.scene, this.state)
+              ? new LinearElementEditor(element, this.scene)
               : null,
           },
           this.scene.getNonDeletedElements(),

src/element/linearElementEditor.ts

@@ -40,11 +40,6 @@ const editorMidPointsCache: {
   zoom: number | null;
 } = { version: null, points: [], zoom: null };
 
-const visiblePointIndexesCache: {
-  points: number[];
-  zoom: number | null;
-  isEditingLinearElement: boolean;
-} = { points: [], zoom: null, isEditingLinearElement: false };
 export class LinearElementEditor {
   public readonly elementId: ExcalidrawElement["id"] & {
     _brand: "excalidrawLinearElementId";
@@ -70,12 +65,7 @@ export class LinearElementEditor {
   public readonly hoverPointIndex: number;
   public readonly segmentMidPointHoveredCoords: Point | null;
 
-  constructor(
-    element: NonDeleted<ExcalidrawLinearElement>,
-    scene: Scene,
-    appState: AppState,
-    editingLinearElement = false,
-  ) {
+  constructor(element: NonDeleted<ExcalidrawLinearElement>, scene: Scene) {
     this.elementId = element.id as string & {
       _brand: "excalidrawLinearElementId";
     };
@@ -443,7 +433,7 @@ export class LinearElementEditor {
       return null;
     }
     const clickedPointIndex = LinearElementEditor.getPointIndexUnderCursor(
-      appState.selectedLinearElement,
+      element,
       appState.zoom,
       scenePointer.x,
       scenePointer.y,
@@ -570,59 +560,6 @@ export class LinearElementEditor {
     return -1;
   }
 
-  static getVisiblePointIndexes(
-    element: NonDeleted<ExcalidrawLinearElement>,
-    appState: AppState,
-  ): typeof visiblePointIndexesCache["points"] {
-    const isEditingLinearElement = !!appState.editingLinearElement;
-    if (appState.editingLinearElement) {
-      // So that when we exit the editor the points are calculated again
-      visiblePointIndexesCache.isEditingLinearElement = true;
-      return element.points.map((_, index) => index);
-    }
-
-    if (
-      visiblePointIndexesCache.points &&
-      visiblePointIndexesCache.zoom === appState.zoom.value &&
-      isEditingLinearElement === visiblePointIndexesCache.isEditingLinearElement
-    ) {
-      return visiblePointIndexesCache.points;
-    }
-
-    LinearElementEditor.updateVisiblePointIndexesCache(element, appState);
-    return visiblePointIndexesCache.points;
-  }
-
-  static updateVisiblePointIndexesCache(
-    element: NonDeleted<ExcalidrawLinearElement>,
-    appState: AppState,
-  ) {
-    const visiblePointIndexes: number[] = [];
-    let previousPoint: Point | null = null;
-    element.points.forEach((point, index) => {
-      let distance = Infinity;
-      if (previousPoint) {
-        distance =
-          distance2d(point[0], point[1], previousPoint[0], previousPoint[1]) *
-          appState.zoom.value;
-      }
-      const isExtremePoint = index === 0 || index === element.points.length - 1;
-      const threshold = 2 * LinearElementEditor.POINT_HANDLE_SIZE;
-      if (isExtremePoint || distance >= threshold) {
-        // hide n-1 point if distance is less than threshold
-        if (isExtremePoint && distance < threshold) {
-          visiblePointIndexes.pop();
-        }
-        visiblePointIndexes.push(index);
-        previousPoint = point;
-      }
-    });
-    visiblePointIndexesCache.points = visiblePointIndexes;
-    visiblePointIndexesCache.zoom = appState.zoom.value;
-    visiblePointIndexesCache.isEditingLinearElement =
-      !!appState.editingLinearElement;
-  }
-
   static handlePointerDown(
     event: React.PointerEvent<HTMLCanvasElement>,
     appState: AppState,
@@ -680,6 +617,15 @@ export class LinearElementEditor {
 
       ret.didAddPoint = true;
       ret.isMidPoint = true;
+      ret.linearElementEditor = {
+        ...linearElementEditor,
+        selectedPointsIndices: element.points[1],
+        pointerDownState: {
+          prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
+          lastClickedPoint: -1,
+        },
+        lastUncommittedPoint: null,
+      };
     }
     if (event.altKey && appState.editingLinearElement) {
       if (linearElementEditor.lastUncommittedPoint == null) {
@@ -716,7 +662,7 @@ export class LinearElementEditor {
     }
 
     const clickedPointIndex = LinearElementEditor.getPointIndexUnderCursor(
-      appState.selectedLinearElement,
+      element,
       appState.zoom,
       scenePointer.x,
       scenePointer.y,
@@ -779,11 +725,7 @@ export class LinearElementEditor {
           }
         : { x: 0, y: 0 },
     };
-    if (ret.didAddPoint) {
-      ret.linearElementEditor = {
-        ...ret.linearElementEditor,
-      };
-    }
+
     return ret;
   }
 
@@ -931,37 +873,25 @@ export class LinearElementEditor {
   }
 
   static getPointIndexUnderCursor(
-    linearElementEditor: LinearElementEditor | null,
+    element: NonDeleted<ExcalidrawLinearElement>,
     zoom: AppState["zoom"],
     x: number,
     y: number,
   ) {
-    if (!linearElementEditor) {
-      return -1;
-    }
-    const element = LinearElementEditor.getElement(
-      linearElementEditor.elementId,
-    );
-    if (!element) {
-      return -1;
-    }
-
     const pointHandles =
       LinearElementEditor.getPointsGlobalCoordinates(element);
-    let counter = visiblePointIndexesCache.points.length;
-
+    let idx = pointHandles.length;
     // loop from right to left because points on the right are rendered over
     // points on the left, thus should take precedence when clicking, if they
     // overlap
-    while (--counter >= 0) {
-      const index = visiblePointIndexesCache.points[counter];
-      const point = pointHandles[index];
+    while (--idx > -1) {
+      const point = pointHandles[idx];
       if (
         distance2d(x, y, point[0], point[1]) * zoom.value <
         // +1px to account for outline stroke
         LinearElementEditor.POINT_HANDLE_SIZE + 1
       ) {
-        return index;
+        return idx;
       }
     }
     return -1;

src/element/newElement.ts

@@ -21,7 +21,12 @@ import { AppState } from "../types";
 import { getElementAbsoluteCoords } from ".";
 import { adjustXYWithRotation } from "../math";
 import { getResizedElementAbsoluteCoords } from "./bounds";
-import { getContainerElement, measureText, wrapText } from "./textElement";
+import {
+  getContainerElement,
+  getContainerDims,
+  measureText,
+  wrapText,
+} from "./textElement";
 import { BOUND_TEXT_PADDING, VERTICAL_ALIGN } from "../constants";
 
 type ElementConstructorOpts = MarkOptional<
@@ -164,7 +169,9 @@ const getAdjustedDimensions = (
   let maxWidth = null;
   const container = getContainerElement(element);
   if (container) {
-    maxWidth = container.width - BOUND_TEXT_PADDING * 2;
+    const containerDims = getContainerDims(container);
+
+    maxWidth = containerDims.width - BOUND_TEXT_PADDING * 2;
   }
   const {
     width: nextWidth,
@@ -224,16 +231,21 @@ const getAdjustedDimensions = (
   // make sure container dimensions are set properly when
   // text editor overflows beyond viewport dimensions
   if (container) {
-    let height = container.height;
-    let width = container.width;
+    const containerDims = getContainerDims(container);
+    let { width, height } = containerDims;
     if (nextHeight > height - BOUND_TEXT_PADDING * 2) {
       height = nextHeight + BOUND_TEXT_PADDING * 2;
     }
     if (nextWidth > width - BOUND_TEXT_PADDING * 2) {
       width = nextWidth + BOUND_TEXT_PADDING * 2;
     }
-    if (height !== container.height || width !== container.width) {
-      mutateElement(container, { height, width });
+    if (height !== containerDims.height || width !== containerDims.height) {
+      const diffHeight = height - containerDims.height;
+      const diffWidth = width - containerDims.width;
+      mutateElement(container, {
+        height: container.height + diffHeight,
+        width: container.width + diffWidth,
+      });
     }
   }
   return {
@@ -259,7 +271,11 @@ export const updateTextElement = (
 ): ExcalidrawTextElement => {
   const container = getContainerElement(element);
   if (container) {
-    text = wrapText(text, getFontString(element), container.width);
+    text = wrapText(
+      text,
+      getFontString(element),
+      getContainerDims(container).width,
+    );
   }
   const dimensions = getAdjustedDimensions(element, text);
   return newElementWith(element, {

src/element/textElement.ts

@@ -16,16 +16,20 @@ export const redrawTextBoundingBox = (
   element: ExcalidrawTextElement,
   container: ExcalidrawElement | null,
 ) => {
-  const maxWidth = container
-    ? container.width - BOUND_TEXT_PADDING * 2
+  let containerDims;
+  if (container) {
+    containerDims = getContainerDims(container);
+  }
+  const maxWidth = containerDims
+    ? containerDims.width - BOUND_TEXT_PADDING * 2
     : undefined;
   let text = element.text;
 
-  if (container) {
+  if (containerDims) {
     text = wrapText(
       element.originalText,
       getFontString(element),
-      container.width,
+      containerDims.width,
     );
   }
   const metrics = measureText(
@@ -37,18 +41,24 @@ export const redrawTextBoundingBox = (
   let coordX = element.x;
   // Resize container and vertically center align the text
   if (container) {
-    let nextHeight = container.height;
-    coordX = container.x + BOUND_TEXT_PADDING;
+    const containerDims = getContainerDims(container);
+    const containerCoords = getBoundTextContainerCoords(container);
+    let nextHeight = containerDims.height;
+    coordX = containerCoords.x + BOUND_TEXT_PADDING;
     if (element.verticalAlign === VERTICAL_ALIGN.TOP) {
-      coordY = container.y + BOUND_TEXT_PADDING;
+      coordY = containerCoords.y + BOUND_TEXT_PADDING;
     } else if (element.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
       coordY =
-        container.y + container.height - metrics.height - BOUND_TEXT_PADDING;
+        containerCoords.y +
+        containerDims.height -
+        metrics.height -
+        BOUND_TEXT_PADDING;
     } else {
-      coordY = container.y + container.height / 2 - metrics.height / 2;
-      if (metrics.height > container.height - BOUND_TEXT_PADDING * 2) {
+      coordY =
+        containerCoords.y + containerDims.height / 2 - metrics.height / 2;
+      if (metrics.height > containerDims.height - BOUND_TEXT_PADDING * 2) {
         nextHeight = metrics.height + BOUND_TEXT_PADDING * 2;
-        coordY = container.y + nextHeight / 2 - metrics.height / 2;
+        coordY = containerCoords.y + nextHeight / 2 - metrics.height / 2;
       }
     }
     mutateElement(container, { height: nextHeight });
@@ -121,7 +131,7 @@ export const handleBindTextResize = (
           text = wrapText(
             textElement.originalText,
             getFontString(textElement),
-            element.width,
+            getContainerDims(element).width,
           );
         }
 
@@ -474,3 +484,28 @@ export const getContainerElement = (
   }
   return null;
 };
+
+export const getContainerDims = (element: ExcalidrawElement) => {
+  if (element.type === "ellipse") {
+    return {
+      width: Math.round((element.width / 2) * Math.sqrt(2)),
+      height: Math.round((element.height / 2) * Math.sqrt(2)),
+    };
+  }
+  return { width: element.width, height: element.height };
+};
+
+export const getBoundTextContainerCoords = (container: ExcalidrawElement) => {
+  if (container.type === "ellipse") {
+    const offsetX = (container.width / 2) * (1 - Math.sqrt(2) / 2);
+    const offsetY = (container.height / 2) * (1 - Math.sqrt(2) / 2);
+    return {
+      x: container.x + offsetX,
+      y: container.y + offsetY,
+    };
+  }
+  return {
+    x: container.x,
+    y: container.y,
+  };
+};

src/element/textWysiwyg.tsx

@@ -19,7 +19,9 @@ import {
   getApproxLineHeight,
   getBoundTextElementId,
   getContainerElement,
+  getContainerDims,
   wrapText,
+  getBoundTextContainerCoords,
 } from "./textElement";
 import {
   actionDecreaseFontSize,
@@ -126,26 +128,28 @@ export const textWysiwyg = ({
           updatedElement,
           editable,
         );
+        const containerDims = getContainerDims(container);
         // using editor.style.height to get the accurate height of text editor
         const editorHeight = Number(editable.style.height.slice(0, -2));
         if (editorHeight > 0) {
           height = editorHeight;
         }
         if (propertiesUpdated) {
-          originalContainerHeight = container.height;
+          originalContainerHeight = containerDims.height;
 
           // update height of the editor after properties updated
           height = updatedElement.height;
         }
         if (!originalContainerHeight) {
-          originalContainerHeight = container.height;
+          originalContainerHeight = containerDims.height;
         }
-        maxWidth = container.width - BOUND_TEXT_PADDING * 2;
-        maxHeight = container.height - BOUND_TEXT_PADDING * 2;
+        maxWidth = containerDims.width - BOUND_TEXT_PADDING * 2;
+        maxHeight = containerDims.height - BOUND_TEXT_PADDING * 2;
         width = maxWidth;
+        const boundTextCoords = getBoundTextContainerCoords(container);
         // The coordinates of text box set a distance of
         // 5px to preserve padding
-        coordX = container.x + BOUND_TEXT_PADDING;
+        coordX = boundTextCoords.x + BOUND_TEXT_PADDING;
         // autogrow container height if text exceeds
         if (height > maxHeight) {
           const diff = Math.min(height - maxHeight, approxLineHeight);
@@ -154,7 +158,7 @@ export const textWysiwyg = ({
         } else if (
           // autoshrink container height until original container height
           // is reached when text is removed
-          container.height > originalContainerHeight &&
+          containerDims.height > originalContainerHeight &&
           height < maxHeight
         ) {
           const diff = Math.min(maxHeight - height, approxLineHeight);
@@ -165,11 +169,14 @@ export const textWysiwyg = ({
         else {
           // vertically center align the text
           if (verticalAlign === VERTICAL_ALIGN.MIDDLE) {
-            coordY = container.y + container.height / 2 - height / 2;
+            coordY = boundTextCoords.y + containerDims.height / 2 - height / 2;
           }
           if (verticalAlign === VERTICAL_ALIGN.BOTTOM) {
             coordY =
-              container.y + container.height - height - BOUND_TEXT_PADDING;
+              boundTextCoords.y +
+              containerDims.height -
+              height -
+              BOUND_TEXT_PADDING;
           }
         }
       }
@@ -303,7 +310,7 @@ export const textWysiwyg = ({
           const actualLineCount = wrapText(
             editable.value,
             font,
-            container!.width,
+            getContainerDims(container!).width,
           ).split("\n").length;
           // This is browser behaviour when setting height to "auto"
           // It sets the height needed for 2 lines even if actual

src/renderer/renderScene.ts

@@ -159,15 +159,13 @@ const strokeGrid = (
 
 const renderSingleLinearPoint = (
   context: CanvasRenderingContext2D,
+  appState: AppState,
   renderConfig: RenderConfig,
   point: Point,
   radius: number,
   isSelected: boolean,
   isPhantomPoint = false,
 ) => {
-  if (!point) {
-    return;
-  }
   context.strokeStyle = "#5e5ad8";
   context.setLineDash([]);
   context.fillStyle = "rgba(255, 255, 255, 0.9)";
@@ -204,16 +202,18 @@ const renderLinearPointHandles = (
   const radius = appState.editingLinearElement
     ? POINT_HANDLE_SIZE
     : POINT_HANDLE_SIZE / 2;
-
-  const visiblePointIndexes = LinearElementEditor.getVisiblePointIndexes(
-    element,
-    appState,
-  );
-  visiblePointIndexes.forEach((index) => {
+  points.forEach((point, idx) => {
     const isSelected =
-      !!appState.editingLinearElement?.selectedPointsIndices?.includes(index);
-    const point = points[index];
-    renderSingleLinearPoint(context, renderConfig, point, radius, isSelected);
+      !!appState.editingLinearElement?.selectedPointsIndices?.includes(idx);
+
+    renderSingleLinearPoint(
+      context,
+      appState,
+      renderConfig,
+      point,
+      radius,
+      isSelected,
+    );
   });
 
   //Rendering segment mid points
@@ -237,6 +237,7 @@ const renderLinearPointHandles = (
       if (appState.editingLinearElement) {
         renderSingleLinearPoint(
           context,
+          appState,
           renderConfig,
           segmentMidPoint,
           radius,
@@ -247,7 +248,7 @@ const renderLinearPointHandles = (
         highlightPoint(segmentMidPoint, context, renderConfig);
         renderSingleLinearPoint(
           context,
-
+          appState,
           renderConfig,
           segmentMidPoint,
           radius,
@@ -257,6 +258,7 @@ const renderLinearPointHandles = (
     } else if (appState.editingLinearElement || points.length === 2) {
       renderSingleLinearPoint(
         context,
+        appState,
         renderConfig,
         segmentMidPoint,
         POINT_HANDLE_SIZE / 2,
@@ -448,22 +450,7 @@ export const _renderScene = ({
       appState.selectedLinearElement &&
       appState.selectedLinearElement.hoverPointIndex >= 0
     ) {
-      const element = LinearElementEditor.getElement(
-        appState.selectedLinearElement.elementId,
-      );
-      if (element) {
-        const visiblePointIndexes = LinearElementEditor.getVisiblePointIndexes(
-          element,
-          appState,
-        );
-        if (
-          visiblePointIndexes.includes(
-            appState.selectedLinearElement.hoverPointIndex,
-          )
-        ) {
-          renderLinearElementPointHighlight(context, appState, renderConfig);
-        }
-      }
+      renderLinearElementPointHighlight(context, appState, renderConfig);
     }
     // Paint selected elements
     if (

src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -10993,9 +10993,6 @@ Object {
     "segmentMidPointHoveredCoords": null,
     "selectedPointsIndices": null,
     "startBindingElement": "keep",
-    "visiblePointIndexes": Array [
-      1,
-    ],
   },
   "selectionElement": null,
   "shouldCacheIgnoreZoom": false,
@@ -11222,9 +11219,6 @@ Object {
     "segmentMidPointHoveredCoords": null,
     "selectedPointsIndices": null,
     "startBindingElement": "keep",
-    "visiblePointIndexes": Array [
-      1,
-    ],
   },
   "selectionElement": null,
   "shouldCacheIgnoreZoom": false,
@@ -11678,9 +11672,6 @@ Object {
     "segmentMidPointHoveredCoords": null,
     "selectedPointsIndices": null,
     "startBindingElement": "keep",
-    "visiblePointIndexes": Array [
-      1,
-    ],
   },
   "selectionElement": null,
   "shouldCacheIgnoreZoom": false,
@@ -12086,9 +12077,6 @@ Object {
     "segmentMidPointHoveredCoords": null,
     "selectedPointsIndices": null,
     "startBindingElement": "keep",
-    "visiblePointIndexes": Array [
-      1,
-    ],
   },
   "selectionElement": null,
   "shouldCacheIgnoreZoom": false,