revert: remove bound-arrows from frames (#7190)

dev-docs/docs/codebase/frames.mdx

@@ -20,13 +20,3 @@ 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.

src/actions/actionDuplicateSelection.tsx

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

src/components/App.tsx

@@ -2554,12 +2554,18 @@ 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;
@@ -3574,11 +3580,6 @@ 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 &&

src/data/restore.ts

@@ -43,7 +43,6 @@ import {
   measureBaseline,
 } from "../element/textElement";
 import { normalizeLink } from "./url";
-import { isValidFrameChild } from "../frame";
 
 type RestoredAppState = Omit<
   AppState,
@@ -397,7 +396,7 @@ const repairBoundElement = (
 };
 
 /**
- * resets `frameId` if no longer applicable.
+ * Remove an element's frameId if its containing frame is non-existent
  *
  * NOTE mutates elements.
  */
@@ -405,16 +404,12 @@ const repairFrameMembership = (
   element: Mutable<ExcalidrawElement>,
   elementsMap: Map<string, Mutable<ExcalidrawElement>>,
 ) => {
-  if (!element.frameId) {
-    return;
-  }
+  if (element.frameId) {
+    const containingFrame = elementsMap.get(element.frameId);
 
-  if (
-    !isValidFrameChild(element) ||
-    // target frame not exists
-    !elementsMap.get(element.frameId)
-  ) {
-    element.frameId = null;
+    if (!containingFrame) {
+      element.frameId = null;
+    }
   }
 };
 
@@ -458,8 +453,6 @@ 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/element/binding.ts

@@ -27,7 +27,6 @@ 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>
@@ -212,15 +211,6 @@ export const bindLinearElement = (
       }),
     });
   }
-  if (linearElement.frameId && !isValidFrameChild(linearElement)) {
-    mutateElement(
-      linearElement,
-      {
-        frameId: null,
-      },
-      false,
-    );
-  }
 };
 
 // Don't bind both ends of a simple segment

src/frame.ts

@@ -323,24 +323,7 @@ export const groupByFrames = (elements: readonly ExcalidrawElement[]) => {
 export const getFrameElements = (
   allElements: ExcalidrawElementsIncludingDeleted,
   frameId: string,
-  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;
-  });
-};
+) => allElements.filter((element) => element.frameId === frameId);
 
 export const getElementsInResizingFrame = (
   allElements: ExcalidrawElementsIncludingDeleted,
@@ -468,14 +451,6 @@ 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 -------------------------------
 
 /**
@@ -514,9 +489,6 @@ export const addElementsToFrame = (
     elementsToAdd,
   )) {
     if (!currTargetFrameChildrenMap.has(element.id)) {
-      if (!isValidFrameChild(element)) {
-        continue;
-      }
       finalElementsToAdd.push(element);
     }
 

src/renderer/renderScene.ts

@@ -69,7 +69,6 @@ import {
 } from "../element/Hyperlink";
 import { renderSnaps } from "./renderSnaps";
 import {
-  isArrowElement,
   isEmbeddableElement,
   isFrameElement,
   isLinearElement,
@@ -985,10 +984,7 @@ const _renderStaticScene = ({
 
           // TODO do we need to check isElementInFrame here?
           if (frame && isElementInFrame(element, elements, appState)) {
-            // do not clip arrows
-            if (!isArrowElement(element)) {
-              frameClip(frame, context, renderConfig, appState);
-            }
+            frameClip(frame, context, renderConfig, appState);
           }
           renderElement(element, rc, context, renderConfig, appState);
           context.restore();