From 354727270c3dfd7fe51bdcea31007289ebf3802c Mon Sep 17 00:00:00 2001 From: Andrey Yamanov Date: Thu, 27 Aug 2026 11:32:43 +0200 Subject: [PATCH 1/4] fix(Board): place a cross-board `swap` arrival instead of cancelling it (CUB-4156) `collisionMode="swap"` treated a widget arriving from another board as strict insertion: the anchor cell had to be empty, and releasing over an occupied one cancelled the whole transfer - both boards snapped back and `onWidgetTransfer` never fired. That made the mode's two halves unusable together. A board that wanted in-board swapping had to either accept drops silently doing nothing, or pick `downscale` and give up the swap; the dashboard Grid container chose the latter, so the swap its authors asked for was never delivered. A cross-board arrival now resolves the way `downscale` does: it keeps its size where the drop cell allows, downscales into the room to its right and below where it does not, and lands in the nearest cell that fits when the anchor is occupied outright. Destination widgets are still never exchanged, pushed, or reflowed - only the arrival moves. In-board drops are untouched and still swap. Removing the strict path exposed a latent bug it had been masking. When a placement is refused, `moveElement` restores the item to the synthetic seed cell one row above (or column left of) the anchor - which sits off the board entirely when the anchor is in row 0. Dropping onto a full row 0 therefore committed the widget at `y: -1`. `landIncoming` now sends a refused arrival to the nearest cell that can hold it, fixing this for every collision mode, not just `swap`. Co-Authored-By: Claude Opus 5 --- .changeset/board-swap-places-arrivals.md | 11 + src/components/layout/Board/Board.docs.mdx | 4 +- src/components/layout/Board/Board.stories.tsx | 6 +- src/components/layout/Board/Board.test.tsx | 127 ++++--- src/components/layout/Board/Board.tsx | 9 +- .../layout/Board/grid-core/collision-modes.ts | 5 +- .../layout/Board/use-board-registry.ts | 314 ++++++++---------- 7 files changed, 251 insertions(+), 225 deletions(-) create mode 100644 .changeset/board-swap-places-arrivals.md diff --git a/.changeset/board-swap-places-arrivals.md b/.changeset/board-swap-places-arrivals.md new file mode 100644 index 000000000..2512a42ab --- /dev/null +++ b/.changeset/board-swap-places-arrivals.md @@ -0,0 +1,11 @@ +--- +'@cube-dev/ui-kit': minor +--- + +`` now **places** a widget arriving from another board instead of cancelling the transfer. + +`swap` used to treat a cross-board arrival as strict insertion: the anchor cell had to be empty, and releasing over an occupied one cancelled the whole transfer — both boards snapped back and `onWidgetTransfer` never fired. That made the mode's two halves unusable together, since a board that wanted in-board swapping had to give up cross-board drops (or pick `downscale` and give up the swap). + +A cross-board arrival now resolves the same way `downscale` does: it keeps its size where the drop cell allows, downscales into the room to its right and below where it does not, and lands in the nearest cell that fits when the anchor is occupied outright. Destination widgets are still never exchanged, pushed, or reflowed to make space — only the arrival moves. In-board drops are unchanged and still swap. + +Also fixes a cross-board landing under any `collisionMode` where a refused placement committed the widget one row above the board (`y: -1`) instead of on it. diff --git a/src/components/layout/Board/Board.docs.mdx b/src/components/layout/Board/Board.docs.mdx index e0d4f181b..9abd9aff9 100644 --- a/src/components/layout/Board/Board.docs.mdx +++ b/src/components/layout/Board/Board.docs.mdx @@ -149,7 +149,7 @@ An aligned board never shrinks its rows, so pair it with `isAutoHeight` on the c - **`compact`** `'vertical' | 'horizontal' | 'free' | null` (default: `'vertical'`) — Compaction behavior. `'vertical'`/`'horizontal'` reflow widgets to remove gaps; `'free'` places each widget exactly where dropped and never pushes its neighbours (blocked from occupied cells unless `allowOverlap`); `null` disables compaction but still resolves collisions the legacy react-grid-layout way. - **`allowOverlap`** `boolean` (default: `false`) — Allow widgets to overlap (stack). It also **switches compaction off**: with it on, `compact="vertical"`/`"horizontal"` no longer reflow, and `preventCollision`/`collisionMode` have no effect at all, since nothing is ever refused. In `'free'` mode it is what distinguishes stacking from blocking a drop onto an occupied cell. - **`preventCollision`** `boolean` (default: `false`) — Block movement into occupied cells instead of pushing. Applies to `'vertical'`, `'horizontal'` and `null` only: `compact="free"` derives blocking from `allowOverlap` and ignores this prop, and `allowOverlap` overrides it everywhere. -- **`collisionMode`** `'revert' | 'downscale' | 'swap'` (default: `'revert'`) — How to resolve a drop the grid would otherwise refuse. Only applies where a collision _blocks_ a move — `compact="free"` (which prevents collisions) or an explicit `preventCollision` — and never under `allowOverlap`, where nothing is blocked. `'revert'` snaps the widget back. `'downscale'` shrinks it into the free space at the drop cell, growing rightward and downward, floored at its `minW`/`minH` (below that it reverts). Within one board, `'swap'` trades places with **one** widget — the one the drop covers most — which takes the cell the drag began at, each keeping as much of its own size as fits there. It never displaces more than that one widget. A drop straddling two widgets trades with one of them rather than refusing, so the swap never blinks away as you sweep across a boundary; ties go to the widget earlier in reading order. Dragging back retraces the original arrangement exactly. For a widget arriving from another board (including a nested board entering its parent), the destination board's `'swap'` mode becomes strict insertion: the requested anchor cell must be empty, the incoming widget keeps its size when possible or downscales into the available room to its right and below, and existing destination widgets are neither exchanged nor reflowed. Releasing over an occupied anchor or where `minW`/`minH` cannot fit cancels the transfer and restores both boards; it does not reuse the last valid preview or search for another slot. No mode ever _grows_ a widget. Applies to single-widget drags. Arrow keys honour it too but never resize anything — each press is a gesture of its own, so a press that shrank a widget would have nothing to restore from, and only an exchange that fits outright is allowed. A multi-widget selection still only moves where it fits outright, and a resize is still blocked by a collision. +- **`collisionMode`** `'revert' | 'downscale' | 'swap'` (default: `'revert'`) — How to resolve a drop the grid would otherwise refuse. Only applies where a collision _blocks_ a move — `compact="free"` (which prevents collisions) or an explicit `preventCollision` — and never under `allowOverlap`, where nothing is blocked. `'revert'` snaps the widget back. `'downscale'` shrinks it into the free space at the drop cell, growing rightward and downward, floored at its `minW`/`minH` (below that it reverts). Within one board, `'swap'` trades places with **one** widget — the one the drop covers most — which takes the cell the drag began at, each keeping as much of its own size as fits there. It never displaces more than that one widget. A drop straddling two widgets trades with one of them rather than refusing, so the swap never blinks away as you sweep across a boundary; ties go to the widget earlier in reading order. Dragging back retraces the original arrangement exactly. For a widget arriving from another board (including a nested board entering its parent), there is no slot on the destination to give back, so `'swap'` resolves as `'downscale'`: the incoming widget keeps its size when possible or downscales into the available room to its right and below, and existing destination widgets are never exchanged. A release over an occupied anchor lands the widget in the nearest cell it fits in — the placement the preview was showing — rather than cancelling the transfer. No mode ever _grows_ a widget. Applies to single-widget drags. Arrow keys honour it too but never resize anything — each press is a gesture of its own, so a press that shrank a widget would have nothing to restore from, and only an exchange that fits outright is allowed. A multi-widget selection still only moves where it fits outright, and a resize is still blocked by a collision. - **`isDraggable`** `boolean` (default: `true`) — Enable dragging for all widgets. A ceiling, not a default: `false` here cannot be re-enabled by a widget, and a `static` layout item is never draggable regardless. - **`isResizable`** `boolean` (default: `true`) — Enable resizing for all widgets. Same ceiling semantics as `isDraggable`. - **`isDroppable`** `boolean` (default: `true`) — Whether the board accepts widgets dropped from _other_ boards. `false` does not affect dragging within the board. @@ -357,7 +357,7 @@ Centre the resize grip on the widget's corner with `resizeGripPlacement="corner" ### Resolving a blocked drop -On a free grid a drop onto occupied cells is refused. `collisionMode` resolves it instead: `'downscale'` shrinks the widget into whatever room is at the drop cell. Within one board, `'swap'` trades places with the widget already there. Across boards, the same mode inserts only at an empty anchor and downscales without moving destination widgets; an invalid release cancels the transfer. None of these paths ever grows a widget. The examples below show all three behaviors. +On a free grid a drop onto occupied cells is refused. `collisionMode` resolves it instead: `'downscale'` shrinks the widget into whatever room is at the drop cell. Within one board, `'swap'` trades places with the widget already there. Across boards, the same mode downscales into an empty cell without exchanging destination widgets, resolving an occupied anchor to the nearest cell that fits. None of these paths ever grows a widget. The examples below show all three behaviors. diff --git a/src/components/layout/Board/Board.stories.tsx b/src/components/layout/Board/Board.stories.tsx index 30f648f18..ba12ed431 100644 --- a/src/components/layout/Board/Board.stories.tsx +++ b/src/components/layout/Board/Board.stories.tsx @@ -684,8 +684,8 @@ const CollisionModesTemplate: StoryFn = () => ( collisionMode="swap" across boards — drag the - incoming widget into empty space on the target; dropping on the blocker - cancels the transfer + incoming widget onto the target; it downscales into the room at the drop + cell, and dropping on the blocker sends it to the nearest cell that fits @@ -743,7 +743,7 @@ CollisionModes.parameters = { docs: { description: { story: - 'A `compact="free"` board refuses a drop onto occupied cells; `collisionMode` resolves it instead. **Downscale** — drag the 4-column widget onto the middle row and it shrinks to the 3 columns free beside the blocker, instead of snapping back. **Swap within one board** — drop one widget onto another and they trade places: the dragged widget takes the other\'s cell, the displaced one takes the cell the drag began at, and each keeps as much of its own size as fits there. Exactly one widget is ever displaced, a drop straddling two of them trades with the one it covers most, and dragging back retraces the original arrangement. **Swap across boards** — the incoming widget can only use an empty anchor, downscales into the room to its right and below, and never moves a destination widget; an occupied anchor cancels the transfer. Neither path ever grows a widget. The default, `"revert"`, is what every other story on this page shows: the widget snaps back.', + 'A `compact="free"` board refuses a drop onto occupied cells; `collisionMode` resolves it instead. **Downscale** — drag the 4-column widget onto the middle row and it shrinks to the 3 columns free beside the blocker, instead of snapping back. **Swap within one board** — drop one widget onto another and they trade places: the dragged widget takes the other\'s cell, the displaced one takes the cell the drag began at, and each keeps as much of its own size as fits there. Exactly one widget is ever displaced, a drop straddling two of them trades with the one it covers most, and dragging back retraces the original arrangement. **Swap across boards** — there is no slot on the destination to trade back, so the arrival downscales into the room to its right and below and never moves a destination widget; an occupied anchor sends it to the nearest cell that fits rather than cancelling. Neither path ever grows a widget. The default, `"revert"`, is what every other story on this page shows: the widget snaps back.', }, }, }; diff --git a/src/components/layout/Board/Board.test.tsx b/src/components/layout/Board/Board.test.tsx index 09e478ad2..2fb2d2130 100644 --- a/src/components/layout/Board/Board.test.tsx +++ b/src/components/layout/Board/Board.test.tsx @@ -12,6 +12,8 @@ import { } from '../../../test'; import { Tab, Tabs } from '../../navigation/Tabs'; +import { isOverlapFree } from './grid-core'; + import { Board } from './index'; import type { ReactNode } from 'react'; @@ -2346,6 +2348,7 @@ describe('Board', () => { function setupCrossBoardSwap( incoming: LayoutItem, targetLayout: LayoutItem[], + mode: 'swap' | 'downscale' = 'swap', ) { const onSourceLayoutChange = vi.fn(); const onTargetLayoutChange = vi.fn(); @@ -2362,7 +2365,7 @@ describe('Board', () => { margin={[0, 0]} containerPadding={[0, 0]} compact="free" - collisionMode="swap" + collisionMode={mode} defaultLayout={[incoming]} onLayoutChange={onSourceLayoutChange} onDragStop={onDragStop} @@ -2379,7 +2382,7 @@ describe('Board', () => { margin={[0, 0]} containerPadding={[0, 0]} compact="free" - collisionMode="swap" + collisionMode={mode} defaultLayout={targetLayout} onLayoutChange={onTargetLayoutChange} > @@ -2476,71 +2479,101 @@ describe('Board', () => { ); }); - it('cancels the transfer when the anchor cell is occupied', () => { - const { - dragTo, - onSourceLayoutChange, - onTargetLayoutChange, - onWidgetTransfer, - onDragStop, - } = setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 2, h: 1 }, [ - { i: 'b', x: 0, y: 0, w: 2, h: 1 }, - ]); + it('places into a free cell when the anchor cell is occupied', () => { + const targetLayout = [{ i: 'b', x: 0, y: 0, w: 2, h: 1 }]; + const { dragTo, onTargetLayoutChange, onWidgetTransfer } = + setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 2, h: 1 }, targetLayout); dragTo(0, 0); - expect(onSourceLayoutChange).not.toHaveBeenCalled(); - expect(onTargetLayoutChange).not.toHaveBeenCalled(); - expect(onWidgetTransfer).not.toHaveBeenCalled(); - expect(onDragStop).toHaveBeenCalledWith( + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + // The occupant keeps its cell: a cross-board arrival never displaces it. + expect(committed.find((it) => it.i === 'b')).toEqual( + expect.objectContaining(targetLayout[0]), + ); + // The nearest free cell in reading order, at the widget's own size. + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 2, y: 0, w: 2, h: 1 }), + ); + expect(isOverlapFree(committed)).toBe(true); + expect(onWidgetTransfer).toHaveBeenCalledWith( expect.objectContaining({ - item: expect.objectContaining({ i: 'a', x: 0, y: 0 }), - layout: [expect.objectContaining({ i: 'a', x: 0, y: 0 })], + fromBoardId: 'source', + toBoardId: 'target', + item: expect.objectContaining({ i: 'a' }), }), ); }); - it('cancels when the available space is below the minimum size', () => { - const { - dragTo, - onSourceLayoutChange, - onTargetLayoutChange, - onWidgetTransfer, - } = setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 4, h: 1, minW: 4 }, [ - { i: 'b', x: 3, y: 0, w: 3, h: 1 }, - ]); + it('places at full size when the anchor cannot honour the minimum size', () => { + const targetLayout = [{ i: 'b', x: 3, y: 0, w: 3, h: 1 }]; + const { dragTo, onTargetLayoutChange, onWidgetTransfer } = + setupCrossBoardSwap( + { i: 'a', x: 0, y: 0, w: 4, h: 1, minW: 4 }, + targetLayout, + ); dragTo(0, 0); - expect(onSourceLayoutChange).not.toHaveBeenCalled(); - expect(onTargetLayoutChange).not.toHaveBeenCalled(); - expect(onWidgetTransfer).not.toHaveBeenCalled(); + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + expect(committed.find((it) => it.i === 'b')).toEqual( + expect.objectContaining(targetLayout[0]), + ); + // `minW: 4` rules out downscaling into the 3 free columns, so the widget + // keeps its size and takes the nearest cell where that size fits. + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 0, y: 1, w: 4, h: 1 }), + ); + expect(isOverlapFree(committed)).toBe(true); + expect(onWidgetTransfer).toHaveBeenCalled(); }); - it('cancels instead of committing the last valid preview', () => { - const { - start, - moveTo, - endAt, - onSourceLayoutChange, - onTargetLayoutChange, - onWidgetTransfer, - } = setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 2, h: 1 }, [ - { i: 'b', x: 2, y: 0, w: 2, h: 1 }, - ]); + it('commits the last valid preview when the release cell is occupied', () => { + const targetLayout = [{ i: 'b', x: 2, y: 0, w: 2, h: 1 }]; + const { start, moveTo, endAt, onTargetLayoutChange, onWidgetTransfer } = + setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 2, h: 1 }, targetLayout); start(); moveTo(0, 0); expect(screen.getAllByTestId('BoardPlaceholder')).toHaveLength(1); moveTo(2, 0); - expect( - screen.queryByTestId('BoardPlaceholder'), - ).not.toBeInTheDocument(); + // The preview holds at the last cell that fit rather than blinking away. + expect(screen.getAllByTestId('BoardPlaceholder')).toHaveLength(1); endAt(2, 0); - expect(onSourceLayoutChange).not.toHaveBeenCalled(); - expect(onTargetLayoutChange).not.toHaveBeenCalled(); - expect(onWidgetTransfer).not.toHaveBeenCalled(); + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 0, y: 0, w: 2, h: 1 }), + ); + expect(committed.find((it) => it.i === 'b')).toEqual( + expect.objectContaining(targetLayout[0]), + ); + expect(onWidgetTransfer).toHaveBeenCalled(); + }); + + it('lands a downscale arrival on the board, never above its first row', () => { + const targetLayout = [{ i: 'b', x: 0, y: 0, w: 6, h: 1 }]; + const { dragTo, onTargetLayoutChange } = setupCrossBoardSwap( + { i: 'a', x: 0, y: 0, w: 2, h: 1 }, + targetLayout, + 'downscale', + ); + + dragTo(0, 0); + + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + // Row 0 is full, so the arrival goes to the row below - not to the + // synthetic seed cell at y = -1 that a refused move restores. + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 0, y: 1, w: 2, h: 1 }), + ); + expect(committed.find((it) => it.i === 'b')).toEqual( + expect.objectContaining(targetLayout[0]), + ); }); it('uses the same insertion rule when dragging from a nested board to its parent', () => { diff --git a/src/components/layout/Board/Board.tsx b/src/components/layout/Board/Board.tsx index 3ab4929dd..90f4d3dc0 100644 --- a/src/components/layout/Board/Board.tsx +++ b/src/components/layout/Board/Board.tsx @@ -304,10 +304,11 @@ export interface CubeBoardProps * snaps the widget back, `'downscale'` shrinks it into the free space at the * drop cell. Within this board, `'swap'` trades places with one widget - the one * the drop covers most - which takes the cell the drag began at (falling back to - * `'downscale'`, then `'revert'`). For a widget arriving from another board, - * `'swap'` never exchanges or reflows destination widgets: the anchor cell must - * be empty, the incoming widget downscales into the room to its right/below, and - * an invalid landing cancels the transfer. No mode ever grows a widget, an + * `'downscale'`, then `'revert'`). For a widget arriving from another board there + * is no slot to trade back, so `'swap'` behaves as `'downscale'`: the widget + * downscales into the room to its right/below and an occupied anchor resolves to + * the nearest cell it fits in rather than cancelling the transfer. No mode ever + * grows a widget, an * in-board swap never displaces more than one widget, and a drop that spans two * widgets trades with one of them rather than blinking away mid-drag. * diff --git a/src/components/layout/Board/grid-core/collision-modes.ts b/src/components/layout/Board/grid-core/collision-modes.ts index b469573c6..186f3c637 100644 --- a/src/components/layout/Board/grid-core/collision-modes.ts +++ b/src/components/layout/Board/grid-core/collision-modes.ts @@ -257,8 +257,9 @@ function exchangeWith( * dense grid stays draggable that way - refusing everything that is not a clean * one-to-one trade would make the mode feel broken exactly where it is needed. * Cross-board callers pass `allowExchange: false`, which turns the same mode into - * empty-anchor downscaling; the registry cancels the transfer if that resolution - * cannot occupy the requested cell. + * downscaling at the drop cell - there is no slot on the destination to trade + * back, and a transfer that cannot resolve is placed in a free cell rather than + * refused, since a widget dropped somewhere has to end up somewhere. */ export function createCollisionResolver( mode: CollisionMode | undefined, diff --git a/src/components/layout/Board/use-board-registry.ts b/src/components/layout/Board/use-board-registry.ts index 901689d91..7557fed42 100644 --- a/src/components/layout/Board/use-board-registry.ts +++ b/src/components/layout/Board/use-board-registry.ts @@ -89,6 +89,37 @@ function hasNewOverlap(before: Set, after: LayoutItem[]): boolean { return false; } +/** + * Where a cross-board arrival ends up when the destination refuses the cell it + * was dropped on. + * + * `moveElement` answers a refused placement by restoring the item to where it + * came from, and for an incoming widget that is the synthetic seed one cell above + * (or left of) the anchor - a cell the pointer never chose, and one that sits off + * the board entirely whenever the anchor is in row 0 or column 0. A widget + * released over a board has to end up somewhere on it, so send it to the nearest + * cell that can hold it instead. The destination's own widgets are the fixed + * `others` here: an arrival is placed around them and never displaces one. + * + * Returns `layout` untouched when the requested cell was honoured, which covers + * both a clean landing and a resolver that downscaled the item into it. + */ +function landIncoming( + layout: LayoutItem[], + others: LayoutItem[], + itemId: string, + x: number, + y: number, + cols: number, + maxRows?: number, +): LayoutItem[] { + const placed = getLayoutItem(layout, itemId); + if (!placed || (placed.x === x && placed.y === y)) return layout; + + const landed = placeInFreeSlot(others, { ...placed, x, y }, cols, maxRows); + return layout.map((l) => (l.i === itemId ? { ...l, ...landed } : l)); +} + /** * Whether any item in `after` has a different size than it had in `before`. */ @@ -140,13 +171,6 @@ export function useBoardRegistry( const previewRef = useRef<{ boardId: string; working: LayoutItem[] } | null>( null, ); - // Whether the latest cross-board preview actually landed at the requested - // cell. A collision resolver may decline a placement, in which case - // `moveElement` restores its synthetic origin; that must not be mistaken for a - // valid target preview and committed somewhere the pointer never selected. - const targetLandingRef = useRef<{ boardId: string; valid: boolean } | null>( - null, - ); // Board content rects captured at drag start. Reading geometry from here (not // live getBoundingClientRect) means the live-reflow preview can't move the // rects that selection/landing depend on -> no feedback loop. @@ -400,7 +424,6 @@ export function useBoardRegistry( affectedRef.current = new Set([boardId]); sourceSnapshotRef.current = cloneLayout(entry.getLayout()); previewRef.current = null; - targetLandingRef.current = null; lastLandingRef.current = { x: item.x, y: item.y }; // Start tracking the live cursor for the ancestor-handoff gate. Keyboard // drags have no pointer, so the gate falls back to the widget anchor. @@ -866,15 +889,6 @@ export function useBoardRegistry( (target: BoardEntry, item: LayoutItem, x: number, y: number) => { const pp = target.getPositionParams(); const compactor = target.getCompactor(); - // `swap` is deliberately source-aware. Inside one board it exchanges two - // widgets; across boards there is no slot on the destination to give back, - // so it becomes strict empty-anchor insertion with downscaling. The strict - // path also skips target compaction: inserting one widget must not reflow or - // push any widget already owned by the destination. - const strictIncomingSwap = - compactor.preventCollision === true && - !compactor.allowOverlap && - target.getCollisionMode() === 'swap'; const carried = previewRef.current?.boardId === target.id @@ -921,7 +935,11 @@ export function useBoardRegistry( // No exchange across boards: the displaced widget would have to be // pushed back the other way, and a transfer moves one widget only. A // widget arriving from elsewhere has no slot here to trade, so `'swap'` - // resolves as `'downscale'`. + // resolves as `'downscale'` - it downscales into the room at the anchor, + // and when the anchor is occupied outright the carried preview simply + // holds at the last cell that did fit. Refusing the frame instead would + // cancel the whole transfer over an occupied cell, which loses the + // widget rather than placing it. resolveCollision: compactor.preventCollision ? createCollisionResolver(target.getCollisionMode(), { cols: pp.cols, @@ -932,9 +950,12 @@ export function useBoardRegistry( : undefined, }, ); - const compacted = strictIncomingSwap - ? cloneLayout(moved) - : [...compactor.compact(moved, pp.cols)]; + const compacted = [ + ...compactor.compact( + landIncoming(moved, base, item.i, x, y, pp.cols, target.getMaxRows()), + pp.cols, + ), + ]; const landed = getLayoutItem(compacted, item.i); // Skip a frame that would *newly* stack widgets on the target (see the same // guard in `moveWithinBoard`): keep the last valid preview instead of @@ -945,32 +966,10 @@ export function useBoardRegistry( !compactor.allowOverlap && hasNewOverlap(overlappingPairs(base), compacted) ) { - if (strictIncomingSwap) { - const snapshot = targetSnapshotsRef.current.get(target.id); - if (snapshot) target.applyLayout(cloneLayout(snapshot), false); - target.setPlaceholders([]); - previewRef.current = null; - targetLandingRef.current = { boardId: target.id, valid: false }; - } - return; - } - - // A prevented collision restores the incoming item to the synthetic origin - // used to make `moveElement` active. For a strict cross-board swap, only the - // exact requested anchor is a valid insertion; an occupied anchor or a fit - // below minW/minH therefore clears the preview and restores the untouched - // target snapshot. - if (strictIncomingSwap && (!landed || landed.x !== x || landed.y !== y)) { - const snapshot = targetSnapshotsRef.current.get(target.id); - if (snapshot) target.applyLayout(cloneLayout(snapshot), false); - target.setPlaceholders([]); - previewRef.current = null; - targetLandingRef.current = { boardId: target.id, valid: false }; return; } previewRef.current = { boardId: target.id, working: compacted }; - targetLandingRef.current = { boardId: target.id, valid: true }; const previewItem = landed ?? { ...item, x, y }; // Apply only the other widgets so the dragged item is never rendered as a @@ -1066,7 +1065,6 @@ export function useBoardRegistry( if (snap) prev?.applyLayout(cloneLayout(snap), false); prev?.setPlaceholders([]); previewRef.current = null; - targetLandingRef.current = null; } if (isGroup) { // The group moves by the delta the grabbed widget travelled from its @@ -1102,7 +1100,6 @@ export function useBoardRegistry( // Drop the carried working layout so the newly entered target seeds a // fresh preview from its own clean snapshot. previewRef.current = null; - targetLandingRef.current = null; // Snapshot the newly entered target once, as the stable base for its // reflow preview. @@ -1146,10 +1143,6 @@ export function useBoardRegistry( const landing = lastLandingRef.current ?? { x: ds.item.x, y: ds.item.y }; const tp = target!.getPositionParams(); const tc = target!.getCompactor(); - const strictIncomingSwap = - tc.preventCollision === true && - !tc.allowOverlap && - target!.getCollisionMode() === 'swap'; // Prefer committing the exact arrangement the user was previewing (item // already placed with the neighbours reflowed around it via continuity). @@ -1159,130 +1152,118 @@ export function useBoardRegistry( ? previewRef.current.working : null; - const hasValidStrictLanding = - targetLandingRef.current?.boardId === target!.id && - targetLandingRef.current.valid && - carried != null; - - // A strict incoming swap is allowed to commit only the exact valid preview - // shown under the pointer. An occupied anchor, a min-size failure, or moving - // from a valid cell onto an invalid one cancels the transfer completely: - // both boards return to their gesture-start snapshots and no controlled - // layout or transfer callback fires. - if (strictIncomingSwap && !hasValidStrictLanding) { - if (source) { - source.applyLayout(cloneLayout(sourceSnapshotRef.current), false); - } - const targetSnapshot = targetSnapshotsRef.current.get(target!.id); - if (targetSnapshot) { - target!.applyLayout(cloneLayout(targetSnapshot), false); - } - } else { - // Remove the item from the source board and compact only after the target - // landing is known to be committable. - if (source) { - const sp = source.getPositionParams(); - const sc = source.getCompactor(); - const remaining = source.getLayout().filter((l) => l.i !== ds.itemId); - source.applyLayout( - [...sc.compact(remaining, sp.cols)], - true, - 'transfer', - ); - } + // Remove the item from the source board and compact it, now that the + // destination has accepted the widget. + if (source) { + const sp = source.getPositionParams(); + const sc = source.getCompactor(); + const remaining = source.getLayout().filter((l) => l.i !== ds.itemId); + source.applyLayout( + [...sc.compact(remaining, sp.cols)], + true, + 'transfer', + ); + } - let finalLayout: LayoutItem[]; - if (carried) { - // The strict path already produced an overlap-free exact placement and - // must not compact the destination widgets around it. - finalLayout = strictIncomingSwap - ? cloneLayout(carried) - : [...tc.compact(cloneLayout(carried), tp.cols)]; - } else { - // No preview frame ran (e.g. a teleport drop). Seed the item just above - // (or left of) its landing cell so `moveElement` actively places it - // rather than no-opping and letting compaction sink it to the bottom. - const newItem: LayoutItem = - tc.type === 'horizontal' - ? { ...ds.item, x: Math.max(0, landing.x) - 1, y: landing.y } - : { ...ds.item, x: landing.x, y: Math.max(0, landing.y) - 1 }; - const base = [ - ...cloneLayout( - ( - targetSnapshotsRef.current.get(target!.id) ?? - target!.getLayout() - ).filter((l) => l.i !== ds.itemId), + let finalLayout: LayoutItem[]; + if (carried) { + finalLayout = [...tc.compact(cloneLayout(carried), tp.cols)]; + } else { + // No preview frame ran (e.g. a teleport drop). Seed the item just above + // (or left of) its landing cell so `moveElement` actively places it + // rather than no-opping and letting compaction sink it to the bottom. + const newItem: LayoutItem = + tc.type === 'horizontal' + ? { ...ds.item, x: Math.max(0, landing.x) - 1, y: landing.y } + : { ...ds.item, x: landing.x, y: Math.max(0, landing.y) - 1 }; + const base = [ + ...cloneLayout( + ( + targetSnapshotsRef.current.get(target!.id) ?? target!.getLayout() + ).filter((l) => l.i !== ds.itemId), + ), + newItem, + ]; + const moved = moveElement( + base, + newItem, + landing.x, + landing.y, + true, + tc.preventCollision, + tc.type, + tp.cols, + tc.allowOverlap, + { + // Cross-board, so no exchange (see `previewOnTarget`). `newItem` is + // built from the drag-start item, so its size is already the one to + // measure against. + resolveCollision: createCollisionResolver( + target!.getCollisionMode(), + { + cols: tp.cols, + maxRows: tp.maxRows, + desired: { w: ds.item.w, h: ds.item.h }, + allowExchange: false, + }, ), - newItem, - ]; - const moved = moveElement( - base, - newItem, - landing.x, - landing.y, - true, - tc.preventCollision, - tc.type, - tp.cols, - tc.allowOverlap, - { - // Cross-board, so no exchange (see `previewOnTarget`). `newItem` is - // built from the drag-start item, so its size is already the one to - // measure against. - resolveCollision: createCollisionResolver( - target!.getCollisionMode(), - { - cols: tp.cols, - maxRows: tp.maxRows, - desired: { w: ds.item.w, h: ds.item.h }, - allowExchange: false, - }, - ), - }, - ); - finalLayout = [...tc.compact(moved, tp.cols)]; - } - // Never commit a drop that *creates* a stack. When the compactor cannot - // resolve overlaps (`compact={null}` / `preventCollision`) a teleport drop - // into an occupied region would otherwise land the item on top of another - // widget; place it in the first free slot instead so the pointer path - // matches the keyboard path. Overlaps the target already had do not trigger - // the reshuffle (they are preserved as-is). `allowOverlap` opts out. - const targetOthers = cloneLayout( - ( - targetSnapshotsRef.current.get(target!.id) ?? target!.getLayout() - ).filter((l) => l.i !== ds.itemId), + }, ); - if ( - !tc.allowOverlap && - hasNewOverlap(overlappingPairs(targetOthers), finalLayout) - ) { - finalLayout = [ - ...targetOthers, - placeInFreeSlot( - targetOthers, - { ...ds.item, x: landing.x, y: landing.y }, + finalLayout = [ + ...tc.compact( + landIncoming( + moved, + base.filter((l) => l.i !== ds.itemId), + ds.itemId, + landing.x, + landing.y, tp.cols, tp.maxRows, ), - ]; - } - target!.applyLayout(finalLayout, true, 'transfer'); - - // Signal the transfer so a controlled app can move the widget's - // declaration into the destination container (positions are already - // reported via each board's onLayoutChange). - onTransferRef.current?.({ - widgetId: ds.itemId, - fromBoardId: ds.sourceBoardId, - toBoardId: target!.id, - item: getLayoutItem(finalLayout, ds.itemId) ?? { - ...ds.item, - x: landing.x, - y: landing.y, - }, - }); + tp.cols, + ), + ]; } + // Never commit a drop that *creates* a stack. When the compactor cannot + // resolve overlaps (`compact={null}` / `preventCollision`) a teleport drop + // into an occupied region would otherwise land the item on top of another + // widget; place it in the first free slot instead so the pointer path + // matches the keyboard path. Overlaps the target already had do not trigger + // the reshuffle (they are preserved as-is). `allowOverlap` opts out. + const targetOthers = cloneLayout( + ( + targetSnapshotsRef.current.get(target!.id) ?? target!.getLayout() + ).filter((l) => l.i !== ds.itemId), + ); + if ( + !tc.allowOverlap && + hasNewOverlap(overlappingPairs(targetOthers), finalLayout) + ) { + finalLayout = [ + ...targetOthers, + placeInFreeSlot( + targetOthers, + { ...ds.item, x: landing.x, y: landing.y }, + tp.cols, + tp.maxRows, + ), + ]; + } + target!.applyLayout(finalLayout, true, 'transfer'); + + // Signal the transfer so a controlled app can move the widget's + // declaration into the destination container (positions are already + // reported via each board's onLayoutChange). + onTransferRef.current?.({ + widgetId: ds.itemId, + fromBoardId: ds.sourceBoardId, + toBoardId: target!.id, + item: getLayoutItem(finalLayout, ds.itemId) ?? { + ...ds.item, + x: landing.x, + y: landing.y, + }, + }); } const ids = new Set(affectedRef.current); @@ -1296,7 +1277,6 @@ export function useBoardRegistry( frozenRectsRef.current.clear(); nestedInDraggedRef.current = new Set(); previewRef.current = null; - targetLandingRef.current = null; lastLandingRef.current = null; setDragState(null); }); From d16e5eadfe5c14458146dd68f6c5e87f9fb91edb Mon Sep 17 00:00:00 2001 From: Andrey Yamanov Date: Thu, 27 Aug 2026 11:45:54 +0200 Subject: [PATCH 2/4] fix(Board): hold the previewed cell when a cross-board sweep crosses a widget `landIncoming` relocated on every refused frame, but `moveElement` only falls back to the synthetic off-board seed on the *first* frame over a board. Every later frame falls back to the cell the previous frame settled on - a real, on-board cell - so relocating there made the placeholder jump to the nearest free slot the moment the pointer swept over a destination widget, and the drop committed that jump instead of what the preview had been showing. It also regressed `downscale`, which used to hold. Only the seeded first frame relocates now; later frames keep the cell they found. Entering a board directly over an occupied cell still places the widget in the nearest cell that fits, which is what the off-board landing fix needed. Reported by Cursor Bugbot on #1370. Co-Authored-By: Claude Opus 5 --- .changeset/board-swap-places-arrivals.md | 2 +- src/components/layout/Board/Board.docs.mdx | 4 +-- src/components/layout/Board/Board.stories.tsx | 5 +-- src/components/layout/Board/Board.test.tsx | 21 +++++++++++ src/components/layout/Board/Board.tsx | 5 +-- .../layout/Board/use-board-registry.ts | 35 ++++++++++++++----- 6 files changed, 56 insertions(+), 16 deletions(-) diff --git a/.changeset/board-swap-places-arrivals.md b/.changeset/board-swap-places-arrivals.md index 2512a42ab..70754704b 100644 --- a/.changeset/board-swap-places-arrivals.md +++ b/.changeset/board-swap-places-arrivals.md @@ -6,6 +6,6 @@ `swap` used to treat a cross-board arrival as strict insertion: the anchor cell had to be empty, and releasing over an occupied one cancelled the whole transfer — both boards snapped back and `onWidgetTransfer` never fired. That made the mode's two halves unusable together, since a board that wanted in-board swapping had to give up cross-board drops (or pick `downscale` and give up the swap). -A cross-board arrival now resolves the same way `downscale` does: it keeps its size where the drop cell allows, downscales into the room to its right and below where it does not, and lands in the nearest cell that fits when the anchor is occupied outright. Destination widgets are still never exchanged, pushed, or reflowed to make space — only the arrival moves. In-board drops are unchanged and still swap. +A cross-board arrival now resolves the same way `downscale` does: it keeps its size where the drop cell allows, downscales into the room to its right and below where it does not, and holds the last cell it fitted in as the pointer sweeps across a destination widget — so releasing over an occupied cell commits what the preview was showing. Entering a board directly over an occupied cell places the widget in the nearest cell that fits. Destination widgets are still never exchanged, pushed, or reflowed to make space — only the arrival moves. In-board drops are unchanged and still swap. Also fixes a cross-board landing under any `collisionMode` where a refused placement committed the widget one row above the board (`y: -1`) instead of on it. diff --git a/src/components/layout/Board/Board.docs.mdx b/src/components/layout/Board/Board.docs.mdx index 9abd9aff9..8b8fe4339 100644 --- a/src/components/layout/Board/Board.docs.mdx +++ b/src/components/layout/Board/Board.docs.mdx @@ -149,7 +149,7 @@ An aligned board never shrinks its rows, so pair it with `isAutoHeight` on the c - **`compact`** `'vertical' | 'horizontal' | 'free' | null` (default: `'vertical'`) — Compaction behavior. `'vertical'`/`'horizontal'` reflow widgets to remove gaps; `'free'` places each widget exactly where dropped and never pushes its neighbours (blocked from occupied cells unless `allowOverlap`); `null` disables compaction but still resolves collisions the legacy react-grid-layout way. - **`allowOverlap`** `boolean` (default: `false`) — Allow widgets to overlap (stack). It also **switches compaction off**: with it on, `compact="vertical"`/`"horizontal"` no longer reflow, and `preventCollision`/`collisionMode` have no effect at all, since nothing is ever refused. In `'free'` mode it is what distinguishes stacking from blocking a drop onto an occupied cell. - **`preventCollision`** `boolean` (default: `false`) — Block movement into occupied cells instead of pushing. Applies to `'vertical'`, `'horizontal'` and `null` only: `compact="free"` derives blocking from `allowOverlap` and ignores this prop, and `allowOverlap` overrides it everywhere. -- **`collisionMode`** `'revert' | 'downscale' | 'swap'` (default: `'revert'`) — How to resolve a drop the grid would otherwise refuse. Only applies where a collision _blocks_ a move — `compact="free"` (which prevents collisions) or an explicit `preventCollision` — and never under `allowOverlap`, where nothing is blocked. `'revert'` snaps the widget back. `'downscale'` shrinks it into the free space at the drop cell, growing rightward and downward, floored at its `minW`/`minH` (below that it reverts). Within one board, `'swap'` trades places with **one** widget — the one the drop covers most — which takes the cell the drag began at, each keeping as much of its own size as fits there. It never displaces more than that one widget. A drop straddling two widgets trades with one of them rather than refusing, so the swap never blinks away as you sweep across a boundary; ties go to the widget earlier in reading order. Dragging back retraces the original arrangement exactly. For a widget arriving from another board (including a nested board entering its parent), there is no slot on the destination to give back, so `'swap'` resolves as `'downscale'`: the incoming widget keeps its size when possible or downscales into the available room to its right and below, and existing destination widgets are never exchanged. A release over an occupied anchor lands the widget in the nearest cell it fits in — the placement the preview was showing — rather than cancelling the transfer. No mode ever _grows_ a widget. Applies to single-widget drags. Arrow keys honour it too but never resize anything — each press is a gesture of its own, so a press that shrank a widget would have nothing to restore from, and only an exchange that fits outright is allowed. A multi-widget selection still only moves where it fits outright, and a resize is still blocked by a collision. +- **`collisionMode`** `'revert' | 'downscale' | 'swap'` (default: `'revert'`) — How to resolve a drop the grid would otherwise refuse. Only applies where a collision _blocks_ a move — `compact="free"` (which prevents collisions) or an explicit `preventCollision` — and never under `allowOverlap`, where nothing is blocked. `'revert'` snaps the widget back. `'downscale'` shrinks it into the free space at the drop cell, growing rightward and downward, floored at its `minW`/`minH` (below that it reverts). Within one board, `'swap'` trades places with **one** widget — the one the drop covers most — which takes the cell the drag began at, each keeping as much of its own size as fits there. It never displaces more than that one widget. A drop straddling two widgets trades with one of them rather than refusing, so the swap never blinks away as you sweep across a boundary; ties go to the widget earlier in reading order. Dragging back retraces the original arrangement exactly. For a widget arriving from another board (including a nested board entering its parent), there is no slot on the destination to give back, so `'swap'` resolves as `'downscale'`: the incoming widget keeps its size when possible or downscales into the available room to its right and below, and existing destination widgets are never exchanged. A release over an occupied anchor commits whatever the preview was showing rather than cancelling the transfer: the widget holds the last cell it fitted in as you sweep across a destination widget, and enters at the nearest fitting cell when the very first cell it is offered is occupied. No mode ever _grows_ a widget. Applies to single-widget drags. Arrow keys honour it too but never resize anything — each press is a gesture of its own, so a press that shrank a widget would have nothing to restore from, and only an exchange that fits outright is allowed. A multi-widget selection still only moves where it fits outright, and a resize is still blocked by a collision. - **`isDraggable`** `boolean` (default: `true`) — Enable dragging for all widgets. A ceiling, not a default: `false` here cannot be re-enabled by a widget, and a `static` layout item is never draggable regardless. - **`isResizable`** `boolean` (default: `true`) — Enable resizing for all widgets. Same ceiling semantics as `isDraggable`. - **`isDroppable`** `boolean` (default: `true`) — Whether the board accepts widgets dropped from _other_ boards. `false` does not affect dragging within the board. @@ -357,7 +357,7 @@ Centre the resize grip on the widget's corner with `resizeGripPlacement="corner" ### Resolving a blocked drop -On a free grid a drop onto occupied cells is refused. `collisionMode` resolves it instead: `'downscale'` shrinks the widget into whatever room is at the drop cell. Within one board, `'swap'` trades places with the widget already there. Across boards, the same mode downscales into an empty cell without exchanging destination widgets, resolving an occupied anchor to the nearest cell that fits. None of these paths ever grows a widget. The examples below show all three behaviors. +On a free grid a drop onto occupied cells is refused. `collisionMode` resolves it instead: `'downscale'` shrinks the widget into whatever room is at the drop cell. Within one board, `'swap'` trades places with the widget already there. Across boards, the same mode downscales into an empty cell without exchanging destination widgets, and an occupied anchor commits the previewed cell instead of cancelling. None of these paths ever grows a widget. The examples below show all three behaviors. diff --git a/src/components/layout/Board/Board.stories.tsx b/src/components/layout/Board/Board.stories.tsx index ba12ed431..39dd6b24a 100644 --- a/src/components/layout/Board/Board.stories.tsx +++ b/src/components/layout/Board/Board.stories.tsx @@ -685,7 +685,8 @@ const CollisionModesTemplate: StoryFn = () => ( collisionMode="swap" across boards — drag the incoming widget onto the target; it downscales into the room at the drop - cell, and dropping on the blocker sends it to the nearest cell that fits + cell, and dropping on the blocker commits the previewed cell instead of + cancelling @@ -743,7 +744,7 @@ CollisionModes.parameters = { docs: { description: { story: - 'A `compact="free"` board refuses a drop onto occupied cells; `collisionMode` resolves it instead. **Downscale** — drag the 4-column widget onto the middle row and it shrinks to the 3 columns free beside the blocker, instead of snapping back. **Swap within one board** — drop one widget onto another and they trade places: the dragged widget takes the other\'s cell, the displaced one takes the cell the drag began at, and each keeps as much of its own size as fits there. Exactly one widget is ever displaced, a drop straddling two of them trades with the one it covers most, and dragging back retraces the original arrangement. **Swap across boards** — there is no slot on the destination to trade back, so the arrival downscales into the room to its right and below and never moves a destination widget; an occupied anchor sends it to the nearest cell that fits rather than cancelling. Neither path ever grows a widget. The default, `"revert"`, is what every other story on this page shows: the widget snaps back.', + 'A `compact="free"` board refuses a drop onto occupied cells; `collisionMode` resolves it instead. **Downscale** — drag the 4-column widget onto the middle row and it shrinks to the 3 columns free beside the blocker, instead of snapping back. **Swap within one board** — drop one widget onto another and they trade places: the dragged widget takes the other\'s cell, the displaced one takes the cell the drag began at, and each keeps as much of its own size as fits there. Exactly one widget is ever displaced, a drop straddling two of them trades with the one it covers most, and dragging back retraces the original arrangement. **Swap across boards** — there is no slot on the destination to trade back, so the arrival downscales into the room to its right and below and never moves a destination widget; an occupied anchor commits the cell the preview was showing rather than cancelling. Neither path ever grows a widget. The default, `"revert"`, is what every other story on this page shows: the widget snaps back.', }, }, }; diff --git a/src/components/layout/Board/Board.test.tsx b/src/components/layout/Board/Board.test.tsx index 2fb2d2130..d47b473ae 100644 --- a/src/components/layout/Board/Board.test.tsx +++ b/src/components/layout/Board/Board.test.tsx @@ -2554,6 +2554,27 @@ describe('Board', () => { expect(onWidgetTransfer).toHaveBeenCalled(); }); + it('holds the last valid preview while sweeping over an occupied cell', () => { + const targetLayout = [{ i: 'b', x: 2, y: 0, w: 2, h: 1 }]; + const { start, moveTo, endAt, onTargetLayoutChange } = + setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 2, h: 1 }, targetLayout); + + start(); + moveTo(4, 0); + moveTo(2, 0); + endAt(2, 0); + + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + // Once a frame has found a cell, an occupied cell holds it there rather + // than hunting for a new one: sweeping across a widget must not make the + // placeholder jump somewhere the pointer never went. (0,0) is free here, + // so a relocating implementation would move it and be visibly wrong. + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 4, y: 0, w: 2, h: 1 }), + ); + }); + it('lands a downscale arrival on the board, never above its first row', () => { const targetLayout = [{ i: 'b', x: 0, y: 0, w: 6, h: 1 }]; const { dragTo, onTargetLayoutChange } = setupCrossBoardSwap( diff --git a/src/components/layout/Board/Board.tsx b/src/components/layout/Board/Board.tsx index 90f4d3dc0..582ce6653 100644 --- a/src/components/layout/Board/Board.tsx +++ b/src/components/layout/Board/Board.tsx @@ -306,8 +306,9 @@ export interface CubeBoardProps * the drop covers most - which takes the cell the drag began at (falling back to * `'downscale'`, then `'revert'`). For a widget arriving from another board there * is no slot to trade back, so `'swap'` behaves as `'downscale'`: the widget - * downscales into the room to its right/below and an occupied anchor resolves to - * the nearest cell it fits in rather than cancelling the transfer. No mode ever + * downscales into the room to its right/below, and an occupied anchor holds the + * cell the preview last found (the nearest fitting one, if the widget arrived + * over an occupied cell) rather than cancelling the transfer. No mode ever * grows a widget, an * in-board swap never displaces more than one widget, and a drop that spans two * widgets trades with one of them rather than blinking away mid-drag. diff --git a/src/components/layout/Board/use-board-registry.ts b/src/components/layout/Board/use-board-registry.ts index 7557fed42..87bb4e86f 100644 --- a/src/components/layout/Board/use-board-registry.ts +++ b/src/components/layout/Board/use-board-registry.ts @@ -90,16 +90,21 @@ function hasNewOverlap(before: Set, after: LayoutItem[]): boolean { } /** - * Where a cross-board arrival ends up when the destination refuses the cell it - * was dropped on. + * Where a cross-board arrival ends up when the destination refuses the very first + * cell it was offered. * * `moveElement` answers a refused placement by restoring the item to where it - * came from, and for an incoming widget that is the synthetic seed one cell above - * (or left of) the anchor - a cell the pointer never chose, and one that sits off - * the board entirely whenever the anchor is in row 0 or column 0. A widget - * released over a board has to end up somewhere on it, so send it to the nearest - * cell that can hold it instead. The destination's own widgets are the fixed - * `others` here: an arrival is placed around them and never displaces one. + * came from. Every frame after the first has a real cell to go back to - the one + * the previous frame settled on - so the preview simply holds there, which is + * what keeps the placeholder from jumping about as the pointer sweeps across an + * occupied widget. The *first* frame on a board has no such cell: its origin is + * the synthetic seed one row above (or column left of) the anchor, which sits off + * the board entirely whenever the anchor is in row 0 or column 0. Holding that + * would preview - and commit - a widget outside the grid. + * + * So only that first frame relocates, to the nearest cell that can hold the + * widget. The destination's own widgets are the fixed `others`: an arrival is + * placed around them and never displaces one. * * Returns `layout` untouched when the requested cell was honoured, which covers * both a clean landing and a resolver that downscaled the item into it. @@ -952,7 +957,19 @@ export function useBoardRegistry( ); const compacted = [ ...compactor.compact( - landIncoming(moved, base, item.i, x, y, pp.cols, target.getMaxRows()), + // Only the seeded first frame may relocate; later frames fall back to + // the cell the previous frame settled on, so the preview holds. + prevItem + ? moved + : landIncoming( + moved, + base, + item.i, + x, + y, + pp.cols, + target.getMaxRows(), + ), pp.cols, ), ]; From 5372cbe2e3ba80bc3ce18980bd5a75ac80a5fd95 Mon Sep 17 00:00:00 2001 From: Andrey Yamanov Date: Thu, 27 Aug 2026 12:24:00 +0200 Subject: [PATCH 3/4] fix(Board): downscale into free room on either side of a blocker `gridBounds` clamps a drag anchor to `cols - w`, which assumes the widget keeps the size it started with. When the blocker sat to the LEFT and the room to its right was narrower than the widget, every anchor a pointer could produce landed inside the blocker, so `downscale` was never offered the room it exists to shrink into and the drop reverted. The mirror case always worked, because column 0 is reachable whatever the widget's width - downscaling worked on one side of a blocker and not the other. A blocked drop now also considers the cells the widget already covers that the clamp put out of reach, taking the largest fit among them. Anchors a pointer could have selected on its own are deliberately excluded, so every drop that resolved before resolves to exactly the same cell. The row axis clamps the same way, so a finite `maxRows` gets the same fix below a blocker. `landIncoming` now reads a refusal off the seed origin rather than off the requested cell, since a resolver is allowed to place the widget somewhere other than the exact cell asked for - which this change makes it do. The cross-board story moves its blocker to the left so it demonstrates the direction that was broken. Co-Authored-By: Claude Opus 5 --- .changeset/board-downscale-clamped-anchors.md | 9 ++ src/components/layout/Board/Board.stories.tsx | 8 +- src/components/layout/Board/Board.test.tsx | 22 +++++ .../Board/grid-core/collision-modes.test.ts | 56 +++++++++++ .../layout/Board/grid-core/collision-modes.ts | 94 ++++++++++++++++--- .../layout/Board/use-board-registry.ts | 32 +++++-- 6 files changed, 195 insertions(+), 26 deletions(-) create mode 100644 .changeset/board-downscale-clamped-anchors.md diff --git a/.changeset/board-downscale-clamped-anchors.md b/.changeset/board-downscale-clamped-anchors.md new file mode 100644 index 000000000..c3a3a7829 --- /dev/null +++ b/.changeset/board-downscale-clamped-anchors.md @@ -0,0 +1,9 @@ +--- +'@cube-dev/ui-kit': patch +--- + +`` (and the cross-board half of `"swap"`) now shrinks a widget into free room on **either** side of a blocker, not just the left. + +`gridBounds` clamps a drag anchor to `cols - w`, which assumes the widget keeps the width it started with. So when the blocker sat to the left and the room to its right was narrower than the widget, every anchor a pointer could produce landed inside the blocker — the mode whose whole job is to shrink the widget into that room never got offered it, and the drop just reverted. Dropping into room on the _left_ always worked, because column 0 is reachable whatever the widget's width. + +A blocked drop now also considers the cells the widget already covers that the clamp put out of the pointer's reach, taking the largest fit among them. Anchors a pointer could have selected on its own are untouched, so every drop that resolved before resolves to exactly the same cell. The row axis clamps the same way, so boards with a finite `maxRows` get the same fix below a blocker. diff --git a/src/components/layout/Board/Board.stories.tsx b/src/components/layout/Board/Board.stories.tsx index 39dd6b24a..079c29ee9 100644 --- a/src/components/layout/Board/Board.stories.tsx +++ b/src/components/layout/Board/Board.stories.tsx @@ -684,9 +684,9 @@ const CollisionModesTemplate: StoryFn = () => ( collisionMode="swap" across boards — drag the - incoming widget onto the target; it downscales into the room at the drop - cell, and dropping on the blocker commits the previewed cell instead of - cancelling + incoming widget onto the room right of the blocker and it downscales + into it, even though 4 columns never fit there; dropping on the blocker + itself commits the previewed cell instead of cancelling @@ -721,7 +721,7 @@ const CollisionModesTemplate: StoryFn = () => ( collisionMode="swap" showGridLines="drag" widgetProps={{ isCard: true }} - defaultLayout={[{ i: 'target-blocker', x: 3, y: 0, w: 3, h: 1 }]} + defaultLayout={[{ i: 'target-blocker', x: 0, y: 0, w: 3, h: 1 }]} > { ); }); + it('downscales into room to the right of a blocker, not just the left', () => { + // The anchor clamp (`cols - w` = 2) puts every cell of the free room out + // of the pointer's reach, so this drop used to miss the downscale + // entirely and land the widget on the next row instead. + const targetLayout = [{ i: 'b', x: 0, y: 0, w: 3, h: 1 }]; + const { start, moveTo, endAt, onTargetLayoutChange } = + setupCrossBoardSwap({ i: 'a', x: 0, y: 0, w: 4, h: 1 }, targetLayout); + + start(); + moveTo(3, 0); + endAt(3, 0); + + const committed = onTargetLayoutChange.mock + .calls[0]![0] as LayoutItem[]; + expect(committed.find((it) => it.i === 'a')).toEqual( + expect.objectContaining({ x: 3, y: 0, w: 3, h: 1 }), + ); + expect(committed.find((it) => it.i === 'b')).toEqual( + expect.objectContaining(targetLayout[0]), + ); + }); + it('lands a downscale arrival on the board, never above its first row', () => { const targetLayout = [{ i: 'b', x: 0, y: 0, w: 6, h: 1 }]; const { dragTo, onTargetLayoutChange } = setupCrossBoardSwap( diff --git a/src/components/layout/Board/grid-core/collision-modes.test.ts b/src/components/layout/Board/grid-core/collision-modes.test.ts index aa2233b50..cf263e32a 100644 --- a/src/components/layout/Board/grid-core/collision-modes.test.ts +++ b/src/components/layout/Board/grid-core/collision-modes.test.ts @@ -216,6 +216,62 @@ describe('collisionMode: downscale', () => { expect(rects(next).a).toBe('0,4 2x2'); }); + it('shrinks into room on either side of a blocker, not just the left', () => { + // `gridBounds` clamps a drag anchor to `cols - w`, so a 4-wide widget on a + // 6-column grid can never be anchored past column 2. With the blocker on the + // left that is every cell of the free room, and the drop used to revert - + // downscaling worked on one side of a blocker and not the other. + const roomOnTheRight = place( + [item('a', 0, 4, 4, 2), item('b', 0, 0, 3, 2)], + 'a', + 2, + 0, + 'downscale', + ); + expect(rects(roomOnTheRight)).toEqual({ a: '3,0 3x2', b: '0,0 3x2' }); + expect(isOverlapFree(roomOnTheRight)).toBe(true); + + // The mirror image, which has always worked because column 0 is reachable. + const roomOnTheLeft = place( + [item('a', 0, 4, 4, 2), item('b', 3, 0, 3, 2)], + 'a', + 0, + 0, + 'downscale', + ); + expect(rects(roomOnTheLeft)).toEqual({ a: '0,0 3x2', b: '3,0 3x2' }); + }); + + it('takes the largest of the cells the anchor clamp hid', () => { + // Free room in columns 3-5, so anchoring at 3 (3 wide) beats 4 (2 wide) and + // 5 (1 wide). All three are past the clamp at column 2. + const next = place( + [item('a', 0, 4, 4, 1), item('b', 0, 0, 3, 1)], + 'a', + 2, + 0, + 'downscale', + ); + + expect(rects(next).a).toBe('3,0 3x1'); + }); + + it('recovers hidden cells on the row axis too', () => { + // The row clamp is the same rule: a 4-tall widget on a 6-row grid cannot be + // anchored past row 2, and the blocker covers rows 0-2 of that column. + const next = place( + [item('a', 4, 0, 2, 4), item('b', 0, 0, 2, 3)], + 'a', + 0, + 2, + 'downscale', + { maxRows: 6 }, + ); + + expect(rects(next).a).toBe('0,3 2x3'); + expect(isOverlapFree(next)).toBe(true); + }); + it('respects the row limit when shrinking downward', () => { const layout = [item('a', 4, 0, 2, 4), item('b', 0, 2, 2, 2)]; const next = place(layout, 'a', 0, 0, 'downscale', { maxRows: 4 }); diff --git a/src/components/layout/Board/grid-core/collision-modes.ts b/src/components/layout/Board/grid-core/collision-modes.ts index 186f3c637..5103ca6b8 100644 --- a/src/components/layout/Board/grid-core/collision-modes.ts +++ b/src/components/layout/Board/grid-core/collision-modes.ts @@ -104,9 +104,55 @@ export function maxFreeRectAt( return best; } +/** + * The cells the widget already covers but a drag could never anchor on. + * + * `gridBounds` clamps a drag anchor to `cols - w` (and `maxRows - h`), which + * assumes the widget keeps the size it started with. So when the blocker is to + * the LEFT of the free room and that room is narrower than the widget, every + * anchor a pointer can produce lands inside the blocker - and the mode whose + * whole job is to shrink the widget into that room never gets offered it. The + * drop just reverts. The mirror case has no such problem, because the anchor at + * column 0 is always reachable: that asymmetry is why downscaling into room on + * the left has always worked and room on the right has not. The row axis clamps + * the same way, so a board with a finite `maxRows` has the same blind spot below + * a blocker. + * + * Only the cells the clamp put out of reach are returned. Anchors a pointer + * could have selected by itself are deliberately absent, so every drop that + * resolves today keeps resolving to exactly the same cell. + */ +function clampedOutAnchors( + item: LayoutItem, + desired: { w: number; h: number }, + limits: { cols: number; maxRows?: number }, +): { x: number; y: number }[] { + const maxRows = limits.maxRows ?? Infinity; + const bounded = Number.isFinite(maxRows); + const xLimit = Math.max(0, limits.cols - desired.w); + const yLimit = bounded ? Math.max(0, maxRows - desired.h) : Infinity; + + const lastX = Math.min(item.x + desired.w - 1, limits.cols - 1); + const lastY = bounded + ? Math.min(item.y + desired.h - 1, maxRows - 1) + : item.y + desired.h - 1; + + const anchors: { x: number; y: number }[] = []; + for (let y = item.y; y <= lastY; y++) { + for (let x = item.x; x <= lastX; x++) { + if (x === item.x && y === item.y) continue; + if (x > xLimit || y > yLimit) anchors.push({ x, y }); + } + } + return anchors; +} + /** * Shrink the placement to the room actually available at the cell it was - * dropped on. + * dropped on, or - when that cell cannot take the widget at any size - at the + * nearest cell the anchor clamp hid from the pointer (see `clampedOutAnchors`). + * The largest fit among those wins, so the widget takes as much of the room it is + * already hovering as there is. */ function downscaleInPlace( layout: Layout, @@ -115,20 +161,44 @@ function downscaleInPlace( desired: { w: number; h: number }, limits: { cols: number; maxRows?: number }, ): LayoutItem[] | null { - const fit = maxFreeRectAt( - others, - { x: item.x, y: item.y }, - desired, - limits, - item, - ); + const fitAt = (anchor: { x: number; y: number }) => + maxFreeRectAt(others, anchor, desired, limits, item); + + let anchor = { x: item.x, y: item.y }; + let fit = fitAt(anchor); + + if (!fit) { + let bestArea = 0; + for (const candidate of clampedOutAnchors(item, desired, limits)) { + const candidateFit = fitAt(candidate); + if (!candidateFit) continue; + const area = candidateFit.w * candidateFit.h; + // `>` keeps the first of several equal-area fits, and the scan runs in + // reading order from the drop cell, so that is the closest one to it. + if (area > bestArea) { + bestArea = area; + fit = candidateFit; + anchor = candidate; + } + } + } + if (!fit) return null; - // Nothing to resolve: the requested size already fits, so the collision was - // not a sizing problem and reverting is still the honest answer. - if (fit.w === item.w && fit.h === item.h) return null; + // Nothing to resolve: the requested cell takes the requested size, so the + // collision was not a sizing problem and reverting is still the honest answer. + if ( + fit.w === item.w && + fit.h === item.h && + anchor.x === item.x && + anchor.y === item.y + ) { + return null; + } return layout.map((l) => - l.i === item.i ? { ...l, w: fit.w, h: fit.h, moved: true } : l, + l.i === item.i + ? { ...l, x: anchor.x, y: anchor.y, w: fit!.w, h: fit!.h, moved: true } + : l, ); } diff --git a/src/components/layout/Board/use-board-registry.ts b/src/components/layout/Board/use-board-registry.ts index 87bb4e86f..61a93db25 100644 --- a/src/components/layout/Board/use-board-registry.ts +++ b/src/components/layout/Board/use-board-registry.ts @@ -106,22 +106,30 @@ function hasNewOverlap(before: Set, after: LayoutItem[]): boolean { * widget. The destination's own widgets are the fixed `others`: an arrival is * placed around them and never displaces one. * - * Returns `layout` untouched when the requested cell was honoured, which covers - * both a clean landing and a resolver that downscaled the item into it. + * A refusal is read off `origin` - the seed itself - rather than off the + * requested cell, because a resolver is allowed to place the widget somewhere + * other than the exact cell asked for (downscaling into room the anchor clamp hid + * from the pointer does exactly that). Only an item sitting back on its origin was + * actually refused; anything else is a resolution and stands. */ function landIncoming( layout: LayoutItem[], others: LayoutItem[], itemId: string, - x: number, - y: number, + origin: { x: number; y: number }, + landing: { x: number; y: number }, cols: number, maxRows?: number, ): LayoutItem[] { const placed = getLayoutItem(layout, itemId); - if (!placed || (placed.x === x && placed.y === y)) return layout; + if (!placed || placed.x !== origin.x || placed.y !== origin.y) return layout; - const landed = placeInFreeSlot(others, { ...placed, x, y }, cols, maxRows); + const landed = placeInFreeSlot( + others, + { ...placed, ...landing }, + cols, + maxRows, + ); return layout.map((l) => (l.i === itemId ? { ...l, ...landed } : l)); } @@ -925,6 +933,9 @@ export function useBoardRegistry( ? { ...item, x: Math.max(0, x) - 1, y } : { ...item, x, y: Math.max(0, y) - 1 }; const working = [...base, dragged]; + // Captured before the move: `moveElement` mutates the item in place, and a + // refusal is recognised by the item being back on this exact cell. + const origin = { x: dragged.x, y: dragged.y }; const moved = moveElement( working, @@ -965,8 +976,8 @@ export function useBoardRegistry( moved, base, item.i, - x, - y, + origin, + { x, y }, pp.cols, target.getMaxRows(), ), @@ -1201,6 +1212,7 @@ export function useBoardRegistry( ), newItem, ]; + const seed = { x: newItem.x, y: newItem.y }; const moved = moveElement( base, newItem, @@ -1232,8 +1244,8 @@ export function useBoardRegistry( moved, base.filter((l) => l.i !== ds.itemId), ds.itemId, - landing.x, - landing.y, + seed, + landing, tp.cols, tp.maxRows, ), From e071e92308131b6eae54319731d19426806ec33e Mon Sep 17 00:00:00 2001 From: Andrey Yamanov Date: Thu, 27 Aug 2026 12:41:10 +0200 Subject: [PATCH 4/4] fix(Board): only recover clamped anchors when the anchor is pinned `clampedOutAnchors` filtered out the cells a pointer could reach, but never checked whether the anchor itself was actually pinned at the limit. A blocked drop at a freely chosen cell therefore recovered the far cells anyway: a 4-wide widget dropped on a 1-wide blocker at column 0 of a 6-column grid landed at column 3, skipping columns 1 and 2 - both free, both reachable - and taking a rectangle outside the footprint it was hovering. For a cross-board arrival this also outranked `landIncoming`, which would have placed it in the nearest free slot. Recovery is now gated on the anchor sitting on the limit, per axis, and slides only along the axis that pinned it. A pinned span ends at the last row/column, so every recovered rectangle now provably lies inside the hovered footprint - which is what the comment claimed all along. Reported by Cursor Bugbot on #1370. Co-Authored-By: Claude Opus 5 --- .changeset/board-downscale-clamped-anchors.md | 2 +- .../Board/grid-core/collision-modes.test.ts | 17 ++++++++ .../layout/Board/grid-core/collision-modes.ts | 42 +++++++++++-------- 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/.changeset/board-downscale-clamped-anchors.md b/.changeset/board-downscale-clamped-anchors.md index c3a3a7829..0a25adf0b 100644 --- a/.changeset/board-downscale-clamped-anchors.md +++ b/.changeset/board-downscale-clamped-anchors.md @@ -6,4 +6,4 @@ `gridBounds` clamps a drag anchor to `cols - w`, which assumes the widget keeps the width it started with. So when the blocker sat to the left and the room to its right was narrower than the widget, every anchor a pointer could produce landed inside the blocker — the mode whose whole job is to shrink the widget into that room never got offered it, and the drop just reverted. Dropping into room on the _left_ always worked, because column 0 is reachable whatever the widget's width. -A blocked drop now also considers the cells the widget already covers that the clamp put out of the pointer's reach, taking the largest fit among them. Anchors a pointer could have selected on its own are untouched, so every drop that resolved before resolves to exactly the same cell. The row axis clamps the same way, so boards with a finite `maxRows` get the same fix below a blocker. +A blocked drop whose anchor is **pinned against the grid edge** now also considers the cells beyond it that the clamp hid, taking the largest fit among them. An anchor short of the limit is the pointer's own choice — every cell between it and the limit was available to aim at — so a blocked drop there still reverts, and every drop that resolved before resolves to exactly the same cell. The row axis clamps the same way, so boards with a finite `maxRows` get the same fix below a blocker. diff --git a/src/components/layout/Board/grid-core/collision-modes.test.ts b/src/components/layout/Board/grid-core/collision-modes.test.ts index cf263e32a..95339cbc9 100644 --- a/src/components/layout/Board/grid-core/collision-modes.test.ts +++ b/src/components/layout/Board/grid-core/collision-modes.test.ts @@ -256,6 +256,23 @@ describe('collisionMode: downscale', () => { expect(rects(next).a).toBe('3,0 3x1'); }); + it('never reaches past a cell the pointer could have aimed at', () => { + // Same 4-wide widget and the same clamp at column 2, but the drop is at + // column 0 - the pointer's own choice, with columns 1 and 2 free and just as + // reachable. Recovering the far columns here would skip over those and land + // the widget three columns from where it was dropped, so a blocked drop that + // is not pinned against the edge still reverts. + const next = place( + [item('a', 0, 4, 4, 1), item('b', 0, 0, 1, 1)], + 'a', + 0, + 0, + 'downscale', + ); + + expect(rects(next).a).toBe('0,4 4x1'); + }); + it('recovers hidden cells on the row axis too', () => { // The row clamp is the same rule: a 4-tall widget on a 6-row grid cannot be // anchored past row 2, and the blocker covers rows 0-2 of that column. diff --git a/src/components/layout/Board/grid-core/collision-modes.ts b/src/components/layout/Board/grid-core/collision-modes.ts index 5103ca6b8..27a77273e 100644 --- a/src/components/layout/Board/grid-core/collision-modes.ts +++ b/src/components/layout/Board/grid-core/collision-modes.ts @@ -105,7 +105,7 @@ export function maxFreeRectAt( } /** - * The cells the widget already covers but a drag could never anchor on. + * The cells beyond a pinned anchor that a drag could never ask for. * * `gridBounds` clamps a drag anchor to `cols - w` (and `maxRows - h`), which * assumes the widget keeps the size it started with. So when the blocker is to @@ -118,9 +118,16 @@ export function maxFreeRectAt( * the same way, so a board with a finite `maxRows` has the same blind spot below * a blocker. * - * Only the cells the clamp put out of reach are returned. Anchors a pointer - * could have selected by itself are deliberately absent, so every drop that - * resolves today keeps resolving to exactly the same cell. + * Nothing is offered unless the anchor is sitting ON the limit, pinned against + * the far edge with the pointer unable to push it further. An anchor short of the + * limit is the pointer's own choice - every cell between it and the limit was + * available to ask for - so a blocked drop there still just reverts, exactly as + * before. Without that gate a widget dropped on an occupied cell would skip over + * free cells the pointer could perfectly well have aimed at. + * + * A pinned anchor's span ends at the last row/column of the grid, so every cell + * returned - and every rectangle that can be measured from one - lies inside the + * footprint the widget is already covering. */ function clampedOutAnchors( item: LayoutItem, @@ -132,27 +139,26 @@ function clampedOutAnchors( const xLimit = Math.max(0, limits.cols - desired.w); const yLimit = bounded ? Math.max(0, maxRows - desired.h) : Infinity; - const lastX = Math.min(item.x + desired.w - 1, limits.cols - 1); - const lastY = bounded - ? Math.min(item.y + desired.h - 1, maxRows - 1) - : item.y + desired.h - 1; - const anchors: { x: number; y: number }[] = []; - for (let y = item.y; y <= lastY; y++) { - for (let x = item.x; x <= lastX; x++) { - if (x === item.x && y === item.y) continue; - if (x > xLimit || y > yLimit) anchors.push({ x, y }); - } + // Each axis is clamped on its own, so each recovers on its own: slide along + // the one the limit actually pinned, never diagonally into open grid. + if (item.x === xLimit) { + const lastX = Math.min(item.x + desired.w - 1, limits.cols - 1); + for (let x = item.x + 1; x <= lastX; x++) anchors.push({ x, y: item.y }); + } + if (bounded && item.y === yLimit) { + const lastY = Math.min(item.y + desired.h - 1, maxRows - 1); + for (let y = item.y + 1; y <= lastY; y++) anchors.push({ x: item.x, y }); } return anchors; } /** * Shrink the placement to the room actually available at the cell it was - * dropped on, or - when that cell cannot take the widget at any size - at the - * nearest cell the anchor clamp hid from the pointer (see `clampedOutAnchors`). - * The largest fit among those wins, so the widget takes as much of the room it is - * already hovering as there is. + * dropped on, or - when that cell cannot take the widget at any size and the + * anchor is pinned against the grid edge - at a cell the clamp hid from the + * pointer (see `clampedOutAnchors`). The largest fit among those wins, so the + * widget takes as much of the room it is already hovering as there is. */ function downscaleInPlace( layout: Layout,