From 855be700ffcacd5122319124b6df8bd22dd76845 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Fri, 3 Jul 2026 14:08:01 +0200 Subject: [PATCH 1/6] feat: Support multiple boards on the same page --- pages/dnd/multiple-boards-test.page.tsx | 59 +++++++ pages/dnd/multiple-boards.page.tsx | 159 ++++++++++++++++++ .../__tests__/board-acquisition.test.tsx | 24 ++- src/board/__tests__/multiple-boards.test.tsx | 159 ++++++++++++++++++ src/board/interfaces.ts | 3 + src/board/internal.tsx | 34 +++- src/board/transition.ts | 3 + .../utils/__tests__/get-hovered-rect.test.ts | 41 +++++ src/board/utils/__tests__/layout.test.ts | 1 + src/board/utils/get-hovered-rect.ts | 7 +- src/board/utils/layout.ts | 2 +- .../dnd-controller/__mocks__/controller.ts | 15 +- .../__tests__/controller.test.tsx | 127 ++++++++++++++ src/internal/dnd-controller/controller.ts | 4 +- .../__tests__/get-next-droppable.test.ts | 36 ++++ src/internal/utils/__tests__/layout.test.ts | 16 +- src/internal/utils/layout.ts | 11 +- .../board-layout/multiple-boards.test.ts | 88 ++++++++++ 18 files changed, 769 insertions(+), 20 deletions(-) create mode 100644 pages/dnd/multiple-boards-test.page.tsx create mode 100644 pages/dnd/multiple-boards.page.tsx create mode 100644 src/board/__tests__/multiple-boards.test.tsx create mode 100644 src/board/utils/__tests__/get-hovered-rect.test.ts create mode 100644 src/internal/dnd-controller/__tests__/controller.test.tsx create mode 100644 test/functional/board-layout/multiple-boards.test.ts diff --git a/pages/dnd/multiple-boards-test.page.tsx b/pages/dnd/multiple-boards-test.page.tsx new file mode 100644 index 00000000..ff7f1cef --- /dev/null +++ b/pages/dnd/multiple-boards-test.page.tsx @@ -0,0 +1,59 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { useState } from "react"; + +import Header from "@cloudscape-design/components/header"; + +import { Board, BoardItem, BoardProps } from "../../lib/components"; +import PageLayout from "../app/page-layout"; +import { boardI18nStrings, boardItemI18nStrings } from "../shared/i18n"; +import { ItemData } from "../shared/interfaces"; +import { createLetterItems, letterWidgets } from "./items"; + +// A functional-test-oriented page: two independent boards seeded with disjoint letter items. +// Board A uses A–D, board B uses E–H, so cross-board leakage (if any) would be obvious. +const boardAItems = createLetterItems([ + ["A", "B"], + ["C", "D"], +])!.boardItems; +const boardBItems = createLetterItems([ + ["E", "F"], + ["G", "H"], +])!.boardItems; + +function LetterBoard({ + boardTestId, + initialItems, +}: { + boardTestId: string; + initialItems: readonly BoardProps.Item[]; +}) { + const [items, setItems] = useState(initialItems); + return ( +
+ setItems(detail.items)} + empty="No items" + renderItem={(item) => { + const widget = letterWidgets[item.id]; + return ( + {widget?.data.title ?? item.id}} i18nStrings={boardItemI18nStrings}> + {item.id} + + ); + }} + /> +
+ ); +} + +export default function MultipleBoardsTestPage() { + return ( + Multiple boards (functional test)}> + + + + ); +} diff --git a/pages/dnd/multiple-boards.page.tsx b/pages/dnd/multiple-boards.page.tsx new file mode 100644 index 00000000..085996e9 --- /dev/null +++ b/pages/dnd/multiple-boards.page.tsx @@ -0,0 +1,159 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { useState } from "react"; + +import ButtonDropdown from "@cloudscape-design/components/button-dropdown"; +import Header from "@cloudscape-design/components/header"; +import SpaceBetween from "@cloudscape-design/components/space-between"; + +import { Board, BoardItem, BoardProps, ItemsPalette } from "../../lib/components"; +import { ItemsPaletteProps } from "../../src/items-palette/interfaces"; +import PageLayout from "../app/page-layout"; +import { ScreenshotArea } from "../screenshot-area"; +import { boardI18nStrings, boardItemI18nStrings, itemsPaletteI18nStrings } from "../shared/i18n"; +import { ItemData } from "../shared/interfaces"; +import { demoWidgets } from "./items"; + +// A small, self-contained board used multiple times on the same page. Each instance keeps its own +// items state and reacts only to its own drag-and-drop interactions. +function DemoBoard({ + boardLabel, + initialItems, +}: { + boardLabel: string; + initialItems: readonly BoardProps.Item[]; +}) { + const [items, setItems] = useState(initialItems); + return ( + setItems(detail.items)} + empty="No items" + renderItem={(item, actions) => ( + {item.data.title}} + footer={item.data.footer} + settings={ + actions.removeItem()} + expandToViewport={true} + /> + } + i18nStrings={boardItemI18nStrings} + > + {item.data.content} + + )} + /> + ); +} + +// A board paired with a palette. Board and palette communicate through the shared d&d controller, +// so items can be inserted from the palette into this board — the legacy Board + ItemsPalette +// sibling pattern, shown here living alongside other boards on the same page. +function BoardWithPalette({ + initialBoardItems, + initialPaletteItems, +}: { + initialBoardItems: readonly BoardProps.Item[]; + initialPaletteItems: readonly ItemsPaletteProps.Item[]; +}) { + const [boardItems, setBoardItems] = useState(initialBoardItems); + const [paletteItems, setPaletteItems] = useState(initialPaletteItems); + return ( + + { + setBoardItems(items); + if (addedItem) { + setPaletteItems((prev) => prev.filter((item) => item.id !== addedItem.id)); + } + if (removedItem) { + setPaletteItems((prev) => [...prev, removedItem].sort((a, b) => a.data.title.localeCompare(b.data.title))); + } + }} + renderItem={(item, actions) => ( + {item.data.title}} + settings={ + actions.removeItem()} + expandToViewport={true} + /> + } + i18nStrings={boardItemI18nStrings} + > + {item.data.content} + + )} + /> +
Add widgets
+ { + const widgetConfig = demoWidgets[item.id]!.data; + return ( + {widgetConfig.title}} i18nStrings={boardItemI18nStrings}> + {context.showPreview ? `(preview) ${widgetConfig.description}` : widgetConfig.description} + + ); + }} + /> +
+ ); +} + +const boardOneItems: readonly BoardProps.Item[] = [ + { id: "1-1", data: { title: "Board 1 · Widget A", description: "", content: "Content A" } }, + { id: "1-2", data: { title: "Board 1 · Widget B", description: "", content: "Content B" } }, + { id: "1-3", data: { title: "Board 1 · Widget C", description: "", content: "Content C" } }, +]; + +const boardTwoItems: readonly BoardProps.Item[] = [ + { id: "2-1", data: { title: "Board 2 · Widget X", description: "", content: "Content X" } }, + { id: "2-2", data: { title: "Board 2 · Widget Y", description: "", content: "Content Y" } }, +]; + +const paletteBoardItems: readonly BoardProps.Item[] = Object.entries(demoWidgets) + .slice(0, 2) + .map(([id, widget]) => ({ id, definition: widget!.definition, data: widget!.data })); + +const paletteItems: readonly ItemsPaletteProps.Item[] = Object.entries(demoWidgets) + .slice(2, 5) + .map(([id, widget]) => ({ id, definition: widget!.definition, data: widget!.data })); + +export default function MultipleBoardsPage() { + return ( + + Multiple boards on the same page}> + +
+
Board 1 (independent)
+ +
+ +
+
Board 2 (independent)
+ +
+ +
+
Board 3 with a palette
+ +
+
+
+
+ ); +} diff --git a/src/board/__tests__/board-acquisition.test.tsx b/src/board/__tests__/board-acquisition.test.tsx index 4051afef..3e1db534 100644 --- a/src/board/__tests__/board-acquisition.test.tsx +++ b/src/board/__tests__/board-acquisition.test.tsx @@ -1,11 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 import { useState } from "react"; -import { act, render, screen } from "@testing-library/react"; -import { expect, test, vi } from "vitest"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import { afterEach, expect, test, vi } from "vitest"; import { Board, BoardProps } from "../../../lib/components"; -import { mockController } from "../../../lib/components/internal/dnd-controller/__mocks__/controller"; +import { mockController, mockDroppables } from "../../../lib/components/internal/dnd-controller/__mocks__/controller"; import { DragAndDropData } from "../../../lib/components/internal/dnd-controller/controller"; import { Coordinates } from "../../../lib/components/internal/utils/coordinates"; import createWrapper from "../../../lib/components/test-utils/dom"; @@ -13,6 +13,20 @@ import { defaultProps } from "./utils"; vi.mock("../../../lib/components/internal/dnd-controller/controller"); +// Placeholder droppable IDs are scoped with a runtime-generated boardId +// (`awsui-placeholder---`), so resolve the target placeholder by its row/col +// suffix instead of hardcoding the full ID. +afterEach(cleanup); + +function getPlaceholderId(row: number, col: number): string { + const suffix = `-${row}-${col}`; + const id = [...mockDroppables].find((droppableId) => String(droppableId).endsWith(suffix)); + if (!id) { + throw new Error(`No placeholder droppable registered for row ${row}, col ${col}.`); + } + return String(id); +} + test("renders acquired item", () => { render(); expect(screen.queryByTestId("acquired-item")).toBeNull(); @@ -30,7 +44,7 @@ test("renders acquired item", () => { act(() => mockController.acquire({ - droppableId: "awsui-placeholder-1-0", + droppableId: getPlaceholderId(1, 0), draggableItem, renderAcquiredItem: () =>
, }), @@ -62,7 +76,7 @@ test("focuses on acquired item's drag handle upon submission", () => { act(() => mockController.acquire({ - droppableId: "awsui-placeholder-1-0", + droppableId: getPlaceholderId(1, 0), draggableItem, renderAcquiredItem: () =>
, }), diff --git a/src/board/__tests__/multiple-boards.test.tsx b/src/board/__tests__/multiple-boards.test.tsx new file mode 100644 index 00000000..fb9ba7bf --- /dev/null +++ b/src/board/__tests__/multiple-boards.test.tsx @@ -0,0 +1,159 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { useState } from "react"; +import { cleanup, render, waitFor } from "@testing-library/react"; +import { afterEach, beforeAll, describe, expect, test, vi } from "vitest"; + +import { KeyCode } from "@cloudscape-design/test-utils-core/utils"; + +import Board from "../../../lib/components/board"; +import { BoardProps } from "../../../lib/components/board"; +import createWrapper, { BoardWrapper } from "../../../lib/components/test-utils/dom"; +import { defaultProps } from "./utils"; + +// These tests exercise two boards rendered on the same page. They use the real (non-mocked) d&d +// controller so that cross-board isolation is validated end to end: both boards subscribe to the +// same singleton controller, so a drag started in one board must not disturb the other. + +describe("Multiple boards on the same page", () => { + beforeAll(() => { + // jsdom does not support this function. + document.elementFromPoint = () => null; + }); + + afterEach(() => { + cleanup(); + }); + + function TwoBoards({ + onItemsChangeA, + onItemsChangeB, + }: { + onItemsChangeA?: BoardProps<{ title: string }>["onItemsChange"]; + onItemsChangeB?: BoardProps<{ title: string }>["onItemsChange"]; + }) { + const [itemsA, setItemsA] = useState(defaultProps.items); + const [itemsB, setItemsB] = useState[]>([ + { id: "b1", data: { title: "Item B1" } }, + { id: "b2", data: { title: "Item B2" } }, + ]); + return ( + <> +
+ { + setItemsA(event.detail.items); + onItemsChangeA?.(event); + }} + /> +
+
+ { + setItemsB(event.detail.items); + onItemsChangeB?.(event); + }} + /> +
+ + ); + } + + function boardA(): BoardWrapper { + return createWrapper(document.querySelector('[data-testid="board-a"]')!).findBoard()!; + } + function boardB(): BoardWrapper { + return createWrapper(document.querySelector('[data-testid="board-b"]')!).findBoard()!; + } + + test("renders two independent boards", () => { + render(); + + expect(boardA().findItemById("1")!.getElement().textContent).toContain("Item 1"); + expect(boardB().findItemById("b1")!.getElement().textContent).toContain("Item B1"); + }); + + test("keyboard reorder in one board does not affect the other", () => { + const onItemsChangeA = vi.fn(); + const onItemsChangeB = vi.fn(); + render(); + + // Reorder the first item down within board A. + const dragHandle = boardA().findItemById("1")!.findDragHandle(); + dragHandle.keydown(KeyCode.enter); + dragHandle.keydown(KeyCode.down); + dragHandle.keydown(KeyCode.down); + dragHandle.keydown(KeyCode.enter); + + // Board A committed a reorder. + expect(onItemsChangeA).toHaveBeenCalledWith( + expect.objectContaining({ + detail: expect.objectContaining({ + movedItem: expect.objectContaining({ id: "1" }), + items: [expect.objectContaining({ id: "2" }), expect.objectContaining({ id: "1" })], + }), + }), + ); + + // Board B was never disturbed. + expect(onItemsChangeB).not.toHaveBeenCalled(); + }); + + test("keyboard resize in one board does not affect the other", () => { + const onItemsChangeA = vi.fn(); + const onItemsChangeB = vi.fn(); + render(); + + const resizeHandle = boardA().findItemById("1")!.findResizeHandle()!; + resizeHandle.keydown(KeyCode.enter); + resizeHandle.keydown(KeyCode.down); + resizeHandle.keydown(KeyCode.down); + resizeHandle.keydown(KeyCode.enter); + + expect(onItemsChangeA).toHaveBeenCalledWith( + expect.objectContaining({ + detail: expect.objectContaining({ resizedItem: expect.objectContaining({ id: "1" }) }), + }), + ); + expect(onItemsChangeB).not.toHaveBeenCalled(); + }); + + test("removing an item from one board does not affect the other", async () => { + const onItemsChangeA = vi.fn(); + const onItemsChangeB = vi.fn(); + render(); + + const removeButton = boardA().findItemById("1")!.findSettings()!.find('[data-testid="remove-button"]')!; + removeButton.click(); + + await waitFor(() => + expect(onItemsChangeA).toHaveBeenCalledWith( + expect.objectContaining({ + detail: expect.objectContaining({ removedItem: expect.objectContaining({ id: "1" }) }), + }), + ), + ); + expect(onItemsChangeB).not.toHaveBeenCalled(); + }); + + test("each board keeps its own items after an interaction", () => { + render(); + + const dragHandle = boardA().findItemById("1")!.findDragHandle(); + dragHandle.keydown(KeyCode.enter); + dragHandle.keydown(KeyCode.down); + dragHandle.keydown(KeyCode.down); + dragHandle.keydown(KeyCode.enter); + + // Board B still renders exactly its own items and none from board A. + // Note: findItemById searches the whole document, so scope the lookup to board B's element. + expect(boardB().find('[data-item-id="b1"]')).not.toBeNull(); + expect(boardB().find('[data-item-id="b2"]')).not.toBeNull(); + expect(boardB().find('[data-item-id="1"]')).toBeNull(); + expect(boardB().find('[data-item-id="2"]')).toBeNull(); + }); +}); diff --git a/src/board/interfaces.ts b/src/board/interfaces.ts index cebdb743..12d990ca 100644 --- a/src/board/interfaces.ts +++ b/src/board/interfaces.ts @@ -161,6 +161,9 @@ export namespace BoardProps { export interface Transition { operation: Operation; interactionType: InteractionType; + // Scopes placeholder droppable IDs to the owning board so that boards sharing a d&d controller + // do not register colliding droppables. Matches the boardId used when rendering placeholders. + boardId: string; itemsLayout: GridLayout; layoutEngine: LayoutEngine; insertionDirection: null | Direction; diff --git a/src/board/internal.tsx b/src/board/internal.tsx index bd3e68da..b953c2cb 100644 --- a/src/board/internal.tsx +++ b/src/board/internal.tsx @@ -1,6 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { ReactNode, useEffect, useRef } from "react"; +import { ReactNode, useEffect, useId, useRef } from "react"; import { usePrevious } from "@dnd-kit/utilities"; import clsx from "clsx"; @@ -49,6 +49,10 @@ export function InternalBoard({ const containerRef = useMergeRefs(containerAccessRef, containerQueryRef); const itemContainerRef = useRef<{ [id: ItemId]: ItemContainerRef }>({}); + // Scopes this board's placeholder droppable IDs so that multiple boards sharing a single d&d + // controller do not register colliding droppables. See createPlaceholdersLayout. + const boardId = useId(); + const isRtl = () => getIsRtl(containerAccessRef.current); useGlobalDragStateStyles(); @@ -125,7 +129,16 @@ export function InternalBoard({ }, [acquiredItemId, previousAcquiredItemElement, acquiredItemElement]); const rows = selectTransitionRows(transitionState) || itemsLayout.rows; - const placeholdersLayout = createPlaceholdersLayout(rows, itemsLayout.columns); + const placeholdersLayout = createPlaceholdersLayout(rows, itemsLayout.columns, boardId); + + // The set of placeholder droppable IDs owned by this board. All boards share one d&d controller, + // which computes collisions against every registered droppable on the page, so a pointer rect + // straddling two adjacent boards can surface a neighbor's placeholder IDs here. Restricting the + // collision IDs to this board's own placeholders keeps each board's transition state (hovered + // cells, collision counts, resulting layout shift) driven only by its own grid. + const ownPlaceholderIds = new Set(placeholdersLayout.items.map((placeholder) => placeholder.id)); + const filterOwnCollisions = (collisionIds: readonly ItemId[]) => + collisionIds.filter((id) => ownPlaceholderIds.has(id)); function isElementOverBoard(rect: Rect) { const board = containerAccessRef.current!; @@ -142,17 +155,29 @@ export function InternalBoard({ } useDragSubscription("start", ({ operation, interactionType, draggableItem, collisionRect, collisionIds }) => { + // The d&d controller broadcasts events to every board subscribed to it (multiple boards can + // share a controller). Reorder and resize operations concern a single, already-placed item, so + // only the board that owns that item should react. Insert operations originate from a palette + // and can target any board, so they are not filtered here (the drop target is resolved via + // collisions / placeholder ownership instead). + const ownsDraggable = itemsLayout.items.some((it) => it.id === draggableItem.id); + if (operation !== "insert" && !ownsDraggable) { + return; + } + dispatch({ type: "init", operation, interactionType, + boardId, itemsLayout, // TODO: resolve any // The code only works assuming the board can take any draggable. // If draggables can be of different types a check of some sort is required here. draggableItem: draggableItem as BoardItemDefinitionBase, draggableRect: collisionRect, - collisionIds: interactionType === "pointer" && isElementOverBoard(collisionRect) ? collisionIds : [], + collisionIds: + interactionType === "pointer" && isElementOverBoard(collisionRect) ? filterOwnCollisions(collisionIds) : [], }); autoScrollHandlers.run(); @@ -161,7 +186,8 @@ export function InternalBoard({ useDragSubscription("update", ({ interactionType, collisionIds, positionOffset, collisionRect }) => { dispatch({ type: "update-with-pointer", - collisionIds: interactionType === "pointer" && isElementOverBoard(collisionRect) ? collisionIds : [], + collisionIds: + interactionType === "pointer" && isElementOverBoard(collisionRect) ? filterOwnCollisions(collisionIds) : [], positionOffset, draggableRect: collisionRect, }); diff --git a/src/board/transition.ts b/src/board/transition.ts index 813ec6c3..6058e798 100644 --- a/src/board/transition.ts +++ b/src/board/transition.ts @@ -35,6 +35,7 @@ interface InitAction { type: "init"; operation: Operation; interactionType: InteractionType; + boardId: string; itemsLayout: GridLayout; draggableItem: BoardItemDefinitionBase; draggableRect: Rect; @@ -105,6 +106,7 @@ function createTransitionReducer({ isRtl }: { isRtl: () => boolean }) { function initTransition({ operation, interactionType, + boardId, itemsLayout, draggableItem, draggableRect, @@ -113,6 +115,7 @@ function initTransition({ const transition: Transition = { operation, interactionType, + boardId, itemsLayout, layoutEngine: new LayoutEngine(itemsLayout), insertionDirection: null, diff --git a/src/board/utils/__tests__/get-hovered-rect.test.ts b/src/board/utils/__tests__/get-hovered-rect.test.ts new file mode 100644 index 00000000..012db390 --- /dev/null +++ b/src/board/utils/__tests__/get-hovered-rect.test.ts @@ -0,0 +1,41 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, test } from "vitest"; + +import { GridLayoutItem } from "../../../internal/interfaces"; +import { getHoveredRect } from "../get-hovered-rect"; + +const placeholders: GridLayoutItem[] = [ + { id: "awsui-placeholder-board-a-0-0", x: 0, y: 0, width: 1, height: 1 }, + { id: "awsui-placeholder-board-a-0-1", x: 1, y: 0, width: 1, height: 1 }, + { id: "awsui-placeholder-board-a-1-0", x: 0, y: 1, width: 1, height: 1 }, + { id: "awsui-placeholder-board-a-1-1", x: 1, y: 1, width: 1, height: 1 }, +]; + +describe("getHoveredRect", () => { + test("computes the bounding rect of the collided placeholders", () => { + const rect = getHoveredRect(["awsui-placeholder-board-a-0-0", "awsui-placeholder-board-a-1-1"], placeholders); + expect(rect).toEqual({ top: 0, left: 0, bottom: 2, right: 2 }); + }); + + // Defensive: an ID that is not in the placeholder set must be skipped, not dereferenced. A raw + // `.find(...)!` here would return `undefined` and throw in the reduce below. This mirrors the + // multi-board case where a stray collision ID from another board could reach this function. + test("ignores collision ids that do not belong to the given placeholders", () => { + const rect = getHoveredRect( + ["awsui-placeholder-board-a-0-0", "awsui-placeholder-board-b-3-3", "awsui-placeholder-board-a-0-1"], + placeholders, + ); + expect(rect).toEqual({ top: 0, left: 0, bottom: 1, right: 2 }); + }); + + test("returns an empty (inverted) rect when no collision id matches", () => { + const rect = getHoveredRect(["awsui-placeholder-board-b-0-0"], placeholders); + expect(rect).toEqual({ + top: Number.POSITIVE_INFINITY, + left: Number.POSITIVE_INFINITY, + bottom: Number.NEGATIVE_INFINITY, + right: Number.NEGATIVE_INFINITY, + }); + }); +}); diff --git a/src/board/utils/__tests__/layout.test.ts b/src/board/utils/__tests__/layout.test.ts index c2ee0b67..cf6a3eec 100644 --- a/src/board/utils/__tests__/layout.test.ts +++ b/src/board/utils/__tests__/layout.test.ts @@ -19,6 +19,7 @@ function createMockTransition( operation, acquiredItem: null, interactionType: "keyboard", + boardId: "test-board", itemsLayout, layoutEngine: new LayoutEngine(itemsLayout), insertionDirection: null, diff --git a/src/board/utils/get-hovered-rect.ts b/src/board/utils/get-hovered-rect.ts index 4a87f949..bfd473e1 100644 --- a/src/board/utils/get-hovered-rect.ts +++ b/src/board/utils/get-hovered-rect.ts @@ -7,7 +7,12 @@ import { GridLayoutItem, ItemId } from "../../internal/interfaces"; * Creates a minimal hovered rectangle (in grid units) that contains all collided placeholders. */ export function getHoveredRect(collisionsIds: readonly ItemId[], placeholders: readonly GridLayoutItem[]) { - const hoveredPlaceholders = collisionsIds.map((id) => placeholders.find((p) => p.id === id)!); + // Only consider collision IDs that belong to the given placeholder set. Callers already scope + // collisions to the owning board, so this is defensive: skip any unknown ID rather than + // dereferencing a missing placeholder (which would throw). + const hoveredPlaceholders = collisionsIds + .map((id) => placeholders.find((p) => p.id === id)) + .filter((placeholder): placeholder is GridLayoutItem => !!placeholder); return hoveredPlaceholders.reduce( (rect, collision) => ({ top: Math.min(rect.top, collision.y), diff --git a/src/board/utils/layout.ts b/src/board/utils/layout.ts index 96286ea3..61fe9dd2 100644 --- a/src/board/utils/layout.ts +++ b/src/board/utils/layout.ts @@ -32,7 +32,7 @@ export function getLayoutRows(transition: Transition) { export function getLayoutPlaceholders(transition: Transition) { const rows = getLayoutRows(transition); const columns = getLayoutColumns(transition); - return createPlaceholdersLayout(rows, columns); + return createPlaceholdersLayout(rows, columns, transition.boardId); } /** diff --git a/src/internal/dnd-controller/__mocks__/controller.ts b/src/internal/dnd-controller/__mocks__/controller.ts index 9c18a308..addf4ecc 100644 --- a/src/internal/dnd-controller/__mocks__/controller.ts +++ b/src/internal/dnd-controller/__mocks__/controller.ts @@ -4,6 +4,7 @@ import { useEffect } from "react"; import { vi } from "vitest"; +import { ItemId } from "../../interfaces"; import { AcquireData, DragAndDropData, DragAndDropEvents } from "../controller"; import { EventEmitter } from "../event-emitter"; @@ -31,6 +32,11 @@ class MockController extends EventEmitter { export const mockController = new MockController(); +// Records droppable IDs registered via useDroppable. Because placeholder IDs are scoped per board +// with a runtime-generated boardId, tests can use this to resolve the actual scoped ID instead of +// hardcoding it. Reset it between tests when needed. +export const mockDroppables = new Set(); + export function useDragSubscription(event: K, handler: DragAndDropEvents[K]) { useEffect(() => mockController.on(event, handler), [event, handler]); } @@ -47,4 +53,11 @@ export function useDraggable() { return mockDraggable; } -export function useDroppable() {} +export function useDroppable({ itemId }: { itemId: ItemId }) { + useEffect(() => { + mockDroppables.add(itemId); + return () => { + mockDroppables.delete(itemId); + }; + }, [itemId]); +} diff --git a/src/internal/dnd-controller/__tests__/controller.test.tsx b/src/internal/dnd-controller/__tests__/controller.test.tsx new file mode 100644 index 00000000..95344b8b --- /dev/null +++ b/src/internal/dnd-controller/__tests__/controller.test.tsx @@ -0,0 +1,127 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { StrictMode, useRef } from "react"; +import { cleanup, render } from "@testing-library/react"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + useDraggable, + useDragSubscription, + useDroppable, +} from "../../../../lib/components/internal/dnd-controller/controller"; +import { BoardItemDefinitionBase, Rect } from "../../../../lib/components/internal/interfaces"; +import { Coordinates } from "../../../../lib/components/internal/utils/coordinates"; + +afterEach(cleanup); + +const draggableItem: BoardItemDefinitionBase = { id: "draggable", data: {}, definition: {} }; +// An empty collision rect keeps collision detection a no-op so the tests focus on subscription and +// droppable registration rather than the collision math. +const collisionRect: Rect = { top: 0, bottom: 0, left: 0, right: 0 }; + +// Registers a droppable and exposes a way to read the droppables visible to a draggable. The d&d +// controller is a page-global singleton, so every actor rendered in a test shares it. +function DndActor({ + droppableId, + onStart, + exposeDroppables, +}: { + droppableId: string; + onStart?: () => void; + exposeDroppables?: (ids: string[]) => void; +}) { + const elementRef = useRef(null); + useDroppable({ + itemId: droppableId, + context: { scale: () => ({ width: 1, height: 1 }) }, + getElement: () => elementRef.current!, + }); + + const draggable = useDraggable({ draggableItem, getCollisionRect: () => collisionRect }); + + useDragSubscription("start", () => { + onStart?.(); + exposeDroppables?.(draggable.getDroppables().map(([id]) => String(id))); + }); + + return ( +
+
+ +
+ ); +} + +describe("shared controller", () => { + test("all components share the singleton controller and receive broadcasts", () => { + const startA = vi.fn(); + const startB = vi.fn(); + let droppablesSeenByA: string[] = []; + + render( + <> + (droppablesSeenByA = ids.sort())} /> + + , + ); + + (document.querySelector('[data-testid="start-a"]') as HTMLButtonElement).click(); + + // The singleton controller broadcasts to every subscriber, which preserves the legacy + // Board + ItemsPalette sibling behavior. Isolation between multiple boards is achieved at the + // board level (scoped placeholder ids + event/collision filtering), not by separate controllers. + expect(startA).toHaveBeenCalledTimes(1); + expect(startB).toHaveBeenCalledTimes(1); + + // The draggable sees both droppables registered in the shared controller. + expect(droppablesSeenByA).toEqual(["a", "b"]); + }); +}); + +describe("droppable lifecycle", () => { + test("unmounting a component unregisters its droppable", () => { + let droppablesSeenByA: string[] = []; + + function App({ showB }: { showB: boolean }) { + return ( + <> + (droppablesSeenByA = ids.sort())} /> + {showB ? : null} + + ); + } + + const { rerender } = render(); + + // Both droppables are registered. + (document.querySelector('[data-testid="start-a"]') as HTMLButtonElement).click(); + expect(droppablesSeenByA).toEqual(["a", "b"]); + + // Unmount the "b" subtree; its droppable must be cleaned up (no stale/leaked entry). + rerender(); + (document.querySelector('[data-testid="start-a"]') as HTMLButtonElement).click(); + expect(droppablesSeenByA).toEqual(["a"]); + }); + + test("under StrictMode double-mount, droppables are not duplicated or stranded", () => { + let droppablesSeenByA: string[] = []; + + // StrictMode intentionally mounts, unmounts, and re-mounts effects in development. The + // addDroppable/removeDroppable effect must be balanced so a droppable is registered exactly + // once after the dust settles (a duplicate or a stranded entry would show up here). + render( + + (droppablesSeenByA = ids.sort())} /> + + , + ); + + (document.querySelector('[data-testid="start-a"]') as HTMLButtonElement).click(); + expect(droppablesSeenByA).toEqual(["a", "b"]); + }); +}); diff --git a/src/internal/dnd-controller/controller.ts b/src/internal/dnd-controller/controller.ts index 533b8750..fc848197 100644 --- a/src/internal/dnd-controller/controller.ts +++ b/src/internal/dnd-controller/controller.ts @@ -160,7 +160,9 @@ class DragAndDropController extends EventEmitter { } } -// Controller is a singleton and is shared between all d&d elements. +// Controller is a singleton and is shared between all d&d elements. Multiple boards on the same +// page therefore share this controller; isolation between them is achieved by scoping placeholder +// droppable IDs per board and filtering events/collisions to the owning board (see InternalBoard). const controller = new DragAndDropController(); export function useDragSubscription(event: K, handler: DragAndDropEvents[K]) { diff --git a/src/internal/item-container/__tests__/get-next-droppable.test.ts b/src/internal/item-container/__tests__/get-next-droppable.test.ts index 77021682..a2b6e40e 100644 --- a/src/internal/item-container/__tests__/get-next-droppable.test.ts +++ b/src/internal/item-container/__tests__/get-next-droppable.test.ts @@ -45,3 +45,39 @@ test("returns next droppable matching the direction", () => { }); expect(next).toBe("2"); }); + +test("only considers droppables from the provided (single board) set", () => { + // When multiple boards share a controller, each board's ItemContainer keyboard insertion only + // walks the droppables it can see. This test simulates that by passing the droppables of a single + // board even though another board's droppable sits closer in the same direction. + const elementMock = getMockElement({ left: 6, right: 4, top: 0, bottom: 0 }); + + const closerOtherBoardDroppable: [string, Droppable] = [ + "other-board-placeholder", + { element: getMockElement({ left: 5, right: 5, top: 0, bottom: 0 }) } as Droppable, + ]; + const sameBoardDroppable: [string, Droppable] = [ + "same-board-placeholder", + { element: getMockElement({ left: 20, right: 30, top: 0, bottom: 0 }) } as Droppable, + ]; + + // Only the same-board droppable is provided, so it must be chosen even though another board's + // droppable would have been closer had it been in scope. + const next = getNextDroppable({ + draggableElement: elementMock, + droppables: [sameBoardDroppable], + direction: "right", + isRtl: false, + }); + expect(next).toBe("same-board-placeholder"); + + // Sanity check: when the other board's droppable IS in scope it wins, confirming the previous + // result was due to scoping and not distance. + const nextWithBoth = getNextDroppable({ + draggableElement: elementMock, + droppables: [sameBoardDroppable, closerOtherBoardDroppable], + direction: "right", + isRtl: false, + }); + expect(nextWithBoth).toBe("other-board-placeholder"); +}); diff --git a/src/internal/utils/__tests__/layout.test.ts b/src/internal/utils/__tests__/layout.test.ts index f14efeaa..28ba6133 100644 --- a/src/internal/utils/__tests__/layout.test.ts +++ b/src/internal/utils/__tests__/layout.test.ts @@ -203,14 +203,20 @@ describe("item setting getters", () => { }); describe("createPlaceholdersLayout", () => { - test("Creates placeholder layout for the given rows, cols", () => { - const layout = createPlaceholdersLayout(3, 2); + test("Creates placeholder layout for the given rows, cols, scoped to the boardId", () => { + const layout = createPlaceholdersLayout(3, 2, "board42"); expect(toString(layout)).toBe( toString([ - ["awsui-placeholder-0-0", "awsui-placeholder-0-1"], - ["awsui-placeholder-1-0", "awsui-placeholder-1-1"], - ["awsui-placeholder-2-0", "awsui-placeholder-2-1"], + ["awsui-placeholder-board42-0-0", "awsui-placeholder-board42-0-1"], + ["awsui-placeholder-board42-1-0", "awsui-placeholder-board42-1-1"], + ["awsui-placeholder-board42-2-0", "awsui-placeholder-board42-2-1"], ]), ); }); + + test("produces disjoint id sets for different boardIds", () => { + const idsA = createPlaceholdersLayout(2, 2, "a").items.map((item) => item.id); + const idsB = createPlaceholdersLayout(2, 2, "b").items.map((item) => item.id); + expect(idsA.some((id) => idsB.includes(id))).toBe(false); + }); }); diff --git a/src/internal/utils/layout.ts b/src/internal/utils/layout.ts index b0fbdb2b..6256e0d2 100644 --- a/src/internal/utils/layout.ts +++ b/src/internal/utils/layout.ts @@ -121,12 +121,19 @@ export function transformItems( return items; } -export function createPlaceholdersLayout(rows: number, columns: number): GridLayout { +/** + * Produces the grid of placeholder drop targets for a board. + * + * The `boardId` scopes the generated placeholder IDs to a single board. All boards on a page share + * one d&d controller, so without scoping every board would register colliding placeholder IDs + * (`awsui-placeholder-0-0`, ...) into the same droppables map and clobber each other. + */ +export function createPlaceholdersLayout(rows: number, columns: number, boardId: string): GridLayout { const layoutItems: GridLayoutItem[] = []; for (let row = 0; row < rows; row++) { for (let col = 0; col < columns; col++) { - layoutItems.push({ id: `awsui-placeholder-${row}-${col}`, x: col, y: row, width: 1, height: 1 }); + layoutItems.push({ id: `awsui-placeholder-${boardId}-${row}-${col}`, x: col, y: row, width: 1, height: 1 }); } } diff --git a/test/functional/board-layout/multiple-boards.test.ts b/test/functional/board-layout/multiple-boards.test.ts new file mode 100644 index 00000000..917a17d6 --- /dev/null +++ b/test/functional/board-layout/multiple-boards.test.ts @@ -0,0 +1,88 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { expect, test } from "vitest"; + +import createWrapper from "../../../lib/components/test-utils/selectors"; +import { setupTest } from "../../utils"; +import { DndPageObject } from "./dnd-page-object"; + +// Scope wrappers to each board via the test-id containers on the page. +const boardA = createWrapper('[data-testid="board-a"]').findBoard(); +const boardB = createWrapper('[data-testid="board-b"]').findBoard(); + +class MultiBoardPageObject extends DndPageObject { + getElementsAttributes(selector: string, attribute: string) { + return this.browser.execute( + (selector, attribute) => + [...document.querySelectorAll(selector)].map((element) => element.getAttribute(attribute)), + selector, + attribute, + ); + } +} + +function itemIds(page: MultiBoardPageObject, boardTestId: string) { + return page.getElementsAttributes(`[data-testid="${boardTestId}"] [data-item-id]`, "data-item-id"); +} + +test( + "pointer reorder in one board does not affect the other", + setupTest("/index.html#/dnd/multiple-boards-test", MultiBoardPageObject, async (page) => { + // Sanity: each board renders its own disjoint items. + await expect(itemIds(page, "board-a")).resolves.toEqual(["A", "B", "C", "D"]); + await expect(itemIds(page, "board-b")).resolves.toEqual(["E", "F", "G", "H"]); + + // Reorder within board A: drag A onto B. + await page.dragAndDropTo( + boardA.findItemById("A").findDragHandle().toSelector(), + boardA.findItemById("B").findDragHandle().toSelector(), + ); + + // Board A reordered; board B is untouched. + await expect(itemIds(page, "board-a")).resolves.toEqual(["B", "A", "C", "D"]); + await expect(itemIds(page, "board-b")).resolves.toEqual(["E", "F", "G", "H"]); + }), +); + +test( + "pointer resize in one board does not affect the other", + setupTest("/index.html#/dnd/multiple-boards-test", MultiBoardPageObject, async (page) => { + const before = await itemIds(page, "board-b"); + + await page.dragAndDropTo( + boardA.findItemById("A").findResizeHandle().toSelector(), + boardA.findItemById("B").findResizeHandle().toSelector(), + ); + + // Board B keeps the same items in the same order. + await expect(itemIds(page, "board-b")).resolves.toEqual(before); + }), +); + +test( + "dragging an item from one board over another board does not crash or move items into it", + setupTest("/index.html#/dnd/multiple-boards-test", MultiBoardPageObject, async (page) => { + await expect(itemIds(page, "board-a")).resolves.toEqual(["A", "B", "C", "D"]); + await expect(itemIds(page, "board-b")).resolves.toEqual(["E", "F", "G", "H"]); + + // Drag an item from board A and release it over an item that lives in board B. Because all + // boards share one d&d controller, the drag rect crosses into board B's droppable + // region. This used to crash: board A received board B's placeholder collision ids and + // dereferenced them in getHoveredRect / appendPath. The regression guards ensure that instead: + // - the page does not crash, + // - no item ever crosses into board B, and + // - board B keeps its exact items and order. + // (Board A may legitimately reorder its own items, since the drag passes over board A's own + // tiles on the way out; that is not what this test asserts.) + await page.dragAndDropTo( + boardA.findItemById("A").findDragHandle().toSelector(), + boardB.findItemById("E").findDragHandle().toSelector(), + ); + + // Board B is untouched: same items, same order, and it never gained board A's item. + await expect(itemIds(page, "board-b")).resolves.toEqual(["E", "F", "G", "H"]); + + // Board A still owns exactly its own four items (order may have changed), and none leaked to B. + await expect(itemIds(page, "board-a").then((ids) => [...ids].sort())).resolves.toEqual(["A", "B", "C", "D"]); + }), +); From 488c826b17ed1ecbd28ad0f3da6ba04e103cdf27 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Fri, 3 Jul 2026 15:12:47 +0200 Subject: [PATCH 2/6] chore: Additional u tests --- .../__tests__/board-acquisition.test.tsx | 84 ++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/src/board/__tests__/board-acquisition.test.tsx b/src/board/__tests__/board-acquisition.test.tsx index 3e1db534..bb338540 100644 --- a/src/board/__tests__/board-acquisition.test.tsx +++ b/src/board/__tests__/board-acquisition.test.tsx @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { useState } from "react"; import { act, cleanup, render, screen } from "@testing-library/react"; -import { afterEach, expect, test, vi } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, test, vi } from "vitest"; import { Board, BoardProps } from "../../../lib/components"; import { mockController, mockDroppables } from "../../../lib/components/internal/dnd-controller/__mocks__/controller"; @@ -11,6 +11,8 @@ import { Coordinates } from "../../../lib/components/internal/utils/coordinates" import createWrapper from "../../../lib/components/test-utils/dom"; import { defaultProps } from "./utils"; +import boardStyles from "../../../lib/components/board/styles.css.js"; + vi.mock("../../../lib/components/internal/dnd-controller/controller"); // Placeholder droppable IDs are scoped with a runtime-generated boardId @@ -55,6 +57,31 @@ test("renders acquired item", () => { expect(screen.queryByTestId("acquired-item")).toBeNull(); }); +test("ignores acquire for a droppable that belongs to another board", () => { + render(); + const draggableItem = { id: "test", data: { title: "Test item" }, definition: {} }; + + act(() => + mockController.start({ + interactionType: "keyboard", + operation: "insert", + draggableItem, + collisionRect: { top: 0, bottom: 0, left: 0, right: 0 }, + coordinates: new Coordinates({ x: 0, y: 0 }), + } as DragAndDropData), + ); + + act(() => + mockController.acquire({ + droppableId: "awsui-placeholder-other-board-1-0", + draggableItem, + renderAcquiredItem: () =>
, + }), + ); + + expect(screen.queryByTestId("acquired-item")).toBeNull(); +}); + function StatefulBoard(props: BoardProps<{ title: string }>) { const [items, setItems] = useState(props.items); return setItems(detail.items)} />; @@ -85,3 +112,58 @@ test("focuses on acquired item's drag handle upon submission", () => { act(() => mockController.submit()); expect(createWrapper().findBoard()!.findItemById("test")!.findDragHandle().getElement()).toHaveFocus(); }); + +describe("pointer collision scoping", () => { + // isElementOverBoard relies on elementFromPoint; jsdom has no layout, so point it at the board. + let boardElement: Element | null = null; + beforeAll(() => { + document.elementFromPoint = () => boardElement; + }); + afterAll(() => { + boardElement = null; + }); + + function hoveredPlaceholderCount() { + return document.querySelectorAll(`.${boardStyles["placeholder--hover"]}`).length; + } + + const draggableItem = { id: "1", data: { title: "Item 1" }, definition: {} }; + const zeroRect = { top: 0, bottom: 0, left: 0, right: 0 }; + + function pointerReorder(collisionIds: string[]) { + act(() => + mockController.start({ + interactionType: "pointer", + operation: "reorder", + draggableItem, + collisionRect: zeroRect, + coordinates: new Coordinates({ x: 0, y: 0 }), + collisionIds: [], + positionOffset: new Coordinates({ x: 0, y: 0 }), + dropTarget: null, + } as unknown as DragAndDropData), + ); + act(() => + mockController.update({ + interactionType: "pointer", + operation: "reorder", + draggableItem, + collisionRect: zeroRect, + coordinates: new Coordinates({ x: 0, y: 0 }), + positionOffset: new Coordinates({ x: 0, y: 0 }), + dropTarget: null, + collisionIds, + } as unknown as DragAndDropData), + ); + } + + test("ignores pointer collision ids that belong to another board", () => { + const { container } = render(); + boardElement = container.querySelector(`.${boardStyles.root}`); + + pointerReorder(["awsui-placeholder-other-board-0-0"]); + + // The foreign id was filtered out, so this board highlights nothing and does not crash. + expect(hoveredPlaceholderCount()).toBe(0); + }); +}); From 53782f466b916f18304902c6085267ff7a0bca16 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Mon, 6 Jul 2026 11:18:23 +0200 Subject: [PATCH 3/6] fix: Infitine loop issues while moving board items from an item palette to a board --- .../__tests__/board-acquisition.test.tsx | 25 ++++++++-- src/board/transition.ts | 22 +++++++- .../utils/__tests__/get-hovered-rect.test.ts | 19 +++---- src/board/utils/get-hovered-rect.ts | 18 +++++-- .../__tests__/item-container.test.tsx | 50 +++++++++++++++++++ src/internal/item-container/index.tsx | 12 +++++ 6 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/board/__tests__/board-acquisition.test.tsx b/src/board/__tests__/board-acquisition.test.tsx index bb338540..4c32164e 100644 --- a/src/board/__tests__/board-acquisition.test.tsx +++ b/src/board/__tests__/board-acquisition.test.tsx @@ -130,7 +130,7 @@ describe("pointer collision scoping", () => { const draggableItem = { id: "1", data: { title: "Item 1" }, definition: {} }; const zeroRect = { top: 0, bottom: 0, left: 0, right: 0 }; - function pointerReorder(collisionIds: string[]) { + function startReorder(collisionIds: string[]) { act(() => mockController.start({ interactionType: "pointer", @@ -138,11 +138,14 @@ describe("pointer collision scoping", () => { draggableItem, collisionRect: zeroRect, coordinates: new Coordinates({ x: 0, y: 0 }), - collisionIds: [], + collisionIds, positionOffset: new Coordinates({ x: 0, y: 0 }), dropTarget: null, } as unknown as DragAndDropData), ); + } + + function updateReorder(collisionIds: string[]) { act(() => mockController.update({ interactionType: "pointer", @@ -161,9 +164,25 @@ describe("pointer collision scoping", () => { const { container } = render(); boardElement = container.querySelector(`.${boardStyles.root}`); - pointerReorder(["awsui-placeholder-other-board-0-0"]); + startReorder([]); + updateReorder(["awsui-placeholder-other-board-0-0"]); // The foreign id was filtered out, so this board highlights nothing and does not crash. expect(hoveredPlaceholderCount()).toBe(0); }); + + // Regression for the "infinite loop in appendPath" crash: collisions that do not map onto this + // board's placeholder grid must never seed or extend the transition path. Before the fix, an + // unmatched collision produced an Infinity rect that poisoned the path, so the next matching + // update looped forever. Feeding an unmatched collision first, then a matching one, must not throw. + test("does not crash when an unmatched collision precedes a matching one", () => { + const { container } = render(); + boardElement = container.querySelector(`.${boardStyles.root}`); + + expect(() => { + startReorder(["awsui-placeholder-other-board-0-0"]); + updateReorder(["awsui-placeholder-other-board-1-1"]); + updateReorder([getPlaceholderId(0, 0)]); + }).not.toThrow(); + }); }); diff --git a/src/board/transition.ts b/src/board/transition.ts index 6058e798..7d17d2ac 100644 --- a/src/board/transition.ts +++ b/src/board/transition.ts @@ -135,7 +135,9 @@ function initTransition({ if (interactionType === "pointer" || operation === "insert") { const collisionRect = getHoveredRect(collisionIds, placeholdersLayout.items); const appendPath = operation === "resize" ? appendResizePath : appendMovePath; - path = layoutItem ? appendPath([], collisionRect) : []; + // No collision rect means the reported collisions do not belong to this board's grid; start with + // an empty path rather than seeding it with an out-of-range position. + path = layoutItem && collisionRect ? appendPath([], collisionRect) : []; } else if (layoutItem) { path = operation === "resize" @@ -243,6 +245,24 @@ function updateTransitionWithPointerEvent( const placeholdersLayout = getLayoutPlaceholders(transition); const collisionRect = getHoveredRect(collisionIds, placeholdersLayout.items); + + // The collisions do not map onto this board's placeholder grid (e.g. they belong to another board + // sharing the controller, or are stale). Treat this like being out of boundaries instead of + // extending the path with an out-of-range position. + if (!collisionRect) { + return { + transition: { + ...transition, + draggableRect, + collisionIds: new Set(), + layoutShift: null, + insertionDirection: null, + }, + removeTransition: null, + announcement: null, + }; + } + const appendPath = transition.operation === "resize" ? appendResizePath : appendMovePath; const path = appendPath(transition.path, collisionRect); diff --git a/src/board/utils/__tests__/get-hovered-rect.test.ts b/src/board/utils/__tests__/get-hovered-rect.test.ts index 012db390..69577d1c 100644 --- a/src/board/utils/__tests__/get-hovered-rect.test.ts +++ b/src/board/utils/__tests__/get-hovered-rect.test.ts @@ -18,9 +18,8 @@ describe("getHoveredRect", () => { expect(rect).toEqual({ top: 0, left: 0, bottom: 2, right: 2 }); }); - // Defensive: an ID that is not in the placeholder set must be skipped, not dereferenced. A raw - // `.find(...)!` here would return `undefined` and throw in the reduce below. This mirrors the - // multi-board case where a stray collision ID from another board could reach this function. + // A stray ID that is not in the placeholder set (e.g. from another board sharing the controller) + // must be skipped, and the rect computed from the matching ones only. test("ignores collision ids that do not belong to the given placeholders", () => { const rect = getHoveredRect( ["awsui-placeholder-board-a-0-0", "awsui-placeholder-board-b-3-3", "awsui-placeholder-board-a-0-1"], @@ -29,13 +28,11 @@ describe("getHoveredRect", () => { expect(rect).toEqual({ top: 0, left: 0, bottom: 1, right: 2 }); }); - test("returns an empty (inverted) rect when no collision id matches", () => { - const rect = getHoveredRect(["awsui-placeholder-board-b-0-0"], placeholders); - expect(rect).toEqual({ - top: Number.POSITIVE_INFINITY, - left: Number.POSITIVE_INFINITY, - bottom: Number.NEGATIVE_INFINITY, - right: Number.NEGATIVE_INFINITY, - }); + // Regression: when NO collision id matches, the function must return null. Previously it returned + // an inverted `±Infinity` rect, which callers fed into appendPath, seeding the transition path + // with an out-of-range position and later throwing "infinite loop in appendPath". + test("returns null when no collision id matches", () => { + expect(getHoveredRect(["awsui-placeholder-board-b-0-0"], placeholders)).toBeNull(); + expect(getHoveredRect([], placeholders)).toBeNull(); }); }); diff --git a/src/board/utils/get-hovered-rect.ts b/src/board/utils/get-hovered-rect.ts index bfd473e1..5c81c442 100644 --- a/src/board/utils/get-hovered-rect.ts +++ b/src/board/utils/get-hovered-rect.ts @@ -1,18 +1,26 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { GridLayoutItem, ItemId } from "../../internal/interfaces"; +import { GridLayoutItem, ItemId, Rect } from "../../internal/interfaces"; /** * Creates a minimal hovered rectangle (in grid units) that contains all collided placeholders. + * + * Returns `null` when none of the collision IDs match the given placeholders. This happens when the + * reported collisions belong to a different board (multiple boards share one d&d controller) or are + * stale relative to the current placeholder grid. Callers must treat `null` as "no hovered cell" + * rather than extending the transition path — a fabricated rect here would seed the path with + * out-of-range coordinates and later break appendPath. */ -export function getHoveredRect(collisionsIds: readonly ItemId[], placeholders: readonly GridLayoutItem[]) { - // Only consider collision IDs that belong to the given placeholder set. Callers already scope - // collisions to the owning board, so this is defensive: skip any unknown ID rather than - // dereferencing a missing placeholder (which would throw). +export function getHoveredRect(collisionsIds: readonly ItemId[], placeholders: readonly GridLayoutItem[]): null | Rect { const hoveredPlaceholders = collisionsIds .map((id) => placeholders.find((p) => p.id === id)) .filter((placeholder): placeholder is GridLayoutItem => !!placeholder); + + if (hoveredPlaceholders.length === 0) { + return null; + } + return hoveredPlaceholders.reduce( (rect, collision) => ({ top: Math.min(rect.top, collision.y), diff --git a/src/internal/item-container/__tests__/item-container.test.tsx b/src/internal/item-container/__tests__/item-container.test.tsx index b006b0ff..4abfe360 100644 --- a/src/internal/item-container/__tests__/item-container.test.tsx +++ b/src/internal/item-container/__tests__/item-container.test.tsx @@ -160,3 +160,53 @@ test("does not renders in portal when item in reorder state by a pointer", () => }); expect(container).toContainElement(getByTestId("drag-handle")); }); + +// Regression: all boards and palettes share one d&d controller, so a palette item and the board +// item created by inserting it can transiently share an id. A palette item (placed=false) whose +// getItemSize throws without a drop context (as ItemsPalette does) must ignore a "resize" event — +// resize only ever targets the placed board item. Before the fix this crashed with +// "no drop context in palette". +describe("shared-controller cross-container events", () => { + const paletteLikeProps: ItemContainerProps = { + ...defaultProps, + placed: false, + getItemSize: (dropContext) => { + if (!dropContext) { + throw new Error("Invariant violation: no drop context in palette."); + } + return { width: 1, minWidth: 1, maxWidth: 1, height: 1, minHeight: 1, maxHeight: 1 }; + }, + }; + + test("a non-placed (palette) container ignores a resize for a same-id item", () => { + render(); + expect(() => + act(() => { + mockController.start({ + interactionType: "pointer", + operation: "resize", + draggableItem: paletteLikeProps.item, + collisionRect: { top: 0, bottom: 0, left: 0, right: 0 }, + coordinates: new Coordinates({ x: 0, y: 0 }), + } as DragAndDropData); + }), + ).not.toThrow(); + }); + + test("a placed (board) container ignores an insert for a same-id item", () => { + // Symmetric guard: an insert targets the non-placed source; the placed board item must not react + // (it would double-handle the same id). + render(); + expect(() => + act(() => { + mockController.start({ + interactionType: "pointer", + operation: "insert", + draggableItem: defaultProps.item, + collisionRect: { top: 0, bottom: 0, left: 0, right: 0 }, + coordinates: new Coordinates({ x: 0, y: 0 }), + } as DragAndDropData); + }), + ).not.toThrow(); + }); +}); diff --git a/src/internal/item-container/index.tsx b/src/internal/item-container/index.tsx index 8af6a1c4..6f3a4735 100644 --- a/src/internal/item-container/index.tsx +++ b/src/internal/item-container/index.tsx @@ -217,6 +217,18 @@ function ItemContainerComponent( dropTarget, }: DragAndDropData) { if (item.id === draggableItem.id) { + // All boards and palettes share a single d&d controller, so this container can receive events + // for an item that merely shares its id — e.g. a palette item and the board item created by + // inserting it coexist with the same id until the app removes the palette copy. The operation + // tells us whether the genuine drag subject is placed: "insert" targets a non-placed item + // (from a palette), while "reorder"/"resize" target a placed board item. If this container's + // placement does not match, it is not the real subject and must ignore the event. (Without + // this, a palette item would react to the board item's resize and crash in getItemSize.) + const isInsert = operation === "insert"; + if (isInsert === placed) { + return; + } + const [width, height] = [collisionRect.right - collisionRect.left, collisionRect.bottom - collisionRect.top]; const pointerOffset = pointerOffsetRef.current; From 11cf8c527720f512229c7e8af833fa1bbfcaae88 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Thu, 9 Jul 2026 16:37:46 +0200 Subject: [PATCH 4/6] fix: Test page behavior --- pages/dnd/multiple-boards.page.tsx | 51 +++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/pages/dnd/multiple-boards.page.tsx b/pages/dnd/multiple-boards.page.tsx index 085996e9..03ab6c81 100644 --- a/pages/dnd/multiple-boards.page.tsx +++ b/pages/dnd/multiple-boards.page.tsx @@ -15,20 +15,27 @@ import { ItemData } from "../shared/interfaces"; import { demoWidgets } from "./items"; // A small, self-contained board used multiple times on the same page. Each instance keeps its own -// items state and reacts only to its own drag-and-drop interactions. +// items state and reacts only to its own drag-and-drop interactions. Because all boards on the page +// share a single d&d controller, an item can be dragged here from the palette too — so we forward +// added/removed items to the shared palette sync to keep the palette consistent across all boards. function DemoBoard({ boardLabel, initialItems, + onPaletteSync, }: { boardLabel: string; initialItems: readonly BoardProps.Item[]; + onPaletteSync: (added?: BoardProps.Item, removed?: BoardProps.Item) => void; }) { const [items, setItems] = useState(initialItems); return ( setItems(detail.items)} + onItemsChange={({ detail: { items, addedItem, removedItem } }) => { + setItems(items); + onPaletteSync(addedItem, removedItem); + }} empty="No items" renderItem={(item, actions) => ( []; - initialPaletteItems: readonly ItemsPaletteProps.Item[]; + paletteItems: readonly ItemsPaletteProps.Item[]; + onPaletteSync: (added?: BoardProps.Item, removed?: BoardProps.Item) => void; }) { const [boardItems, setBoardItems] = useState(initialBoardItems); - const [paletteItems, setPaletteItems] = useState(initialPaletteItems); return ( { setBoardItems(items); - if (addedItem) { - setPaletteItems((prev) => prev.filter((item) => item.id !== addedItem.id)); - } - if (removedItem) { - setPaletteItems((prev) => [...prev, removedItem].sort((a, b) => a.data.title.localeCompare(b.data.title))); - } + onPaletteSync(addedItem, removedItem); }} renderItem={(item, actions) => ( [] = Object.entries .map(([id, widget]) => ({ id, definition: widget!.definition, data: widget!.data })); export default function MultipleBoardsPage() { + // The palette items live here at the page level. Since every board shares a single d&d controller, + // dropping a palette item onto any board must remove it from the palette (and removing a widget from + // any board re-adds it), keeping the palette consistent regardless of which board receives the item. + const [currentPaletteItems, setCurrentPaletteItems] = useState(paletteItems); + const syncPalette = (added?: BoardProps.Item, removed?: BoardProps.Item) => { + if (added) { + setCurrentPaletteItems((prev) => prev.filter((item) => item.id !== added.id)); + } + if (removed) { + setCurrentPaletteItems((prev) => [...prev, removed].sort((a, b) => a.data.title.localeCompare(b.data.title))); + } + }; + return ( Multiple boards on the same page}>
Board 1 (independent)
- +
Board 2 (independent)
- +
Board 3 with a palette
- +
From f57bba6ad8c6b7f758ef167f9cc3faeebd80209c Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Thu, 9 Jul 2026 17:30:58 +0200 Subject: [PATCH 5/6] chore: Adjust testing page --- pages/dnd/multiple-boards.page.tsx | 68 ++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/pages/dnd/multiple-boards.page.tsx b/pages/dnd/multiple-boards.page.tsx index 03ab6c81..51317763 100644 --- a/pages/dnd/multiple-boards.page.tsx +++ b/pages/dnd/multiple-boards.page.tsx @@ -106,7 +106,10 @@ function BoardWithPalette({ items={paletteItems} i18nStrings={itemsPaletteI18nStrings} renderItem={(item, context) => { - const widgetConfig = demoWidgets[item.id]!.data; + // Render from the item's own data, not a lookup in `demoWidgets`. Items re-added to the + // palette after being removed from any board (e.g. Board 1/2 ids like "1-1") are not keys + // in `demoWidgets`, so a lookup there would be undefined and crash the page. + const widgetConfig = item.data; return ( {widgetConfig.title}} i18nStrings={boardItemI18nStrings}> {context.showPreview ? `(preview) ${widgetConfig.description}` : widgetConfig.description} @@ -118,15 +121,56 @@ function BoardWithPalette({ ); } +// Seed the demo boards with a multi-row layout (2 columns per item → a 2×2 grid). The Board engine +// only floats items straight up when an item is removed — it never shifts them sideways. With a +// single-row layout, removing a middle item would leave a permanent gap (nothing below to float up). +// A multi-row layout lets the item below float up to fill the gap, so removals reflow cleanly. const boardOneItems: readonly BoardProps.Item[] = [ - { id: "1-1", data: { title: "Board 1 · Widget A", description: "", content: "Content A" } }, - { id: "1-2", data: { title: "Board 1 · Widget B", description: "", content: "Content B" } }, - { id: "1-3", data: { title: "Board 1 · Widget C", description: "", content: "Content C" } }, + { + id: "1-1", + columnSpan: 2, + columnOffset: { 4: 0, 6: 0 }, + data: { title: "Board 1 · Widget A", description: "", content: "Content A" }, + }, + { + id: "1-2", + columnSpan: 2, + columnOffset: { 4: 2, 6: 2 }, + data: { title: "Board 1 · Widget B", description: "", content: "Content B" }, + }, + { + id: "1-3", + columnSpan: 2, + columnOffset: { 4: 0, 6: 0 }, + data: { title: "Board 1 · Widget C", description: "", content: "Content C" }, + }, + { + id: "1-4", + columnSpan: 2, + columnOffset: { 4: 2, 6: 2 }, + data: { title: "Board 1 · Widget D", description: "", content: "Content D" }, + }, ]; const boardTwoItems: readonly BoardProps.Item[] = [ - { id: "2-1", data: { title: "Board 2 · Widget X", description: "", content: "Content X" } }, - { id: "2-2", data: { title: "Board 2 · Widget Y", description: "", content: "Content Y" } }, + { + id: "2-1", + columnSpan: 2, + columnOffset: { 4: 0, 6: 0 }, + data: { title: "Board 2 · Widget X", description: "", content: "Content X" }, + }, + { + id: "2-2", + columnSpan: 2, + columnOffset: { 4: 2, 6: 2 }, + data: { title: "Board 2 · Widget Y", description: "", content: "Content Y" }, + }, + { + id: "2-3", + columnSpan: 2, + columnOffset: { 4: 0, 6: 0 }, + data: { title: "Board 2 · Widget Z", description: "", content: "Content Z" }, + }, ]; const paletteBoardItems: readonly BoardProps.Item[] = Object.entries(demoWidgets) @@ -137,16 +181,22 @@ const paletteItems: readonly ItemsPaletteProps.Item[] = Object.entries .slice(2, 5) .map(([id, widget]) => ({ id, definition: widget!.definition, data: widget!.data })); +// Ids of the widgets that originate from the palette. Only these return to the palette when removed +// from a board — a board's own initial items (e.g. "1-1", "2-1") are not palette widgets, so removing +// them just drops them without re-populating the palette. +const paletteOriginIds = new Set(paletteItems.map((item) => item.id)); + export default function MultipleBoardsPage() { // The palette items live here at the page level. Since every board shares a single d&d controller, - // dropping a palette item onto any board must remove it from the palette (and removing a widget from - // any board re-adds it), keeping the palette consistent regardless of which board receives the item. + // dropping a palette item onto any board must remove it from the palette, keeping the palette + // consistent regardless of which board receives the item. Removing a palette-origin widget from any + // board re-adds it; removing a board's own initial item does not add it to the palette. const [currentPaletteItems, setCurrentPaletteItems] = useState(paletteItems); const syncPalette = (added?: BoardProps.Item, removed?: BoardProps.Item) => { if (added) { setCurrentPaletteItems((prev) => prev.filter((item) => item.id !== added.id)); } - if (removed) { + if (removed && paletteOriginIds.has(removed.id)) { setCurrentPaletteItems((prev) => [...prev, removed].sort((a, b) => a.data.title.localeCompare(b.data.title))); } }; From 2a26cc9ce1782ac8fa7bbc6eeba6a0359f153b14 Mon Sep 17 00:00:00 2001 From: Georgii Lobko Date: Thu, 9 Jul 2026 17:57:39 +0200 Subject: [PATCH 6/6] chore: Adjust testing page --- pages/dnd/multiple-boards.page.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pages/dnd/multiple-boards.page.tsx b/pages/dnd/multiple-boards.page.tsx index 51317763..9db0709a 100644 --- a/pages/dnd/multiple-boards.page.tsx +++ b/pages/dnd/multiple-boards.page.tsx @@ -59,10 +59,10 @@ function DemoBoard({ ); } -// A board paired with a palette. Board and palette communicate through the shared d&d controller, -// so items can be inserted from the palette into this board — the legacy Board + ItemsPalette -// sibling pattern, shown here living alongside other boards on the same page. The palette items -// live at the page level so that dropping a palette item onto any board removes it from the palette. +// A board rendered next to the shared `ItemsPalette`. This board is not special — because all boards +// share one d&d controller, palette widgets can be dropped onto any board on the page. The palette is +// simply placed here for layout. The palette items live at the page level so that dropping a palette +// item onto any board removes it from the palette. function BoardWithPalette({ initialBoardItems, paletteItems, @@ -101,7 +101,7 @@ function BoardWithPalette({ )} /> -
Add widgets
+
Add widgets (drag onto any board)
Multiple boards on the same page}>
-
Board 1 (independent)
+
Board 1
-
Board 2 (independent)
+
Board 2
-
Board 3 with a palette
+
Board 3