Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix virtualizer drag and drop with layout gaps #7886

Merged
merged 2 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions packages/@react-stately/layout/src/GridLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {DropTarget, DropTargetDelegate, ItemDropTarget, Key, Node} from '@react-types/shared';
import {InvalidationContext, Layout, LayoutInfo, Point, Rect, Size} from '@react-stately/virtualizer';
import {InvalidationContext, Layout, LayoutInfo, Rect, Size} from '@react-stately/virtualizer';

export interface GridLayoutOptions {
/**
Expand Down Expand Up @@ -102,6 +102,7 @@ export class GridLayout<T, O extends GridLayoutOptions = GridLayoutOptions> exte
// Compute the number of rows and columns needed to display the content
let columns = Math.floor(visibleWidth / (minItemSize.width + minSpace.width));
let numColumns = Math.max(1, Math.min(maxColumns, columns));
this.numColumns = numColumns;

// Compute the available width (minus the space between items)
let width = visibleWidth - (minSpace.width * Math.max(0, numColumns));
Expand Down Expand Up @@ -223,7 +224,46 @@ export class GridLayout<T, O extends GridLayoutOptions = GridLayoutOptions> exte
x += this.virtualizer!.visibleRect.x;
y += this.virtualizer!.visibleRect.y;

let key = this.virtualizer!.keyAtPoint(new Point(x, y));
// Find the closest item within on either side of the point using the gap width.
let key: Key | null = null;
if (this.numColumns === 1) {
let searchRect = new Rect(x, Math.max(0, y - this.gap.height), 1, this.gap.height * 2);
let candidates = this.getVisibleLayoutInfos(searchRect);
let minDistance = Infinity;
for (let candidate of candidates) {
// Ignore items outside the search rect, e.g. persisted keys.
if (!candidate.rect.intersects(searchRect)) {
continue;
}

let yDist = Math.abs(candidate.rect.y - x);
let maxYDist = Math.abs(candidate.rect.maxY - x);
Comment on lines +239 to +240
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe I'm having a brain fart and not visualizing this quite right, but shouldn't this be candidate.rect.y - y instead? Like comparing the candidate's top/bottom edge's Y position with the current point to find the closest edge -> reiterating on the next candidate to find the candidate that is closest to the drop point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, copy pasta

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let dist = Math.min(yDist, maxYDist);
if (dist < minDistance) {
minDistance = dist;
key = candidate.key;
}
}
} else {
let searchRect = new Rect(Math.max(0, x - this.gap.width), y, this.gap.width * 2, 1);
let candidates = this.getVisibleLayoutInfos(searchRect);
let minDistance = Infinity;
for (let candidate of candidates) {
// Ignore items outside the search rect, e.g. persisted keys.
if (!candidate.rect.intersects(searchRect)) {
continue;
}

let xDist = Math.abs(candidate.rect.x - x);
let maxXDist = Math.abs(candidate.rect.maxX - x);
let dist = Math.min(xDist, maxXDist);
if (dist < minDistance) {
minDistance = dist;
key = candidate.key;
}
}
}

let layoutInfo = key != null ? this.getLayoutInfo(key) : null;
if (!layoutInfo) {
return {type: 'root'};
Expand Down
23 changes: 21 additions & 2 deletions packages/@react-stately/layout/src/ListLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {Collection, DropTarget, DropTargetDelegate, ItemDropTarget, Key, Node} from '@react-types/shared';
import {getChildNodes} from '@react-stately/collections';
import {InvalidationContext, Layout, LayoutInfo, Point, Rect, Size} from '@react-stately/virtualizer';
import {InvalidationContext, Layout, LayoutInfo, Rect, Size} from '@react-stately/virtualizer';

export interface ListLayoutOptions {
/**
Expand Down Expand Up @@ -505,7 +505,26 @@ export class ListLayout<T, O extends ListLayoutOptions = ListLayoutOptions> exte
x += this.virtualizer!.visibleRect.x;
y += this.virtualizer!.visibleRect.y;

let key = this.virtualizer!.keyAtPoint(new Point(x, y));
// Find the closest item within on either side of the point using the gap width.
let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, this.gap * 2);
let candidates = this.getVisibleLayoutInfos(searchRect);
let key: Key | null = null;
let minDistance = Infinity;
for (let candidate of candidates) {
// Ignore items outside the search rect, e.g. persisted keys.
if (!candidate.rect.intersects(searchRect)) {
continue;
}

let yDist = Math.abs(candidate.rect.y - x);
let maxYDist = Math.abs(candidate.rect.maxY - x);
let dist = Math.min(yDist, maxYDist);
if (dist < minDistance) {
minDistance = dist;
key = candidate.key;
}
}

if (key == null || this.virtualizer!.collection.size === 0) {
return {type: 'root'};
}
Expand Down
26 changes: 16 additions & 10 deletions packages/@react-stately/layout/src/TableLayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,17 +542,23 @@ export class TableLayout<T, O extends TableLayoutProps = TableLayoutProps> exten
x += this.virtualizer!.visibleRect.x;
y += this.virtualizer!.visibleRect.y;

// Custom variation of this.virtualizer.keyAtPoint that ignores body
// Find the closest item within on either side of the point using the gap width.
let searchRect = new Rect(x, Math.max(0, y - this.gap), 1, this.gap * 2);
let candidates = this.getVisibleLayoutInfos(searchRect);
let key: Key | null = null;
let point = new Point(x, y);
let rectAtPoint = new Rect(point.x, point.y, 1, 1);
let layoutInfos = this.virtualizer!.layout.getVisibleLayoutInfos(rectAtPoint).filter(info => info.type === 'row');

// Layout may return multiple layout infos in the case of
// persisted keys, so find the first one that actually intersects.
for (let layoutInfo of layoutInfos) {
if (layoutInfo.rect.intersects(rectAtPoint)) {
key = layoutInfo.key;
let minDistance = Infinity;
for (let candidate of candidates) {
// Ignore items outside the search rect, e.g. persisted keys.
if (candidate.type !== 'row' || !candidate.rect.intersects(searchRect)) {
continue;
}

let yDist = Math.abs(candidate.rect.y - x);
let maxYDist = Math.abs(candidate.rect.maxY - x);
let dist = Math.min(yDist, maxYDist);
if (dist < minDistance) {
minDistance = dist;
key = candidate.key;
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/react-aria-components/stories/ListBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ export function VirtualizedListBoxDnd() {
<Virtualizer
layout={ListLayout}
layoutOptions={{
rowHeight: 25
rowHeight: 25,
gap: 8
}}>
<ListBox
className={styles.menu}
Expand Down