adobe / adobe/react-spectrum

Virtualized Tree/GridList drop indicator is offset when the collection is scrolled by an ancestor

Open
#10,597 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
15.9k
Forks
1.6k
Avg merge
3d 9m
Merged PRs (30d)
59

Description

## Provide a general summary of the issue here

When a virtualized Tree or GridList with dragAndDropHooks sits inside a scrollable ancestor (so the ancestor scrolls while the collection itself stays at scrollTop 0), the drop indicator drifts away from the pointer during a drag. The offset grows with how far the ancestor has scrolled and eventually the indicator lands off screen entirely.

## Expected Behavior

The drop indicator should track the pointer accurately no matter whether the collection scrolled itself or an ancestor scrolled around it.

## Current Behavior

Virtualizer hardcodes allowsWindowScrolling: true (packages/react-aria-components/src/Virtualizer.tsx, lines 109 and 140). Because of that, visibleRect on the virtualizer state folds in viewportOffset, a value ScrollView tracks whenever an ancestor scroll moves the collection element above y=0 in the viewport (packages/react-aria/src/virtualizer/ScrollView.tsx, the onScroll branch for target !== ref.current).

ListLayout.getDropTargetFromPoint does this (packages/react-stately/src/layout/ListLayout.ts, around line 740):

x += this.virtualizer.visibleRect.x;
y += this.virtualizer.visibleRect.y;

to turn the incoming pointer position into content space coordinates. That incoming x, y already comes from useDrop (packages/react-aria/src/dnd/useDrop.ts), computed as event.clientX/clientY minus the collection element's getBoundingClientRect(). Since getBoundingClientRect reflects the element's current position on screen, that value already accounts for however far the ancestor has scrolled the element. Adding visibleRect on top adds the same ancestor offset a second time, and the error grows with scroll depth.

## Possible Solution

This looks like it belongs in ListLayout.getDropTargetFromPoint (and the matching spot in TableLayout), where only the collection's own internal scroll offset should be added rather than the whole visibleRect once viewportOffset is nonzero. We are not fully sure this is the cleanest fix given allowsWindowScrolling is meant to support real window scrolling too, so we would rather describe the behavior than assert the exact patch. Downstream we worked around it by subclassing ListLayout, reading the collection element's own scrollLeft and scrollTop directly, and adjusting the point passed into super.getDropTargetFromPoint so its addition of visibleRect nets out to just that local scroll value. Happy to share that subclass or open a PR if that direction sounds reasonable.

## Context

We hit this in Phi, Sigma Computing's internal design system, where a Tree with drag and drop reordering is nested inside an outer scrollable panel. With enough rows the outer panel scrolls instead of the tree itself, and the drop indicator becomes unusable after a couple of screens of scroll. We have a local workaround, but wanted to report the root cause since it would affect any virtualized collection with drag and drop nested inside a scrollable ancestor, not just our case.

## Steps to Reproduce

1. Render a Tree or GridList from react-aria-components with dragAndDropHooks enabled and enough rows to be virtualized.
2. Put it inside an outer div with overflow auto and a fixed height, where the outer div is what actually scrolls rather than the collection's own scrollbar.
3. Scroll the outer div down so the collection element's bounding rect top moves above the outer div's visible top edge.
4. Start dragging a row and move the pointer near the top, middle, and bottom of the visible rows.
5. Compare the rendered drop indicator against the pointer. It will be offset, growing with how far the outer div has scrolled, and disappears once the offset exceeds the visible area.

We can put together a StackBlitz if the trace above is not enough to reproduce locally.

## Version

react-aria-components 1.21.1, react-stately 3.50.0, react-aria 3.52.1

## Browsers

Chrome, Firefox, Safari

## Operating System

macOS

Contributor guide

Open the contributing guide

Research direction

Start by tracing pointer coordinates through packages/react-aria/src/dnd/useDrop.ts and packages/react-stately/src/layout/ListLayout.ts, then compare the corresponding logic in TableLayout.ts with viewportOffset handling in packages/react-aria/src/virtualizer/ScrollView.tsx and packages/react-aria-components/src/Virtualizer.tsx. Reproduce the reported nested-scroll case with a virtualized Tree or GridList and drag and drop enabled; done means the indicator tracks the pointer after the ancestor scrolls.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.