refactor useLoadMore to get rid of scroll handlers

Author: LFDanLu

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

@@ -11,8 +11,8 @@
  */
 
 import {Collection, Node} from '@react-types/shared';
-import {RefObject, useCallback, useRef} from 'react';
-import {useEvent} from './useEvent';
+import {RefObject, useRef} from 'react';
+import {useEffectEvent} from './useEffectEvent';
 import {useLayoutEffect} from './useLayoutEffect';
 
 export interface LoadMoreProps {
@@ -28,82 +28,62 @@ export interface LoadMoreProps {
    * @default 1
    */
   scrollOffset?: number,
-  // TODO: will need to refactor the existing components that use items
-  // this is a breaking change but this isn't documented and is in the react-aria/utils package so might be ok? Maybe can move this to a different package?
-  // /** The data currently loaded. */
-  // items?: any,
-  collection?: Collection<Node<unknown>>
+  // TODO: this is a breaking change but this isn't documented and is in the react-aria/utils package so might be ok? Maybe can move this to a different package?
+  collection?: Collection<Node<unknown>>,
+  /**
+   * A ref to a sentinel element that is positioned at the end of the list's content. The visibility of this senetinel
+   * with respect to the scrollable region and its offset determines if we should load more.
+   */
+  sentinelRef: RefObject<HTMLElement | null>
 }
 
 export function useLoadMore(props: LoadMoreProps, ref: RefObject<HTMLElement | null>) {
-  let {isLoading, onLoadMore, scrollOffset = 1, collection} = props;
-
-  // Handle scrolling, and call onLoadMore when nearing the bottom.
-  let onScroll = useCallback(() => {
-    if (ref.current && !isLoading && onLoadMore) {
-      let shouldLoadMore = ref.current.scrollHeight - ref.current.scrollTop - ref.current.clientHeight < ref.current.clientHeight * scrollOffset;
-
-      if (shouldLoadMore) {
-        onLoadMore();
-      }
-    }
-  }, [onLoadMore, isLoading, ref, scrollOffset]);
-
+  let {isLoading, onLoadMore, scrollOffset = 1, collection, sentinelRef} = props;
   let lastCollection = useRef(collection);
+
   // If we are in a loading state when this hook is called and a collection is provided, we can assume that the collection will update in the future so we don't
   // want to trigger another loadMore until the collection has updated as a result of the load.
   // TODO: If the load doesn't end up updating the collection even after completion, this flag could get stuck as true. However, not tracking
   // this means we could end up calling onLoadMore multiple times if isLoading changes but the collection takes time to update
   let collectionAwaitingUpdate = useRef(collection && isLoading);
+  let sentinelObserver = useRef<IntersectionObserver>(null);
+
+  let triggerLoadMore = useEffectEvent((entries: IntersectionObserverEntry[]) => {
+    // Only one entry should exist so this should be ok. Also use "isIntersecting" over an equality check of 0 since it seems like there is cases where
+    // a intersection ratio of 0 can be reported when isIntersecting is actually true
+    if (entries[0].isIntersecting && !isLoading && !(collection && collectionAwaitingUpdate.current) && onLoadMore) {
+      onLoadMore();
+      if (collection !== null && lastCollection.current !== null) {
+        collectionAwaitingUpdate.current = true;
+      }
+    }
+  });
+
+  // TODO: maybe can optimize creating the intersection observer by adding it in a useLayoutEffect but would need said effect to run every render
+  // so that we can catch when ref.current exists or is modified (maybe return a callbackRef?) and then would need to check if scrollOffset changed.
   useLayoutEffect(() => {
     if (!ref.current) {
       return;
     }
 
-    // Only update this flag if the collection changes when we aren't loading. Guard against the addition of a loading spinner when a load starts
-    // which mutates the collection? Alternatively, the user might wipe the collection during load
-    // If collection isn't provided, update flag
+    // Only update this flag if the collection changes when we aren't loading. Guards against cases like the addition of a loading spinner when a load starts or if the user
+    // temporarily wipes the collection during loading which isn't the collection update via fetch which we are waiting for.
+    // If collection isn't provided (aka for RSP components which won't provide a collection), flip flag to false so we still trigger onLoadMore
     if (collection !== lastCollection.current && !isLoading) {
       collectionAwaitingUpdate.current = false;
     }
 
-    let observer = new IntersectionObserver((entries) => {
-      let allItemsInView = true;
-      entries.forEach((entry) => {
-        // TODO this is problematic if the entry is a long row since a part of it will always out of view meaning the intersectionRatio is < 1
-        if (entry.intersectionRatio < 1) {
-          allItemsInView = false;
-        }
-      });
-
-      if (allItemsInView && !isLoading && !(collection && collectionAwaitingUpdate.current) && onLoadMore) {
-        onLoadMore();
-        if (collection !== null && lastCollection.current !== null) {
-          collectionAwaitingUpdate.current = true;
-        }
-      }
-    }, {root: ref.current});
-
-    // TODO: right now this is using the Virtualizer's div that wraps the rows, but perhaps should be the items themselves
-    // This also has various problems because we'll need to figure out what is the proper element to compare the scroll container height to
-
-    let lastElement = ref.current.querySelector('[role="presentation"]');
-    // let lastElement = ref.current.querySelector('[role="presentation"]>[role="presentation"]:last-child');
-    if (lastElement) {
-      observer.observe(lastElement);
+    sentinelObserver.current = new IntersectionObserver(triggerLoadMore, {root: ref.current, rootMargin: `0px 0px ${100 * scrollOffset}% 0px`});
+    if (sentinelRef?.current) {
+      // console.log('observing', sentinelRef.current.outerHTML)
+      sentinelObserver.current.observe(sentinelRef.current);
     }
 
     lastCollection.current = collection;
     return () => {
-      if (observer) {
-        observer.disconnect();
+      if (sentinelObserver.current) {
+        sentinelObserver.current.disconnect();
       }
     };
-  }, [isLoading, onLoadMore, props, ref, collection]);
-
-
-  // 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);
+  }, [isLoading, triggerLoadMore, ref, collection, scrollOffset, sentinelRef]);
 }

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

@@ -13,6 +13,7 @@
 // @ts-ignore
 import {flushSync} from 'react-dom';
 import {getScrollLeft} from './utils';
+import {inertValue, useEffectEvent, useEvent, useLayoutEffect, useObjectRef, useResizeObserver} from '@react-aria/utils';
 import React, {
   CSSProperties,
   ForwardedRef,
@@ -25,7 +26,6 @@ import React, {
   useState
 } from 'react';
 import {Rect, Size} from '@react-stately/virtualizer';
-import {useEffectEvent, useEvent, useLayoutEffect, useObjectRef, useResizeObserver} from '@react-aria/utils';
 import {useLocale} from '@react-aria/i18n';
 
 interface ScrollViewProps extends HTMLAttributes<HTMLElement> {
@@ -35,26 +35,32 @@ interface ScrollViewProps extends HTMLAttributes<HTMLElement> {
   innerStyle?: CSSProperties,
   onScrollStart?: () => void,
   onScrollEnd?: () => void,
-  scrollDirection?: 'horizontal' | 'vertical' | 'both'
+  scrollDirection?: 'horizontal' | 'vertical' | 'both',
+  sentinelRef: React.RefObject<HTMLDivElement | null>
 }
 
+interface ScrollViewOptions extends Omit<ScrollViewProps, 'sentinelRef'> {}
+
 function ScrollView(props: ScrollViewProps, ref: ForwardedRef<HTMLDivElement | null>) {
+  let {sentinelRef, ...otherProps} = props;
   ref = useObjectRef(ref);
-  let {scrollViewProps, contentProps} = useScrollView(props, ref);
+  let {scrollViewProps, contentProps} = useScrollView(otherProps, ref);
 
   return (
     <div role="presentation" {...scrollViewProps} ref={ref}>
       <div {...contentProps}>
         {props.children}
       </div>
+      {/* @ts-ignore - compatibility with React < 19 */}
+      <div data-testid="loadMoreSentinel" ref={sentinelRef} style={{height: 1, width: 1}} inert={inertValue(true)} />
     </div>
   );
 }
 
 const ScrollViewForwardRef = React.forwardRef(ScrollView);
 export {ScrollViewForwardRef as ScrollView};
 
-export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement | null>) {
+export function useScrollView(props: ScrollViewOptions, ref: RefObject<HTMLElement | null>) {
   let {
     contentSize,
     onVisibleRectChange,
@@ -135,7 +141,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject<HTMLElement
   // Attach event directly to ref so RAC Virtualizer doesn't need to send props upward.
   useEvent(ref, 'scroll', onScroll);
 
-   
+
   useEffect(() => {
     return () => {
       if (state.scrollTimeout != null) {

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

@@ -13,7 +13,7 @@
 import {Collection, Key, RefObject} from '@react-types/shared';
 import {Layout, Rect, ReusableView, useVirtualizerState} from '@react-stately/virtualizer';
 import {mergeProps, useLoadMore, useObjectRef} from '@react-aria/utils';
-import React, {ForwardedRef, HTMLAttributes, ReactElement, ReactNode, useCallback} from 'react';
+import React, {ForwardedRef, HTMLAttributes, ReactElement, ReactNode, useCallback, useRef} from 'react';
 import {ScrollView} from './ScrollView';
 import {VirtualizerItem} from './VirtualizerItem';
 
@@ -68,21 +68,25 @@ export const Virtualizer = React.forwardRef(function Virtualizer<T extends objec
     layoutOptions
   });
 
-  useLoadMore({isLoading, onLoadMore, scrollOffset: 1}, ref);
+  let sentinelRef = useRef(null);
+  useLoadMore({isLoading, onLoadMore, scrollOffset: 1, sentinelRef}, ref);
   let onVisibleRectChange = useCallback((rect: Rect) => {
     state.setVisibleRect(rect);
   }, [state]);
 
   return (
-    <ScrollView
-      {...mergeProps(otherProps, {onVisibleRectChange})}
-      ref={ref}
-      contentSize={state.contentSize}
-      onScrollStart={state.startScrolling}
-      onScrollEnd={state.endScrolling}
-      scrollDirection={scrollDirection}>
-      {renderChildren(null, state.visibleViews, renderWrapper || defaultRenderWrapper)}
-    </ScrollView>
+    <>
+      <ScrollView
+        {...mergeProps(otherProps, {onVisibleRectChange})}
+        ref={ref}
+        sentinelRef={sentinelRef}
+        contentSize={state.contentSize}
+        onScrollStart={state.startScrolling}
+        onScrollEnd={state.endScrolling}
+        scrollDirection={scrollDirection}>
+        {renderChildren(null, state.visibleViews, renderWrapper || defaultRenderWrapper)}
+      </ScrollView>
+    </>
   );
 }) as <T extends object, V, O>(props: VirtualizerProps<T, V, O> & {ref?: RefObject<HTMLDivElement | null>}) => ReactElement;
 

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

@@ -11,7 +11,7 @@
  */
 
 jest.mock('@react-aria/utils/src/scrollIntoView');
-import {act, fireEvent, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
+import {act, fireEvent, pointerMap, render, setupIntersectionObserverMock, within} from '@react-spectrum/test-utils-internal';
 import {Card, CardView, GalleryLayout, GridLayout, WaterfallLayout} from '../';
 import {composeStories} from '@storybook/react';
 import {Content} from '@react-spectrum/view';
@@ -1186,6 +1186,10 @@ describe('CardView', function () {
       ${'Grid layout'}      | ${GridLayout}
       ${'Gallery layout'}   | ${GalleryLayout}
     `('$Name CardView should call loadMore when scrolling to the bottom', async function ({layout}) {
+      let observe = jest.fn();
+      let observer = setupIntersectionObserverMock({
+        observe
+      });
       let scrollHeightMock = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 3000);
       let onLoadMore = jest.fn();
       let tree = render(<DynamicCardView layout={layout} onLoadMore={onLoadMore} />);
@@ -1196,10 +1200,13 @@ describe('CardView', function () {
 
       let cards = tree.getAllByRole('gridcell');
       expect(cards).toBeTruthy();
+      let sentinel = tree.getByTestId('loadMoreSentinel');
+      expect(observe).toHaveBeenLastCalledWith(sentinel);
 
       let grid = tree.getByRole('grid');
       grid.scrollTop = 3000;
       fireEvent.scroll(grid);
+      act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
       expect(onLoadMore).toHaveBeenCalledTimes(1);
       scrollHeightMock.mockReset();
     });

packages/@react-spectrum/combobox/test/ComboBox.test.js

@@ -11,7 +11,7 @@
  */
 
 jest.mock('@react-aria/live-announcer');
-import {act, fireEvent, pointerMap, render, simulateDesktop, simulateMobile, waitFor, within} from '@react-spectrum/test-utils-internal';
+import {act, fireEvent, pointerMap, render, setupIntersectionObserverMock, simulateDesktop, simulateMobile, waitFor, within} from '@react-spectrum/test-utils-internal';
 import {announce} from '@react-aria/live-announcer';
 import {Button} from '@react-spectrum/button';
 import {chain} from '@react-aria/utils';
@@ -2074,6 +2074,7 @@ describe('ComboBox', function () {
       jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000);
     });
     it('onLoadMore is called on initial open', async () => {
+      let observer = setupIntersectionObserverMock();
       load = jest
         .fn()
         .mockImplementationOnce(() => {
@@ -2111,6 +2112,7 @@ describe('ComboBox', function () {
       expect(listbox).toBeVisible();
       // update size, virtualizer raf kicks in
       act(() => {jest.advanceTimersToNextTimer();});
+      act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
       // onLoadMore queued by previous timer, run it now
       act(() => {jest.advanceTimersToNextTimer();});
 
@@ -2137,6 +2139,7 @@ describe('ComboBox', function () {
     });
 
     it('onLoadMore is not called on when previously opened', async () => {
+      let observer = setupIntersectionObserverMock();
       load = jest
       .fn()
       .mockImplementationOnce(() => {
@@ -2173,6 +2176,7 @@ describe('ComboBox', function () {
       expect(listbox).toBeVisible();
       // update size, virtualizer raf kicks in
       act(() => {jest.advanceTimersToNextTimer();});
+      act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
       // onLoadMore queued by previous timer, run it now
       act(() => {jest.advanceTimersToNextTimer();});
 

packages/@react-spectrum/listbox/test/ListBox.test.js

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {act, fireEvent, mockClickDefault, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
+import {act, fireEvent, mockClickDefault, pointerMap, render, setupIntersectionObserverMock, within} from '@react-spectrum/test-utils-internal';
 import Bell from '@spectrum-icons/workflow/Bell';
 import {FocusExample} from '../stories/ListBox.stories';
 import {Item, ListBox, Section} from '../';
@@ -886,6 +886,10 @@ describe('ListBox', function () {
     });
 
     it('should fire onLoadMore when scrolling near the bottom', function () {
+      let observe = jest.fn();
+      let observer = setupIntersectionObserverMock({
+        observe
+      });
       // Mock clientHeight to match maxHeight prop
       let maxHeight = 200;
       jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => maxHeight);
@@ -903,7 +907,7 @@ describe('ListBox', function () {
 
         return 48;
       });
-      let {getByRole} = render(
+      let {getByRole, getByTestId} = render(
         <Provider theme={theme}>
           <ListBox aria-label="listbox" items={items} maxHeight={maxHeight} onLoadMore={onLoadMore}>
             {item => <Item key={item.name}>{item.name}</Item>}
@@ -916,6 +920,8 @@ describe('ListBox', function () {
       let listbox = getByRole('listbox');
       let options = within(listbox).getAllByRole('option');
       expect(options.length).toBe(6); // each row is 48px tall, listbox is 200px. 5 rows fit. + 1/3 overscan
+      let sentinel = getByTestId('loadMoreSentinel');
+      expect(observe).toHaveBeenLastCalledWith(sentinel);
 
       listbox.scrollTop = 250;
       fireEvent.scroll(listbox);
@@ -929,13 +935,15 @@ describe('ListBox', function () {
       // there are no more items to load at this height, so loadMore is only called twice
       listbox.scrollTop = 5000;
       fireEvent.scroll(listbox);
+      act(() => {observer.instance.triggerCallback([{isIntersecting: true}]);});
       act(() => jest.runAllTimers());
 
       expect(onLoadMore).toHaveBeenCalledTimes(1);
       scrollHeightMock.mockReset();
     });
 
     it('should fire onLoadMore if there aren\'t enough items to fill the ListBox ', async function () {
+      let observer = setupIntersectionObserverMock();
       // Mock clientHeight to match maxHeight prop
       let maxHeight = 300;
       jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => maxHeight);
@@ -1000,9 +1008,11 @@ describe('ListBox', function () {
       let {getByRole} = render(
         <AsyncListBox />
       );
+
       await act(async () => {
         jest.runAllTimers();
       });
+      await act(async () => {observer.instance.triggerCallback([{isIntersecting: true}]);});
 
       let listbox = getByRole('listbox');
       let options = within(listbox).getAllByRole('option');

packages/@react-spectrum/table/src/TableViewBase.tsx

@@ -584,8 +584,8 @@ function TableVirtualizer<T>(props: TableVirtualizerProps<T>) {
       columnWidths: columnResizeState.columnWidths
     }), [columnResizeState.columnWidths])
   });
-
-  useLoadMore({isLoading, onLoadMore, scrollOffset: 1}, bodyRef);
+  let sentinelRef = useRef(null);
+  useLoadMore({isLoading, onLoadMore, scrollOffset: 1, sentinelRef}, bodyRef);
   let onVisibleRectChange = useCallback((rect: Rect) => {
     state.setVisibleRect(rect);
   }, [state]);
@@ -694,6 +694,7 @@ function TableVirtualizer<T>(props: TableVirtualizerProps<T>) {
             }}
             innerStyle={{overflow: 'visible'}}
             ref={bodyRef}
+            sentinelRef={sentinelRef}
             contentSize={state.contentSize}
             onVisibleRectChange={onVisibleRectChangeMemo}
             onScrollStart={state.startScrolling}