fix: elements in non-existing frame getting removed (#6708)

Co-authored-by: dwelle <[email protected]>

src/data/restore.ts

@@ -370,6 +370,24 @@ const repairBoundElement = (
   }
 };
 
+/**
+ * Remove an element's frameId if its containing frame is non-existent
+ *
+ * NOTE mutates elements.
+ */
+const repairFrameMembership = (
+  element: Mutable<ExcalidrawElement>,
+  elementsMap: Map<string, Mutable<ExcalidrawElement>>,
+) => {
+  if (element.frameId) {
+    const containingFrame = elementsMap.get(element.frameId);
+
+    if (!containingFrame) {
+      element.frameId = null;
+    }
+  }
+};
+
 export const restoreElements = (
   elements: ImportedDataState["elements"],
   /** NOTE doesn't serve for reconciliation */
@@ -410,6 +428,10 @@ export const restoreElements = (
   // repair binding. Mutates elements.
   const restoredElementsMap = arrayToMap(restoredElements);
   for (const element of restoredElements) {
+    if (element.frameId) {
+      repairFrameMembership(element, restoredElementsMap);
+    }
+
     if (isTextElement(element) && element.containerId) {
       repairBoundElement(element, restoredElementsMap);
     } else if (element.boundElements) {

src/frame.ts

@@ -304,7 +304,7 @@ export const groupsAreCompletelyOutOfFrame = (
 /**
  * Returns a map of frameId to frame elements. Includes empty frames.
  */
-export const groupByFrames = (elements: ExcalidrawElementsIncludingDeleted) => {
+export const groupByFrames = (elements: readonly ExcalidrawElement[]) => {
   const frameElementsMap = new Map<
     ExcalidrawElement["id"],
     ExcalidrawElement[]
@@ -591,6 +591,7 @@ export const updateFrameMembershipOfSelectedElements = (
 
   elementsToFilter.forEach((element) => {
     if (
+      element.frameId &&
       !isFrameElement(element) &&
       !isElementInFrame(element, allElements, appState)
     ) {
@@ -598,7 +599,9 @@ export const updateFrameMembershipOfSelectedElements = (
     }
   });
 
-  return removeElementsFromFrame(allElements, [...elementsToRemove], appState);
+  return elementsToRemove.size > 0
+    ? removeElementsFromFrame(allElements, [...elementsToRemove], appState)
+    : allElements;
 };
 
 /**

src/zindex.ts

@@ -376,9 +376,22 @@ function shift(
   ) => ExcalidrawElement[] | readonly ExcalidrawElement[],
   elementsToBeMoved?: readonly ExcalidrawElement[],
 ) {
-  let rootElements = elements.filter((element) => isRootElement(element));
+  const elementsMap = arrayToMap(elements);
   const frameElementsMap = groupByFrames(elements);
 
+  // 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
+  for (const frameId of frameElementsMap.keys()) {
+    if (!elementsMap.has(frameId)) {
+      frameElementsMap.delete(frameId);
+    }
+  }
+
   // shift the root elements first
   rootElements = shiftFunction(
     rootElements,