Explorar el Código

fix: Frame dimensions change by stats don't include new elements (#9568)

Márk Tolmács hace 1 mes
padre
commit
8e27de2cdc

+ 115 - 34
packages/excalidraw/components/Stats/Dimension.tsx

@@ -7,6 +7,9 @@ import {
 } from "@excalidraw/element";
 } from "@excalidraw/element";
 import { resizeSingleElement } from "@excalidraw/element";
 import { resizeSingleElement } from "@excalidraw/element";
 import { isImageElement } from "@excalidraw/element";
 import { isImageElement } from "@excalidraw/element";
+import { isFrameLikeElement } from "@excalidraw/element";
+import { getElementsInResizingFrame } from "@excalidraw/element";
+import { replaceAllElementsInFrame } from "@excalidraw/element";
 
 
 import type { ExcalidrawElement } from "@excalidraw/element/types";
 import type { ExcalidrawElement } from "@excalidraw/element/types";
 
 
@@ -15,7 +18,10 @@ import type { Scene } from "@excalidraw/element";
 import DragInput from "./DragInput";
 import DragInput from "./DragInput";
 import { getStepSizedValue, isPropertyEditable } from "./utils";
 import { getStepSizedValue, isPropertyEditable } from "./utils";
 
 
-import type { DragInputCallbackType } from "./DragInput";
+import type {
+  DragFinishedCallbackType,
+  DragInputCallbackType,
+} from "./DragInput";
 import type { AppState } from "../../types";
 import type { AppState } from "../../types";
 
 
 interface DimensionDragInputProps {
 interface DimensionDragInputProps {
@@ -43,6 +49,8 @@ const handleDimensionChange: DragInputCallbackType<
   originalAppState,
   originalAppState,
   instantChange,
   instantChange,
   scene,
   scene,
+  app,
+  setAppState,
 }) => {
 }) => {
   const elementsMap = scene.getNonDeletedElementsMap();
   const elementsMap = scene.getNonDeletedElementsMap();
   const origElement = originalElements[0];
   const origElement = originalElements[0];
@@ -153,6 +161,7 @@ const handleDimensionChange: DragInputCallbackType<
       return;
       return;
     }
     }
 
 
+    // User types in a value to stats then presses Enter
     if (nextValue !== undefined) {
     if (nextValue !== undefined) {
       const nextWidth = Math.max(
       const nextWidth = Math.max(
         property === "width"
         property === "width"
@@ -184,52 +193,123 @@ const handleDimensionChange: DragInputCallbackType<
         },
         },
       );
       );
 
 
+      // Handle frame membership update for resized frames
+      if (isFrameLikeElement(latestElement)) {
+        const nextElementsInFrame = getElementsInResizingFrame(
+          scene.getElementsIncludingDeleted(),
+          latestElement,
+          originalAppState,
+          scene.getNonDeletedElementsMap(),
+        );
+
+        const updatedElements = replaceAllElementsInFrame(
+          scene.getElementsIncludingDeleted(),
+          nextElementsInFrame,
+          latestElement,
+          app,
+        );
+
+        scene.replaceAllElements(updatedElements);
+      }
+
       return;
       return;
     }
     }
-    const changeInWidth = property === "width" ? accumulatedChange : 0;
-    const changeInHeight = property === "height" ? accumulatedChange : 0;
 
 
-    let nextWidth = Math.max(0, origElement.width + changeInWidth);
-    if (property === "width") {
-      if (shouldChangeByStepSize) {
-        nextWidth = getStepSizedValue(nextWidth, STEP_SIZE);
-      } else {
-        nextWidth = Math.round(nextWidth);
+    // Stats slider is dragged
+    {
+      const changeInWidth = property === "width" ? accumulatedChange : 0;
+      const changeInHeight = property === "height" ? accumulatedChange : 0;
+
+      let nextWidth = Math.max(0, origElement.width + changeInWidth);
+      if (property === "width") {
+        if (shouldChangeByStepSize) {
+          nextWidth = getStepSizedValue(nextWidth, STEP_SIZE);
+        } else {
+          nextWidth = Math.round(nextWidth);
+        }
       }
       }
-    }
 
 
-    let nextHeight = Math.max(0, origElement.height + changeInHeight);
-    if (property === "height") {
-      if (shouldChangeByStepSize) {
-        nextHeight = getStepSizedValue(nextHeight, STEP_SIZE);
-      } else {
-        nextHeight = Math.round(nextHeight);
+      let nextHeight = Math.max(0, origElement.height + changeInHeight);
+      if (property === "height") {
+        if (shouldChangeByStepSize) {
+          nextHeight = getStepSizedValue(nextHeight, STEP_SIZE);
+        } else {
+          nextHeight = Math.round(nextHeight);
+        }
       }
       }
-    }
 
 
-    if (keepAspectRatio) {
-      if (property === "width") {
-        nextHeight = Math.round((nextWidth / aspectRatio) * 100) / 100;
-      } else {
-        nextWidth = Math.round(nextHeight * aspectRatio * 100) / 100;
+      if (keepAspectRatio) {
+        if (property === "width") {
+          nextHeight = Math.round((nextWidth / aspectRatio) * 100) / 100;
+        } else {
+          nextWidth = Math.round(nextHeight * aspectRatio * 100) / 100;
+        }
+      }
+
+      nextHeight = Math.max(MIN_WIDTH_OR_HEIGHT, nextHeight);
+      nextWidth = Math.max(MIN_WIDTH_OR_HEIGHT, nextWidth);
+
+      resizeSingleElement(
+        nextWidth,
+        nextHeight,
+        latestElement,
+        origElement,
+        originalElementsMap,
+        scene,
+        property === "width" ? "e" : "s",
+        {
+          shouldMaintainAspectRatio: keepAspectRatio,
+        },
+      );
+
+      // Handle highlighting frame element candidates
+      if (isFrameLikeElement(latestElement)) {
+        const nextElementsInFrame = getElementsInResizingFrame(
+          scene.getElementsIncludingDeleted(),
+          latestElement,
+          originalAppState,
+          scene.getNonDeletedElementsMap(),
+        );
+
+        setAppState({
+          elementsToHighlight: nextElementsInFrame,
+        });
       }
       }
     }
     }
+  }
+};
 
 
-    nextHeight = Math.max(MIN_WIDTH_OR_HEIGHT, nextHeight);
-    nextWidth = Math.max(MIN_WIDTH_OR_HEIGHT, nextWidth);
+const handleDragFinished: DragFinishedCallbackType = ({
+  setAppState,
+  app,
+  originalElements,
+  originalAppState,
+}) => {
+  const elementsMap = app.scene.getNonDeletedElementsMap();
+  const origElement = originalElements?.[0];
+  const latestElement = origElement && elementsMap.get(origElement.id);
+
+  // Handle frame membership update for resized frames
+  if (latestElement && isFrameLikeElement(latestElement)) {
+    const nextElementsInFrame = getElementsInResizingFrame(
+      app.scene.getElementsIncludingDeleted(),
+      latestElement,
+      originalAppState,
+      app.scene.getNonDeletedElementsMap(),
+    );
 
 
-    resizeSingleElement(
-      nextWidth,
-      nextHeight,
+    const updatedElements = replaceAllElementsInFrame(
+      app.scene.getElementsIncludingDeleted(),
+      nextElementsInFrame,
       latestElement,
       latestElement,
-      origElement,
-      originalElementsMap,
-      scene,
-      property === "width" ? "e" : "s",
-      {
-        shouldMaintainAspectRatio: keepAspectRatio,
-      },
+      app,
     );
     );
+
+    app.scene.replaceAllElements(updatedElements);
+
+    setAppState({
+      elementsToHighlight: null,
+    });
   }
   }
 };
 };
 
 
@@ -269,6 +349,7 @@ const DimensionDragInput = ({
       scene={scene}
       scene={scene}
       appState={appState}
       appState={appState}
       property={property}
       property={property}
+      dragFinishedCallback={handleDragFinished}
     />
     />
   );
   );
 };
 };

+ 25 - 1
packages/excalidraw/components/Stats/DragInput.tsx

@@ -11,7 +11,7 @@ import type { ElementsMap, ExcalidrawElement } from "@excalidraw/element/types";
 
 
 import type { Scene } from "@excalidraw/element";
 import type { Scene } from "@excalidraw/element";
 
 
-import { useApp } from "../App";
+import { useApp, useExcalidrawSetAppState } from "../App";
 import { InlineIcon } from "../InlineIcon";
 import { InlineIcon } from "../InlineIcon";
 
 
 import { SMALLEST_DELTA } from "./utils";
 import { SMALLEST_DELTA } from "./utils";
@@ -36,6 +36,15 @@ export type DragInputCallbackType<
   property: P;
   property: P;
   originalAppState: AppState;
   originalAppState: AppState;
   setInputValue: (value: number) => void;
   setInputValue: (value: number) => void;
+  app: ReturnType<typeof useApp>;
+  setAppState: ReturnType<typeof useExcalidrawSetAppState>;
+}) => void;
+
+export type DragFinishedCallbackType<E = ExcalidrawElement> = (props: {
+  app: ReturnType<typeof useApp>;
+  setAppState: ReturnType<typeof useExcalidrawSetAppState>;
+  originalElements: readonly E[] | null;
+  originalAppState: AppState;
 }) => void;
 }) => void;
 
 
 interface StatsDragInputProps<
 interface StatsDragInputProps<
@@ -54,6 +63,7 @@ interface StatsDragInputProps<
   appState: AppState;
   appState: AppState;
   /** how many px you need to drag to get 1 unit change */
   /** how many px you need to drag to get 1 unit change */
   sensitivity?: number;
   sensitivity?: number;
+  dragFinishedCallback?: DragFinishedCallbackType;
 }
 }
 
 
 const StatsDragInput = <
 const StatsDragInput = <
@@ -71,8 +81,10 @@ const StatsDragInput = <
   scene,
   scene,
   appState,
   appState,
   sensitivity = 1,
   sensitivity = 1,
+  dragFinishedCallback,
 }: StatsDragInputProps<T, E>) => {
 }: StatsDragInputProps<T, E>) => {
   const app = useApp();
   const app = useApp();
+  const setAppState = useExcalidrawSetAppState();
   const inputRef = useRef<HTMLInputElement>(null);
   const inputRef = useRef<HTMLInputElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
   const labelRef = useRef<HTMLDivElement>(null);
 
 
@@ -137,6 +149,8 @@ const StatsDragInput = <
         property,
         property,
         originalAppState: appState,
         originalAppState: appState,
         setInputValue: (value) => setInputValue(String(value)),
         setInputValue: (value) => setInputValue(String(value)),
+        app,
+        setAppState,
       });
       });
       app.syncActionResult({
       app.syncActionResult({
         captureUpdate: CaptureUpdateAction.IMMEDIATELY,
         captureUpdate: CaptureUpdateAction.IMMEDIATELY,
@@ -263,6 +277,8 @@ const StatsDragInput = <
                       scene,
                       scene,
                       originalAppState,
                       originalAppState,
                       setInputValue: (value) => setInputValue(String(value)),
                       setInputValue: (value) => setInputValue(String(value)),
+                      app,
+                      setAppState,
                     });
                     });
 
 
                     stepChange = 0;
                     stepChange = 0;
@@ -287,6 +303,14 @@ const StatsDragInput = <
                 captureUpdate: CaptureUpdateAction.IMMEDIATELY,
                 captureUpdate: CaptureUpdateAction.IMMEDIATELY,
               });
               });
 
 
+              // Notify implementors
+              dragFinishedCallback?.({
+                app,
+                setAppState,
+                originalElements,
+                originalAppState,
+              });
+
               lastPointer = null;
               lastPointer = null;
               accumulatedChange = 0;
               accumulatedChange = 0;
               stepChange = 0;
               stepChange = 0;

+ 83 - 2
packages/excalidraw/components/Stats/MultiDimension.tsx

@@ -2,7 +2,12 @@ import { pointFrom, type GlobalPoint } from "@excalidraw/math";
 import { useMemo } from "react";
 import { useMemo } from "react";
 
 
 import { MIN_WIDTH_OR_HEIGHT } from "@excalidraw/common";
 import { MIN_WIDTH_OR_HEIGHT } from "@excalidraw/common";
-import { updateBoundElements } from "@excalidraw/element";
+import {
+  getElementsInResizingFrame,
+  isFrameLikeElement,
+  replaceAllElementsInFrame,
+  updateBoundElements,
+} from "@excalidraw/element";
 import {
 import {
   rescalePointsInElement,
   rescalePointsInElement,
   resizeSingleElement,
   resizeSingleElement,
@@ -25,7 +30,10 @@ import DragInput from "./DragInput";
 import { getAtomicUnits, getStepSizedValue, isPropertyEditable } from "./utils";
 import { getAtomicUnits, getStepSizedValue, isPropertyEditable } from "./utils";
 import { getElementsInAtomicUnit } from "./utils";
 import { getElementsInAtomicUnit } from "./utils";
 
 
-import type { DragInputCallbackType } from "./DragInput";
+import type {
+  DragFinishedCallbackType,
+  DragInputCallbackType,
+} from "./DragInput";
 import type { AtomicUnit } from "./utils";
 import type { AtomicUnit } from "./utils";
 import type { AppState } from "../../types";
 import type { AppState } from "../../types";
 
 
@@ -153,6 +161,8 @@ const handleDimensionChange: DragInputCallbackType<
   nextValue,
   nextValue,
   scene,
   scene,
   property,
   property,
+  setAppState,
+  app,
 }) => {
 }) => {
   const elementsMap = scene.getNonDeletedElementsMap();
   const elementsMap = scene.getNonDeletedElementsMap();
   const atomicUnits = getAtomicUnits(originalElements, originalAppState);
   const atomicUnits = getAtomicUnits(originalElements, originalAppState);
@@ -239,6 +249,25 @@ const handleDimensionChange: DragInputCallbackType<
               shouldInformMutation: false,
               shouldInformMutation: false,
             },
             },
           );
           );
+
+          // Handle frame membership update for resized frames
+          if (isFrameLikeElement(latestElement)) {
+            const nextElementsInFrame = getElementsInResizingFrame(
+              scene.getElementsIncludingDeleted(),
+              latestElement,
+              originalAppState,
+              scene.getNonDeletedElementsMap(),
+            );
+
+            const updatedElements = replaceAllElementsInFrame(
+              scene.getElementsIncludingDeleted(),
+              nextElementsInFrame,
+              latestElement,
+              app,
+            );
+
+            scene.replaceAllElements(updatedElements);
+          }
         }
         }
       }
       }
     }
     }
@@ -250,6 +279,7 @@ const handleDimensionChange: DragInputCallbackType<
 
 
   const changeInWidth = property === "width" ? accumulatedChange : 0;
   const changeInWidth = property === "width" ? accumulatedChange : 0;
   const changeInHeight = property === "height" ? accumulatedChange : 0;
   const changeInHeight = property === "height" ? accumulatedChange : 0;
+  const elementsToHighlight: ExcalidrawElement[] = [];
 
 
   for (const atomicUnit of atomicUnits) {
   for (const atomicUnit of atomicUnits) {
     const elementsInUnit = getElementsInAtomicUnit(
     const elementsInUnit = getElementsInAtomicUnit(
@@ -342,13 +372,63 @@ const handleDimensionChange: DragInputCallbackType<
             shouldInformMutation: false,
             shouldInformMutation: false,
           },
           },
         );
         );
+
+        // Handle highlighting frame element candidates
+        if (isFrameLikeElement(latestElement)) {
+          const nextElementsInFrame = getElementsInResizingFrame(
+            scene.getElementsIncludingDeleted(),
+            latestElement,
+            originalAppState,
+            scene.getNonDeletedElementsMap(),
+          );
+
+          elementsToHighlight.push(...nextElementsInFrame);
+        }
       }
       }
     }
     }
   }
   }
 
 
+  setAppState({
+    elementsToHighlight,
+  });
+
   scene.triggerUpdate();
   scene.triggerUpdate();
 };
 };
 
 
+const handleDragFinished: DragFinishedCallbackType = ({
+  setAppState,
+  app,
+  originalElements,
+  originalAppState,
+}) => {
+  const elementsMap = app.scene.getNonDeletedElementsMap();
+  const origElement = originalElements?.[0];
+  const latestElement = origElement && elementsMap.get(origElement.id);
+
+  // Handle frame membership update for resized frames
+  if (latestElement && isFrameLikeElement(latestElement)) {
+    const nextElementsInFrame = getElementsInResizingFrame(
+      app.scene.getElementsIncludingDeleted(),
+      latestElement,
+      originalAppState,
+      app.scene.getNonDeletedElementsMap(),
+    );
+
+    const updatedElements = replaceAllElementsInFrame(
+      app.scene.getElementsIncludingDeleted(),
+      nextElementsInFrame,
+      latestElement,
+      app,
+    );
+
+    app.scene.replaceAllElements(updatedElements);
+
+    setAppState({
+      elementsToHighlight: null,
+    });
+  }
+};
+
 const MultiDimension = ({
 const MultiDimension = ({
   property,
   property,
   elements,
   elements,
@@ -396,6 +476,7 @@ const MultiDimension = ({
       appState={appState}
       appState={appState}
       property={property}
       property={property}
       scene={scene}
       scene={scene}
+      dragFinishedCallback={handleDragFinished}
     />
     />
   );
   );
 };
 };

+ 193 - 0
packages/excalidraw/components/Stats/stats.test.tsx

@@ -737,3 +737,196 @@ describe("stats for multiple elements", () => {
     expect(newGroupHeight).toBeCloseTo(500, 4);
     expect(newGroupHeight).toBeCloseTo(500, 4);
   });
   });
 });
 });
+
+describe("frame resizing behavior", () => {
+  beforeEach(async () => {
+    localStorage.clear();
+    renderStaticScene.mockClear();
+    reseed(7);
+    setDateTimeForTests("201933152653");
+
+    await render(<Excalidraw handleKeyboardGlobally={true} />);
+
+    API.setElements([]);
+
+    fireEvent.contextMenu(GlobalTestState.interactiveCanvas, {
+      button: 2,
+      clientX: 1,
+      clientY: 1,
+    });
+    const contextMenu = UI.queryContextMenu();
+    fireEvent.click(queryByTestId(contextMenu!, "stats")!);
+    stats = UI.queryStats();
+  });
+
+  beforeAll(() => {
+    mockBoundingClientRect();
+  });
+
+  afterAll(() => {
+    restoreOriginalGetBoundingClientRect();
+  });
+
+  it("should add shapes to frame when resizing frame to encompass them", () => {
+    // Create a frame
+    const frame = API.createElement({
+      type: "frame",
+      x: 0,
+      y: 0,
+      width: 100,
+      height: 100,
+    });
+
+    // Create a rectangle outside the frame
+    const rectangle = API.createElement({
+      type: "rectangle",
+      x: 150,
+      y: 50,
+      width: 50,
+      height: 50,
+    });
+
+    API.setElements([frame, rectangle]);
+
+    // Initially, rectangle should not be in the frame
+    expect(rectangle.frameId).toBe(null);
+
+    // Select the frame
+    API.setAppState({
+      selectedElementIds: {
+        [frame.id]: true,
+      },
+    });
+
+    elementStats = stats?.querySelector("#elementStats");
+
+    // Find the width input and update it to encompass the rectangle
+    const widthInput = UI.queryStatsProperty("W")?.querySelector(
+      ".drag-input",
+    ) as HTMLInputElement;
+
+    expect(widthInput).toBeDefined();
+    expect(widthInput.value).toBe("100");
+
+    // Resize frame to width 250, which should encompass the rectangle
+    UI.updateInput(widthInput, "250");
+
+    // After resizing, the rectangle should now be part of the frame
+    expect(h.elements.find((el) => el.id === rectangle.id)?.frameId).toBe(
+      frame.id,
+    );
+  });
+
+  it("should add multiple shapes when frame encompasses them through height resize", () => {
+    const frame = API.createElement({
+      type: "frame",
+      x: 0,
+      y: 0,
+      width: 200,
+      height: 100,
+    });
+
+    const rectangle1 = API.createElement({
+      type: "rectangle",
+      x: 50,
+      y: 150,
+      width: 50,
+      height: 50,
+    });
+
+    const rectangle2 = API.createElement({
+      type: "rectangle",
+      x: 100,
+      y: 180,
+      width: 40,
+      height: 40,
+    });
+
+    API.setElements([frame, rectangle1, rectangle2]);
+
+    // Initially, rectangles should not be in the frame
+    expect(rectangle1.frameId).toBe(null);
+    expect(rectangle2.frameId).toBe(null);
+
+    // Select the frame
+    API.setAppState({
+      selectedElementIds: {
+        [frame.id]: true,
+      },
+    });
+
+    elementStats = stats?.querySelector("#elementStats");
+
+    // Resize frame height to encompass both rectangles
+    const heightInput = UI.queryStatsProperty("H")?.querySelector(
+      ".drag-input",
+    ) as HTMLInputElement;
+
+    // Resize frame to height 250, which should encompass both rectangles
+    UI.updateInput(heightInput, "250");
+
+    // After resizing, both rectangles should now be part of the frame
+    expect(h.elements.find((el) => el.id === rectangle1.id)?.frameId).toBe(
+      frame.id,
+    );
+    expect(h.elements.find((el) => el.id === rectangle2.id)?.frameId).toBe(
+      frame.id,
+    );
+  });
+
+  it("should not affect shapes that remain outside frame after resize", () => {
+    const frame = API.createElement({
+      type: "frame",
+      x: 0,
+      y: 0,
+      width: 100,
+      height: 100,
+    });
+
+    const insideRect = API.createElement({
+      type: "rectangle",
+      x: 120,
+      y: 50,
+      width: 30,
+      height: 30,
+    });
+
+    const outsideRect = API.createElement({
+      type: "rectangle",
+      x: 300,
+      y: 50,
+      width: 30,
+      height: 30,
+    });
+
+    API.setElements([frame, insideRect, outsideRect]);
+
+    // Initially, both rectangles should not be in the frame
+    expect(insideRect.frameId).toBe(null);
+    expect(outsideRect.frameId).toBe(null);
+
+    // Select the frame
+    API.setAppState({
+      selectedElementIds: {
+        [frame.id]: true,
+      },
+    });
+
+    elementStats = stats?.querySelector("#elementStats");
+
+    // Resize frame width to 200, which should only encompass insideRect
+    const widthInput = UI.queryStatsProperty("W")?.querySelector(
+      ".drag-input",
+    ) as HTMLInputElement;
+
+    UI.updateInput(widthInput, "200");
+
+    // After resizing, only insideRect should be in the frame
+    expect(h.elements.find((el) => el.id === insideRect.id)?.frameId).toBe(
+      frame.id,
+    );
+    expect(h.elements.find((el) => el.id === outsideRect.id)?.frameId).toBe(
+      null,
+    );
+  });
+});