added an explanatory comment

dev-docs/docs/codebase/frames.mdx

@@ -20,3 +20,13 @@ Frames should be ordered where frame children come first, followed by the frame
 ```
 
 If not oredered correctly, the editor will still function, but the elements may not be rendered and clipped correctly. Further, the renderer relies on this ordering for performance optimizations.
+
+# Arrows
+
+An arrow can be a child of a frame only if it has no binding (either start or end) to any other element, regardless of whether the bound element is inside the frame or not.
+
+This ensures that when an arrow is bound to an element outside the frame, it's rendered and behaves correctly.
+
+Therefore, when an arrow (that's a child of a frame) gets bound to an element, it's automatically removed from the frame.
+
+Bound-arrow is duplicated alongside a frame only if the arrow start is bound to an element within that frame.

package.json

@@ -53,7 +53,7 @@
     "pwacompat": "2.0.17",
     "react": "18.2.0",
     "react-dom": "18.2.0",
-    "roughjs": "4.6.4",
+    "roughjs": "4.6.5",
     "sass": "1.51.0",
     "socket.io-client": "2.3.1",
     "tunnel-rat": "0.1.2"

src/actions/actionClipboard.tsx

@@ -10,34 +10,26 @@ import { actionDeleteSelected } from "./actionDeleteSelected";
 import { exportCanvas } from "../data/index";
 import { getNonDeletedElements, isTextElement } from "../element";
 import { t } from "../i18n";
-import { isFirefox } from "../constants";
 
 export const actionCopy = register({
   name: "copy",
   trackEvent: { category: "element" },
-  perform: async (elements, appState, _, app) => {
+  perform: (elements, appState, _, app) => {
     const elementsToCopy = app.scene.getSelectedElements({
       selectedElementIds: appState.selectedElementIds,
       includeBoundTextElement: true,
       includeElementsInFrames: true,
     });
 
-    try {
+    copyToClipboard(elementsToCopy, app.files);
-      await copyToClipboard(elementsToCopy, app.files);
-    } catch (error: any) {
-      return {
-        commitToHistory: false,
-        appState: {
-          ...appState,
-          errorMessage: error.message,
-        },
-      };
-    }
 
     return {
       commitToHistory: false,
     };
   },
+  predicate: (elements, appState, appProps, app) => {
+    return app.device.isMobile && !!navigator.clipboard;
+  },
   contextItemLabel: "labels.copy",
   // don't supply a shortcut since we handle this conditionally via onCopy event
   keyTest: undefined,
@@ -46,91 +38,15 @@ export const actionCopy = register({
 export const actionPaste = register({
   name: "paste",
   trackEvent: { category: "element" },
-  perform: async (elements, appState, data, app) => {
+  perform: (elements: any, appStates: any, data, app) => {
-    const MIME_TYPES: Record<string, string> = {};
+    app.pasteFromClipboard(null);
-    try {
-      try {
-        const clipboardItems = await navigator.clipboard?.read();
-        for (const item of clipboardItems) {
-          for (const type of item.types) {
-            try {
-              const blob = await item.getType(type);
-              MIME_TYPES[type] = await blob.text();
-            } catch (error: any) {
-              console.warn(
-                `Cannot retrieve ${type} from clipboardItem: ${error.message}`,
-              );
-            }
-          }
-        }
-        if (Object.keys(MIME_TYPES).length === 0) {
-          console.warn(
-            "No clipboard data found from clipboard.read(). Falling back to clipboard.readText()",
-          );
-          // throw so we fall back onto clipboard.readText()
-          throw new Error("No clipboard data found");
-        }
-      } catch (error: any) {
-        try {
-          MIME_TYPES["text/plain"] = await navigator.clipboard?.readText();
-        } catch (error: any) {
-          console.warn(`Cannot readText() from clipboard: ${error.message}`);
-          if (isFirefox) {
-            return {
-              commitToHistory: false,
-              appState: {
-                ...appState,
-                errorMessage: t("hints.firefox_clipboard_write"),
-              },
-            };
-          }
-          throw error;
-        }
-      }
-    } catch (error: any) {
-      console.error(`actionPaste: ${error.message}`);
-      return {
-        commitToHistory: false,
-        appState: {
-          ...appState,
-          errorMessage: error.message,
-        },
-      };
-    }
-    try {
-      console.log("actionPaste (1)", { MIME_TYPES });
-      const event = new ClipboardEvent("paste", {
-        clipboardData: new DataTransfer(),
-      });
-      for (const [type, value] of Object.entries(MIME_TYPES)) {
-        try {
-          event.clipboardData?.setData(type, value);
-        } catch (error: any) {
-          console.warn(
-            `Cannot set ${type} as clipboardData item: ${error.message}`,
-          );
-        }
-      }
-      event.clipboardData?.types.forEach((type) => {
-        console.log(
-          `actionPaste (2) event.clipboardData?.getData(${type})`,
-          event.clipboardData?.getData(type),
-        );
-      });
-      app.pasteFromClipboard(event);
-    } catch (error: any) {
-      return {
-        commitToHistory: false,
-        appState: {
-          ...appState,
-          errorMessage: error.message,
-        },
-      };
-    }
     return {
       commitToHistory: false,
     };
   },
+  predicate: (elements, appState, appProps, app) => {
+    return app.device.isMobile && !!navigator.clipboard;
+  },
   contextItemLabel: "labels.paste",
   // don't supply a shortcut since we handle this conditionally via onCopy event
   keyTest: undefined,

src/actions/actionDuplicateSelection.tsx

@@ -155,7 +155,12 @@ const duplicateElements = (
             groupId,
           ).flatMap((element) =>
             isFrameElement(element)
-              ? [...getFrameElements(elements, element.id), element]
+              ? [
+                  ...getFrameElements(elements, element.id, {
+                    includeBoundArrows: true,
+                  }),
+                  element,
+                ]
               : [element],
           );
 
@@ -181,7 +186,9 @@ const duplicateElements = (
           continue;
         }
         if (isElementAFrame) {
-          const elementsInFrame = getFrameElements(sortedElements, element.id);
+          const elementsInFrame = getFrameElements(sortedElements, element.id, {
+            includeBoundArrows: true,
+          });
 
           elementsWithClones.push(
             ...markAsProcessed([

src/actions/actionProperties.test.tsx

@@ -1,167 +0,0 @@
-import { Excalidraw } from "../packages/excalidraw/index";
-import { queryByTestId } from "@testing-library/react";
-import { render } from "../tests/test-utils";
-import { UI } from "../tests/helpers/ui";
-import { API } from "../tests/helpers/api";
-import { COLOR_PALETTE, DEFAULT_ELEMENT_BACKGROUND_PICKS } from "../colors";
-import { FONT_FAMILY, STROKE_WIDTH } from "../constants";
-
-const { h } = window;
-
-describe("element locking", () => {
-  beforeEach(async () => {
-    await render(<Excalidraw />);
-  });
-
-  describe("properties when tool selected", () => {
-    it("should show active background top picks", () => {
-      UI.clickTool("rectangle");
-
-      const color = DEFAULT_ELEMENT_BACKGROUND_PICKS[1];
-
-      // just in case we change it in the future
-      expect(color).not.toBe(COLOR_PALETTE.transparent);
-
-      h.setState({
-        currentItemBackgroundColor: color,
-      });
-      const activeColor = queryByTestId(
-        document.body,
-        `color-top-pick-${color}`,
-      );
-      expect(activeColor).toHaveClass("active");
-    });
-
-    it("should show fill style when background non-transparent", () => {
-      UI.clickTool("rectangle");
-
-      const color = DEFAULT_ELEMENT_BACKGROUND_PICKS[1];
-
-      // just in case we change it in the future
-      expect(color).not.toBe(COLOR_PALETTE.transparent);
-
-      h.setState({
-        currentItemBackgroundColor: color,
-        currentItemFillStyle: "hachure",
-      });
-      const hachureFillButton = queryByTestId(document.body, `fill-hachure`);
-
-      expect(hachureFillButton).toHaveClass("active");
-      h.setState({
-        currentItemFillStyle: "solid",
-      });
-      const solidFillStyle = queryByTestId(document.body, `fill-solid`);
-      expect(solidFillStyle).toHaveClass("active");
-    });
-
-    it("should not show fill style when background transparent", () => {
-      UI.clickTool("rectangle");
-
-      h.setState({
-        currentItemBackgroundColor: COLOR_PALETTE.transparent,
-        currentItemFillStyle: "hachure",
-      });
-      const hachureFillButton = queryByTestId(document.body, `fill-hachure`);
-
-      expect(hachureFillButton).toBe(null);
-    });
-
-    it("should show horizontal text align for text tool", () => {
-      UI.clickTool("text");
-
-      h.setState({
-        currentItemTextAlign: "right",
-      });
-
-      const centerTextAlign = queryByTestId(document.body, `align-right`);
-      expect(centerTextAlign).toBeChecked();
-    });
-  });
-
-  describe("properties when elements selected", () => {
-    it("should show active styles when single element selected", () => {
-      const rect = API.createElement({
-        type: "rectangle",
-        backgroundColor: "red",
-        fillStyle: "cross-hatch",
-      });
-      h.elements = [rect];
-      API.setSelectedElements([rect]);
-
-      const crossHatchButton = queryByTestId(document.body, `fill-cross-hatch`);
-      expect(crossHatchButton).toHaveClass("active");
-    });
-
-    it("should not show fill style selected element's background is transparent", () => {
-      const rect = API.createElement({
-        type: "rectangle",
-        backgroundColor: COLOR_PALETTE.transparent,
-        fillStyle: "cross-hatch",
-      });
-      h.elements = [rect];
-      API.setSelectedElements([rect]);
-
-      const crossHatchButton = queryByTestId(document.body, `fill-cross-hatch`);
-      expect(crossHatchButton).toBe(null);
-    });
-
-    it("should highlight common stroke width of selected elements", () => {
-      const rect1 = API.createElement({
-        type: "rectangle",
-        strokeWidth: STROKE_WIDTH.thin,
-      });
-      const rect2 = API.createElement({
-        type: "rectangle",
-        strokeWidth: STROKE_WIDTH.thin,
-      });
-      h.elements = [rect1, rect2];
-      API.setSelectedElements([rect1, rect2]);
-
-      const thinStrokeWidthButton = queryByTestId(
-        document.body,
-        `strokeWidth-thin`,
-      );
-      expect(thinStrokeWidthButton).toBeChecked();
-    });
-
-    it("should not highlight any stroke width button if no common style", () => {
-      const rect1 = API.createElement({
-        type: "rectangle",
-        strokeWidth: STROKE_WIDTH.thin,
-      });
-      const rect2 = API.createElement({
-        type: "rectangle",
-        strokeWidth: STROKE_WIDTH.bold,
-      });
-      h.elements = [rect1, rect2];
-      API.setSelectedElements([rect1, rect2]);
-
-      expect(queryByTestId(document.body, `strokeWidth-thin`)).not.toBe(null);
-      expect(
-        queryByTestId(document.body, `strokeWidth-thin`),
-      ).not.toBeChecked();
-      expect(
-        queryByTestId(document.body, `strokeWidth-bold`),
-      ).not.toBeChecked();
-      expect(
-        queryByTestId(document.body, `strokeWidth-extraBold`),
-      ).not.toBeChecked();
-    });
-
-    it("should show properties of different element types when selected", () => {
-      const rect = API.createElement({
-        type: "rectangle",
-        strokeWidth: STROKE_WIDTH.bold,
-      });
-      const text = API.createElement({
-        type: "text",
-        fontFamily: FONT_FAMILY.Cascadia,
-      });
-      h.elements = [rect, text];
-      API.setSelectedElements([rect, text]);
-
-      expect(queryByTestId(document.body, `strokeWidth-bold`)).toBeChecked();
-      expect(queryByTestId(document.body, `font-family-code`)).toBeChecked();
-    });
-  });
-});

src/actions/actionProperties.tsx

@@ -1,4 +1,4 @@
-import { AppState, Primitive } from "../../src/types";
+import { AppState } from "../../src/types";
 import {
   DEFAULT_ELEMENT_BACKGROUND_COLOR_PALETTE,
   DEFAULT_ELEMENT_BACKGROUND_PICKS,
@@ -51,7 +51,6 @@ import {
   DEFAULT_FONT_SIZE,
   FONT_FAMILY,
   ROUNDNESS,
-  STROKE_WIDTH,
   VERTICAL_ALIGN,
 } from "../constants";
 import {
@@ -83,6 +82,7 @@ import { getLanguage, t } from "../i18n";
 import { KEYS } from "../keys";
 import { randomInteger } from "../random";
 import {
+  canChangeRoundness,
   canHaveArrowheads,
   getCommonAttributeOfSelectedElements,
   getSelectedElements,
@@ -118,44 +118,25 @@ export const changeProperty = (
   });
 };
 
-export const getFormValue = function <T extends Primitive>(
+export const getFormValue = function <T>(
   elements: readonly ExcalidrawElement[],
   appState: AppState,
   getAttribute: (element: ExcalidrawElement) => T,
-  isRelevantElement: true | ((element: ExcalidrawElement) => boolean),
+  defaultValue: T,
-  defaultValue: T | ((isSomeElementSelected: boolean) => T),
 ): T {
   const editingElement = appState.editingElement;
   const nonDeletedElements = getNonDeletedElements(elements);
-
+  return (
-  let ret: T | null = null;
+    (editingElement && getAttribute(editingElement)) ??
-
+    (isSomeElementSelected(nonDeletedElements, appState)
-  if (editingElement) {
+      ? getCommonAttributeOfSelectedElements(
-    ret = getAttribute(editingElement);
+          nonDeletedElements,
-  }
-
-  if (!ret) {
-    const hasSelection = isSomeElementSelected(nonDeletedElements, appState);
-
-    if (hasSelection) {
-      ret =
-        getCommonAttributeOfSelectedElements(
-          isRelevantElement === true
-            ? nonDeletedElements
-            : nonDeletedElements.filter((el) => isRelevantElement(el)),
           appState,
           getAttribute,
-        ) ??
+        )
-        (typeof defaultValue === "function"
+      : defaultValue) ??
-          ? defaultValue(true)
+    defaultValue
-          : defaultValue);
+  );
-    } else {
-      ret =
-        typeof defaultValue === "function" ? defaultValue(false) : defaultValue;
-    }
-  }
-
-  return ret;
 };
 
 const offsetElementAfterFontResize = (
@@ -266,7 +247,6 @@ export const actionChangeStrokeColor = register({
           elements,
           appState,
           (element) => element.strokeColor,
-          true,
           appState.currentItemStrokeColor,
         )}
         onChange={(color) => updateData({ currentItemStrokeColor: color })}
@@ -309,7 +289,6 @@ export const actionChangeBackgroundColor = register({
           elements,
           appState,
           (element) => element.backgroundColor,
-          true,
           appState.currentItemBackgroundColor,
         )}
         onChange={(color) => updateData({ currentItemBackgroundColor: color })}
@@ -359,28 +338,23 @@ export const actionChangeFillStyle = register({
               } (${getShortcutKey("Alt-Click")})`,
               icon: allElementsZigZag ? FillZigZagIcon : FillHachureIcon,
               active: allElementsZigZag ? true : undefined,
-              testId: `fill-hachure`,
             },
             {
               value: "cross-hatch",
               text: t("labels.crossHatch"),
               icon: FillCrossHatchIcon,
-              testId: `fill-cross-hatch`,
             },
             {
               value: "solid",
               text: t("labels.solid"),
               icon: FillSolidIcon,
-              testId: `fill-solid`,
             },
           ]}
           value={getFormValue(
             elements,
             appState,
             (element) => element.fillStyle,
-            (element) => element.hasOwnProperty("fillStyle"),
+            appState.currentItemFillStyle,
-            (hasSelection) =>
-              hasSelection ? null : appState.currentItemFillStyle,
           )}
           onClick={(value, event) => {
             const nextValue =
@@ -419,31 +393,26 @@ export const actionChangeStrokeWidth = register({
         group="stroke-width"
         options={[
           {
-            value: STROKE_WIDTH.thin,
+            value: 1,
             text: t("labels.thin"),
             icon: StrokeWidthBaseIcon,
-            testId: "strokeWidth-thin",
           },
           {
-            value: STROKE_WIDTH.bold,
+            value: 2,
             text: t("labels.bold"),
             icon: StrokeWidthBoldIcon,
-            testId: "strokeWidth-bold",
           },
           {
-            value: STROKE_WIDTH.extraBold,
+            value: 4,
             text: t("labels.extraBold"),
             icon: StrokeWidthExtraBoldIcon,
-            testId: "strokeWidth-extraBold",
           },
         ]}
         value={getFormValue(
           elements,
           appState,
           (element) => element.strokeWidth,
-          (element) => element.hasOwnProperty("strokeWidth"),
+          appState.currentItemStrokeWidth,
-          (hasSelection) =>
-            hasSelection ? null : appState.currentItemStrokeWidth,
         )}
         onChange={(value) => updateData(value)}
       />
@@ -492,9 +461,7 @@ export const actionChangeSloppiness = register({
           elements,
           appState,
           (element) => element.roughness,
-          (element) => element.hasOwnProperty("roughness"),
+          appState.currentItemRoughness,
-          (hasSelection) =>
-            hasSelection ? null : appState.currentItemRoughness,
         )}
         onChange={(value) => updateData(value)}
       />
@@ -542,9 +509,7 @@ export const actionChangeStrokeStyle = register({
           elements,
           appState,
           (element) => element.strokeStyle,
-          (element) => element.hasOwnProperty("strokeStyle"),
+          appState.currentItemStrokeStyle,
-          (hasSelection) =>
-            hasSelection ? null : appState.currentItemStrokeStyle,
         )}
         onChange={(value) => updateData(value)}
       />
@@ -584,7 +549,6 @@ export const actionChangeOpacity = register({
             elements,
             appState,
             (element) => element.opacity,
-            true,
             appState.currentItemOpacity,
           ) ?? undefined
         }
@@ -643,12 +607,7 @@ export const actionChangeFontSize = register({
             }
             return null;
           },
-          (element) =>
+          appState.currentItemFontSize || DEFAULT_FONT_SIZE,
-            isTextElement(element) || getBoundTextElement(element) !== null,
-          (hasSelection) =>
-            hasSelection
-              ? null
-              : appState.currentItemFontSize || DEFAULT_FONT_SIZE,
         )}
         onChange={(value) => updateData(value)}
       />
@@ -733,25 +692,21 @@ export const actionChangeFontFamily = register({
       value: FontFamilyValues;
       text: string;
       icon: JSX.Element;
-      testId: string;
     }[] = [
       {
         value: FONT_FAMILY.Virgil,
         text: t("labels.handDrawn"),
         icon: FreedrawIcon,
-        testId: "font-family-virgil",
       },
       {
         value: FONT_FAMILY.Helvetica,
         text: t("labels.normal"),
         icon: FontFamilyNormalIcon,
-        testId: "font-family-normal",
       },
       {
         value: FONT_FAMILY.Cascadia,
         text: t("labels.code"),
         icon: FontFamilyCodeIcon,
-        testId: "font-family-code",
       },
     ];
 
@@ -774,12 +729,7 @@ export const actionChangeFontFamily = register({
               }
               return null;
             },
-            (element) =>
+            appState.currentItemFontFamily || DEFAULT_FONT_FAMILY,
-              isTextElement(element) || getBoundTextElement(element) !== null,
-            (hasSelection) =>
-              hasSelection
-                ? null
-                : appState.currentItemFontFamily || DEFAULT_FONT_FAMILY,
           )}
           onChange={(value) => updateData(value)}
         />
@@ -856,10 +806,7 @@ export const actionChangeTextAlign = register({
               }
               return null;
             },
-            (element) =>
+            appState.currentItemTextAlign,
-              isTextElement(element) || getBoundTextElement(element) !== null,
-            (hasSelection) =>
-              hasSelection ? null : appState.currentItemTextAlign,
           )}
           onChange={(value) => updateData(value)}
         />
@@ -935,9 +882,7 @@ export const actionChangeVerticalAlign = register({
               }
               return null;
             },
-            (element) =>
+            VERTICAL_ALIGN.MIDDLE,
-              isTextElement(element) || getBoundTextElement(element) !== null,
-            (hasSelection) => (hasSelection ? null : VERTICAL_ALIGN.MIDDLE),
           )}
           onChange={(value) => updateData(value)}
         />
@@ -1002,9 +947,9 @@ export const actionChangeRoundness = register({
             appState,
             (element) =>
               hasLegacyRoundness ? null : element.roundness ? "round" : "sharp",
-            (element) => element.hasOwnProperty("roundness"),
+            (canChangeRoundness(appState.activeTool.type) &&
-            (hasSelection) =>
+              appState.currentItemRoundness) ||
-              hasSelection ? null : appState.currentItemRoundness,
+              null,
           )}
           onChange={(value) => updateData(value)}
         />
@@ -1098,7 +1043,6 @@ export const actionChangeArrowhead = register({
                 isLinearElement(element) && canHaveArrowheads(element.type)
                   ? element.startArrowhead
                   : appState.currentItemStartArrowhead,
-              true,
               appState.currentItemStartArrowhead,
             )}
             onChange={(value) => updateData({ position: "start", type: value })}
@@ -1145,7 +1089,6 @@ export const actionChangeArrowhead = register({
                 isLinearElement(element) && canHaveArrowheads(element.type)
                   ? element.endArrowhead
                   : appState.currentItemEndArrowhead,
-              true,
               appState.currentItemEndArrowhead,
             )}
             onChange={(value) => updateData({ position: "end", type: value })}

src/clipboard.ts

@@ -118,7 +118,7 @@ export const copyToClipboard = async (
     await copyTextToSystemClipboard(json);
   } catch (error: any) {
     PREFER_APP_CLIPBOARD = true;
-    throw error;
+    console.error(error);
   }
 };
 
@@ -193,7 +193,7 @@ const maybeParseHTMLPaste = (event: ClipboardEvent) => {
  *  via async clipboard API if supported)
  */
 const getSystemClipboard = async (
-  event: ClipboardEvent,
+  event: ClipboardEvent | null,
   isPlainPaste = false,
 ): Promise<
   | { type: "text"; value: string }
@@ -205,7 +205,10 @@ const getSystemClipboard = async (
       return { type: "mixedContent", value: mixedContent };
     }
 
-    const text = event.clipboardData?.getData("text/plain");
+    const text = event
+      ? event.clipboardData?.getData("text/plain")
+      : probablySupportsClipboardReadText &&
+        (await navigator.clipboard.readText());
 
     return { type: "text", value: (text || "").trim() };
   } catch {
@@ -217,7 +220,7 @@ const getSystemClipboard = async (
  * Attempts to parse clipboard. Prefers system clipboard.
  */
 export const parseClipboard = async (
-  event: ClipboardEvent,
+  event: ClipboardEvent | null,
   isPlainPaste = false,
 ): Promise<ClipboardData> => {
   const systemClipboard = await getSystemClipboard(event, isPlainPaste);

src/components/Actions.tsx

@@ -11,6 +11,7 @@ import {
   hasBackground,
   hasStrokeStyle,
   hasStrokeWidth,
+  hasText,
 } from "../scene";
 import { SHAPES } from "../shapes";
 import { AppClassProperties, UIAppState, Zoom } from "../types";
@@ -19,7 +20,7 @@ import Stack from "./Stack";
 import { ToolButton } from "./ToolButton";
 import { hasStrokeColor } from "../scene/comparisons";
 import { trackEvent } from "../analytics";
-import { hasBoundTextElement, isTextElement } from "../element/typeChecks";
+import { hasBoundTextElement } from "../element/typeChecks";
 import clsx from "clsx";
 import { actionToggleZenMode } from "../actions";
 import { Tooltip } from "./Tooltip";
@@ -65,8 +66,7 @@ export const SelectedShapeActions = ({
   const isRTL = document.documentElement.getAttribute("dir") === "rtl";
 
   const showFillIcons =
-    (hasBackground(appState.activeTool.type) &&
+    hasBackground(appState.activeTool.type) ||
-      !isTransparent(appState.currentItemBackgroundColor)) ||
     targetElements.some(
       (element) =>
         hasBackground(element.type) && !isTransparent(element.backgroundColor),
@@ -123,15 +123,14 @@ export const SelectedShapeActions = ({
         <>{renderAction("changeRoundness")}</>
       )}
 
-      {(appState.activeTool.type === "text" ||
+      {(hasText(appState.activeTool.type) ||
-        targetElements.some(isTextElement)) && (
+        targetElements.some((element) => hasText(element.type))) && (
         <>
           {renderAction("changeFontSize")}
 
           {renderAction("changeFontFamily")}
 
-          {(appState.activeTool.type === "text" ||
+          {suppportsHorizontalAlign(targetElements) &&
-            suppportsHorizontalAlign(targetElements)) &&
             renderAction("changeTextAlign")}
         </>
       )}

src/components/App.tsx

@@ -1275,12 +1275,6 @@ class App extends React.Component<AppProps, AppState> {
                             top={this.state.contextMenu.top}
                             left={this.state.contextMenu.left}
                             actionManager={this.actionManager}
-                            onClose={(cb) => {
-                              this.setState({ contextMenu: null }, () => {
-                                this.focusContainer();
-                                cb?.();
-                              });
-                            }}
                           />
                         )}
                         <StaticCanvas
@@ -2201,21 +2195,14 @@ class App extends React.Component<AppProps, AppState> {
   };
 
   public pasteFromClipboard = withBatchedUpdates(
-    async (event: ClipboardEvent) => {
+    async (event: ClipboardEvent | null) => {
       const isPlainPaste = !!(IS_PLAIN_PASTE && event);
 
-      console.warn(
-        "pasteFromClipboard",
-        event?.clipboardData?.types,
-        event?.clipboardData?.getData("text/plain"),
-      );
-
       // #686
       const target = document.activeElement;
       const isExcalidrawActive =
         this.excalidrawContainerRef.current?.contains(target);
       if (event && !isExcalidrawActive) {
-        console.log("exit (1)");
         return;
       }
 
@@ -2228,7 +2215,6 @@ class App extends React.Component<AppProps, AppState> {
         (!(elementUnderCursor instanceof HTMLCanvasElement) ||
           isWritableElement(target))
       ) {
-        console.log("exit (2)");
         return;
       }
 
@@ -2568,18 +2554,12 @@ class App extends React.Component<AppProps, AppState> {
 
         const lineHeight = getDefaultLineHeight(textElementProps.fontFamily);
         if (text.length) {
-          const topLayerFrame = this.getTopLayerFrameAtSceneCoords({
-            x,
-            y: currentY,
-          });
-
           const element = newTextElement({
             ...textElementProps,
             x,
             y: currentY,
             text,
             lineHeight,
-            frameId: topLayerFrame ? topLayerFrame.id : null,
           });
           acc.push(element);
           currentY += element.height + LINE_GAP;
@@ -3594,6 +3574,11 @@ class App extends React.Component<AppProps, AppState> {
     return getElementsAtPosition(elements, (element) =>
       hitTest(element, this.state, this.frameNameBoundsCache, x, y),
     ).filter((element) => {
+      // arrows don't clip even if they're children of frames,
+      // so always allow hitbox regardless of beinging contained in frame
+      if (isArrowElement(element)) {
+        return true;
+      }
       // hitting a frame's element from outside the frame is not considered a hit
       const containingFrame = getContainingFrame(element);
       return containingFrame &&
@@ -3765,9 +3750,32 @@ class App extends React.Component<AppProps, AppState> {
       return;
     }
 
+    let { x: sceneX, y: sceneY } = viewportCoordsToSceneCoords(
+      event,
+      this.state,
+    );
+
     const selectedElements = this.scene.getSelectedElements(this.state);
 
     if (selectedElements.length === 1 && isLinearElement(selectedElements[0])) {
+      if (!event[KEYS.CTRL_OR_CMD]) {
+        // If double clicked without any ctrl/cmd modifier on top of a point,
+        // toggle split mode for that point. Else, treat as regular double click.
+        const pointUnderCursorIndex =
+          LinearElementEditor.getPointIndexUnderCursor(
+            selectedElements[0],
+            this.state.zoom,
+            sceneX,
+            sceneY,
+          );
+        if (pointUnderCursorIndex >= 0) {
+          LinearElementEditor.toggleSegmentSplitAtIndex(
+            selectedElements[0],
+            pointUnderCursorIndex,
+          );
+          return;
+        }
+      }
       if (
         event[KEYS.CTRL_OR_CMD] &&
         (!this.state.editingLinearElement ||
@@ -3791,11 +3799,6 @@ class App extends React.Component<AppProps, AppState> {
 
     resetCursor(this.interactiveCanvas);
 
-    let { x: sceneX, y: sceneY } = viewportCoordsToSceneCoords(
-      event,
-      this.state,
-    );
-
     const selectedGroupIds = getSelectedGroupIds(this.state);
 
     if (selectedGroupIds.length > 0) {

src/components/ColorPicker/TopPicks.tsx

@@ -55,7 +55,6 @@ export const TopPicks = ({
           type="button"
           title={color}
           onClick={() => onChange(color)}
-          data-testid={`color-top-pick-${color}`}
         >
           <div className="color-picker__button-outline" />
         </button>

src/components/ContextMenu.tsx

@@ -9,7 +9,11 @@ import {
 } from "../actions/shortcuts";
 import { Action } from "../actions/types";
 import { ActionManager } from "../actions/manager";
-import { useExcalidrawAppState, useExcalidrawElements } from "./App";
+import {
+  useExcalidrawAppState,
+  useExcalidrawElements,
+  useExcalidrawSetAppState,
+} from "./App";
 import React from "react";
 
 export type ContextMenuItem = typeof CONTEXT_MENU_SEPARATOR | Action;
@@ -21,14 +25,14 @@ type ContextMenuProps = {
   items: ContextMenuItems;
   top: number;
   left: number;
-  onClose: (cb?: () => void) => void;
 };
 
 export const CONTEXT_MENU_SEPARATOR = "separator";
 
 export const ContextMenu = React.memo(
-  ({ actionManager, items, top, left, onClose }: ContextMenuProps) => {
+  ({ actionManager, items, top, left }: ContextMenuProps) => {
     const appState = useExcalidrawAppState();
+    const setAppState = useExcalidrawSetAppState();
     const elements = useExcalidrawElements();
 
     const filteredItems = items.reduce((acc: ContextMenuItem[], item) => {
@@ -50,7 +54,7 @@ export const ContextMenu = React.memo(
 
     return (
       <Popover
-        onCloseRequest={() => onClose()}
+        onCloseRequest={() => setAppState({ contextMenu: null })}
         top={top}
         left={left}
         fitInViewport={true}
@@ -98,7 +102,7 @@ export const ContextMenu = React.memo(
                   // we need update state before executing the action in case
                   // the action uses the appState it's being passed (that still
                   // contains a defined contextMenu) to return the next state.
-                  onClose(() => {
+                  setAppState({ contextMenu: null }, () => {
                     actionManager.executeAction(item, "contextMenu");
                   });
                 }}

src/constants.ts

@@ -302,12 +302,6 @@ export const ROUGHNESS = {
   cartoonist: 2,
 } as const;
 
-export const STROKE_WIDTH = {
-  thin: 1,
-  bold: 2,
-  extraBold: 4,
-} as const;
-
 export const DEFAULT_ELEMENT_PROPS: {
   strokeColor: ExcalidrawElement["strokeColor"];
   backgroundColor: ExcalidrawElement["backgroundColor"];

src/data/__snapshots__/transform.test.ts.snap

@@ -6,11 +6,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "backgroundColor": "#d8f5a2",
   "boundElements": [
     {
-      "id": "id41",
+      "id": "id40",
       "type": "arrow",
     },
     {
-      "id": "id42",
+      "id": "id41",
       "type": "arrow",
     },
   ],
@@ -45,7 +45,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id42",
+      "id": "id41",
       "type": "arrow",
     },
   ],
@@ -97,12 +97,12 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
-      0.5,
+      0,
     ],
     [
-      394.5,
+      395,
-      34.5,
+      35,
     ],
   ],
   "roughness": 1,
@@ -110,7 +110,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": {
-    "elementId": "id43",
+    "elementId": "id42",
     "focus": -0.08139534883720931,
     "gap": 1,
   },
@@ -150,11 +150,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      399.5,
+      400,
       0,
     ],
   ],
@@ -186,7 +186,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id41",
+      "id": "id40",
       "type": "arrow",
     },
   ],
@@ -222,7 +222,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "baseline": 0,
   "boundElements": [
     {
-      "id": "id44",
+      "id": "id43",
       "type": "arrow",
     },
   ],
@@ -266,7 +266,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "baseline": 0,
   "boundElements": [
     {
-      "id": "id44",
+      "id": "id43",
       "type": "arrow",
     },
   ],
@@ -309,7 +309,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id45",
+      "id": "id44",
       "type": "text",
     },
   ],
@@ -317,7 +317,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "endBinding": {
     "elementId": "text-2",
     "focus": 0,
-    "gap": 205,
+    "gap": 5,
   },
   "fillStyle": "solid",
   "frameId": null,
@@ -331,11 +331,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -355,7 +355,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "updated": 1,
   "version": 3,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 255,
   "y": 239,
 }
@@ -367,7 +367,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id44",
+  "containerId": "id43",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -395,7 +395,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 130,
-  "x": 240,
+  "x": 340,
   "y": 226.5,
 }
 `;
@@ -406,13 +406,13 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id34",
+      "id": "id33",
       "type": "text",
     },
   ],
   "endArrowhead": "arrow",
   "endBinding": {
-    "elementId": "id36",
+    "elementId": "id35",
     "focus": 0,
     "gap": 1,
   },
@@ -428,11 +428,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -441,7 +441,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": {
-    "elementId": "id35",
+    "elementId": "id34",
     "focus": 0,
     "gap": 1,
   },
@@ -452,7 +452,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "updated": 1,
   "version": 3,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 255,
   "y": 239,
 }
@@ -464,7 +464,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id33",
+  "containerId": "id32",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -492,7 +492,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 130,
-  "x": 240,
+  "x": 340,
   "y": 226.5,
 }
 `;
@@ -503,7 +503,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id33",
+      "id": "id32",
       "type": "arrow",
     },
   ],
@@ -538,7 +538,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id33",
+      "id": "id32",
       "type": "arrow",
     },
   ],
@@ -562,7 +562,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "version": 2,
   "versionNonce": Any<Number>,
   "width": 100,
-  "x": 355,
+  "x": 555,
   "y": 189,
 }
 `;
@@ -573,13 +573,13 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id38",
+      "id": "id37",
       "type": "text",
     },
   ],
   "endArrowhead": "arrow",
   "endBinding": {
-    "elementId": "id40",
+    "elementId": "id39",
     "focus": 0,
     "gap": 1,
   },
@@ -595,11 +595,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -608,7 +608,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": {
-    "elementId": "id39",
+    "elementId": "id38",
     "focus": 0,
     "gap": 1,
   },
@@ -619,7 +619,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "updated": 1,
   "version": 3,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 255,
   "y": 239,
 }
@@ -631,7 +631,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id37",
+  "containerId": "id36",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -659,7 +659,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 130,
-  "x": 240,
+  "x": 340,
   "y": 226.5,
 }
 `;
@@ -671,7 +671,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "baseline": 0,
   "boundElements": [
     {
-      "id": "id37",
+      "id": "id36",
       "type": "arrow",
     },
   ],
@@ -715,7 +715,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "baseline": 0,
   "boundElements": [
     {
-      "id": "id37",
+      "id": "id36",
       "type": "arrow",
     },
   ],
@@ -747,7 +747,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "versionNonce": Any<Number>,
   "verticalAlign": "top",
   "width": 100,
-  "x": 355,
+  "x": 555,
   "y": 226.5,
 }
 `;
@@ -801,11 +801,11 @@ exports[`Test Transform > should transform linear elements 1`] = `
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -821,7 +821,7 @@ exports[`Test Transform > should transform linear elements 1`] = `
   "updated": 1,
   "version": 1,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 100,
   "y": 20,
 }
@@ -846,11 +846,11 @@ exports[`Test Transform > should transform linear elements 2`] = `
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -866,7 +866,7 @@ exports[`Test Transform > should transform linear elements 2`] = `
   "updated": 1,
   "version": 1,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 450,
   "y": 20,
 }
@@ -895,7 +895,7 @@ exports[`Test Transform > should transform linear elements 3`] = `
       0,
     ],
     [
-      100,
+      300,
       0,
     ],
   ],
@@ -911,7 +911,7 @@ exports[`Test Transform > should transform linear elements 3`] = `
   "updated": 1,
   "version": 1,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 100,
   "y": 60,
 }
@@ -940,7 +940,7 @@ exports[`Test Transform > should transform linear elements 4`] = `
       0,
     ],
     [
-      100,
+      300,
       0,
     ],
   ],
@@ -956,7 +956,7 @@ exports[`Test Transform > should transform linear elements 4`] = `
   "updated": 1,
   "version": 1,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 450,
   "y": 60,
 }
@@ -1221,6 +1221,56 @@ exports[`Test Transform > should transform text element 2`] = `
 `;
 
 exports[`Test Transform > should transform to labelled arrows when label provided for arrows 1`] = `
+{
+  "angle": 0,
+  "backgroundColor": "transparent",
+  "boundElements": [
+    {
+      "id": "id28",
+      "type": "text",
+    },
+  ],
+  "endArrowhead": "arrow",
+  "endBinding": null,
+  "fillStyle": "solid",
+  "frameId": null,
+  "groupIds": [],
+  "height": 0,
+  "id": Any<String>,
+  "isDeleted": false,
+  "lastCommittedPoint": null,
+  "link": null,
+  "locked": false,
+  "opacity": 100,
+  "points": [
+    [
+      0,
+      0,
+    ],
+    [
+      300,
+      0,
+    ],
+  ],
+  "roughness": 1,
+  "roundness": null,
+  "seed": Any<Number>,
+  "startArrowhead": null,
+  "startBinding": null,
+  "strokeColor": "#1e1e1e",
+  "strokeStyle": "solid",
+  "strokeWidth": 2,
+  "type": "arrow",
+  "updated": 1,
+  "version": 1,
+  "versionNonce": Any<Number>,
+  "width": 300,
+  "x": 100,
+  "y": 100,
+}
+`;
+
+exports[`Test Transform > should transform to labelled arrows when label provided for arrows 2`] = `
 {
   "angle": 0,
   "backgroundColor": "transparent",
@@ -1244,11 +1294,11 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -1264,13 +1314,13 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "updated": 1,
   "version": 1,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 100,
-  "y": 100,
+  "y": 200,
 }
 `;
 
-exports[`Test Transform > should transform to labelled arrows when label provided for arrows 2`] = `
+exports[`Test Transform > should transform to labelled arrows when label provided for arrows 3`] = `
 {
   "angle": 0,
   "backgroundColor": "transparent",
@@ -1285,7 +1335,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": 0,
+  "height": 130,
   "id": Any<String>,
   "isDeleted": false,
   "lastCommittedPoint": null,
@@ -1294,11 +1344,11 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -1307,20 +1357,20 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": null,
-  "strokeColor": "#1e1e1e",
+  "strokeColor": "#1098ad",
   "strokeStyle": "solid",
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 1,
+  "version": 2,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
   "x": 100,
-  "y": 200,
+  "y": 300,
 }
 `;
 
-exports[`Test Transform > should transform to labelled arrows when label provided for arrows 3`] = `
+exports[`Test Transform > should transform to labelled arrows when label provided for arrows 4`] = `
 {
   "angle": 0,
   "backgroundColor": "transparent",
@@ -1344,11 +1394,11 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "opacity": 100,
   "points": [
     [
-      0.5,
+      0,
       0,
     ],
     [
-      99.5,
+      300,
       0,
     ],
   ],
@@ -1364,57 +1414,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "updated": 1,
   "version": 2,
   "versionNonce": Any<Number>,
-  "width": 100,
+  "width": 300,
-  "x": 100,
-  "y": 300,
-}
-`;
-
-exports[`Test Transform > should transform to labelled arrows when label provided for arrows 4`] = `
-{
-  "angle": 0,
-  "backgroundColor": "transparent",
-  "boundElements": [
-    {
-      "id": "id32",
-      "type": "text",
-    },
-  ],
-  "endArrowhead": "arrow",
-  "endBinding": null,
-  "fillStyle": "solid",
-  "frameId": null,
-  "groupIds": [],
-  "height": 130,
-  "id": Any<String>,
-  "isDeleted": false,
-  "lastCommittedPoint": null,
-  "link": null,
-  "locked": false,
-  "opacity": 100,
-  "points": [
-    [
-      0.5,
-      0,
-    ],
-    [
-      99.5,
-      0,
-    ],
-  ],
-  "roughness": 1,
-  "roundness": null,
-  "seed": Any<Number>,
-  "startArrowhead": null,
-  "startBinding": null,
-  "strokeColor": "#1098ad",
-  "strokeStyle": "solid",
-  "strokeWidth": 2,
-  "type": "arrow",
-  "updated": 1,
-  "version": 2,
-  "versionNonce": Any<Number>,
-  "width": 100,
   "x": 100,
   "y": 400,
 }
@@ -1426,7 +1426,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id25",
+  "containerId": "id24",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1454,7 +1454,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 130,
-  "x": 85,
+  "x": 185,
   "y": 87.5,
 }
 `;
@@ -1465,7 +1465,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id26",
+  "containerId": "id25",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1493,7 +1493,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 200,
-  "x": 50,
+  "x": 150,
   "y": 187.5,
 }
 `;
@@ -1504,7 +1504,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id27",
+  "containerId": "id26",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1533,7 +1533,7 @@ LABELLED ARROW",
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 150,
-  "x": 75,
+  "x": 175,
   "y": 275,
 }
 `;
@@ -1544,7 +1544,7 @@ exports[`Test Transform > should transform to labelled arrows when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id28",
+  "containerId": "id27",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1573,7 +1573,7 @@ LABELLED ARROW",
   "versionNonce": Any<Number>,
   "verticalAlign": "middle",
   "width": 150,
-  "x": 75,
+  "x": 175,
   "y": 375,
 }
 `;
@@ -1584,7 +1584,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id19",
+      "id": "id18",
       "type": "text",
     },
   ],
@@ -1619,7 +1619,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id20",
+      "id": "id19",
       "type": "text",
     },
   ],
@@ -1654,7 +1654,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id21",
+      "id": "id20",
       "type": "text",
     },
   ],
@@ -1689,7 +1689,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "#fff3bf",
   "boundElements": [
     {
-      "id": "id22",
+      "id": "id21",
       "type": "text",
     },
   ],
@@ -1724,7 +1724,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id23",
+      "id": "id22",
       "type": "text",
     },
   ],
@@ -1759,7 +1759,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "#ffec99",
   "boundElements": [
     {
-      "id": "id24",
+      "id": "id23",
       "type": "text",
     },
   ],
@@ -1794,7 +1794,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id13",
+  "containerId": "id12",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1833,7 +1833,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id14",
+  "containerId": "id13",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1873,7 +1873,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id15",
+  "containerId": "id14",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1915,7 +1915,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id16",
+  "containerId": "id15",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1955,7 +1955,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id17",
+  "containerId": "id16",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,
@@ -1996,7 +1996,7 @@ exports[`Test Transform > should transform to text containers when label provide
   "backgroundColor": "transparent",
   "baseline": 0,
   "boundElements": null,
-  "containerId": "id18",
+  "containerId": "id17",
   "fillStyle": "solid",
   "fontFamily": 1,
   "fontSize": 20,

src/data/restore.ts

@@ -43,6 +43,7 @@ import {
   measureBaseline,
 } from "../element/textElement";
 import { normalizeLink } from "./url";
+import { isValidFrameChild } from "../frame";
 
 type RestoredAppState = Omit<
   AppState,
@@ -284,6 +285,9 @@ const restoreElement = (
         points,
         x,
         y,
+        segmentSplitIndices: element.segmentSplitIndices
+          ? [...element.segmentSplitIndices]
+          : [],
       });
     }
 
@@ -396,7 +400,7 @@ const repairBoundElement = (
 };
 
 /**
- * Remove an element's frameId if its containing frame is non-existent
+ * resets `frameId` if no longer applicable.
  *
  * NOTE mutates elements.
  */
@@ -404,12 +408,16 @@ const repairFrameMembership = (
   element: Mutable<ExcalidrawElement>,
   elementsMap: Map<string, Mutable<ExcalidrawElement>>,
 ) => {
-  if (element.frameId) {
+  if (!element.frameId) {
-    const containingFrame = elementsMap.get(element.frameId);
+    return;
+  }
 
-    if (!containingFrame) {
+  if (
-      element.frameId = null;
+    !isValidFrameChild(element) ||
-    }
+    // target frame not exists
+    !elementsMap.get(element.frameId)
+  ) {
+    element.frameId = null;
   }
 };
 
@@ -453,6 +461,8 @@ export const restoreElements = (
   // repair binding. Mutates elements.
   const restoredElementsMap = arrayToMap(restoredElements);
   for (const element of restoredElements) {
+    // repair frame membership *after* bindings we do in restoreElement()
+    // since we rely on bindings to be correct
     if (element.frameId) {
       repairFrameMembership(element, restoredElementsMap);
     }

src/data/transform.test.ts

@@ -5,31 +5,7 @@ import {
 } from "./transform";
 import { ExcalidrawArrowElement } from "../element/types";
 
-const opts = { regenerateIds: false };
-
 describe("Test Transform", () => {
-  it("should generate id unless opts.regenerateIds is set to false explicitly", () => {
-    const elements = [
-      {
-        type: "rectangle",
-        x: 100,
-        y: 100,
-        id: "rect-1",
-      },
-    ];
-    let data = convertToExcalidrawElements(
-      elements as ExcalidrawElementSkeleton[],
-    );
-    expect(data.length).toBe(1);
-    expect(data[0].id).toBe("id0");
-
-    data = convertToExcalidrawElements(
-      elements as ExcalidrawElementSkeleton[],
-      opts,
-    );
-    expect(data[0].id).toBe("rect-1");
-  });
-
   it("should transform regular shapes", () => {
     const elements = [
       {
@@ -83,7 +59,6 @@ describe("Test Transform", () => {
 
     convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     ).forEach((ele) => {
       expect(ele).toMatchSnapshot({
         seed: expect.any(Number),
@@ -112,7 +87,6 @@ describe("Test Transform", () => {
     ];
     convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     ).forEach((ele) => {
       expect(ele).toMatchSnapshot({
         seed: expect.any(Number),
@@ -154,7 +128,6 @@ describe("Test Transform", () => {
     ];
     const excaldrawElements = convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     );
 
     expect(excaldrawElements.length).toBe(4);
@@ -237,7 +210,6 @@ describe("Test Transform", () => {
     ];
     const excaldrawElements = convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     );
 
     expect(excaldrawElements.length).toBe(12);
@@ -295,7 +267,6 @@ describe("Test Transform", () => {
     ];
     const excaldrawElements = convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     );
 
     expect(excaldrawElements.length).toBe(8);
@@ -329,7 +300,6 @@ describe("Test Transform", () => {
       ];
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
 
       expect(excaldrawElements.length).toBe(4);
@@ -351,7 +321,7 @@ describe("Test Transform", () => {
       });
 
       expect(text).toMatchObject({
-        x: 240,
+        x: 340,
         y: 226.5,
         type: "text",
         text: "HELLO WORLD!!",
@@ -371,7 +341,7 @@ describe("Test Transform", () => {
       });
 
       expect(ellipse).toMatchObject({
-        x: 355,
+        x: 555,
         y: 189,
         type: "ellipse",
         boundElements: [
@@ -413,10 +383,10 @@ describe("Test Transform", () => {
 
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
 
       expect(excaldrawElements.length).toBe(4);
+
       const [arrow, text1, text2, text3] = excaldrawElements;
 
       expect(arrow).toMatchObject({
@@ -436,7 +406,7 @@ describe("Test Transform", () => {
       });
 
       expect(text1).toMatchObject({
-        x: 240,
+        x: 340,
         y: 226.5,
         type: "text",
         text: "HELLO WORLD!!",
@@ -456,7 +426,7 @@ describe("Test Transform", () => {
       });
 
       expect(text3).toMatchObject({
-        x: 355,
+        x: 555,
         y: 226.5,
         type: "text",
         boundElements: [
@@ -529,7 +499,6 @@ describe("Test Transform", () => {
 
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
 
       expect(excaldrawElements.length).toBe(5);
@@ -578,7 +547,6 @@ describe("Test Transform", () => {
 
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
 
       expect(excaldrawElements.length).toBe(4);
@@ -632,18 +600,17 @@ describe("Test Transform", () => {
 
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
 
       expect(excaldrawElements.length).toBe(4);
-      const [, , arrow, text] = excaldrawElements;
+      const [, , arrow] = excaldrawElements;
       expect(arrow).toMatchObject({
         type: "arrow",
         x: 255,
         y: 239,
         boundElements: [
           {
-            id: text.id,
+            id: "id46",
             type: "text",
           },
         ],
@@ -683,18 +650,17 @@ describe("Test Transform", () => {
       ];
       const excaldrawElements = convertToExcalidrawElements(
         elements as ExcalidrawElementSkeleton[],
-        opts,
       );
       expect(excaldrawElements.length).toBe(2);
       const [arrow, rect] = excaldrawElements;
       expect((arrow as ExcalidrawArrowElement).endBinding).toStrictEqual({
         elementId: "rect-1",
         focus: 0,
-        gap: 205,
+        gap: 5,
       });
       expect(rect.boundElements).toStrictEqual([
         {
-          id: arrow.id,
+          id: "id47",
           type: "arrow",
         },
       ]);
@@ -726,7 +692,6 @@ describe("Test Transform", () => {
     ];
     const excaldrawElements = convertToExcalidrawElements(
       elements as ExcalidrawElementSkeleton[],
-      opts,
     );
 
     expect(excaldrawElements.length).toBe(1);

src/data/transform.ts

@@ -39,8 +39,6 @@ import {
 } from "../element/types";
 import { MarkOptional } from "../utility-types";
 import { assertNever, getFontString } from "../utils";
-import { getSizeFromPoints } from "../points";
-import { randomId } from "../random";
 
 export type ValidLinearElement = {
   type: "arrow" | "line";
@@ -161,7 +159,7 @@ export type ExcalidrawElementSkeleton =
     } & Partial<ExcalidrawImageElement>);
 
 const DEFAULT_LINEAR_ELEMENT_PROPS = {
-  width: 100,
+  width: 300,
   height: 0,
 };
 
@@ -359,48 +357,6 @@ const bindLinearElementToElement = (
       );
     }
   }
-
-  // Update start/end points by 0.5 so bindings don't overlap with start/end bound element coordinates.
-  const endPointIndex = linearElement.points.length - 1;
-  const delta = 0.5;
-  const newPoints = JSON.parse(JSON.stringify(linearElement.points));
-  // left to right so shift the arrow towards right
-  if (
-    linearElement.points[endPointIndex][0] >
-    linearElement.points[endPointIndex - 1][0]
-  ) {
-    newPoints[0][0] = delta;
-    newPoints[endPointIndex][0] -= delta;
-  }
-
-  // right to left so shift the arrow towards left
-  if (
-    linearElement.points[endPointIndex][0] <
-    linearElement.points[endPointIndex - 1][0]
-  ) {
-    newPoints[0][0] = -delta;
-    newPoints[endPointIndex][0] += delta;
-  }
-  // top to bottom so shift the arrow towards top
-  if (
-    linearElement.points[endPointIndex][1] >
-    linearElement.points[endPointIndex - 1][1]
-  ) {
-    newPoints[0][1] = delta;
-    newPoints[endPointIndex][1] -= delta;
-  }
-
-  // bottom to top so shift the arrow towards bottom
-  if (
-    linearElement.points[endPointIndex][1] <
-    linearElement.points[endPointIndex - 1][1]
-  ) {
-    newPoints[0][1] = -delta;
-    newPoints[endPointIndex][1] += delta;
-  }
-
-  Object.assign(linearElement, { points: newPoints });
-
   return {
     linearElement,
     startBoundElement,
@@ -428,28 +384,18 @@ class ElementStore {
 }
 
 export const convertToExcalidrawElements = (
-  elementsSkeleton: ExcalidrawElementSkeleton[] | null,
+  elements: ExcalidrawElementSkeleton[] | null,
-  opts?: { regenerateIds: boolean },
 ) => {
-  if (!elementsSkeleton) {
+  if (!elements) {
     return [];
   }
-  const elements: ExcalidrawElementSkeleton[] = JSON.parse(
-    JSON.stringify(elementsSkeleton),
-  );
 
   const elementStore = new ElementStore();
   const elementsWithIds = new Map<string, ExcalidrawElementSkeleton>();
-  const oldToNewElementIdMap = new Map<string, string>();
 
   // Create individual elements
   for (const element of elements) {
     let excalidrawElement: ExcalidrawElement;
-    const originalId = element.id;
-    if (opts?.regenerateIds !== false) {
-      Object.assign(element, { id: randomId() });
-    }
-
     switch (element.type) {
       case "rectangle":
       case "ellipse":
@@ -498,11 +444,6 @@ export const convertToExcalidrawElements = (
           ],
           ...element,
         });
-
-        Object.assign(
-          excalidrawElement,
-          getSizeFromPoints(excalidrawElement.points),
-        );
         break;
       }
       case "text": {
@@ -558,9 +499,6 @@ export const convertToExcalidrawElements = (
     } else {
       elementStore.add(excalidrawElement);
       elementsWithIds.set(excalidrawElement.id, element);
-      if (originalId) {
-        oldToNewElementIdMap.set(originalId, excalidrawElement.id);
-      }
     }
   }
 
@@ -586,18 +524,6 @@ export const convertToExcalidrawElements = (
               element.type === "arrow" ? element?.start : undefined;
             const originalEnd =
               element.type === "arrow" ? element?.end : undefined;
-            if (originalStart && originalStart.id) {
-              const newStartId = oldToNewElementIdMap.get(originalStart.id);
-              if (newStartId) {
-                Object.assign(originalStart, { id: newStartId });
-              }
-            }
-            if (originalEnd && originalEnd.id) {
-              const newEndId = oldToNewElementIdMap.get(originalEnd.id);
-              if (newEndId) {
-                Object.assign(originalEnd, { id: newEndId });
-              }
-            }
             const { linearElement, startBoundElement, endBoundElement } =
               bindLinearElementToElement(
                 container as ExcalidrawArrowElement,
@@ -613,23 +539,13 @@ export const convertToExcalidrawElements = (
         } else {
           switch (element.type) {
             case "arrow": {
-              const { start, end } = element;
-              if (start && start.id) {
-                const newStartId = oldToNewElementIdMap.get(start.id);
-                Object.assign(start, { id: newStartId });
-              }
-              if (end && end.id) {
-                const newEndId = oldToNewElementIdMap.get(end.id);
-                Object.assign(end, { id: newEndId });
-              }
               const { linearElement, startBoundElement, endBoundElement } =
                 bindLinearElementToElement(
                   excalidrawElement as ExcalidrawArrowElement,
-                  start,
+                  element.start,
-                  end,
+                  element.end,
                   elementStore,
                 );
-
               elementStore.add(linearElement);
               elementStore.add(startBoundElement);
               elementStore.add(endBoundElement);

src/element/Hyperlink.tsx

@@ -392,7 +392,7 @@ export const getLinkHandleFromCoords = (
   [x1, y1, x2, y2]: Bounds,
   angle: number,
   appState: Pick<UIAppState, "zoom">,
-): Bounds => {
+): [x: number, y: number, width: number, height: number] => {
   const size = DEFAULT_LINK_SIZE;
   const linkWidth = size / appState.zoom.value;
   const linkHeight = size / appState.zoom.value;

src/element/binding.ts

@@ -27,6 +27,7 @@ import { LinearElementEditor } from "./linearElementEditor";
 import { arrayToMap, tupleToCoors } from "../utils";
 import { KEYS } from "../keys";
 import { getBoundTextElement, handleBindTextResize } from "./textElement";
+import { isValidFrameChild } from "../frame";
 
 export type SuggestedBinding =
   | NonDeleted<ExcalidrawBindableElement>
@@ -211,6 +212,15 @@ export const bindLinearElement = (
       }),
     });
   }
+  if (linearElement.frameId && !isValidFrameChild(linearElement)) {
+    mutateElement(
+      linearElement,
+      {
+        frameId: null,
+      },
+      false,
+    );
+  }
 };
 
 // Don't bind both ends of a simple segment

src/element/bounds.ts

@@ -34,12 +34,7 @@ export type RectangleBox = {
 type MaybeQuadraticSolution = [number | null, number | null] | false;
 
 // x and y position of top left corner, x and y position of bottom right corner
-export type Bounds = readonly [
+export type Bounds = readonly [x1: number, y1: number, x2: number, y2: number];
-  minX: number,
-  minY: number,
-  maxX: number,
-  maxY: number,
-];
 
 export class ElementBounds {
   private static boundsCache = new WeakMap<
@@ -68,7 +63,7 @@ export class ElementBounds {
   }
 
   private static calculateBounds(element: ExcalidrawElement): Bounds {
-    let bounds: Bounds;
+    let bounds: [number, number, number, number];
 
     const [x1, y1, x2, y2, cx, cy] = getElementAbsoluteCoords(element);
 
@@ -392,7 +387,7 @@ const getCubicBezierCurveBound = (
 export const getMinMaxXYFromCurvePathOps = (
   ops: Op[],
   transformXY?: (x: number, y: number) => [number, number],
-): Bounds => {
+): [number, number, number, number] => {
   let currentP: Point = [0, 0];
 
   const { minX, minY, maxX, maxY } = ops.reduce(
@@ -440,9 +435,9 @@ export const getMinMaxXYFromCurvePathOps = (
   return [minX, minY, maxX, maxY];
 };
 
-export const getBoundsFromPoints = (
+const getBoundsFromPoints = (
   points: ExcalidrawFreeDrawElement["points"],
-): Bounds => {
+): [number, number, number, number] => {
   let minX = Infinity;
   let minY = Infinity;
   let maxX = -Infinity;
@@ -594,7 +589,7 @@ const getLinearElementRotatedBounds = (
   element: ExcalidrawLinearElement,
   cx: number,
   cy: number,
-): Bounds => {
+): [number, number, number, number] => {
   if (element.points.length < 2) {
     const [pointX, pointY] = element.points[0];
     const [x, y] = rotate(
@@ -605,7 +600,7 @@ const getLinearElementRotatedBounds = (
       element.angle,
     );
 
-    let coords: Bounds = [x, y, x, y];
+    let coords: [number, number, number, number] = [x, y, x, y];
     const boundTextElement = getBoundTextElement(element);
     if (boundTextElement) {
       const coordsWithBoundText = LinearElementEditor.getMinMaxXYWithBoundText(
@@ -630,7 +625,12 @@ const getLinearElementRotatedBounds = (
   const transformXY = (x: number, y: number) =>
     rotate(element.x + x, element.y + y, cx, cy, element.angle);
   const res = getMinMaxXYFromCurvePathOps(ops, transformXY);
-  let coords: Bounds = [res[0], res[1], res[2], res[3]];
+  let coords: [number, number, number, number] = [
+    res[0],
+    res[1],
+    res[2],
+    res[3],
+  ];
   const boundTextElement = getBoundTextElement(element);
   if (boundTextElement) {
     const coordsWithBoundText = LinearElementEditor.getMinMaxXYWithBoundText(
@@ -692,7 +692,7 @@ export const getResizedElementAbsoluteCoords = (
   nextWidth: number,
   nextHeight: number,
   normalizePoints: boolean,
-): Bounds => {
+): [number, number, number, number] => {
   if (!(isLinearElement(element) || isFreeDrawElement(element))) {
     return [
       element.x,
@@ -709,7 +709,7 @@ export const getResizedElementAbsoluteCoords = (
     normalizePoints,
   );
 
-  let bounds: Bounds;
+  let bounds: [number, number, number, number];
 
   if (isFreeDrawElement(element)) {
     // Free Draw
@@ -740,8 +740,8 @@ export const getResizedElementAbsoluteCoords = (
 export const getElementPointsCoords = (
   element: ExcalidrawLinearElement,
   points: readonly (readonly [number, number])[],
-): Bounds => {
+): [number, number, number, number] => {
-  // This might be computationally heavey
+  // This might be computationally heavy
   const gen = rough.generator();
   const curve =
     element.roundness == null

src/element/collision.ts

@@ -494,9 +494,7 @@ const hitTestFreeDrawElement = (
   // for filled freedraw shapes, support
   // selecting from inside
   if (shape && shape.sets.length) {
-    return element.fillStyle === "solid"
+    return hitTestCurveInside(shape, x, y, "round");
-      ? hitTestCurveInside(shape, x, y, "round")
-      : hitTestRoughShape(shape, x, y, threshold);
   }
 
   return false;

src/element/dragElements.ts

@@ -1,5 +1,5 @@
 import { updateBoundElements } from "./binding";
-import { Bounds, getCommonBounds } from "./bounds";
+import { getCommonBounds } from "./bounds";
 import { mutateElement } from "./mutateElement";
 import { getPerfectElementSize } from "./sizeHelpers";
 import { NonDeletedExcalidrawElement } from "./types";
@@ -8,11 +8,7 @@ import { getBoundTextElement } from "./textElement";
 import { isSelectedViaGroup } from "../groups";
 import { getGridPoint } from "../math";
 import Scene from "../scene/Scene";
-import {
+import { isFrameElement } from "./typeChecks";
-  isArrowElement,
-  isBoundToContainer,
-  isFrameElement,
-} from "./typeChecks";
 
 export const dragSelectedElements = (
   pointerDownState: PointerDownState,
@@ -39,41 +35,44 @@ export const dragSelectedElements = (
   if (frames.length > 0) {
     const elementsInFrames = scene
       .getNonDeletedElements()
-      .filter((e) => !isBoundToContainer(e))
       .filter((e) => e.frameId !== null)
       .filter((e) => frames.includes(e.frameId!));
 
     elementsInFrames.forEach((element) => elementsToUpdate.add(element));
   }
 
-  const commonBounds = getCommonBounds(
-    Array.from(elementsToUpdate).map(
-      (el) => pointerDownState.originalElements.get(el.id) ?? el,
-    ),
-  );
-  const adjustedOffset = calculateOffset(
-    commonBounds,
-    offset,
-    snapOffset,
-    gridSize,
-  );
-
   elementsToUpdate.forEach((element) => {
-    updateElementCoords(pointerDownState, element, adjustedOffset);
+    updateElementCoords(
+      pointerDownState,
+      element,
+      offset,
+      snapOffset,
+      gridSize,
+    );
     // update coords of bound text only if we're dragging the container directly
     // (we don't drag the group that it's part of)
     if (
-      // Don't update coords of arrow label since we calculate its position during render
-      !isArrowElement(element) &&
       // container isn't part of any group
       // (perf optim so we don't check `isSelectedViaGroup()` in every case)
-      (!element.groupIds.length ||
+      !element.groupIds.length ||
-        // container is part of a group, but we're dragging the container directly
+      // container is part of a group, but we're dragging the container directly
-        (appState.editingGroupId && !isSelectedViaGroup(appState, element)))
+      (appState.editingGroupId && !isSelectedViaGroup(appState, element))
     ) {
       const textElement = getBoundTextElement(element);
-      if (textElement) {
+      if (
-        updateElementCoords(pointerDownState, textElement, adjustedOffset);
+        textElement &&
+        // when container is added to a frame, so will its bound text
+        // so the text is already in `elementsToUpdate` and we should avoid
+        // updating its coords again
+        (!textElement.frameId || !frames.includes(textElement.frameId))
+      ) {
+        updateElementCoords(
+          pointerDownState,
+          textElement,
+          offset,
+          snapOffset,
+          gridSize,
+        );
       }
     }
     updateBoundElements(element, {
@@ -82,20 +81,23 @@ export const dragSelectedElements = (
   });
 };
 
-const calculateOffset = (
+const updateElementCoords = (
-  commonBounds: Bounds,
+  pointerDownState: PointerDownState,
+  element: NonDeletedExcalidrawElement,
   dragOffset: { x: number; y: number },
   snapOffset: { x: number; y: number },
   gridSize: AppState["gridSize"],
-): { x: number; y: number } => {
+) => {
-  const [x, y] = commonBounds;
+  const originalElement =
-  let nextX = x + dragOffset.x + snapOffset.x;
+    pointerDownState.originalElements.get(element.id) ?? element;
-  let nextY = y + dragOffset.y + snapOffset.y;
+
+  let nextX = originalElement.x + dragOffset.x + snapOffset.x;
+  let nextY = originalElement.y + dragOffset.y + snapOffset.y;
 
   if (snapOffset.x === 0 || snapOffset.y === 0) {
     const [nextGridX, nextGridY] = getGridPoint(
-      x + dragOffset.x,
+      originalElement.x + dragOffset.x,
-      y + dragOffset.y,
+      originalElement.y + dragOffset.y,
       gridSize,
     );
 
@@ -107,22 +109,6 @@ const calculateOffset = (
       nextY = nextGridY;
     }
   }
-  return {
-    x: nextX - x,
-    y: nextY - y,
-  };
-};
-
-const updateElementCoords = (
-  pointerDownState: PointerDownState,
-  element: NonDeletedExcalidrawElement,
-  dragOffset: { x: number; y: number },
-) => {
-  const originalElement =
-    pointerDownState.originalElements.get(element.id) ?? element;
-
-  const nextX = originalElement.x + dragOffset.x;
-  const nextY = originalElement.y + dragOffset.y;
 
   mutateElement(element, {
     x: nextX,

src/element/embeddable.ts

@@ -48,9 +48,6 @@ const RE_VALTOWN =
 const RE_GENERIC_EMBED =
   /^<(?:iframe|blockquote)[\s\S]*?\s(?:src|href)=["']([^"']*)["'][\s\S]*?>$/i;
 
-const RE_GIPHY =
-  /giphy.com\/(?:clips|embed|gifs)\/[a-zA-Z0-9]*?-?([a-zA-Z0-9]+)(?:[^a-zA-Z0-9]|$)/;
-
 const ALLOWED_DOMAINS = new Set([
   "youtube.com",
   "youtu.be",
@@ -63,7 +60,6 @@ const ALLOWED_DOMAINS = new Set([
   "*.simplepdf.eu",
   "stackblitz.com",
   "val.town",
-  "giphy.com",
 ]);
 
 const createSrcDoc = (body: string) => {
@@ -313,10 +309,6 @@ export const extractSrc = (htmlString: string): string => {
     return gistMatch[1];
   }
 
-  if (RE_GIPHY.test(htmlString)) {
-    return `https://giphy.com/embed/${RE_GIPHY.exec(htmlString)![1]}`;
-  }
-
   const match = htmlString.match(RE_GENERIC_EMBED);
   if (match && match.length === 2) {
     return match[1];

src/element/linearElementEditor.ts

@@ -21,7 +21,6 @@ import {
 } from "../math";
 import { getElementAbsoluteCoords, getLockedLinearCursorAlignSize } from ".";
 import {
-  Bounds,
   getCurvePathOps,
   getElementPointsCoords,
   getMinMaxXYFromCurvePathOps,
@@ -548,7 +547,10 @@ export class LinearElementEditor {
     endPointIndex: number,
   ) {
     let segmentMidPoint = centerPoint(startPoint, endPoint);
-    if (element.points.length > 2 && element.roundness) {
+    const splits = element.segmentSplitIndices || [];
+    const treatAsCurve =
+      splits.includes(endPointIndex) || splits.includes(endPointIndex - 1);
+    if (element.points.length > 2 && (element.roundness || treatAsCurve)) {
       const controlPoints = getControlPointsForBezierCurve(
         element,
         element.points[endPointIndex],
@@ -1043,13 +1045,15 @@ export class LinearElementEditor {
     let offsetX = 0;
     let offsetY = 0;
 
-    const isDeletingOriginPoint = pointIndices.includes(0);
+    const indexSet = new Set(pointIndices);
+
+    const isDeletingOriginPoint = indexSet.has(0);
 
     // if deleting first point, make the next to be [0,0] and recalculate
     // positions of the rest with respect to it
     if (isDeletingOriginPoint) {
       const firstNonDeletedPoint = element.points.find((point, idx) => {
-        return !pointIndices.includes(idx);
+        return !indexSet.has(idx);
       });
       if (firstNonDeletedPoint) {
         offsetX = firstNonDeletedPoint[0];
@@ -1058,7 +1062,7 @@ export class LinearElementEditor {
     }
 
     const nextPoints = element.points.reduce((acc: Point[], point, idx) => {
-      if (!pointIndices.includes(idx)) {
+      if (!indexSet.has(idx)) {
         acc.push(
           !acc.length ? [0, 0] : [point[0] - offsetX, point[1] - offsetY],
         );
@@ -1066,7 +1070,22 @@ export class LinearElementEditor {
       return acc;
     }, []);
 
-    LinearElementEditor._updatePoints(element, nextPoints, offsetX, offsetY);
+    const splits: number[] = [];
+    (element.segmentSplitIndices || []).forEach((index) => {
+      if (!indexSet.has(index)) {
+        let shift = 0;
+        for (const pointIndex of pointIndices) {
+          if (index > pointIndex) {
+            shift++;
+          }
+        }
+        splits.push(index - shift);
+      }
+    });
+
+    LinearElementEditor._updatePoints(element, nextPoints, offsetX, offsetY, {
+      segmentSplitIndices: splits.sort((a, b) => a - b),
+    });
   }
 
   static addPoints(
@@ -1205,9 +1224,13 @@ export class LinearElementEditor {
       midpoint,
       ...element.points.slice(segmentMidpoint.index!),
     ];
+    const splits = (element.segmentSplitIndices || []).map((index) =>
+      index >= segmentMidpoint.index! ? index + 1 : index,
+    );
 
     mutateElement(element, {
       points,
+      segmentSplitIndices: splits.sort((a, b) => a - b),
     });
 
     ret.pointerDownState = {
@@ -1227,7 +1250,11 @@ export class LinearElementEditor {
     nextPoints: readonly Point[],
     offsetX: number,
     offsetY: number,
-    otherUpdates?: { startBinding?: PointBinding; endBinding?: PointBinding },
+    otherUpdates?: {
+      startBinding?: PointBinding;
+      endBinding?: PointBinding;
+      segmentSplitIndices?: number[];
+    },
   ) {
     const nextCoords = getElementPointsCoords(element, nextPoints);
     const prevCoords = getElementPointsCoords(element, element.points);
@@ -1317,7 +1344,7 @@ export class LinearElementEditor {
 
   static getMinMaxXYWithBoundText = (
     element: ExcalidrawLinearElement,
-    elementBounds: Bounds,
+    elementBounds: [number, number, number, number],
     boundTextElement: ExcalidrawTextElementWithContainer,
   ): [number, number, number, number, number, number] => {
     let [x1, y1, x2, y2] = elementBounds;
@@ -1473,6 +1500,27 @@ export class LinearElementEditor {
 
     return coords;
   };
+
+  static toggleSegmentSplitAtIndex(
+    element: NonDeleted<ExcalidrawLinearElement>,
+    index: number,
+  ) {
+    let found = false;
+    const splitIndices = (element.segmentSplitIndices || []).filter((idx) => {
+      if (idx === index) {
+        found = true;
+        return false;
+      }
+      return true;
+    });
+    if (!found) {
+      splitIndices.push(index);
+    }
+
+    mutateElement(element, {
+      segmentSplitIndices: splitIndices.sort((a, b) => a - b),
+    });
+  }
 }
 
 const normalizeSelectedPoints = (

src/element/mutateElement.ts

@@ -25,7 +25,7 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
 
   // casting to any because can't use `in` operator
   // (see https://github.com/microsoft/TypeScript/issues/21732)
-  const { points, fileId } = updates as any;
+  const { points, fileId, segmentSplitIndices } = updates as any;
 
   if (typeof points !== "undefined") {
     updates = { ...getSizeFromPoints(points), ...updates };
@@ -86,6 +86,7 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
   if (
     typeof updates.height !== "undefined" ||
     typeof updates.width !== "undefined" ||
+    typeof segmentSplitIndices !== "undefined" ||
     typeof fileId != "undefined" ||
     typeof points !== "undefined"
   ) {

src/element/newElement.ts

@@ -374,6 +374,7 @@ export const newLinearElement = (
     endBinding: null,
     startArrowhead: opts.startArrowhead || null,
     endArrowhead: opts.endArrowhead || null,
+    segmentSplitIndices: [],
   };
 };
 

src/element/resizeTest.ts

@@ -13,7 +13,6 @@ import {
   MaybeTransformHandleType,
 } from "./transformHandles";
 import { AppState, Zoom } from "../types";
-import { Bounds } from "./bounds";
 
 const isInsideTransformHandle = (
   transformHandle: TransformHandle,
@@ -88,7 +87,7 @@ export const getElementWithTransformHandleType = (
 };
 
 export const getTransformHandleTypeFromCoords = (
-  [x1, y1, x2, y2]: Bounds,
+  [x1, y1, x2, y2]: readonly [number, number, number, number],
   scenePointerX: number,
   scenePointerY: number,
   zoom: Zoom,

src/element/transformHandles.ts

@@ -4,7 +4,7 @@ import {
   PointerType,
 } from "./types";
 
-import { Bounds, getElementAbsoluteCoords } from "./bounds";
+import { getElementAbsoluteCoords } from "./bounds";
 import { rotate } from "../math";
 import { InteractiveCanvasAppState, Zoom } from "../types";
 import { isTextElement } from ".";
@@ -23,7 +23,7 @@ export type TransformHandleDirection =
 
 export type TransformHandleType = TransformHandleDirection | "rotation";
 
-export type TransformHandle = Bounds;
+export type TransformHandle = [number, number, number, number];
 export type TransformHandles = Partial<{
   [T in TransformHandleType]: TransformHandle;
 }>;

src/element/types.ts

@@ -195,6 +195,7 @@ export type ExcalidrawLinearElement = _ExcalidrawElementBase &
   Readonly<{
     type: "line" | "arrow";
     points: readonly Point[];
+    segmentSplitIndices: readonly number[] | null;
     lastCommittedPoint: Point | null;
     startBinding: PointBinding | null;
     endBinding: PointBinding | null;

src/frame.test.tsx

@@ -123,7 +123,7 @@ describe("adding elements to frames", () => {
   const commonTestCases = async (
     func: typeof resizeFrameOverElement | typeof dragElementIntoFrame,
   ) => {
-    describe.skip("when frame is in a layer below", async () => {
+    describe("when frame is in a layer below", async () => {
       it("should add an element", async () => {
         h.elements = [frame, rect2];
 
@@ -167,7 +167,7 @@ describe("adding elements to frames", () => {
       });
     });
 
-    describe.skip("when frame is in a layer above", async () => {
+    describe("when frame is in a layer above", async () => {
       it("should add an element", async () => {
         h.elements = [rect2, frame];
 
@@ -212,7 +212,7 @@ describe("adding elements to frames", () => {
     });
 
     describe("when frame is in an inner layer", async () => {
-      it.skip("should add elements", async () => {
+      it("should add elements", async () => {
         h.elements = [rect2, frame, rect3];
 
         func(frame, rect2);
@@ -223,7 +223,7 @@ describe("adding elements to frames", () => {
         expectEqualIds([rect2, rect3, frame]);
       });
 
-      it.skip("should add elements when there are other other elements in between", async () => {
+      it("should add elements when there are other other elements in between", async () => {
         h.elements = [rect2, rect1, frame, rect4, rect3];
 
         func(frame, rect2);
@@ -234,7 +234,7 @@ describe("adding elements to frames", () => {
         expectEqualIds([rect1, rect2, rect3, frame, rect4]);
       });
 
-      it.skip("should add elements when there are other elements in between and the order is reversed", async () => {
+      it("should add elements when there are other elements in between and the order is reversed", async () => {
         h.elements = [rect3, rect4, frame, rect2, rect1];
 
         func(frame, rect2);
@@ -289,7 +289,7 @@ describe("adding elements to frames", () => {
   describe("resizing frame over elements", async () => {
     await commonTestCases(resizeFrameOverElement);
 
-    it.skip("resizing over text containers and labelled arrows", async () => {
+    it("resizing over text containers and labelled arrows", async () => {
       await resizingTest(
         "rectangle",
         ["frame", "rectangle", "text"],
@@ -339,7 +339,7 @@ describe("adding elements to frames", () => {
       // );
     });
 
-    it.skip("should add arrow bound with text when frame is in a layer below", async () => {
+    it("should add arrow bound with text when frame is in a layer below", async () => {
       h.elements = [frame, arrow, text];
 
       resizeFrameOverElement(frame, arrow);
@@ -359,7 +359,7 @@ describe("adding elements to frames", () => {
       expectEqualIds([arrow, text, frame]);
     });
 
-    it.skip("should add arrow bound with text when frame is in an inner layer", async () => {
+    it("should add arrow bound with text when frame is in an inner layer", async () => {
       h.elements = [arrow, frame, text];
 
       resizeFrameOverElement(frame, arrow);
@@ -371,7 +371,7 @@ describe("adding elements to frames", () => {
   });
 
   describe("resizing frame over elements but downwards", async () => {
-    it.skip("should add elements when frame is in a layer below", async () => {
+    it("should add elements when frame is in a layer below", async () => {
       h.elements = [frame, rect1, rect2, rect3, rect4];
 
       resizeFrameOverElement(frame, rect4);
@@ -382,7 +382,7 @@ describe("adding elements to frames", () => {
       expectEqualIds([rect2, rect3, frame, rect4, rect1]);
     });
 
-    it.skip("should add elements when frame is in a layer above", async () => {
+    it("should add elements when frame is in a layer above", async () => {
       h.elements = [rect1, rect2, rect3, rect4, frame];
 
       resizeFrameOverElement(frame, rect4);
@@ -393,7 +393,7 @@ describe("adding elements to frames", () => {
       expectEqualIds([rect1, rect2, rect3, frame, rect4]);
     });
 
-    it.skip("should add elements when frame is in an inner layer", async () => {
+    it("should add elements when frame is in an inner layer", async () => {
       h.elements = [rect1, rect2, frame, rect3, rect4];
 
       resizeFrameOverElement(frame, rect4);
@@ -408,7 +408,7 @@ describe("adding elements to frames", () => {
   describe("dragging elements into the frame", async () => {
     await commonTestCases(dragElementIntoFrame);
 
-    it.skip("should drag element inside, duplicate it and keep it in frame", () => {
+    it("should drag element inside, duplicate it and keep it in frame", () => {
       h.elements = [frame, rect2];
 
       dragElementIntoFrame(frame, rect2);
@@ -422,7 +422,7 @@ describe("adding elements to frames", () => {
       expectEqualIds([rect2_copy, rect2, frame]);
     });
 
-    it.skip("should drag element inside, duplicate it and remove it from frame", () => {
+    it("should drag element inside, duplicate it and remove it from frame", () => {
       h.elements = [frame, rect2];
 
       dragElementIntoFrame(frame, rect2);

src/frame.ts

@@ -19,10 +19,10 @@ import { mutateElement } from "./element/mutateElement";
 import { AppClassProperties, AppState, StaticCanvasAppState } from "./types";
 import { getElementsWithinSelection, getSelectedElements } from "./scene";
 import { isFrameElement } from "./element";
+import { moveOneRight } from "./zindex";
 import { getElementsInGroup, selectGroupsFromGivenElements } from "./groups";
 import Scene, { ExcalidrawElementsIncludingDeleted } from "./scene/Scene";
 import { getElementLineSegments } from "./element/bounds";
-import { doLineSegmentsIntersect } from "./packages/utils";
 
 // --------------------------- Frame State ------------------------------------
 export const bindElementsToFramesAfterDuplication = (
@@ -56,21 +56,130 @@ export const bindElementsToFramesAfterDuplication = (
   }
 };
 
-export function isElementIntersectingFrame(
+// --------------------------- Frame Geometry ---------------------------------
-  element: ExcalidrawElement,
+class Point {
-  frame: ExcalidrawFrameElement,
+  x: number;
-) {
+  y: number;
-  const frameLineSegments = getElementLineSegments(frame);
 
-  const elementLineSegments = getElementLineSegments(element);
+  constructor(x: number, y: number) {
+    this.x = x;
+    this.y = y;
+  }
+}
 
-  const intersecting = frameLineSegments.some((frameLineSegment) =>
+class LineSegment {
-    elementLineSegments.some((elementLineSegment) =>
+  first: Point;
-      doLineSegmentsIntersect(frameLineSegment, elementLineSegment),
+  second: Point;
-    ),
-  );
 
-  return intersecting;
+  constructor(pointA: Point, pointB: Point) {
+    this.first = pointA;
+    this.second = pointB;
+  }
+
+  public getBoundingBox(): [Point, Point] {
+    return [
+      new Point(
+        Math.min(this.first.x, this.second.x),
+        Math.min(this.first.y, this.second.y),
+      ),
+      new Point(
+        Math.max(this.first.x, this.second.x),
+        Math.max(this.first.y, this.second.y),
+      ),
+    ];
+  }
+}
+
+// https://martin-thoma.com/how-to-check-if-two-line-segments-intersect/
+class FrameGeometry {
+  private static EPSILON = 0.000001;
+
+  private static crossProduct(a: Point, b: Point) {
+    return a.x * b.y - b.x * a.y;
+  }
+
+  private static doBoundingBoxesIntersect(
+    a: [Point, Point],
+    b: [Point, Point],
+  ) {
+    return (
+      a[0].x <= b[1].x &&
+      a[1].x >= b[0].x &&
+      a[0].y <= b[1].y &&
+      a[1].y >= b[0].y
+    );
+  }
+
+  private static isPointOnLine(a: LineSegment, b: Point) {
+    const aTmp = new LineSegment(
+      new Point(0, 0),
+      new Point(a.second.x - a.first.x, a.second.y - a.first.y),
+    );
+    const bTmp = new Point(b.x - a.first.x, b.y - a.first.y);
+    const r = this.crossProduct(aTmp.second, bTmp);
+    return Math.abs(r) < this.EPSILON;
+  }
+
+  private static isPointRightOfLine(a: LineSegment, b: Point) {
+    const aTmp = new LineSegment(
+      new Point(0, 0),
+      new Point(a.second.x - a.first.x, a.second.y - a.first.y),
+    );
+    const bTmp = new Point(b.x - a.first.x, b.y - a.first.y);
+    return this.crossProduct(aTmp.second, bTmp) < 0;
+  }
+
+  private static lineSegmentTouchesOrCrossesLine(
+    a: LineSegment,
+    b: LineSegment,
+  ) {
+    return (
+      this.isPointOnLine(a, b.first) ||
+      this.isPointOnLine(a, b.second) ||
+      (this.isPointRightOfLine(a, b.first)
+        ? !this.isPointRightOfLine(a, b.second)
+        : this.isPointRightOfLine(a, b.second))
+    );
+  }
+
+  private static doLineSegmentsIntersect(
+    a: [readonly [number, number], readonly [number, number]],
+    b: [readonly [number, number], readonly [number, number]],
+  ) {
+    const aSegment = new LineSegment(
+      new Point(a[0][0], a[0][1]),
+      new Point(a[1][0], a[1][1]),
+    );
+    const bSegment = new LineSegment(
+      new Point(b[0][0], b[0][1]),
+      new Point(b[1][0], b[1][1]),
+    );
+
+    const box1 = aSegment.getBoundingBox();
+    const box2 = bSegment.getBoundingBox();
+    return (
+      this.doBoundingBoxesIntersect(box1, box2) &&
+      this.lineSegmentTouchesOrCrossesLine(aSegment, bSegment) &&
+      this.lineSegmentTouchesOrCrossesLine(bSegment, aSegment)
+    );
+  }
+
+  public static isElementIntersectingFrame(
+    element: ExcalidrawElement,
+    frame: ExcalidrawFrameElement,
+  ) {
+    const frameLineSegments = getElementLineSegments(frame);
+
+    const elementLineSegments = getElementLineSegments(element);
+
+    const intersecting = frameLineSegments.some((frameLineSegment) =>
+      elementLineSegments.some((elementLineSegment) =>
+        this.doLineSegmentsIntersect(frameLineSegment, elementLineSegment),
+      ),
+    );
+
+    return intersecting;
+  }
 }
 
 export const getElementsCompletelyInFrame = (
@@ -98,7 +207,10 @@ export const isElementContainingFrame = (
 export const getElementsIntersectingFrame = (
   elements: readonly ExcalidrawElement[],
   frame: ExcalidrawFrameElement,
-) => elements.filter((element) => isElementIntersectingFrame(element, frame));
+) =>
+  elements.filter((element) =>
+    FrameGeometry.isElementIntersectingFrame(element, frame),
+  );
 
 export const elementsAreInFrameBounds = (
   elements: readonly ExcalidrawElement[],
@@ -124,7 +236,7 @@ export const elementOverlapsWithFrame = (
 ) => {
   return (
     elementsAreInFrameBounds([element], frame) ||
-    isElementIntersectingFrame(element, frame) ||
+    FrameGeometry.isElementIntersectingFrame(element, frame) ||
     isElementContainingFrame([frame], element, frame)
   );
 };
@@ -161,7 +273,7 @@ export const groupsAreAtLeastIntersectingTheFrame = (
   return !!elementsInGroup.find(
     (element) =>
       elementsAreInFrameBounds([element], frame) ||
-      isElementIntersectingFrame(element, frame),
+      FrameGeometry.isElementIntersectingFrame(element, frame),
   );
 };
 
@@ -182,7 +294,7 @@ export const groupsAreCompletelyOutOfFrame = (
     elementsInGroup.find(
       (element) =>
         elementsAreInFrameBounds([element], frame) ||
-        isElementIntersectingFrame(element, frame),
+        FrameGeometry.isElementIntersectingFrame(element, frame),
     ) === undefined
   );
 };
@@ -211,7 +323,24 @@ export const groupByFrames = (elements: readonly ExcalidrawElement[]) => {
 export const getFrameElements = (
   allElements: ExcalidrawElementsIncludingDeleted,
   frameId: string,
-) => allElements.filter((element) => element.frameId === frameId);
+  opts?: { includeBoundArrows?: boolean },
+) => {
+  return allElements.filter((element) => {
+    if (element.frameId === frameId) {
+      return true;
+    }
+    if (opts?.includeBoundArrows && element.type === "arrow") {
+      const bindingId = element.startBinding?.elementId;
+      if (bindingId) {
+        const boundElement = Scene.getScene(element)?.getElement(bindingId);
+        if (boundElement?.frameId === frameId) {
+          return true;
+        }
+      }
+    }
+    return false;
+  });
+};
 
 export const getElementsInResizingFrame = (
   allElements: ExcalidrawElementsIncludingDeleted,
@@ -242,7 +371,7 @@ export const getElementsInResizingFrame = (
   );
 
   for (const element of elementsNotCompletelyInFrame) {
-    if (!isElementIntersectingFrame(element, frame)) {
+    if (!FrameGeometry.isElementIntersectingFrame(element, frame)) {
       if (element.groupIds.length === 0) {
         nextElementsInFrame.delete(element);
       }
@@ -339,6 +468,14 @@ export const getContainingFrame = (
   return null;
 };
 
+export const isValidFrameChild = (element: ExcalidrawElement) => {
+  return (
+    element.type !== "frame" &&
+    // arrows that are bound to elements cannot be frame children
+    (element.type !== "arrow" || (!element.startBinding && !element.endBinding))
+  );
+};
+
 // --------------------------- Frame Operations -------------------------------
 
 /**
@@ -351,17 +488,20 @@ export const addElementsToFrame = (
   elementsToAdd: NonDeletedExcalidrawElement[],
   frame: ExcalidrawFrameElement,
 ) => {
-  const { currTargetFrameChildrenMap } = allElements.reduce(
+  const { allElementsIndexMap, currTargetFrameChildrenMap } =
-    (acc, element, index) => {
+    allElements.reduce(
-      if (element.frameId === frame.id) {
+      (acc, element, index) => {
-        acc.currTargetFrameChildrenMap.set(element.id, true);
+        acc.allElementsIndexMap.set(element.id, index);
-      }
+        if (element.frameId === frame.id) {
-      return acc;
+          acc.currTargetFrameChildrenMap.set(element.id, true);
-    },
+        }
-    {
+        return acc;
-      currTargetFrameChildrenMap: new Map<ExcalidrawElement["id"], true>(),
+      },
-    },
+      {
-  );
+        allElementsIndexMap: new Map<ExcalidrawElement["id"], number>(),
+        currTargetFrameChildrenMap: new Map<ExcalidrawElement["id"], true>(),
+      },
+    );
 
   const suppliedElementsToAddSet = new Set(elementsToAdd.map((el) => el.id));
 
@@ -374,6 +514,9 @@ export const addElementsToFrame = (
     elementsToAdd,
   )) {
     if (!currTargetFrameChildrenMap.has(element.id)) {
+      if (!isValidFrameChild(element)) {
+        continue;
+      }
       finalElementsToAdd.push(element);
     }
 
@@ -387,6 +530,66 @@ export const addElementsToFrame = (
     }
   }
 
+  const finalElementsToAddSet = new Set(finalElementsToAdd.map((el) => el.id));
+
+  const nextElements: ExcalidrawElement[] = [];
+
+  const processedElements = new Set<ExcalidrawElement["id"]>();
+
+  for (const element of allElements) {
+    if (processedElements.has(element.id)) {
+      continue;
+    }
+
+    processedElements.add(element.id);
+
+    if (
+      finalElementsToAddSet.has(element.id) ||
+      (element.frameId && element.frameId === frame.id)
+    ) {
+      // will be added in bulk once we process target frame
+      continue;
+    }
+
+    // target frame
+    if (element.id === frame.id) {
+      const currFrameChildren = getFrameElements(allElements, frame.id);
+      currFrameChildren.forEach((child) => {
+        processedElements.add(child.id);
+      });
+
+      // if not found, add all children on top by assigning the lowest index
+      const targetFrameIndex = allElementsIndexMap.get(frame.id) ?? -1;
+
+      const { newChildren_left, newChildren_right } = finalElementsToAdd.reduce(
+        (acc, element) => {
+          // if index not found, add on top of current frame children
+          const elementIndex = allElementsIndexMap.get(element.id) ?? Infinity;
+          if (elementIndex < targetFrameIndex) {
+            acc.newChildren_left.push(element);
+          } else {
+            acc.newChildren_right.push(element);
+          }
+          return acc;
+        },
+        {
+          newChildren_left: [] as ExcalidrawElement[],
+          newChildren_right: [] as ExcalidrawElement[],
+        },
+      );
+
+      nextElements.push(
+        ...newChildren_left,
+        ...currFrameChildren,
+        ...newChildren_right,
+        element,
+      );
+      continue;
+    }
+
+    nextElements.push(element);
+  }
+
   for (const element of finalElementsToAdd) {
     mutateElement(
       element,
@@ -396,7 +599,8 @@ export const addElementsToFrame = (
       false,
     );
   }
-  return allElements.slice();
+
+  return nextElements;
 };
 
 export const removeElementsFromFrame = (
@@ -404,34 +608,20 @@ export const removeElementsFromFrame = (
   elementsToRemove: NonDeletedExcalidrawElement[],
   appState: AppState,
 ) => {
-  const _elementsToRemove = new Map<
+  const _elementsToRemove: ExcalidrawElement[] = [];
-    ExcalidrawElement["id"],
-    ExcalidrawElement
-  >();
-
-  const toRemoveElementsByFrame = new Map<
-    ExcalidrawFrameElement["id"],
-    ExcalidrawElement[]
-  >();
 
   for (const element of elementsToRemove) {
     if (element.frameId) {
-      _elementsToRemove.set(element.id, element);
+      _elementsToRemove.push(element);
-
-      const arr = toRemoveElementsByFrame.get(element.frameId) || [];
-      arr.push(element);
 
       const boundTextElement = getBoundTextElement(element);
       if (boundTextElement) {
-        _elementsToRemove.set(boundTextElement.id, boundTextElement);
+        _elementsToRemove.push(boundTextElement);
-        arr.push(boundTextElement);
       }
-
-      toRemoveElementsByFrame.set(element.frameId, arr);
     }
   }
 
-  for (const [, element] of _elementsToRemove) {
+  for (const element of _elementsToRemove) {
     mutateElement(
       element,
       {
@@ -441,7 +631,13 @@ export const removeElementsFromFrame = (
     );
   }
 
-  return allElements.slice();
+  const nextElements = moveOneRight(
+    allElements,
+    appState,
+    Array.from(_elementsToRemove),
+  );
+
+  return nextElements;
 };
 
 export const removeAllElementsFromFrame = (

src/math.ts

@@ -505,7 +505,3 @@ export const rangeIntersection = (
 
   return null;
 };
-
-export const isValueInRange = (value: number, min: number, max: number) => {
-  return value >= min && value <= max;
-};

src/packages/bbox.ts

@@ -1,65 +0,0 @@
-import { Bounds } from "../element/bounds";
-import { Point } from "../types";
-
-export type LineSegment = [Point, Point];
-
-export function getBBox(line: LineSegment): Bounds {
-  return [
-    Math.min(line[0][0], line[1][0]),
-    Math.min(line[0][1], line[1][1]),
-    Math.max(line[0][0], line[1][0]),
-    Math.max(line[0][1], line[1][1]),
-  ];
-}
-
-export function crossProduct(a: Point, b: Point) {
-  return a[0] * b[1] - b[0] * a[1];
-}
-
-export function doBBoxesIntersect(a: Bounds, b: Bounds) {
-  return a[0] <= b[2] && a[2] >= b[0] && a[1] <= b[3] && a[3] >= b[1];
-}
-
-export function translate(a: Point, b: Point): Point {
-  return [a[0] - b[0], a[1] - b[1]];
-}
-
-const EPSILON = 0.000001;
-
-export function isPointOnLine(l: LineSegment, p: Point) {
-  const p1 = translate(l[1], l[0]);
-  const p2 = translate(p, l[0]);
-
-  const r = crossProduct(p1, p2);
-
-  return Math.abs(r) < EPSILON;
-}
-
-export function isPointRightOfLine(l: LineSegment, p: Point) {
-  const p1 = translate(l[1], l[0]);
-  const p2 = translate(p, l[0]);
-
-  return crossProduct(p1, p2) < 0;
-}
-
-export function isLineSegmentTouchingOrCrossingLine(
-  a: LineSegment,
-  b: LineSegment,
-) {
-  return (
-    isPointOnLine(a, b[0]) ||
-    isPointOnLine(a, b[1]) ||
-    (isPointRightOfLine(a, b[0])
-      ? !isPointRightOfLine(a, b[1])
-      : isPointRightOfLine(a, b[1]))
-  );
-}
-
-// https://martin-thoma.com/how-to-check-if-two-line-segments-intersect/
-export function doLineSegmentsIntersect(a: LineSegment, b: LineSegment) {
-  return (
-    doBBoxesIntersect(getBBox(a), getBBox(b)) &&
-    isLineSegmentTouchingOrCrossingLine(a, b) &&
-    isLineSegmentTouchingOrCrossingLine(b, a)
-  );
-}

src/packages/excalidraw/.size-limit.json

@@ -1,12 +1,12 @@
 [
   {
     "path": "dist/excalidraw.production.min.js",
-    "limit": "320 kB"
+    "limit": "305 kB"
   },
   {
     "path": "dist/excalidraw-assets/locales",
     "name": "dist/excalidraw-assets/locales",
-    "limit": "290 kB"
+    "limit": "270 kB"
   },
   {
     "path": "dist/excalidraw-assets/vendor-*.js",

src/packages/excalidraw/CHANGELOG.md

@@ -15,9 +15,7 @@ Please add the latest change on the top under the correct section.
 
 ### Features
 
-- Export `elementsOverlappingBBox`, `isElementInsideBBox`, `elementPartiallyOverlapsWithOrContainsBBox` helpers for filtering/checking if elements within bounds. [#6727](https://github.com/excalidraw/excalidraw/pull/6727)
+- Add `selected` prop for `MainMenu.Item` and `MainMenu.ItemCustom` components to indicate active state. [7078](https://github.com/excalidraw/excalidraw/pull/7078)
-- Regenerate ids by default when using transform api and also update bindings by 0.5px to avoid possible overlapping [#7195](https://github.com/excalidraw/excalidraw/pull/7195)
-- Add `selected` prop for `MainMenu.Item` and `MainMenu.ItemCustom` components to indicate active state. [#7078](https://github.com/excalidraw/excalidraw/pull/7078)
 
 ## 0.16.1 (2023-09-21)
 

src/packages/excalidraw/index.tsx

@@ -254,9 +254,3 @@ export { DefaultSidebar } from "../../components/DefaultSidebar";
 
 export { normalizeLink } from "../../data/url";
 export { convertToExcalidrawElements } from "../../data/transform";
-
-export {
-  elementsOverlappingBBox,
-  isElementInsideBBox,
-  elementPartiallyOverlapsWithOrContainsBBox,
-} from "../withinBounds";

src/packages/utils.ts

@@ -229,12 +229,6 @@ export const exportToClipboard = async (
   }
 };
 
-export * from "./bbox";
-export {
-  elementsOverlappingBBox,
-  isElementInsideBBox,
-  elementPartiallyOverlapsWithOrContainsBBox,
-} from "./withinBounds";
 export { serializeAsJSON, serializeLibraryAsJSON } from "../data/json";
 export {
   loadFromBlob,

src/packages/withinBounds.test.ts

@@ -1,262 +0,0 @@
-import { Bounds } from "../element/bounds";
-import { API } from "../tests/helpers/api";
-import {
-  elementPartiallyOverlapsWithOrContainsBBox,
-  elementsOverlappingBBox,
-  isElementInsideBBox,
-} from "./withinBounds";
-
-const makeElement = (x: number, y: number, width: number, height: number) =>
-  API.createElement({
-    type: "rectangle",
-    x,
-    y,
-    width,
-    height,
-  });
-
-const makeBBox = (
-  minX: number,
-  minY: number,
-  maxX: number,
-  maxY: number,
-): Bounds => [minX, minY, maxX, maxY];
-
-describe("isElementInsideBBox()", () => {
-  it("should return true if element is fully inside", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // bbox contains element
-    expect(isElementInsideBBox(makeElement(0, 0, 100, 100), bbox)).toBe(true);
-    expect(isElementInsideBBox(makeElement(10, 10, 90, 90), bbox)).toBe(true);
-  });
-
-  it("should return false if element is only partially overlapping", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // element contains bbox
-    expect(isElementInsideBBox(makeElement(-10, -10, 110, 110), bbox)).toBe(
-      false,
-    );
-
-    // element overlaps bbox from top-left
-    expect(isElementInsideBBox(makeElement(-10, -10, 100, 100), bbox)).toBe(
-      false,
-    );
-    // element overlaps bbox from top-right
-    expect(isElementInsideBBox(makeElement(90, -10, 100, 100), bbox)).toBe(
-      false,
-    );
-    // element overlaps bbox from bottom-left
-    expect(isElementInsideBBox(makeElement(-10, 90, 100, 100), bbox)).toBe(
-      false,
-    );
-    // element overlaps bbox from bottom-right
-    expect(isElementInsideBBox(makeElement(90, 90, 100, 100), bbox)).toBe(
-      false,
-    );
-  });
-
-  it("should return false if element outside", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // outside diagonally
-    expect(isElementInsideBBox(makeElement(110, 110, 100, 100), bbox)).toBe(
-      false,
-    );
-
-    // outside on the left
-    expect(isElementInsideBBox(makeElement(-110, 10, 50, 50), bbox)).toBe(
-      false,
-    );
-    // outside on the right
-    expect(isElementInsideBBox(makeElement(110, 10, 50, 50), bbox)).toBe(false);
-    // outside on the top
-    expect(isElementInsideBBox(makeElement(10, -110, 50, 50), bbox)).toBe(
-      false,
-    );
-    // outside on the bottom
-    expect(isElementInsideBBox(makeElement(10, 110, 50, 50), bbox)).toBe(false);
-  });
-
-  it("should return true if bbox contains element and flag enabled", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // element contains bbox
-    expect(
-      isElementInsideBBox(makeElement(-10, -10, 110, 110), bbox, true),
-    ).toBe(true);
-
-    // bbox contains element
-    expect(isElementInsideBBox(makeElement(0, 0, 100, 100), bbox)).toBe(true);
-    expect(isElementInsideBBox(makeElement(10, 10, 90, 90), bbox)).toBe(true);
-  });
-});
-
-describe("elementPartiallyOverlapsWithOrContainsBBox()", () => {
-  it("should return true if element overlaps, is inside, or contains", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // bbox contains element
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(0, 0, 100, 100),
-        bbox,
-      ),
-    ).toBe(true);
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(10, 10, 90, 90),
-        bbox,
-      ),
-    ).toBe(true);
-
-    // element contains bbox
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(-10, -10, 110, 110),
-        bbox,
-      ),
-    ).toBe(true);
-
-    // element overlaps bbox from top-left
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(-10, -10, 100, 100),
-        bbox,
-      ),
-    ).toBe(true);
-    // element overlaps bbox from top-right
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(90, -10, 100, 100),
-        bbox,
-      ),
-    ).toBe(true);
-    // element overlaps bbox from bottom-left
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(-10, 90, 100, 100),
-        bbox,
-      ),
-    ).toBe(true);
-    // element overlaps bbox from bottom-right
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(90, 90, 100, 100),
-        bbox,
-      ),
-    ).toBe(true);
-  });
-
-  it("should return false if element does not overlap", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    // outside diagonally
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(110, 110, 100, 100),
-        bbox,
-      ),
-    ).toBe(false);
-
-    // outside on the left
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(-110, 10, 50, 50),
-        bbox,
-      ),
-    ).toBe(false);
-    // outside on the right
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(110, 10, 50, 50),
-        bbox,
-      ),
-    ).toBe(false);
-    // outside on the top
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(10, -110, 50, 50),
-        bbox,
-      ),
-    ).toBe(false);
-    // outside on the bottom
-    expect(
-      elementPartiallyOverlapsWithOrContainsBBox(
-        makeElement(10, 110, 50, 50),
-        bbox,
-      ),
-    ).toBe(false);
-  });
-});
-
-describe("elementsOverlappingBBox()", () => {
-  it("should return elements that overlap bbox", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    const rectOutside = makeElement(110, 110, 100, 100);
-    const rectInside = makeElement(10, 10, 90, 90);
-    const rectContainingBBox = makeElement(-10, -10, 110, 110);
-    const rectOverlappingTopLeft = makeElement(-10, -10, 50, 50);
-
-    expect(
-      elementsOverlappingBBox({
-        bounds: bbox,
-        type: "overlap",
-        elements: [
-          rectOutside,
-          rectInside,
-          rectContainingBBox,
-          rectOverlappingTopLeft,
-        ],
-      }),
-    ).toEqual([rectInside, rectContainingBBox, rectOverlappingTopLeft]);
-  });
-
-  it("should return elements inside/containing bbox", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    const rectOutside = makeElement(110, 110, 100, 100);
-    const rectInside = makeElement(10, 10, 90, 90);
-    const rectContainingBBox = makeElement(-10, -10, 110, 110);
-    const rectOverlappingTopLeft = makeElement(-10, -10, 50, 50);
-
-    expect(
-      elementsOverlappingBBox({
-        bounds: bbox,
-        type: "contain",
-        elements: [
-          rectOutside,
-          rectInside,
-          rectContainingBBox,
-          rectOverlappingTopLeft,
-        ],
-      }),
-    ).toEqual([rectInside, rectContainingBBox]);
-  });
-
-  it("should return elements inside bbox", () => {
-    const bbox = makeBBox(0, 0, 100, 100);
-
-    const rectOutside = makeElement(110, 110, 100, 100);
-    const rectInside = makeElement(10, 10, 90, 90);
-    const rectContainingBBox = makeElement(-10, -10, 110, 110);
-    const rectOverlappingTopLeft = makeElement(-10, -10, 50, 50);
-
-    expect(
-      elementsOverlappingBBox({
-        bounds: bbox,
-        type: "inside",
-        elements: [
-          rectOutside,
-          rectInside,
-          rectContainingBBox,
-          rectOverlappingTopLeft,
-        ],
-      }),
-    ).toEqual([rectInside]);
-  });
-
-  // TODO test linear, freedraw, and diamond element types (+rotated)
-});

src/packages/withinBounds.ts

@@ -1,206 +0,0 @@
-import type {
-  ExcalidrawElement,
-  ExcalidrawFreeDrawElement,
-  ExcalidrawLinearElement,
-  NonDeletedExcalidrawElement,
-} from "../element/types";
-import {
-  isArrowElement,
-  isFreeDrawElement,
-  isLinearElement,
-  isTextElement,
-} from "../element/typeChecks";
-import { isValueInRange, rotatePoint } from "../math";
-import type { Point } from "../types";
-import { Bounds } from "../element/bounds";
-
-type Element = NonDeletedExcalidrawElement;
-type Elements = readonly NonDeletedExcalidrawElement[];
-
-type Points = readonly Point[];
-
-/** @returns vertices relative to element's top-left [0,0] position  */
-const getNonLinearElementRelativePoints = (
-  element: Exclude<
-    Element,
-    ExcalidrawLinearElement | ExcalidrawFreeDrawElement
-  >,
-): [TopLeft: Point, TopRight: Point, BottomRight: Point, BottomLeft: Point] => {
-  if (element.type === "diamond") {
-    return [
-      [element.width / 2, 0],
-      [element.width, element.height / 2],
-      [element.width / 2, element.height],
-      [0, element.height / 2],
-    ];
-  }
-  return [
-    [0, 0],
-    [0 + element.width, 0],
-    [0 + element.width, element.height],
-    [0, element.height],
-  ];
-};
-
-/** @returns vertices relative to element's top-left [0,0] position  */
-const getElementRelativePoints = (element: ExcalidrawElement): Points => {
-  if (isLinearElement(element) || isFreeDrawElement(element)) {
-    return element.points;
-  }
-  return getNonLinearElementRelativePoints(element);
-};
-
-const getMinMaxPoints = (points: Points) => {
-  const ret = points.reduce(
-    (limits, [x, y]) => {
-      limits.minY = Math.min(limits.minY, y);
-      limits.minX = Math.min(limits.minX, x);
-
-      limits.maxX = Math.max(limits.maxX, x);
-      limits.maxY = Math.max(limits.maxY, y);
-
-      return limits;
-    },
-    {
-      minX: Infinity,
-      minY: Infinity,
-      maxX: -Infinity,
-      maxY: -Infinity,
-      cx: 0,
-      cy: 0,
-    },
-  );
-
-  ret.cx = (ret.maxX + ret.minX) / 2;
-  ret.cy = (ret.maxY + ret.minY) / 2;
-
-  return ret;
-};
-
-const getRotatedBBox = (element: Element): Bounds => {
-  const points = getElementRelativePoints(element);
-
-  const { cx, cy } = getMinMaxPoints(points);
-  const centerPoint: Point = [cx, cy];
-
-  const rotatedPoints = points.map((point) =>
-    rotatePoint([point[0], point[1]], centerPoint, element.angle),
-  );
-  const { minX, minY, maxX, maxY } = getMinMaxPoints(rotatedPoints);
-
-  return [
-    minX + element.x,
-    minY + element.y,
-    maxX + element.x,
-    maxY + element.y,
-  ];
-};
-
-export const isElementInsideBBox = (
-  element: Element,
-  bbox: Bounds,
-  eitherDirection = false,
-): boolean => {
-  const elementBBox = getRotatedBBox(element);
-
-  const elementInsideBbox =
-    bbox[0] <= elementBBox[0] &&
-    bbox[2] >= elementBBox[2] &&
-    bbox[1] <= elementBBox[1] &&
-    bbox[3] >= elementBBox[3];
-
-  if (!eitherDirection) {
-    return elementInsideBbox;
-  }
-
-  if (elementInsideBbox) {
-    return true;
-  }
-
-  return (
-    elementBBox[0] <= bbox[0] &&
-    elementBBox[2] >= bbox[2] &&
-    elementBBox[1] <= bbox[1] &&
-    elementBBox[3] >= bbox[3]
-  );
-};
-
-export const elementPartiallyOverlapsWithOrContainsBBox = (
-  element: Element,
-  bbox: Bounds,
-): boolean => {
-  const elementBBox = getRotatedBBox(element);
-
-  return (
-    (isValueInRange(elementBBox[0], bbox[0], bbox[2]) ||
-      isValueInRange(bbox[0], elementBBox[0], elementBBox[2])) &&
-    (isValueInRange(elementBBox[1], bbox[1], bbox[3]) ||
-      isValueInRange(bbox[1], elementBBox[1], elementBBox[3]))
-  );
-};
-
-export const elementsOverlappingBBox = ({
-  elements,
-  bounds,
-  type,
-  errorMargin = 0,
-}: {
-  elements: Elements;
-  bounds: Bounds;
-  /** safety offset. Defaults to 0. */
-  errorMargin?: number;
-  /**
-   * - overlap: elements overlapping or inside bounds
-   * - contain: elements inside bounds or bounds inside elements
-   * - inside: elements inside bounds
-   **/
-  type: "overlap" | "contain" | "inside";
-}) => {
-  const adjustedBBox: Bounds = [
-    bounds[0] - errorMargin,
-    bounds[1] - errorMargin,
-    bounds[2] + errorMargin,
-    bounds[3] + errorMargin,
-  ];
-
-  const includedElementSet = new Set<string>();
-
-  for (const element of elements) {
-    if (includedElementSet.has(element.id)) {
-      continue;
-    }
-
-    const isOverlaping =
-      type === "overlap"
-        ? elementPartiallyOverlapsWithOrContainsBBox(element, adjustedBBox)
-        : type === "inside"
-        ? isElementInsideBBox(element, adjustedBBox)
-        : isElementInsideBBox(element, adjustedBBox, true);
-
-    if (isOverlaping) {
-      includedElementSet.add(element.id);
-
-      if (element.boundElements) {
-        for (const boundElement of element.boundElements) {
-          includedElementSet.add(boundElement.id);
-        }
-      }
-
-      if (isTextElement(element) && element.containerId) {
-        includedElementSet.add(element.containerId);
-      }
-
-      if (isArrowElement(element)) {
-        if (element.startBinding) {
-          includedElementSet.add(element.startBinding.elementId);
-        }
-
-        if (element.endBinding) {
-          includedElementSet.add(element.endBinding?.elementId);
-        }
-      }
-    }
-  }
-
-  return elements.filter((element) => includedElementSet.has(element.id));
-};

src/renderer/renderScene.ts

@@ -69,6 +69,7 @@ import {
 } from "../element/Hyperlink";
 import { renderSnaps } from "./renderSnaps";
 import {
+  isArrowElement,
   isEmbeddableElement,
   isFrameElement,
   isLinearElement,
@@ -165,6 +166,21 @@ const fillCircle = (
   }
 };
 
+const fillSquare = (
+  context: CanvasRenderingContext2D,
+  cx: number,
+  cy: number,
+  side: number,
+  stroke = true,
+) => {
+  context.beginPath();
+  context.rect(cx - side / 2, cy - side / 2, side, side);
+  context.fill();
+  if (stroke) {
+    context.stroke();
+  }
+};
+
 const strokeGrid = (
   context: CanvasRenderingContext2D,
   gridSize: number,
@@ -223,6 +239,7 @@ const renderSingleLinearPoint = (
   point: Point,
   radius: number,
   isSelected: boolean,
+  renderAsSquare: boolean,
   isPhantomPoint = false,
 ) => {
   context.strokeStyle = "#5e5ad8";
@@ -234,13 +251,29 @@ const renderSingleLinearPoint = (
     context.fillStyle = "rgba(177, 151, 252, 0.7)";
   }
 
-  fillCircle(
+  const effectiveRadius = radius / appState.zoom.value;
-    context,
+
-    point[0],
+  if (renderAsSquare) {
-    point[1],
+    fillSquare(
-    radius / appState.zoom.value,
+      context,
-    !isPhantomPoint,
+      point[0],
-  );
+      point[1],
+      effectiveRadius * 2,
+      !isPhantomPoint,
+    );
+  } else {
+    fillCircle(context, point[0], point[1], effectiveRadius, !isPhantomPoint);
+  }
+};
+
+const isLinearPointAtIndexSquared = (
+  element: NonDeleted<ExcalidrawLinearElement>,
+  index: number,
+) => {
+  const splitting = element.segmentSplitIndices
+    ? element.segmentSplitIndices.includes(index)
+    : false;
+  return element.roundness ? splitting : !splitting;
 };
 
 const renderLinearPointHandles = (
@@ -264,7 +297,14 @@ const renderLinearPointHandles = (
     const isSelected =
       !!appState.editingLinearElement?.selectedPointsIndices?.includes(idx);
 
-    renderSingleLinearPoint(context, appState, point, radius, isSelected);
+    renderSingleLinearPoint(
+      context,
+      appState,
+      point,
+      radius,
+      isSelected,
+      isLinearPointAtIndexSquared(element, idx),
+    );
   });
 
   //Rendering segment mid points
@@ -292,6 +332,7 @@ const renderLinearPointHandles = (
           segmentMidPoint,
           radius,
           false,
+          false,
         );
         highlightPoint(segmentMidPoint, context, appState);
       } else {
@@ -302,6 +343,7 @@ const renderLinearPointHandles = (
           segmentMidPoint,
           radius,
           false,
+          false,
         );
       }
     } else if (appState.editingLinearElement || points.length === 2) {
@@ -311,6 +353,7 @@ const renderLinearPointHandles = (
         segmentMidPoint,
         POINT_HANDLE_SIZE / 2,
         false,
+        false,
         true,
       );
     }
@@ -323,16 +366,16 @@ const highlightPoint = (
   point: Point,
   context: CanvasRenderingContext2D,
   appState: InteractiveCanvasAppState,
+  renderAsSquare = false,
 ) => {
   context.fillStyle = "rgba(105, 101, 219, 0.4)";
+  const radius = LinearElementEditor.POINT_HANDLE_SIZE / appState.zoom.value;
 
-  fillCircle(
+  if (renderAsSquare) {
-    context,
+    fillSquare(context, point[0], point[1], radius * 2, false);
-    point[0],
+  } else {
-    point[1],
+    fillCircle(context, point[0], point[1], radius, false);
-    LinearElementEditor.POINT_HANDLE_SIZE / appState.zoom.value,
+  }
-    false,
-  );
 };
 const renderLinearElementPointHighlight = (
   context: CanvasRenderingContext2D,
@@ -354,10 +397,15 @@ const renderLinearElementPointHighlight = (
     element,
     hoverPointIndex,
   );
+
   context.save();
   context.translate(appState.scrollX, appState.scrollY);
-
+  highlightPoint(
-  highlightPoint(point, context, appState);
+    point,
+    context,
+    appState,
+    isLinearPointAtIndexSquared(element, hoverPointIndex),
+  );
   context.restore();
 };
 
@@ -984,7 +1032,10 @@ const _renderStaticScene = ({
 
           // TODO do we need to check isElementInFrame here?
           if (frame && isElementInFrame(element, elements, appState)) {
-            frameClip(frame, context, renderConfig, appState);
+            // do not clip arrows
+            if (!isArrowElement(element)) {
+              frameClip(frame, context, renderConfig, appState);
+            }
           }
           renderElement(element, rc, context, renderConfig, appState);
           context.restore();

src/scene/Shape.ts

@@ -14,6 +14,7 @@ import { generateFreeDrawShape } from "../renderer/renderElement";
 import { isTransparent, assertNever } from "../utils";
 import { simplify } from "points-on-curve";
 import { ROUGHNESS } from "../constants";
+import { Point } from "../types";
 
 const getDashArrayDashed = (strokeWidth: number) => [8, 8 + strokeWidth];
 
@@ -228,18 +229,44 @@ export const _generateElementShape = (
 
       // points array can be empty in the beginning, so it is important to add
       // initial position to it
-      const points = element.points.length ? element.points : [[0, 0]];
+      const points = element.points.length
+        ? element.points
+        : ([[0, 0]] as Point[]);
 
       // curve is always the first element
       // this simplifies finding the curve for an element
+      const splits = element.segmentSplitIndices || [];
       if (!element.roundness) {
-        if (options.fill) {
+        if (splits.length === 0) {
-          shape = [generator.polygon(points as [number, number][], options)];
+          if (options.fill) {
+            shape = [generator.polygon(points as [number, number][], options)];
+          } else {
+            shape = [
+              generator.linearPath(points as [number, number][], options),
+            ];
+          }
         } else {
-          shape = [generator.linearPath(points as [number, number][], options)];
+          const splitInverse: number[] = [];
+          const splitSet = new Set(splits);
+          for (let i = 0; i < points.length; i++) {
+            if (!splitSet.has(i)) {
+              splitInverse.push(i);
+            }
+          }
+          shape = [
+            generator.curve(
+              computeMultipleCurvesFromSplits(points, splitInverse),
+              options,
+            ),
+          ];
         }
       } else {
-        shape = [generator.curve(points as [number, number][], options)];
+        shape = [
+          generator.curve(
+            computeMultipleCurvesFromSplits(points, splits),
+            options,
+          ),
+        ];
       }
 
       // add lines only in arrow
@@ -376,3 +403,22 @@ export const _generateElementShape = (
     }
   }
 };
+
+const computeMultipleCurvesFromSplits = (
+  points: readonly Point[],
+  splits: readonly number[],
+): [number, number][][] => {
+  const pointList: Point[][] = [];
+  let currentIndex = 0;
+  for (const index of splits) {
+    const slice = points.slice(currentIndex, index + 1);
+    if (slice.length) {
+      pointList.push([...slice]);
+    }
+    currentIndex = index;
+  }
+  if (currentIndex < points.length - 1) {
+    pointList.push(points.slice(currentIndex));
+  }
+  return pointList as [number, number][][];
+};

src/scene/comparisons.ts

@@ -39,6 +39,8 @@ export const canChangeRoundness = (type: string) =>
   type === "line" ||
   type === "diamond";
 
+export const hasText = (type: string) => type === "text";
+
 export const canHaveArrowheads = (type: string) => type === "arrow";
 
 export const getElementAtPosition = (

src/scene/export.ts

@@ -1,10 +1,6 @@
 import rough from "roughjs/bin/rough";
 import { NonDeletedExcalidrawElement } from "../element/types";
-import {
+import { getCommonBounds, getElementAbsoluteCoords } from "../element/bounds";
-  Bounds,
-  getCommonBounds,
-  getElementAbsoluteCoords,
-} from "../element/bounds";
 import { renderSceneToSvg, renderStaticScene } from "../renderer/renderScene";
 import { distance, isOnlyExportingSingleFrame } from "../utils";
 import { AppState, BinaryFiles } from "../types";
@@ -225,7 +221,7 @@ export const exportToSvg = async (
 const getCanvasSize = (
   elements: readonly NonDeletedExcalidrawElement[],
   exportPadding: number,
-): Bounds => {
+): [number, number, number, number] => {
   // we should decide if we are exporting the whole canvas
   // if so, we are not clipping elements in the frame
   // and therefore, we should not do anything special

src/scene/index.ts

@@ -14,6 +14,7 @@ export {
   canHaveArrowheads,
   canChangeRoundness,
   getElementAtPosition,
+  hasText,
   getElementsAtPosition,
 } from "./comparisons";
 export { getNormalizedZoom } from "./zoom";

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

@@ -14255,6 +14255,217 @@ exports[`regression tests > should group elements and ungroup them > [end of tes
 
 exports[`regression tests > should group elements and ungroup them > [end of test] number of renders 1`] = `21`;
 
+exports[`regression tests > should show fill icons when element has non transparent background > [end of test] appState 1`] = `
+{
+  "activeEmbeddable": null,
+  "activeTool": {
+    "customType": null,
+    "lastActiveTool": null,
+    "locked": false,
+    "type": "selection",
+  },
+  "collaborators": Map {},
+  "contextMenu": null,
+  "currentChartType": "bar",
+  "currentItemBackgroundColor": "#ffc9c9",
+  "currentItemEndArrowhead": "arrow",
+  "currentItemFillStyle": "solid",
+  "currentItemFontFamily": 1,
+  "currentItemFontSize": 20,
+  "currentItemOpacity": 100,
+  "currentItemRoughness": 1,
+  "currentItemRoundness": "round",
+  "currentItemStartArrowhead": null,
+  "currentItemStrokeColor": "#1e1e1e",
+  "currentItemStrokeStyle": "solid",
+  "currentItemStrokeWidth": 2,
+  "currentItemTextAlign": "left",
+  "cursorButton": "up",
+  "defaultSidebarDockedPreference": false,
+  "draggingElement": null,
+  "editingElement": null,
+  "editingFrame": null,
+  "editingGroupId": null,
+  "editingLinearElement": null,
+  "elementsToHighlight": null,
+  "errorMessage": null,
+  "exportBackground": true,
+  "exportEmbedScene": false,
+  "exportScale": 1,
+  "exportWithDarkMode": false,
+  "fileHandle": null,
+  "frameRendering": {
+    "clip": true,
+    "enabled": true,
+    "name": true,
+    "outline": true,
+  },
+  "frameToHighlight": null,
+  "gridSize": null,
+  "height": 768,
+  "isBindingEnabled": true,
+  "isLoading": false,
+  "isResizing": false,
+  "isRotating": false,
+  "lastPointerDownWith": "mouse",
+  "multiElement": null,
+  "name": "Untitled-201933152653",
+  "objectsSnapModeEnabled": false,
+  "offsetLeft": 0,
+  "offsetTop": 0,
+  "openDialog": null,
+  "openMenu": null,
+  "openPopup": "elementBackground",
+  "openSidebar": null,
+  "originSnapOffset": null,
+  "pasteDialog": {
+    "data": null,
+    "shown": false,
+  },
+  "penDetected": false,
+  "penMode": false,
+  "pendingImageElementId": null,
+  "previousSelectedElementIds": {},
+  "resizingElement": null,
+  "scrollX": 0,
+  "scrollY": 0,
+  "scrolledOutside": false,
+  "selectedElementIds": {
+    "id0": true,
+  },
+  "selectedElementsAreBeingDragged": false,
+  "selectedGroupIds": {},
+  "selectedLinearElement": null,
+  "selectionElement": null,
+  "shouldCacheIgnoreZoom": false,
+  "showHyperlinkPopup": false,
+  "showStats": false,
+  "showWelcomeScreen": true,
+  "snapLines": [],
+  "startBoundElement": null,
+  "suggestedBindings": [],
+  "theme": "light",
+  "toast": null,
+  "viewBackgroundColor": "#ffffff",
+  "viewModeEnabled": false,
+  "width": 1024,
+  "zenModeEnabled": false,
+  "zoom": {
+    "value": 1,
+  },
+}
+`;
+
+exports[`regression tests > should show fill icons when element has non transparent background > [end of test] history 1`] = `
+{
+  "recording": false,
+  "redoStack": [],
+  "stateHistory": [
+    {
+      "appState": {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": {},
+        "selectedGroupIds": {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": [],
+    },
+    {
+      "appState": {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": {
+          "id0": true,
+        },
+        "selectedGroupIds": {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": [
+        {
+          "angle": 0,
+          "backgroundColor": "transparent",
+          "boundElements": null,
+          "fillStyle": "solid",
+          "frameId": null,
+          "groupIds": [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "link": null,
+          "locked": false,
+          "opacity": 100,
+          "roughness": 1,
+          "roundness": {
+            "type": 3,
+          },
+          "seed": 1278240551,
+          "strokeColor": "#1e1e1e",
+          "strokeStyle": "solid",
+          "strokeWidth": 2,
+          "type": "rectangle",
+          "updated": 1,
+          "version": 2,
+          "versionNonce": 453191,
+          "width": 10,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+    {
+      "appState": {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": {
+          "id0": true,
+        },
+        "selectedGroupIds": {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": [
+        {
+          "angle": 0,
+          "backgroundColor": "#ffc9c9",
+          "boundElements": null,
+          "fillStyle": "solid",
+          "frameId": null,
+          "groupIds": [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "link": null,
+          "locked": false,
+          "opacity": 100,
+          "roughness": 1,
+          "roundness": {
+            "type": 3,
+          },
+          "seed": 1278240551,
+          "strokeColor": "#1e1e1e",
+          "strokeStyle": "solid",
+          "strokeWidth": 2,
+          "type": "rectangle",
+          "updated": 1,
+          "version": 3,
+          "versionNonce": 2019559783,
+          "width": 10,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`regression tests > should show fill icons when element has non transparent background > [end of test] number of elements 1`] = `0`;
+
+exports[`regression tests > should show fill icons when element has non transparent background > [end of test] number of renders 1`] = `9`;
+
 exports[`regression tests > single-clicking on a subgroup of a selected group should not alter selection > [end of test] appState 1`] = `
 {
   "activeEmbeddable": null,

src/tests/helpers/api.ts

@@ -94,7 +94,7 @@ export class API {
     angle?: number;
     id?: string;
     isDeleted?: boolean;
-    frameId?: ExcalidrawElement["id"] | null;
+    frameId?: ExcalidrawElement["id"];
     groupIds?: string[];
     // generic element props
     strokeColor?: ExcalidrawGenericElement["strokeColor"];

src/tests/linearElementEditor.test.tsx

@@ -1202,29 +1202,5 @@ describe("Test Linear Elements", () => {
         }),
       );
     });
-
-    it("should not update label position when arrow dragged", () => {
-      createTwoPointerLinearElement("arrow");
-      let arrow = h.elements[0] as ExcalidrawLinearElement;
-      createBoundTextElement(DEFAULT_TEXT, arrow);
-      let label = h.elements[1] as ExcalidrawTextElementWithContainer;
-      expect(arrow.x).toBe(20);
-      expect(arrow.y).toBe(20);
-      expect(label.x).toBe(0);
-      expect(label.y).toBe(0);
-      mouse.reset();
-      mouse.select(arrow);
-      mouse.select(label);
-      mouse.downAt(arrow.x, arrow.y);
-      mouse.moveTo(arrow.x + 20, arrow.y + 30);
-      mouse.up(arrow.x + 20, arrow.y + 30);
-
-      arrow = h.elements[0] as ExcalidrawLinearElement;
-      label = h.elements[1] as ExcalidrawTextElementWithContainer;
-      expect(arrow.x).toBe(80);
-      expect(arrow.y).toBe(100);
-      expect(label.x).toBe(0);
-      expect(label.y).toBe(0);
-    });
   });
 });

src/tests/packages/__snapshots__/excalidraw.test.tsx.snap

@@ -535,7 +535,6 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
             >
               <button
                 class="color-picker__button active"
-                data-testid="color-top-pick-#ffffff"
                 style="--swatch-color: #ffffff;"
                 title="#ffffff"
                 type="button"
@@ -546,7 +545,6 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
               </button>
               <button
                 class="color-picker__button"
-                data-testid="color-top-pick-#f8f9fa"
                 style="--swatch-color: #f8f9fa;"
                 title="#f8f9fa"
                 type="button"
@@ -557,7 +555,6 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
               </button>
               <button
                 class="color-picker__button"
-                data-testid="color-top-pick-#f5faff"
                 style="--swatch-color: #f5faff;"
                 title="#f5faff"
                 type="button"
@@ -568,7 +565,6 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
               </button>
               <button
                 class="color-picker__button"
-                data-testid="color-top-pick-#fffce8"
                 style="--swatch-color: #fffce8;"
                 title="#fffce8"
                 type="button"
@@ -579,7 +575,6 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
               </button>
               <button
                 class="color-picker__button"
-                data-testid="color-top-pick-#fdf8f6"
                 style="--swatch-color: #fdf8f6;"
                 title="#fdf8f6"
                 type="button"

src/tests/regressionTests.test.tsx

@@ -1089,6 +1089,20 @@ describe("regression tests", () => {
     });
     assertSelectedElements(rect3);
   });
+
+  it("should show fill icons when element has non transparent background", async () => {
+    UI.clickTool("rectangle");
+    expect(screen.queryByText(/fill/i)).not.toBeNull();
+    mouse.down();
+    mouse.up(10, 10);
+    expect(screen.queryByText(/fill/i)).toBeNull();
+    togglePopover("Background");
+    UI.clickOnTestId("color-red");
+    // select rectangle
+    mouse.reset();
+    mouse.click();
+    expect(screen.queryByText(/fill/i)).not.toBeNull();
+  });
 });
 
 it(

src/tests/zindex.test.tsx

@@ -12,11 +12,6 @@ import {
 import { AppState } from "../types";
 import { API } from "./helpers/api";
 import { selectGroupsForSelectedElements } from "../groups";
-import {
-  ExcalidrawElement,
-  ExcalidrawFrameElement,
-  ExcalidrawSelectionElement,
-} from "../element/types";
 
 // Unmount ReactDOM from root
 ReactDOM.unmountComponentAtNode(document.getElementById("root")!);
@@ -28,15 +23,9 @@ beforeEach(() => {
 
 const { h } = window;
 
-type ExcalidrawElementType = Exclude<
-  ExcalidrawElement,
-  ExcalidrawSelectionElement
->["type"];
-
 const populateElements = (
   elements: {
     id: string;
-    type?: ExcalidrawElementType;
     isDeleted?: boolean;
     isSelected?: boolean;
     groupIds?: string[];
@@ -45,7 +34,6 @@ const populateElements = (
     width?: number;
     height?: number;
     containerId?: string;
-    frameId?: ExcalidrawFrameElement["id"];
   }[],
   appState?: Partial<AppState>,
 ) => {
@@ -62,11 +50,9 @@ const populateElements = (
       width = 100,
       height = 100,
       containerId = null,
-      frameId = null,
-      type,
     }) => {
       const element = API.createElement({
-        type: type ?? (containerId ? "text" : "rectangle"),
+        type: containerId ? "text" : "rectangle",
         id,
         isDeleted,
         x,
@@ -75,7 +61,6 @@ const populateElements = (
         height,
         groupIds,
         containerId,
-        frameId: frameId || null,
       });
       if (isSelected) {
         selectedElementIds[element.id] = true;
@@ -131,8 +116,6 @@ const assertZindex = ({
     isSelected?: true;
     groupIds?: string[];
     containerId?: string;
-    frameId?: ExcalidrawFrameElement["id"];
-    type?: ExcalidrawElementType;
   }[];
   appState?: Partial<AppState>;
   operations: [Actions, string[]][];
@@ -1200,285 +1183,3 @@ describe("z-index manipulation", () => {
     });
   });
 });
-
-describe("z-indexing with frames", () => {
-  beforeEach(async () => {
-    await render(<Excalidraw />);
-  });
-
-  // naming scheme:
-  // F#   ... frame element
-  // F#_# ... frame child of F# (rectangle)
-  // R#   ... unrelated element (rectangle)
-
-  it("moving whole frame by one (normalized)", () => {
-    // normalized frame order
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +1
-        [actionBringForward, ["R1", "F1_1", "F1_2", "F1", "R2"]],
-        // +1
-        [actionBringForward, ["R1", "R2", "F1_1", "F1_2", "F1"]],
-        // noop
-        [actionBringForward, ["R1", "R2", "F1_1", "F1_2", "F1"]],
-        // -1
-        [actionSendBackward, ["R1", "F1_1", "F1_2", "F1", "R2"]],
-        // -1
-        [actionSendBackward, ["F1_1", "F1_2", "F1", "R1", "R2"]],
-        // noop
-        [actionSendBackward, ["F1_1", "F1_2", "F1", "R1", "R2"]],
-      ],
-    });
-  });
-
-  it("moving whole frame by one (DENORMALIZED)", () => {
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +1
-        [actionBringForward, ["R1", "F1_1", "F1", "F1_2", "R2"]],
-        // +1
-        [actionBringForward, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-        // noop
-        [actionBringForward, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R1" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +1
-        [actionBringForward, ["R1", "F1_1", "F1", "R2", "F1_2"]],
-        // +1
-        [actionBringForward, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-        // noop
-        [actionBringForward, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "R1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R2" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R3" },
-      ],
-      operations: [
-        // +1
-        [actionBringForward, ["R1", "F1_1", "R2", "F1", "R3", "F1_2"]],
-        // +1
-        // FIXME incorrect, should put F1_1 after R3
-        [actionBringForward, ["R1", "R2", "F1_1", "R3", "F1", "F1_2"]],
-        // +1
-        // FIXME should be noop from previous step after it's fixed
-        [actionBringForward, ["R1", "R2", "R3", "F1_1", "F1", "F1_2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "R1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R2" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R3" },
-      ],
-      operations: [
-        // -1
-        [actionSendBackward, ["F1_1", "F1", "R1", "F1_2", "R2", "R3"]],
-        // -1
-        [actionSendBackward, ["F1_1", "F1", "F1_2", "R1", "R2", "R3"]],
-      ],
-    });
-  });
-
-  it("moving selected frame children by one (normalized)", () => {
-    // normalized frame order
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1", isSelected: true },
-        { id: "F1_2", frameId: "F1" },
-        { id: "F1", type: "frame" },
-        { id: "R1" },
-      ],
-      operations: [
-        // +1
-        [actionBringForward, ["F1_2", "F1_1", "F1", "R1"]],
-        // noop
-        [actionBringForward, ["F1_2", "F1_1", "F1", "R1"]],
-      ],
-    });
-
-    // normalized frame order, multiple frames
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1", isSelected: true },
-        { id: "F1_2", frameId: "F1" },
-        { id: "F1", type: "frame" },
-        { id: "R1" },
-        { id: "F2_1", frameId: "F2", isSelected: true },
-        { id: "F2_2", frameId: "F2" },
-        { id: "F2", type: "frame" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +1
-        [
-          actionBringForward,
-          ["F1_2", "F1_1", "F1", "R1", "F2_2", "F2_1", "F2", "R2"],
-        ],
-        // noop
-        [
-          actionBringForward,
-          ["F1_2", "F1_1", "F1", "R1", "F2_2", "F2_1", "F2", "R2"],
-        ],
-      ],
-    });
-  });
-
-  it("moving selected frame children by one (DENORMALIZED)", () => {
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1", isSelected: true },
-        { id: "F1", type: "frame" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R1" },
-      ],
-      operations: [
-        // +1
-        // NOTE not sure what we wanna do here
-        [actionBringForward, ["F1", "F1_2", "F1_1", "R1"]],
-        // noop
-        [actionBringForward, ["F1", "F1_2", "F1_1", "R1"]],
-        // -1
-        [actionSendBackward, ["F1", "F1_1", "F1_2", "R1"]],
-        // noop
-        [actionSendBackward, ["F1", "F1_1", "F1_2", "R1"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1", isSelected: true },
-        { id: "R1" },
-        { id: "F1", type: "frame" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +1
-        // NOTE not sure what we wanna do here
-        [actionBringForward, ["R1", "F1", "F1_2", "F1_1", "R2"]],
-        // noop
-        [actionBringForward, ["R1", "F1", "F1_2", "F1_1", "R2"]],
-        // -1
-        [actionSendBackward, ["R1", "F1", "F1_1", "F1_2", "R2"]],
-        // noop
-        [actionSendBackward, ["R1", "F1", "F1_1", "F1_2", "R2"]],
-      ],
-    });
-  });
-
-  it("moving whole frame to front/end", () => {
-    // normalized frame order
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +∞
-        [actionBringToFront, ["R1", "R2", "F1_1", "F1_2", "F1"]],
-        // noop
-        [actionBringToFront, ["R1", "R2", "F1_1", "F1_2", "F1"]],
-        // -∞
-        [actionSendToBack, ["F1_1", "F1_2", "F1", "R1", "R2"]],
-        // noop
-        [actionSendToBack, ["F1_1", "F1_2", "F1", "R1", "R2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +∞
-        [actionBringToFront, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-        // noop
-        [actionBringToFront, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-        // -∞
-        [actionSendToBack, ["F1_1", "F1", "F1_2", "R1", "R2"]],
-        // noop
-        [actionSendToBack, ["F1_1", "F1", "F1_2", "R1", "R2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R1" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R2" },
-      ],
-      operations: [
-        // +∞
-        [actionBringToFront, ["R1", "R2", "F1_1", "F1", "F1_2"]],
-      ],
-    });
-
-    // DENORMALIZED FRAME ORDER
-    assertZindex({
-      elements: [
-        { id: "F1_1", frameId: "F1" },
-        { id: "R1" },
-        { id: "F1", type: "frame", isSelected: true },
-        { id: "R2" },
-        { id: "F1_2", frameId: "F1" },
-        { id: "R3" },
-      ],
-      operations: [
-        // +1
-        [actionBringToFront, ["R1", "R2", "R3", "F1_1", "F1", "F1_2"]],
-      ],
-    });
-  });
-});

src/types.ts

@@ -695,12 +695,3 @@ export type KeyboardModifiersObject = {
   altKey: boolean;
   metaKey: boolean;
 };
-
-export type Primitive =
-  | number
-  | string
-  | boolean
-  | bigint
-  | symbol
-  | null
-  | undefined;

src/zindex.ts

@@ -1,14 +1,16 @@
 import { bumpVersion } from "./element/mutateElement";
 import { isFrameElement } from "./element/typeChecks";
-import { ExcalidrawElement, ExcalidrawFrameElement } from "./element/types";
+import { ExcalidrawElement } from "./element/types";
+import { groupByFrames } from "./frame";
 import { getElementsInGroup } from "./groups";
 import { getSelectedElements } from "./scene";
 import Scene from "./scene/Scene";
 import { AppState } from "./types";
 import { arrayToMap, findIndex, findLastIndex } from "./utils";
 
-const isOfTargetFrame = (element: ExcalidrawElement, frameId: string) => {
+// elements that do not belong to a frame are considered a root element
-  return element.frameId === frameId || element.id === frameId;
+const isRootElement = (element: ExcalidrawElement) => {
+  return !element.frameId;
 };
 
 /**
@@ -33,7 +35,6 @@ const getIndicesToMove = (
       ? elementsToBeMoved
       : getSelectedElements(elements, appState, {
           includeBoundTextElement: true,
-          includeElementsInFrames: true,
         }),
   );
   while (++index < elements.length) {
@@ -105,26 +106,6 @@ const getTargetIndexAccountingForBinding = (
   }
 };
 
-const getContiguousFrameRangeElements = (
-  allElements: readonly ExcalidrawElement[],
-  frameId: ExcalidrawFrameElement["id"],
-) => {
-  let rangeStart = -1;
-  let rangeEnd = -1;
-  allElements.forEach((element, index) => {
-    if (isOfTargetFrame(element, frameId)) {
-      if (rangeStart === -1) {
-        rangeStart = index;
-      }
-      rangeEnd = index;
-    }
-  });
-  if (rangeStart === -1) {
-    return [];
-  }
-  return allElements.slice(rangeStart, rangeEnd + 1);
-};
-
 /**
  * Returns next candidate index that's available to be moved to. Currently that
  *  is a non-deleted element, and not inside a group (unless we're editing it).
@@ -134,11 +115,6 @@ const getTargetIndex = (
   elements: readonly ExcalidrawElement[],
   boundaryIndex: number,
   direction: "left" | "right",
-  /**
-   * Frame id if moving frame children.
-   * If whole frame (including all children) is being moved, supply `null`.
-   */
-  containingFrame: ExcalidrawFrameElement["id"] | null,
 ) => {
   const sourceElement = elements[boundaryIndex];
 
@@ -146,9 +122,6 @@ const getTargetIndex = (
     if (element.isDeleted) {
       return false;
     }
-    if (containingFrame) {
-      return element.frameId === containingFrame;
-    }
     // if we're editing group, find closest sibling irrespective of whether
     // there's a different-group element between them (for legacy reasons)
     if (appState.editingGroupId) {
@@ -159,12 +132,8 @@ const getTargetIndex = (
 
   const candidateIndex =
     direction === "left"
-      ? findLastIndex(
+      ? findLastIndex(elements, indexFilter, Math.max(0, boundaryIndex - 1))
-          elements,
+      : findIndex(elements, indexFilter, boundaryIndex + 1);
-          (el) => indexFilter(el),
-          Math.max(0, boundaryIndex - 1),
-        )
-      : findIndex(elements, (el) => indexFilter(el), boundaryIndex + 1);
 
   const nextElement = elements[candidateIndex];
 
@@ -187,19 +156,6 @@ const getTargetIndex = (
     }
   }
 
-  if (
-    !containingFrame &&
-    (nextElement.frameId || nextElement.type === "frame")
-  ) {
-    const frameElements = getContiguousFrameRangeElements(
-      elements,
-      nextElement.frameId || nextElement.id,
-    );
-    return direction === "left"
-      ? elements.indexOf(frameElements[0])
-      : elements.indexOf(frameElements[frameElements.length - 1]);
-  }
-
   if (!nextElement.groupIds.length) {
     return (
       getTargetIndexAccountingForBinding(nextElement, elements, direction) ??
@@ -239,12 +195,13 @@ const getTargetElementsMap = <T extends ExcalidrawElement>(
   }, {} as Record<string, ExcalidrawElement>);
 };
 
-const shiftElementsByOne = (
+const _shiftElements = (
   elements: readonly ExcalidrawElement[],
   appState: AppState,
   direction: "left" | "right",
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  const indicesToMove = getIndicesToMove(elements, appState);
+  const indicesToMove = getIndicesToMove(elements, appState, elementsToBeMoved);
   const targetElementsMap = getTargetElementsMap(elements, indicesToMove);
   let groupedIndices = toContiguousGroups(indicesToMove);
 
@@ -252,30 +209,16 @@ const shiftElementsByOne = (
     groupedIndices = groupedIndices.reverse();
   }
 
-  const selectedFrames = new Set<ExcalidrawFrameElement["id"]>(
-    indicesToMove
-      .filter((idx) => elements[idx].type === "frame")
-      .map((idx) => elements[idx].id),
-  );
-
   groupedIndices.forEach((indices, i) => {
     const leadingIndex = indices[0];
     const trailingIndex = indices[indices.length - 1];
     const boundaryIndex = direction === "left" ? leadingIndex : trailingIndex;
 
-    const containingFrame = indices.some((idx) => {
-      const el = elements[idx];
-      return el.frameId && selectedFrames.has(el.frameId);
-    })
-      ? null
-      : elements[boundaryIndex]?.frameId;
-
     const targetIndex = getTargetIndex(
       appState,
       elements,
       boundaryIndex,
       direction,
-      containingFrame,
     );
 
     if (targetIndex === -1 || boundaryIndex === targetIndex) {
@@ -320,25 +263,34 @@ const shiftElementsByOne = (
   });
 };
 
-const shiftElementsToEnd = (
+const shiftElements = (
+  appState: AppState,
+  elements: readonly ExcalidrawElement[],
+  direction: "left" | "right",
+  elementsToBeMoved?: readonly ExcalidrawElement[],
+) => {
+  return shift(
+    elements,
+    appState,
+    direction,
+    _shiftElements,
+    elementsToBeMoved,
+  );
+};
+
+const _shiftElementsToEnd = (
   elements: readonly ExcalidrawElement[],
   appState: AppState,
   direction: "left" | "right",
-  containingFrame: ExcalidrawFrameElement["id"] | null,
-  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  const indicesToMove = getIndicesToMove(elements, appState, elementsToBeMoved);
+  const indicesToMove = getIndicesToMove(elements, appState);
   const targetElementsMap = getTargetElementsMap(elements, indicesToMove);
   const displacedElements: ExcalidrawElement[] = [];
 
   let leadingIndex: number;
   let trailingIndex: number;
   if (direction === "left") {
-    if (containingFrame) {
+    if (appState.editingGroupId) {
-      leadingIndex = findIndex(elements, (el) =>
-        isOfTargetFrame(el, containingFrame),
-      );
-    } else if (appState.editingGroupId) {
       const groupElements = getElementsInGroup(
         elements,
         appState.editingGroupId,
@@ -353,11 +305,7 @@ const shiftElementsToEnd = (
 
     trailingIndex = indicesToMove[indicesToMove.length - 1];
   } else {
-    if (containingFrame) {
+    if (appState.editingGroupId) {
-      trailingIndex = findLastIndex(elements, (el) =>
-        isOfTargetFrame(el, containingFrame),
-      );
-    } else if (appState.editingGroupId) {
       const groupElements = getElementsInGroup(
         elements,
         appState.editingGroupId,
@@ -373,10 +321,6 @@ const shiftElementsToEnd = (
     leadingIndex = indicesToMove[0];
   }
 
-  if (leadingIndex === -1) {
-    leadingIndex = 0;
-  }
-
   for (let index = leadingIndex; index < trailingIndex + 1; index++) {
     if (!indicesToMove.includes(index)) {
       displacedElements.push(elements[index]);
@@ -405,123 +349,121 @@ const shiftElementsToEnd = (
       ];
 };
 
-function shiftElementsAccountingForFrames(
+const shiftElementsToEnd = (
-  allElements: readonly ExcalidrawElement[],
+  elements: readonly ExcalidrawElement[],
+  appState: AppState,
+  direction: "left" | "right",
+  elementsToBeMoved?: readonly ExcalidrawElement[],
+) => {
+  return shift(
+    elements,
+    appState,
+    direction,
+    _shiftElementsToEnd,
+    elementsToBeMoved,
+  );
+};
+
+function shift(
+  elements: readonly ExcalidrawElement[],
   appState: AppState,
   direction: "left" | "right",
   shiftFunction: (
-    elements: readonly ExcalidrawElement[],
+    elements: ExcalidrawElement[],
     appState: AppState,
     direction: "left" | "right",
-    containingFrame: ExcalidrawFrameElement["id"] | null,
     elementsToBeMoved?: readonly ExcalidrawElement[],
   ) => ExcalidrawElement[] | readonly ExcalidrawElement[],
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) {
-  const elementsToMove = arrayToMap(
+  const elementsMap = arrayToMap(elements);
-    getSelectedElements(allElements, appState, {
+  const frameElementsMap = groupByFrames(elements);
-      includeBoundTextElement: true,
+
-      includeElementsInFrames: true,
+  // in case root is non-existent, we promote children elements to root
-    }),
+  let rootElements = elements.filter(
+    (element) =>
+      isRootElement(element) ||
+      (element.frameId && !elementsMap.has(element.frameId)),
   );
-
+  // and remove non-existet root
-  const frameAwareContiguousElementsToMove: {
+  for (const frameId of frameElementsMap.keys()) {
-    regularElements: ExcalidrawElement[];
+    if (!elementsMap.has(frameId)) {
-    frameChildren: Map<ExcalidrawFrameElement["id"], ExcalidrawElement[]>;
+      frameElementsMap.delete(frameId);
-  } = { regularElements: [], frameChildren: new Map() };
-
-  const fullySelectedFrames = new Set<ExcalidrawFrameElement["id"]>();
-
-  for (const element of allElements) {
-    if (elementsToMove.has(element.id) && isFrameElement(element)) {
-      fullySelectedFrames.add(element.id);
     }
   }
 
-  for (const element of allElements) {
+  // shift the root elements first
-    if (elementsToMove.has(element.id)) {
+  rootElements = shiftFunction(
-      if (
+    rootElements,
-        isFrameElement(element) ||
-        (element.frameId && fullySelectedFrames.has(element.frameId))
-      ) {
-        frameAwareContiguousElementsToMove.regularElements.push(element);
-      } else if (!element.frameId) {
-        frameAwareContiguousElementsToMove.regularElements.push(element);
-      } else {
-        const frameChildren =
-          frameAwareContiguousElementsToMove.frameChildren.get(
-            element.frameId,
-          ) || [];
-        frameChildren.push(element);
-        frameAwareContiguousElementsToMove.frameChildren.set(
-          element.frameId,
-          frameChildren,
-        );
-      }
-    }
-  }
-
-  let nextElements = allElements;
-
-  const frameChildrenSets = Array.from(
-    frameAwareContiguousElementsToMove.frameChildren.entries(),
-  );
-
-  for (const [frameId, children] of frameChildrenSets) {
-    nextElements = shiftFunction(
-      allElements,
-      appState,
-      direction,
-      frameId,
-      children,
-    );
-  }
-
-  return shiftFunction(
-    nextElements,
     appState,
     direction,
-    null,
+    elementsToBeMoved,
-    frameAwareContiguousElementsToMove.regularElements,
+  ) as ExcalidrawElement[];
-  );
+
+  // shift the elements in frames if needed
+  frameElementsMap.forEach((frameElements, frameId) => {
+    if (!appState.selectedElementIds[frameId]) {
+      frameElementsMap.set(
+        frameId,
+        shiftFunction(
+          frameElements,
+          appState,
+          direction,
+          elementsToBeMoved,
+        ) as ExcalidrawElement[],
+      );
+    }
+  });
+
+  // return the final elements
+  let finalElements: ExcalidrawElement[] = [];
+
+  rootElements.forEach((element) => {
+    if (isFrameElement(element)) {
+      finalElements = [
+        ...finalElements,
+        ...(frameElementsMap.get(element.id) ?? []),
+        element,
+      ];
+    } else {
+      finalElements = [...finalElements, element];
+    }
+  });
+
+  return finalElements;
 }
 
 // public API
 // -----------------------------------------------------------------------------
 
 export const moveOneLeft = (
-  allElements: readonly ExcalidrawElement[],
+  elements: readonly ExcalidrawElement[],
   appState: AppState,
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  return shiftElementsByOne(allElements, appState, "left");
+  return shiftElements(appState, elements, "left", elementsToBeMoved);
 };
 
 export const moveOneRight = (
-  allElements: readonly ExcalidrawElement[],
+  elements: readonly ExcalidrawElement[],
   appState: AppState,
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  return shiftElementsByOne(allElements, appState, "right");
+  return shiftElements(appState, elements, "right", elementsToBeMoved);
 };
 
 export const moveAllLeft = (
-  allElements: readonly ExcalidrawElement[],
+  elements: readonly ExcalidrawElement[],
   appState: AppState,
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  return shiftElementsAccountingForFrames(
+  return shiftElementsToEnd(elements, appState, "left", elementsToBeMoved);
-    allElements,
-    appState,
-    "left",
-    shiftElementsToEnd,
-  );
 };
 
 export const moveAllRight = (
-  allElements: readonly ExcalidrawElement[],
+  elements: readonly ExcalidrawElement[],
   appState: AppState,
+  elementsToBeMoved?: readonly ExcalidrawElement[],
 ) => {
-  return shiftElementsAccountingForFrames(
+  return shiftElementsToEnd(elements, appState, "right", elementsToBeMoved);
-    allElements,
-    appState,
-    "right",
-    shiftElementsToEnd,
-  );
 };

yarn.lock

@@ -6459,10 +6459,10 @@ rollup@^3.25.2:
   optionalDependencies:
     fsevents "~2.3.2"
 
-roughjs@4.6.4:
+roughjs@4.6.5:
-  version "4.6.4"
+  version "4.6.5"
-  resolved "https://registry.yarnpkg.com/roughjs/-/roughjs-4.6.4.tgz#b6f39b44645854a6e0a4a28b078368701eb7f939"
+  resolved "https://registry.yarnpkg.com/roughjs/-/roughjs-4.6.5.tgz#1db965cf1a043cb7f05181dd7d119f7960fba8d8"
-  integrity sha512-s6EZ0BntezkFYMf/9mGn7M8XGIoaav9QQBCnJROWB3brUWQ683Q2LbRD/hq0Z3bAJ/9NVpU/5LpiTWvQMyLDhw==
+  integrity sha512-4Q6XBbZWlp8yj1uipq2bQ1CPlxMhW/ukufwkuhh+2L79utk+O5kMSbfVh4UNBMtKJ3PxHQ9Ou3ncNt1iQcphJA==
   dependencies:
     hachure-fill "^0.5.2"
     path-data-parser "^0.1.0"