Load more collection element support in RAC Table and Tree (#6494)

* rough approach for having user provide loading element to rac collection element

* bare minimum for table loader support

* add first and last row/cell data-attributes

* updating collection to auto propagate dependencies and debugging example stories

* adding stories for empty state + loading variations in tree and table

* updating verdaccio to not crash on newer macbooks

* rip out first/last row and fix lint

* add TableLoader to index so that I can test in s2

* add isTableEmpty and colspan for table loader

also some debugging of crashes

* add renderProps to loader

* omit loader from the collection size in table

* fix bugs with keyboard navigation and load more in table

* Fix Tree keyboard nav issues and add tests

* fix keyboard nav with loaders via keyboard delegate item filtering logic

it makes more sense for the keyboard delegate to handle what keys should be returned on up/down/pageUp/etc

* clean up and use hook for loader row attributes

* refactor so that empty load more is expected to be a part of renderEmptyState

* useLoadMore support in RAC Table (#6538)

* initial implementation of load more hook

* initial stab at scrollable body table

* fix lint and readd content height change check

* update attributes and test virtualizer + loadmore

* remove option in prep for fix

* fix for loadMore and update virtualized RAC table stories

* get rid of loaderHeight option

* move hook to separate package

* add tests

* fix some stuff from rebase

* fix test for 16/17

* refactor useLoadOnScroll so it can replace useVirtualizer internals

* inconsistent 19 test is fixed now

* fix another 19 test

* fix crash when dragging over loading spinner

* only render loadmore spinner in dnd table if there are items

* get rid of todo now that layout has been refactored

* move logic to render after drop indicator when row is before logic into Collection

* move useLoadMore to util package for now

punting refactor and removal of virtualizer to later see note

* pull loadmore hook out of rac table

* rename loader collection element and update layout for variable loader height

* forgot to remove deps

* fix esm test?

* update useloadmore logic and replace useVirtualizer internals

note this removes the "wasLoading || ref.current.scrollHeight !== lastContentSize.current" portion of the logic. Previously useVirtualizer onVisibleRectChange was the one that would handle loading more items if opening a combobox dropdown during/after initial load, not the layouteffect. However, now that that is replaced by useLoadMore onScroll handler, we need the layouteffect to handle that

* remove useVirtualizer

* fix tests

* fix erroneous conditional hooks call

was causing a different order of hooks calls when a table with renderEmptyState went from empty to having a rows

* review comments and updating Tree loader naming for consistency

* dont apply width directly on columnheader if virtualizer

fix for S2, the wrapper around the columnheader already has the calculated width applied on it so we dont need it on the columnheader as well. This makes padding values applied on the columnheader take up room from current width instead of making the columnheader extra wide

* fix lint

* add items check to useLoadMore

interim fix to prevent some extra onLoadMore calls. RAC will eventually make use of the collection when the hook is moved inside the component whereas RSP table cant benefit from this since its collection is always rebuilt and thus non stable. Items are also provided to the TableBody and cant be accessed from with tablevirtualizer

* add display contents and debug sticky loader

* clean up from review

* test and story updates from review comments

---------

Co-authored-by: Devon Govett <[email protected]>

Author: LFDanLu

packages/@react-aria/dnd/src/ListDropTargetDelegate.ts

@@ -95,7 +95,11 @@ export class ListDropTargetDelegate implements DropTargetDelegate {
       }
     }
 
-    let items = [...this.collection];
+    // TODO: assume that only item type items are valid drop targets. This is to prevent a crash when dragging over the loader
+    // row since it doesn't have a data-key set on it. Will eventually need to handle the case with drag and drop and loaders located between rows aka tree.
+    // Can see https://github.com/adobe/react-spectrum/pull/4210/files#diff-21e555e0c597a28215e36137f5be076a65a1e1456c92cd0fdd60f866929aae2a for additional logic
+    // that may need to happen then
+    let items = [...this.collection].filter(item => item.type === 'item');
     let low = 0;
     let high = items.length;
     while (low < high) {

packages/@react-aria/grid/src/GridKeyboardDelegate.ts

@@ -101,7 +101,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
     }
 
     // Find the next item
-    key = this.findNextKey(key);
+    key = this.findNextKey(key, (item => item.type === 'item'));
     if (key != null) {
       // If focus was on a cell, focus the cell with the same index in the next row.
       if (this.isCell(startItem)) {
@@ -128,7 +128,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
     }
 
     // Find the previous item
-    key = this.findPreviousKey(key);
+    key = this.findPreviousKey(key, item => item.type === 'item');
     if (key != null) {
       // If focus was on a cell, focus the cell with the same index in the previous row.
       if (this.isCell(startItem)) {
@@ -232,7 +232,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
     }
 
     // Find the first row
-    key = this.findNextKey();
+    key = this.findNextKey(null, item => item.type === 'item');
 
     // If global flag is set (or if focus mode is cell), focus the first cell in the first row.
     if ((key != null && item && this.isCell(item) && global) || this.focusMode === 'cell') {
@@ -262,7 +262,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
     }
 
     // Find the last row
-    key = this.findPreviousKey();
+    key = this.findPreviousKey(null, item => item.type === 'item');
 
     // If global flag is set (or if focus mode is cell), focus the last cell in the last row.
     if ((key != null && item && this.isCell(item) && global) || this.focusMode === 'cell') {
@@ -303,13 +303,13 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
 
     while (itemRect && (itemRect.y + itemRect.height) < pageY) {
       let nextKey = this.getKeyBelow(key);
-      itemRect = this.layoutDelegate.getItemRect(nextKey);
-
-      // Guard against case where maxY of the last key is barely less than pageY due to rounding
-      // and thus it attempts to set key to null
-      if (nextKey != null) {
-        key = nextKey;
+      // If nextKey is undefined, we've reached the last row already
+      if (nextKey == null) {
+        break;
       }
+
+      itemRect = this.layoutDelegate.getItemRect(nextKey);
+      key = nextKey;
     }
 
     return key;
@@ -345,7 +345,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
         }
       }
 
-      key = this.findNextKey(key);
+      key = this.findNextKey(key, item => item.type === 'item');
 
       // Wrap around when reaching the end of the collection
       if (key == null && !hasWrapped) {

packages/@react-aria/table/src/useTableCell.ts

@@ -46,7 +46,6 @@ export interface TableCellAria {
  */
 export function useTableCell<T>(props: AriaTableCellProps, state: TableState<T>, ref: RefObject<FocusableElement | null>): TableCellAria {
   let {gridCellProps, isPressed} = useGridCell(props, state, ref);
-
   let columnKey = props.node.column.key;
   if (state.collection.rowHeaderColumnKeys.has(columnKey)) {
     gridCellProps.role = 'rowheader';

packages/@react-aria/utils/src/index.ts

@@ -41,3 +41,4 @@ export {isVirtualClick, isVirtualPointerEvent} from './isVirtualEvent';
 export {useEffectEvent} from './useEffectEvent';
 export {useDeepMemo} from './useDeepMemo';
 export {useFormReset} from './useFormReset';
+export {useLoadMore} from './useLoadMore';

packages/@react-aria/utils/src/useLoadMore.ts

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2024 Adobe. All rights reserved.
+ * This file is licensed to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
+ * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+ * OF ANY KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+
+import {RefObject, useCallback, useRef} from 'react';
+import {useEvent} from './useEvent';
+// eslint-disable-next-line rulesdir/useLayoutEffectRule
+import {useLayoutEffect} from './useLayoutEffect';
+
+export interface LoadMoreProps {
+  /** Whether data is currently being loaded. */
+  isLoading?: boolean,
+  /** Handler that is called when more items should be loaded, e.g. while scrolling near the bottom.  */
+  onLoadMore?: () => void,
+  /**
+   * The amount of offset from the bottom of your scrollable region that should trigger load more.
+   * Uses a percentage value relative to the scroll body's client height. Load more is then triggered
+   * when your current scroll position's distance from the bottom of the currently loaded list of items is less than
+   * or equal to the provided value. (e.g. 1 = 100% of the scroll region's height).
+   * @default 1
+   */
+  scrollOffset?: number,
+  /** The data currently loaded. */
+  items?: any[]
+}
+
+export function useLoadMore(props: LoadMoreProps, ref: RefObject<HTMLElement | null>) {
+  let {isLoading, onLoadMore, scrollOffset = 1, items} = props;
+
+  // Handle scrolling, and call onLoadMore when nearing the bottom.
+  let isLoadingRef = useRef(isLoading);
+  let prevProps = useRef(props);
+  let onScroll = useCallback(() => {
+    if (ref.current && !isLoadingRef.current && onLoadMore) {
+      let shouldLoadMore = ref.current.scrollHeight - ref.current.scrollTop - ref.current.clientHeight < ref.current.clientHeight * scrollOffset;
+
+      if (shouldLoadMore) {
+        isLoadingRef.current = true;
+        onLoadMore();
+      }
+    }
+  }, [onLoadMore, ref, scrollOffset]);
+
+  let lastItems = useRef(items);
+  useLayoutEffect(() => {
+    // Only update isLoadingRef if props object actually changed,
+    // not if a local state change occurred.
+    if (props !== prevProps.current) {
+      isLoadingRef.current = isLoading;
+      prevProps.current = props;
+    }
+
+    // TODO: Eventually this hook will move back into RAC during which we will accept the collection as a option to this hook.
+    // We will only load more if the collection has changed after the last load to prevent multiple onLoadMore from being called
+    // while the data from the last onLoadMore is being processed by RAC collection.
+    let shouldLoadMore = ref?.current
+      && !isLoadingRef.current
+      && onLoadMore
+      && (!items || items !== lastItems.current)
+      && ref.current.clientHeight === ref.current.scrollHeight;
+
+    if (shouldLoadMore) {
+      isLoadingRef.current = true;
+      onLoadMore?.();
+    }
+
+    lastItems.current = items;
+  }, [isLoading, onLoadMore, props, ref]);
+
+  // TODO: maybe this should still just return scroll props?
+  // Test against case where the ref isn't defined when this is called
+  // Think this was a problem when trying to attach to the scrollable body of the table in OnLoadMoreTableBodyScroll
+  useEvent(ref, 'scroll', onScroll);
+}

packages/@react-aria/virtualizer/src/Virtualizer.tsx

@@ -11,8 +11,8 @@
  */
 
 import {Collection, Key, RefObject} from '@react-types/shared';
-import {Layout, Rect, ReusableView, useVirtualizerState, VirtualizerState} from '@react-stately/virtualizer';
-import {mergeProps, useLayoutEffect} from '@react-aria/utils';
+import {Layout, Rect, ReusableView, useVirtualizerState} from '@react-stately/virtualizer';
+import {mergeProps, useLoadMore} from '@react-aria/utils';
 import React, {HTMLAttributes, ReactElement, ReactNode, useCallback, useRef} from 'react';
 import {ScrollView} from './ScrollView';
 import {VirtualizerItem} from './VirtualizerItem';
@@ -69,11 +69,14 @@ function Virtualizer<T extends object, V extends ReactNode, O>(props: Virtualize
     layoutOptions
   });
 
-  let {virtualizerProps, scrollViewProps} = useVirtualizer(props, state, ref);
+  useLoadMore({isLoading, onLoadMore, scrollOffset: 1}, ref);
+  let onVisibleRectChange = useCallback((rect: Rect) => {
+    state.setVisibleRect(rect);
+  }, [state]);
 
   return (
     <ScrollView
-      {...mergeProps(otherProps, virtualizerProps, scrollViewProps)}
+      {...mergeProps(otherProps, {onVisibleRectChange})}
       ref={ref}
       contentSize={state.contentSize}
       onScrollStart={state.startScrolling}
@@ -85,66 +88,6 @@ function Virtualizer<T extends object, V extends ReactNode, O>(props: Virtualize
   );
 }
 
-interface VirtualizerOptions {
-  tabIndex?: number,
-  focusedKey?: Key,
-  isLoading?: boolean,
-  onLoadMore?: () => void
-}
-
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
-export function useVirtualizer<T extends object, V extends ReactNode, W>(props: VirtualizerOptions, state: VirtualizerState<T, V>, ref: RefObject<HTMLElement | null>) {
-  let {isLoading, onLoadMore} = props;
-  let {setVisibleRect, virtualizer} = state;
-
-  // Handle scrolling, and call onLoadMore when nearing the bottom.
-  let isLoadingRef = useRef(isLoading);
-  let prevProps = useRef(props);
-  let onVisibleRectChange = useCallback((rect: Rect) => {
-    setVisibleRect(rect);
-
-    if (!isLoadingRef.current && onLoadMore) {
-      let scrollOffset = virtualizer.contentSize.height - rect.height * 2;
-      if (rect.y > scrollOffset) {
-        isLoadingRef.current = true;
-        onLoadMore();
-      }
-    }
-  }, [onLoadMore, setVisibleRect, virtualizer]);
-
-  let lastContentSize = useRef(0);
-  useLayoutEffect(() => {
-    // Only update isLoadingRef if props object actually changed,
-    // not if a local state change occurred.
-    let wasLoading = isLoadingRef.current;
-    if (props !== prevProps.current) {
-      isLoadingRef.current = isLoading;
-      prevProps.current = props;
-    }
-
-    let shouldLoadMore = !isLoadingRef.current
-      && onLoadMore
-      && state.contentSize.height > 0
-      && state.contentSize.height <= state.virtualizer.visibleRect.height
-      // Only try loading more if the content size changed, or if we just finished
-      // loading and still have room for more items.
-      && (wasLoading || state.contentSize.height !== lastContentSize.current);
-
-    if (shouldLoadMore) {
-      isLoadingRef.current = true;
-      onLoadMore();
-    }
-    lastContentSize.current = state.contentSize.height;
-  }, [state.contentSize, state.virtualizer, isLoading, onLoadMore, props]);
-
-  return {
-    virtualizerProps: {},
-    scrollViewProps: {
-      onVisibleRectChange
-    }
-  };
-}
-
 // forwardRef doesn't support generic parameters, so cast the result to the correct type
 // https://stackoverflow.com/questions/58469229/react-with-typescript-generics-while-using-react-forwardref
 const _Virtualizer = React.forwardRef(Virtualizer) as <T extends object, V, O>(props: VirtualizerProps<T, V, O> & {ref?: RefObject<HTMLDivElement | null>}) => ReactElement;

packages/@react-aria/virtualizer/src/index.ts

@@ -12,7 +12,7 @@
 
 export type {RTLOffsetType} from './utils';
 export type {VirtualizerItemOptions} from './useVirtualizerItem';
-export {useVirtualizer, Virtualizer} from './Virtualizer';
+export {Virtualizer} from './Virtualizer';
 export {useVirtualizerItem} from './useVirtualizerItem';
 export {VirtualizerItem, layoutInfoToStyle} from './VirtualizerItem';
 export {ScrollView, useScrollView} from './ScrollView';

packages/@react-spectrum/card/test/CardView.test.js

@@ -1186,6 +1186,7 @@ describe('CardView', function () {
       ${'Grid layout'}      | ${GridLayout}
       ${'Gallery layout'}   | ${GalleryLayout}
     `('$Name CardView should call loadMore when scrolling to the bottom', async function ({layout}) {
+      let scrollHeightMock = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 3000);
       let onLoadMore = jest.fn();
       let tree = render(<DynamicCardView layout={layout} onLoadMore={onLoadMore} />);
 
@@ -1199,13 +1200,8 @@ describe('CardView', function () {
       let grid = tree.getByRole('grid');
       grid.scrollTop = 3000;
       fireEvent.scroll(grid);
-      // TODO: look into and address difference in behavior
-      let isReact19 = parseInt(React.version, 10) >= 19;
-      if (isReact19 && layout !== GridLayout) {
-        expect(onLoadMore).toHaveBeenCalledTimes(2);
-      } else {
-        expect(onLoadMore).toHaveBeenCalledTimes(1);
-      }
+      expect(onLoadMore).toHaveBeenCalledTimes(1);
+      scrollHeightMock.mockReset();
     });
 
     it.each`