From 0cfa53b7649c5f856bd73b58cb147b04f15e5b39 Mon Sep 17 00:00:00 2001 From: Christopher Tangonan <161169629+cTangonan123@users.noreply.github.com> Date: Tue, 15 Jul 2025 03:43:42 -0700 Subject: [PATCH] fix: aligning and distributing elements and nested groups while editing a group (#9721) --- packages/element/src/align.ts | 11 +- packages/element/src/distribute.ts | 11 +- packages/element/src/groups.ts | 77 ++++ packages/element/tests/align.test.tsx | 420 ++++++++++++++++++ packages/excalidraw/actions/actionAlign.tsx | 15 +- .../excalidraw/actions/actionDistribute.tsx | 9 +- 6 files changed, 534 insertions(+), 9 deletions(-) diff --git a/packages/element/src/align.ts b/packages/element/src/align.ts index 546bbbfa48..3068aee8d1 100644 --- a/packages/element/src/align.ts +++ b/packages/element/src/align.ts @@ -1,6 +1,8 @@ +import type { AppState } from "@excalidraw/excalidraw/types"; + import { updateBoundElements } from "./binding"; import { getCommonBoundingBox } from "./bounds"; -import { getMaximumGroups } from "./groups"; +import { getSelectedElementsByGroup } from "./groups"; import type { Scene } from "./Scene"; @@ -16,11 +18,12 @@ export const alignElements = ( selectedElements: ExcalidrawElement[], alignment: Alignment, scene: Scene, + appState: Readonly, ): ExcalidrawElement[] => { - const elementsMap = scene.getNonDeletedElementsMap(); - const groups: ExcalidrawElement[][] = getMaximumGroups( + const groups: ExcalidrawElement[][] = getSelectedElementsByGroup( selectedElements, - elementsMap, + scene.getNonDeletedElementsMap(), + appState, ); const selectionBoundingBox = getCommonBoundingBox(selectedElements); diff --git a/packages/element/src/distribute.ts b/packages/element/src/distribute.ts index da79837da6..add3522acc 100644 --- a/packages/element/src/distribute.ts +++ b/packages/element/src/distribute.ts @@ -1,7 +1,9 @@ +import type { AppState } from "@excalidraw/excalidraw/types"; + import { getCommonBoundingBox } from "./bounds"; import { newElementWith } from "./mutateElement"; -import { getMaximumGroups } from "./groups"; +import { getSelectedElementsByGroup } from "./groups"; import type { ElementsMap, ExcalidrawElement } from "./types"; @@ -14,6 +16,7 @@ export const distributeElements = ( selectedElements: ExcalidrawElement[], elementsMap: ElementsMap, distribution: Distribution, + appState: Readonly, ): ExcalidrawElement[] => { const [start, mid, end, extent] = distribution.axis === "x" @@ -21,7 +24,11 @@ export const distributeElements = ( : (["minY", "midY", "maxY", "height"] as const); const bounds = getCommonBoundingBox(selectedElements); - const groups = getMaximumGroups(selectedElements, elementsMap) + const groups = getSelectedElementsByGroup( + selectedElements, + elementsMap, + appState, + ) .map((group) => [group, getCommonBoundingBox(group)] as const) .sort((a, b) => a[1][mid] - b[1][mid]); diff --git a/packages/element/src/groups.ts b/packages/element/src/groups.ts index 1cd1536e11..40f787a01b 100644 --- a/packages/element/src/groups.ts +++ b/packages/element/src/groups.ts @@ -7,6 +7,8 @@ import type { Mutable } from "@excalidraw/common/utility-types"; import { getBoundTextElement } from "./textElement"; +import { isBoundToContainer } from "./typeChecks"; + import { makeNextSelectedElementIds, getSelectedElements } from "./selection"; import type { @@ -402,3 +404,78 @@ export const getNewGroupIdsForDuplication = ( return copy; }; + +// given a list of selected elements, return the element grouped by their immediate group selected state +// in the case if only one group is selected and all elements selected are within the group, it will respect group hierarchy in accordance to their nested grouping order +export const getSelectedElementsByGroup = ( + selectedElements: ExcalidrawElement[], + elementsMap: ElementsMap, + appState: Readonly, +): ExcalidrawElement[][] => { + const selectedGroupIds = getSelectedGroupIds(appState); + const unboundElements = selectedElements.filter( + (element) => !isBoundToContainer(element), + ); + const groups: Map = new Map(); + const elements: Map = new Map(); + + // helper function to add an element to the elements map + const addToElementsMap = (element: ExcalidrawElement) => { + // elements + const currentElementMembers = elements.get(element.id) || []; + const boundTextElement = getBoundTextElement(element, elementsMap); + + if (boundTextElement) { + currentElementMembers.push(boundTextElement); + } + elements.set(element.id, [...currentElementMembers, element]); + }; + + // helper function to add an element to the groups map + const addToGroupsMap = (element: ExcalidrawElement, groupId: string) => { + // groups + const currentGroupMembers = groups.get(groupId) || []; + const boundTextElement = getBoundTextElement(element, elementsMap); + + if (boundTextElement) { + currentGroupMembers.push(boundTextElement); + } + groups.set(groupId, [...currentGroupMembers, element]); + }; + + // helper function to handle the case where a single group is selected + // and all elements selected are within the group, it will respect group hierarchy in accordance to + // their nested grouping order + const handleSingleSelectedGroupCase = ( + element: ExcalidrawElement, + selectedGroupId: GroupId, + ) => { + const indexOfSelectedGroupId = element.groupIds.indexOf(selectedGroupId, 0); + const nestedGroupCount = element.groupIds.slice( + 0, + indexOfSelectedGroupId, + ).length; + return nestedGroupCount > 0 + ? addToGroupsMap(element, element.groupIds[indexOfSelectedGroupId - 1]) + : addToElementsMap(element); + }; + + const isAllInSameGroup = selectedElements.every((element) => + isSelectedViaGroup(appState, element), + ); + + unboundElements.forEach((element) => { + const selectedGroupId = getSelectedGroupIdForElement( + element, + appState.selectedGroupIds, + ); + if (!selectedGroupId) { + addToElementsMap(element); + } else if (selectedGroupIds.length === 1 && isAllInSameGroup) { + handleSingleSelectedGroupCase(element, selectedGroupId); + } else { + addToGroupsMap(element, selectedGroupId); + } + }); + return Array.from(groups.values()).concat(Array.from(elements.values())); +}; diff --git a/packages/element/tests/align.test.tsx b/packages/element/tests/align.test.tsx index afffb72cb4..b796793690 100644 --- a/packages/element/tests/align.test.tsx +++ b/packages/element/tests/align.test.tsx @@ -589,4 +589,424 @@ describe("aligning", () => { expect(API.getSelectedElements()[2].x).toEqual(250); expect(API.getSelectedElements()[3].x).toEqual(150); }); + + const createGroupAndSelectInEditGroupMode = () => { + UI.clickTool("rectangle"); + mouse.down(); + mouse.up(100, 100); + + UI.clickTool("rectangle"); + mouse.down(0, 0); + mouse.up(100, 100); + + // select the first element. + // The second rectangle is already reselected because it was the last element created + mouse.reset(); + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(10, 0); + mouse.click(); + }); + + API.executeAction(actionGroup); + mouse.reset(); + mouse.moveTo(10, 0); + mouse.doubleClick(); + + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.click(); + mouse.moveTo(100, 100); + mouse.click(); + }); + }; + + it("aligns elements within a group while in group edit mode correctly to the top", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignTop); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(0); + }); + it("aligns elements within a group while in group edit mode correctly to the bottom", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignBottom); + + expect(API.getSelectedElements()[0].y).toEqual(100); + expect(API.getSelectedElements()[1].y).toEqual(100); + }); + it("aligns elements within a group while in group edit mode correctly to the left", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignLeft); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(0); + }); + it("aligns elements within a group while in group edit mode correctly to the right", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignRight); + + expect(API.getSelectedElements()[0].x).toEqual(100); + expect(API.getSelectedElements()[1].x).toEqual(100); + }); + it("aligns elements within a group while in group edit mode correctly to the vertical center", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignVerticallyCentered); + + expect(API.getSelectedElements()[0].y).toEqual(50); + expect(API.getSelectedElements()[1].y).toEqual(50); + }); + it("aligns elements within a group while in group edit mode correctly to the horizontal center", () => { + createGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignHorizontallyCentered); + + expect(API.getSelectedElements()[0].x).toEqual(50); + expect(API.getSelectedElements()[1].x).toEqual(50); + }); + + const createNestedGroupAndSelectInEditGroupMode = () => { + UI.clickTool("rectangle"); + mouse.down(); + mouse.up(100, 100); + + UI.clickTool("rectangle"); + mouse.down(0, 0); + mouse.up(100, 100); + + // Select the first element. + // The second rectangle is already reselected because it was the last element created + mouse.reset(); + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(10, 0); + mouse.click(); + }); + + API.executeAction(actionGroup); + + mouse.reset(); + mouse.moveTo(200, 200); + // create third element + UI.clickTool("rectangle"); + mouse.down(0, 0); + mouse.up(100, 100); + + // third element is already selected, select the initial group and group together + mouse.reset(); + + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(10, 0); + mouse.click(); + }); + + API.executeAction(actionGroup); + + // double click to enter edit mode + mouse.doubleClick(); + + // select nested group and other element within the group + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(200, 200); + mouse.click(); + }); + }; + + it("aligns element and nested group while in group edit mode correctly to the top", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignTop); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(0); + }); + it("aligns element and nested group while in group edit mode correctly to the bottom", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignBottom); + + expect(API.getSelectedElements()[0].y).toEqual(100); + expect(API.getSelectedElements()[1].y).toEqual(200); + expect(API.getSelectedElements()[2].y).toEqual(200); + }); + it("aligns element and nested group while in group edit mode correctly to the left", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignLeft); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(0); + }); + it("aligns element and nested group while in group edit mode correctly to the right", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignRight); + + expect(API.getSelectedElements()[0].x).toEqual(100); + expect(API.getSelectedElements()[1].x).toEqual(200); + expect(API.getSelectedElements()[2].x).toEqual(200); + }); + it("aligns element and nested group while in group edit mode correctly to the vertical center", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignVerticallyCentered); + + expect(API.getSelectedElements()[0].y).toEqual(50); + expect(API.getSelectedElements()[1].y).toEqual(150); + expect(API.getSelectedElements()[2].y).toEqual(100); + }); + it("aligns elements and nested group within a group while in group edit mode correctly to the horizontal center", () => { + createNestedGroupAndSelectInEditGroupMode(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignHorizontallyCentered); + + expect(API.getSelectedElements()[0].x).toEqual(50); + expect(API.getSelectedElements()[1].x).toEqual(150); + expect(API.getSelectedElements()[2].x).toEqual(100); + }); + + const createAndSelectSingleGroup = () => { + UI.clickTool("rectangle"); + mouse.down(); + mouse.up(100, 100); + + UI.clickTool("rectangle"); + mouse.down(0, 0); + mouse.up(100, 100); + + // Select the first element. + // The second rectangle is already reselected because it was the last element created + mouse.reset(); + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(10, 0); + mouse.click(); + }); + + API.executeAction(actionGroup); + }; + + it("aligns elements within a single-selected group correctly to the top", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignTop); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(0); + }); + it("aligns elements within a single-selected group correctly to the bottom", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignBottom); + + expect(API.getSelectedElements()[0].y).toEqual(100); + expect(API.getSelectedElements()[1].y).toEqual(100); + }); + it("aligns elements within a single-selected group correctly to the left", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignLeft); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(0); + }); + it("aligns elements within a single-selected group correctly to the right", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignRight); + + expect(API.getSelectedElements()[0].x).toEqual(100); + expect(API.getSelectedElements()[1].x).toEqual(100); + }); + it("aligns elements within a single-selected group correctly to the vertical center", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + + API.executeAction(actionAlignVerticallyCentered); + + expect(API.getSelectedElements()[0].y).toEqual(50); + expect(API.getSelectedElements()[1].y).toEqual(50); + }); + it("aligns elements within a single-selected group correctly to the horizontal center", () => { + createAndSelectSingleGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + + API.executeAction(actionAlignHorizontallyCentered); + + expect(API.getSelectedElements()[0].x).toEqual(50); + expect(API.getSelectedElements()[1].x).toEqual(50); + }); + + const createAndSelectSingleGroupWithNestedGroup = () => { + UI.clickTool("rectangle"); + mouse.down(); + mouse.up(100, 100); + + UI.clickTool("rectangle"); + mouse.down(0, 0); + mouse.up(100, 100); + + // Select the first element. + // The second rectangle is already reselected because it was the last element created + mouse.reset(); + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.moveTo(10, 0); + mouse.click(); + }); + + API.executeAction(actionGroup); + + mouse.reset(); + UI.clickTool("rectangle"); + mouse.down(200, 200); + mouse.up(100, 100); + + // Add group to current selection + mouse.restorePosition(10, 0); + Keyboard.withModifierKeys({ shift: true }, () => { + mouse.click(); + }); + + // Create the nested group + API.executeAction(actionGroup); + }; + it("aligns elements within a single-selected group containing a nested group correctly to the top", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignTop); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(0); + }); + it("aligns elements within a single-selected group containing a nested group correctly to the bottom", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignBottom); + + expect(API.getSelectedElements()[0].y).toEqual(100); + expect(API.getSelectedElements()[1].y).toEqual(200); + expect(API.getSelectedElements()[2].y).toEqual(200); + }); + it("aligns elements within a single-selected group containing a nested group correctly to the left", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignLeft); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(0); + }); + it("aligns elements within a single-selected group containing a nested group correctly to the right", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignRight); + + expect(API.getSelectedElements()[0].x).toEqual(100); + expect(API.getSelectedElements()[1].x).toEqual(200); + expect(API.getSelectedElements()[2].x).toEqual(200); + }); + it("aligns elements within a single-selected group containing a nested group correctly to the vertical center", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].y).toEqual(0); + expect(API.getSelectedElements()[1].y).toEqual(100); + expect(API.getSelectedElements()[2].y).toEqual(200); + + API.executeAction(actionAlignVerticallyCentered); + + expect(API.getSelectedElements()[0].y).toEqual(50); + expect(API.getSelectedElements()[1].y).toEqual(150); + expect(API.getSelectedElements()[2].y).toEqual(100); + }); + it("aligns elements within a single-selected group containing a nested group correctly to the horizontal center", () => { + createAndSelectSingleGroupWithNestedGroup(); + + expect(API.getSelectedElements()[0].x).toEqual(0); + expect(API.getSelectedElements()[1].x).toEqual(100); + expect(API.getSelectedElements()[2].x).toEqual(200); + + API.executeAction(actionAlignHorizontallyCentered); + + expect(API.getSelectedElements()[0].x).toEqual(50); + expect(API.getSelectedElements()[1].x).toEqual(150); + expect(API.getSelectedElements()[2].x).toEqual(100); + }); }); diff --git a/packages/excalidraw/actions/actionAlign.tsx b/packages/excalidraw/actions/actionAlign.tsx index de5cd2c1e4..63a887635b 100644 --- a/packages/excalidraw/actions/actionAlign.tsx +++ b/packages/excalidraw/actions/actionAlign.tsx @@ -10,6 +10,8 @@ import { alignElements } from "@excalidraw/element"; import { CaptureUpdateAction } from "@excalidraw/element"; +import { getSelectedElementsByGroup } from "@excalidraw/element"; + import type { ExcalidrawElement } from "@excalidraw/element/types"; import type { Alignment } from "@excalidraw/element"; @@ -38,7 +40,11 @@ export const alignActionsPredicate = ( ) => { const selectedElements = app.scene.getSelectedElements(appState); return ( - selectedElements.length > 1 && + getSelectedElementsByGroup( + selectedElements, + app.scene.getNonDeletedElementsMap(), + appState as Readonly, + ).length > 1 && // TODO enable aligning frames when implemented properly !selectedElements.some((el) => isFrameLikeElement(el)) ); @@ -52,7 +58,12 @@ const alignSelectedElements = ( ) => { const selectedElements = app.scene.getSelectedElements(appState); - const updatedElements = alignElements(selectedElements, alignment, app.scene); + const updatedElements = alignElements( + selectedElements, + alignment, + app.scene, + appState, + ); const updatedElementsMap = arrayToMap(updatedElements); diff --git a/packages/excalidraw/actions/actionDistribute.tsx b/packages/excalidraw/actions/actionDistribute.tsx index bd823ec01a..f02906741c 100644 --- a/packages/excalidraw/actions/actionDistribute.tsx +++ b/packages/excalidraw/actions/actionDistribute.tsx @@ -10,6 +10,8 @@ import { distributeElements } from "@excalidraw/element"; import { CaptureUpdateAction } from "@excalidraw/element"; +import { getSelectedElementsByGroup } from "@excalidraw/element"; + import type { ExcalidrawElement } from "@excalidraw/element/types"; import type { Distribution } from "@excalidraw/element"; @@ -31,7 +33,11 @@ import type { AppClassProperties, AppState } from "../types"; const enableActionGroup = (appState: AppState, app: AppClassProperties) => { const selectedElements = app.scene.getSelectedElements(appState); return ( - selectedElements.length > 1 && + getSelectedElementsByGroup( + selectedElements, + app.scene.getNonDeletedElementsMap(), + appState as Readonly, + ).length > 2 && // TODO enable distributing frames when implemented properly !selectedElements.some((el) => isFrameLikeElement(el)) ); @@ -49,6 +55,7 @@ const distributeSelectedElements = ( selectedElements, app.scene.getNonDeletedElementsMap(), distribution, + appState, ); const updatedElementsMap = arrayToMap(updatedElements);