useLoadMore support in RAC Table

packages/@react-aria/loading/README.md

@@ -0,0 +1,3 @@
+# @react-aria/loading
+
+This package is part of [react-spectrum](https://github.com/adobe/react-spectrum). See the repo for more details.

packages/@react-aria/loading/index.ts

@@ -0,0 +1,13 @@
+/*
+ * 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.
+ */
+
+export * from './src';

packages/@react-aria/loading/package.json

@@ -0,0 +1,34 @@
+{
+  "name": "@react-aria/loading",
+  "version": "3.0.0-alpha.1",
+  "description": "Spectrum UI components in React",
+  "license": "Apache-2.0",
+  "main": "dist/main.js",
+  "module": "dist/module.js",
+  "exports": {
+    "types": "./dist/types.d.ts",
+    "import": "./dist/import.mjs",
+    "require": "./dist/main.js"
+  },
+  "types": "dist/types.d.ts",
+  "source": "src/index.ts",
+  "files": [
+    "dist",
+    "src"
+  ],
+  "sideEffects": false,
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/adobe/react-spectrum"
+  },
+  "dependencies": {
+    "@react-aria/utils": "^3.24.1",
+    "@swc/helpers": "^0.5.0"
+  },
+  "peerDependencies": {
+    "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0"
+  },
+  "publishConfig": {
+    "access": "public"
+  }
+}

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

@@ -0,0 +1,15 @@
+/*
+ * 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.
+ */
+
+export {useLoadOnScroll} from './useLoadOnScroll';
+
+export type {LoadOnScrollProps} from './useLoadOnScroll';

packages/@react-aria/loading/src/useLoadOnScroll.ts

@@ -0,0 +1,88 @@
+/*
+ * 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, useMemo, useRef} from 'react';
+import {useLayoutEffect} from '@react-aria/utils';
+
+export interface LoadOnScrollProps {
+  /** 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,
+  // TODO: decide on default here
+  /**
+   * The amount of offset (in pixels) from the bottom of your scrollable region that should trigger load more.
+   * @default 25
+   */
+  scrollOffset?: number
+}
+
+// TODO: discuss if it would be ok to just attach event to ref...
+interface LoadOnScrollAria {
+  /** Props for the scrollable region. */
+  scrollViewProps: {
+    onScroll: () => void
+  }
+}
+
+export function useLoadOnScroll(props: LoadOnScrollProps, ref: RefObject<HTMLElement | null>): LoadOnScrollAria {
+  let {isLoading, onLoadMore, scrollOffset = 25} = 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 < scrollOffset;
+
+      if (shouldLoadMore) {
+        isLoadingRef.current = true;
+        onLoadMore();
+      }
+
+    }
+  }, [onLoadMore, ref, scrollOffset]);
+
+  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;
+    }
+
+    // TODO: this actually calls loadmore twice in succession on intial load because after the first load
+    // the scrollable element hasn't yet recieved its new height with the newly loaded items... Because of RAC collection needing two renders?
+    let shouldLoadMore = ref?.current
+      && !isLoadingRef.current
+      && onLoadMore
+      && ref.current.clientHeight === ref.current.scrollHeight
+      // Only try loading more if the content size changed, or if we just finished
+      // loading and still have room for more items.
+      && (wasLoading || ref.current.scrollHeight !== lastContentSize.current);
+
+    if (shouldLoadMore) {
+      isLoadingRef.current = true;
+      onLoadMore?.();
+    }
+    lastContentSize.current = ref.current?.scrollHeight || 0;
+  }, [isLoading, onLoadMore, props, ref]);
+
+
+  return useMemo(() => ({
+    scrollViewProps: {
+      onScroll
+    }
+  }), [onScroll]);
+}

packages/@react-aria/virtualizer/package.json

@@ -24,6 +24,7 @@
   "dependencies": {
     "@react-aria/i18n": "^3.11.1",
     "@react-aria/interactions": "^3.21.3",
+    "@react-aria/loading": "3.0.0-alpha.1",
     "@react-aria/utils": "^3.24.1",
     "@react-stately/virtualizer": "^3.7.1",
     "@react-types/shared": "^3.23.1",

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

@@ -12,9 +12,10 @@
 
 import {Collection, Key} from '@react-types/shared';
 import {Layout, Rect, ReusableView, useVirtualizerState, VirtualizerState} from '@react-stately/virtualizer';
-import {mergeProps, useLayoutEffect} from '@react-aria/utils';
+import {mergeProps} from '@react-aria/utils';
 import React, {createContext, HTMLAttributes, ReactElement, ReactNode, RefObject, useCallback, useMemo, useRef} from 'react';
 import {ScrollView} from './ScrollView';
+import {useLoadOnScroll} from '@react-aria/loading';
 import {VirtualizerItem} from './VirtualizerItem';
 
 interface VirtualizerProps<T extends object, V, O> extends Omit<HTMLAttributes<HTMLElement>, 'children'> {
@@ -98,48 +99,21 @@ interface VirtualizerOptions {
 // 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, W>, ref: RefObject<HTMLElement | null>) {
   let {isLoading, onLoadMore} = props;
-  let {setVisibleRect, virtualizer} = state;
+  let {setVisibleRect} = state;
+  let scrollOffset = ref.current?.clientHeight;
+
+  // TODO: The ref previously provided was the table's ref. That ref didn't actually do anything in the old implementation
+  // but now the ref provided to useVirtualizer MUST be the scrollable ref so its a breaking change kinda?
+  // TODO: Also, now that we use ref.currnet.clientHeight/scrollHeight/etc instead of relying on the rect to figure out if we should load more
+  // this will break peoples tests since they need to mock scrollHeight to simulate the virtualizer's total contents height...
+  let {scrollViewProps: {onScroll}} = useLoadOnScroll({isLoading, onLoadMore, scrollOffset}, ref);
 
-  // Handle scrolling, and call onLoadMore when nearing the bottom.
-  let isLoadingRef = useRef(isLoading);
-  let prevProps = useRef(props);
   let onVisibleRectChange = useCallback((rect: Rect) => {
     setVisibleRect(rect);
+    onScroll();
+  }, [setVisibleRect, onScroll]);
 
-    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]);
-
+  // TODO: would've liked it if I didn't have to preseve these and just attach onScroll directly to the scroll ref but it would be breaking.
   return {
     virtualizerProps: {},
     scrollViewProps: {

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`

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

@@ -2122,8 +2122,8 @@ describe('ComboBox', function () {
 
         return 40;
       });
-      // scrollHeight is for useVirutalizerItem to mock its getSize()
-      scrollHeightSpy = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 32);
+      // scrollHeight is now mocking the virtualizer's total content height
+      scrollHeightSpy = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 104);
     });
     afterEach(() => {
       clientHeightSpy.mockRestore();

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

@@ -837,7 +837,13 @@ describe('ListBox', function () {
         items.push({name: 'Test ' + i});
       }
       // total height if all are rendered would be about 100 * 48px = 4800px
+      let scrollHeightMock = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(function () {
+        if (this.getAttribute('role') === 'listbox') {
+          return 4800;
+        }
 
+        return 48;
+      });
       let {getByRole} = render(
         <Provider theme={theme}>
           <ListBox aria-label="listbox" items={items} maxHeight={maxHeight} onLoadMore={onLoadMore}>
@@ -867,6 +873,7 @@ describe('ListBox', function () {
       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 () {
@@ -893,12 +900,7 @@ describe('ListBox', function () {
       let listbox = getByRole('listbox');
       let options = within(listbox).getAllByRole('option');
       expect(options.length).toBe(5);
-      let isReact19 = parseInt(React.version, 10) >= 19;
-      if (isReact19) {
-        expect(onLoadMore).toHaveBeenCalledTimes(2);
-      } else {
-        expect(onLoadMore).toHaveBeenCalledTimes(1);
-      }
+      expect(onLoadMore).toHaveBeenCalledTimes(1);
     });
   });
 

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

@@ -574,7 +574,7 @@ function TableVirtualizer<T>(props: TableVirtualizerProps<T>) {
     onLoadMore
   }), [otherProps.tabIndex, focusedKey, isLoading, onLoadMore]);
 
-  let {virtualizerProps, scrollViewProps: {onVisibleRectChange}} = useVirtualizer(memoedVirtualizerProps, state, domRef);
+  let {virtualizerProps, scrollViewProps: {onVisibleRectChange}} = useVirtualizer(memoedVirtualizerProps, state, bodyRef);
   let onVisibleRectChangeMemo = useCallback(rect => {
     setTableWidth(rect.width);
     onVisibleRectChange(rect);

packages/@react-spectrum/table/test/Table.test.js

@@ -4167,6 +4167,7 @@ export let tableTests = () => {
     });
 
     it('should fire onLoadMore when scrolling near the bottom', function () {
+      let scrollHeightMock = jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 4100);
       let items = [];
       for (let i = 1; i <= 100; i++) {
         items.push({id: i, foo: 'Foo ' + i, bar: 'Bar ' + i});
@@ -4208,6 +4209,7 @@ export let tableTests = () => {
       act(() => {jest.runAllTimers();});
 
       expect(onLoadMore).toHaveBeenCalledTimes(1);
+      scrollHeightMock.mockReset();
     });
 
     it('should automatically fire onLoadMore if there aren\'t enough items to fill the Table', function () {

packages/@react-stately/layout/src/TableLayout.ts

@@ -18,7 +18,7 @@ import {LayoutNode, ListLayout, ListLayoutOptions, ListLayoutProps} from './List
 import {TableCollection} from '@react-types/table';
 import {TableColumnLayout} from '@react-stately/table';
 
-export interface TableLayoutOptions<T> extends ListLayoutOptions<T> {
+export interface TableLayoutOptions<T> extends Omit<ListLayoutOptions<T>, 'loaderHeight'> {
   scrollContainer?: 'table' | 'body'
 }
 
@@ -45,7 +45,7 @@ export class TableLayout<T> extends ListLayout<T> {
   }
 
   private columnsChanged(newCollection: TableCollection<T>, oldCollection: TableCollection<T> | null) {
-    return !oldCollection || 
+    return !oldCollection ||
       newCollection.columns !== oldCollection.columns &&
       newCollection.columns.length !== oldCollection.columns.length ||
       newCollection.columns.some((c, i) =>
@@ -75,6 +75,10 @@ export class TableLayout<T> extends ListLayout<T> {
     super.validate(invalidationContext);
   }
 
+  // TODO: in the RAC case, we don't explicity accept loadingState on the TableBody, but note that if the user
+  // does happen to set loadingState="loading" or loadingState="loadingMore" coincidentally, the isLoading
+  // part of this code will trigger and the layout will reserve more room for the loading spinner which we actually only use
+  // in RSP
   protected buildCollection(): LayoutNode[] {
     // Track whether we were previously loading. This is used to adjust the animations of async loading vs inserts.
     let loadingState = this.collection.body.props.loadingState;
@@ -299,6 +303,7 @@ export class TableLayout<T> extends ListLayout<T> {
       case 'headerrow':
         return this.buildHeaderRow(node, x, y);
       case 'item':
+      case 'loader':
         return this.buildRow(node, x, y);
       case 'column':
       case 'placeholder':
@@ -336,6 +341,12 @@ export class TableLayout<T> extends ListLayout<T> {
       }
     }
 
+    // TODO: perhaps make a separate buildLoader? Do we need to differentiate the layoutInfo information?
+    // I think the below is ok for now since we can just treat nested loaders/load more as rows
+    if (node.type === 'loader') {
+      height = this.rowHeight;
+    }
+
     this.setChildHeights(children, height);
 
     rect.width = this.layoutInfos.get(this.collection.head?.key ?? 'header').rect.width;

packages/react-aria-components/package.json

@@ -42,6 +42,7 @@
     "@react-aria/color": "3.0.0-beta.33",
     "@react-aria/focus": "^3.17.1",
     "@react-aria/interactions": "^3.21.3",
+    "@react-aria/loading": "3.0.0-alpha.1",
     "@react-aria/menu": "^3.14.1",
     "@react-aria/toolbar": "3.0.0-beta.5",
     "@react-aria/tree": "3.0.0-alpha.1",