react-spectrum Fix virtualizer persisted keys with drag and drop

Depends on #6640

This fixes the virtualizer persistedKeys during drag and drop so that keyboard navigation works correctly and focus isn't lost. Since we only render one of the drop positions (before or after), we need to normalize the key we persist accordingly so the correct one is in the DOM and able to get focus.

I've also changed the API on Virtualizer and collection renderers in general to accept persistedKeys as a prop rather than a single focusedKey. This enables multiple keys to be persisted and is more extensible for future use cases.

Finally, based on feedback in https://github.com/adobe/react-spectrum/pull/6631#discussion_r1657916779, I've renamed the Layout validate method to update to make it easier to understand. This was originally pulled from v2 CollectionView but since we are doing a breaking change we can rename it now.

Jun 28 '24 19:06 devongovett

Build successful! 🎉

Jun 28 '24 19:06 rspbot

Hmm yeah that seems to be an issue with persisted keys in TableLayout...

Jun 28 '24 22:06 devongovett

Ok fixed 3 issues:

TableLayout was using the wrong indices to calculate persisted keys. When we perform an incremental layout, the index should be a count of the items that we actually have built, not all of the nodes in the collection. For example, if we start layout from the middle, there will be items before that we didn't layout. This matches how firstVisibleRow and lastVisibleRow are computed.
In Spectrum, revert back to only persisting either the focused key or the drop target, not both. In RAC we need to persist both to avoid the dragged element going out of view, getting removed from the DOM, and triggering the end of the drag session due to this layout effect. This doesn't happen in Spectrum for some reason. On the flip side, we can't persist both in Spectrum because when dragging ends, there seems to be a race where the focusedKey isn't updated to the dropped item right away and we scroll back to the original focused item from before the drop. I can't reproduce this in RAC. Not quite sure what the difference is...
@LFDanLu noticed that the async loading story was scrolling to the top, because we were placing the loading indicator in the wrong spot.

Jun 28 '24 23:06 devongovett

Build successful! 🎉

Jun 28 '24 23:06 rspbot

Brain dump before break:

Actually I did manage to reproduce (2) in RSP as well, so we should persist the drag target as well as the drop target. It only occurs when the drag target element is removed from the dom and not reused by another view, most commonly with variable row heights.

The other issue with scrolling to the top on drop also only seems to occur in the non-util handlers stories, I think because the onDrop is async there and we refocus in DragManager's end method prior to the items being moved therefore scrolling to the wrong position. If you delay that by a micro task it works, but this breaks a lot of tests.

Jun 29 '24 01:06 devongovett

Build successful! 🎉

Jul 09 '24 20:07 rspbot

Build successful! 🎉

Jul 09 '24 21:07 rspbot

## API Changes

unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any', access: 'private' } unknown top level export { type: 'any', access: 'private' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'identifier', name: 'Column' } unknown top level export { type: 'identifier', name: 'Column' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown type { type: 'link' } unknown type { type: 'link' } unknown type { type: 'link' } unknown type { type: 'link' } unknown type { type: 'link' } unknown type { type: 'link' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' } unknown top level export { type: 'any' }

@react-aria/virtualizer

Virtualizer

 Virtualizer<O, T extends {}, V extends ReactNode> {
   children: (string, {}) => ReactNode
   collection: Collection<{}>
-  focusedKey?: Key
   isLoading?: boolean
   layout: Layout<{}, O>
   layoutOptions?: O
   onLoadMore?: () => void
+  persistedKeys?: Set<Key> | null
   renderWrapper?: RenderWrapper<{}, ReactNode>
   scrollDirection?: 'horizontal' | 'vertical' | 'both'
   sizeToFit?: 'width' | 'height'
 }

@react-stately/layout

GridLayout

 GridLayout<O = any, T> {
   constructor: (GridLayoutOptions) => void
   getContentSize: () => Size
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
   getDropTargetLayoutInfo: (ItemDropTarget) => LayoutInfo
   getLayoutInfo: (Key) => LayoutInfo | null
   getVisibleLayoutInfos: (Rect) => Array<LayoutInfo>
-  validate: () => void
+  update: () => void
 }

ListLayout

 ListLayout<O = any, T> {
   constructor: (ListLayoutOptions) => void
   getContentSize: () => void
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
   getDropTargetLayoutInfo: (ItemDropTarget) => LayoutInfo
   getLayoutInfo: (Key) => void
   getVisibleLayoutInfos: (Rect) => void
+  update: (InvalidationContext<O>) => void
   updateItemSize: (Key, Size) => void
-  validate: (InvalidationContext<O>) => void
 }

TableLayout

 TableLayout<O extends TableLayoutProps = TableLayoutProps, T> {
   constructor: (ListLayoutOptions) => void
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
   getDropTargetLayoutInfo: (ItemDropTarget) => LayoutInfo
   getVisibleLayoutInfos: (Rect) => void
-  validate: (InvalidationContext<TableLayoutProps>) => void
+  update: (InvalidationContext<TableLayoutProps>) => void
 }

@react-stately/virtualizer

Layout

 Layout<O = any, T extends {}> {
   getContentSize: () => Size
   getDropTargetLayoutInfo: (ItemDropTarget) => LayoutInfo
   getItemRect: (Key) => Rect
   getLayoutInfo: (Key) => LayoutInfo | null
   getVisibleLayoutInfos: (Rect) => Array<LayoutInfo>
   getVisibleRect: () => Rect
   shouldInvalidate: (Rect, Rect) => boolean
+  update: (InvalidationContext<O>) => void
   updateItemSize: (Key, Size) => boolean
-  validate: (InvalidationContext<O>) => void
   virtualizer: Virtualizer<{}, any>
 }

Jul 09 '24 21:07 rspbot

In all versions, /story/listview-drag-and-drop-util-handlers--drop-onto-root dragging via mouse does not scroll to the dropped item. Dragging via the keyboard does. Might be fixed by the scroll anchoring?

/story/listview-drag-and-drop--drag-within-scroll Item overlaps/spacing can be wrong after dnd. Select item 1 via keyboard, hit 'end' key, drop the item. Note that there's a big gap between the last two items and two drop indicators will appear if you try to drop in that area. It works fine if you use mouse, I assume because then everything will have been laid out during the scrolling.

Those are the only issues I've found so far, and they aren't that big of issues.

Tested on Chrome and Safari, keyboard and mouse, RAC and RSP.

Jul 09 '24 23:07 snowystinger

dragging via mouse does not scroll to the dropped item

Looks like this was the case before as well. Probably due to scroll into view only occurring in keyboard modality...

Item overlaps/spacing can be wrong after dnd

This is strange, but the behavior is better than before where it didn't scroll at all. 😂

Jul 10 '24 17:07 devongovett