feat: Support keyboard focusing loading spinners in collection components #8326

LFDanLu · 2025-05-30T17:59:50Z

Closes

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

rspbot · 2025-05-30T18:07:57Z

Build successful! 🎉

LFDanLu · 2025-05-30T20:19:01Z

packages/@react-aria/listbox/src/useOption.ts

    optionProps['aria-posinset'] = Number.isNaN(index) ? undefined : index + 1;
-    optionProps['aria-setsize'] = getItemCount(state.collection);
+    // TODO: this is not great, but the loader sentinel is always in the collection even when loading isn't currently in progress.
+    // This same issue applies to the other collection elements, namely the row counts calculated for the top level parent element
+    optionProps['aria-setsize'] = getItemCount(state.collection, (node) => node.type === 'item' || (node.type === 'loader' && node.props.isLoading));


these are kinda weird in RAC/S2 when there are sections, the setsize is still based on the whole collection but the posinset is based off the index within the section since the collection calculated index is different in new collections

rspbot · 2025-05-30T21:32:44Z

Build successful! 🎉

rspbot · 2025-05-30T22:11:18Z

Build successful! 🎉

LFDanLu · 2025-05-30T22:32:39Z

packages/@react-stately/list/src/useListState.ts

+        const diff: number = (cachedItemNodes?.length ?? 0) - (itemNodes?.length ?? 0);
+        // Look up the start item's index in the cached node array. We can't rely on the node's index
+        // because it maybe in a section and thus have a index relative to that section
+        // TODO: the collection keys/item are not guarenteed to be in order
+        // (aka the loader will be the first node in the keymap if the collection was empty and then loaded items)
+        // this means we should ideally traverse through the key's prevKey until we find a focusable node but
+        // that is a bigger refactor, will handle later. For now continue assuming that the loader is the last element
+        let startItemIndex = 0;
+        if (startItem) {


To avoid further bloat, will be handling this refactor later. Perhaps something can be done in the RAC collection level but otherwise we should maybe consider refactoring this to use the old focused key's prevKey instead of index since the collection's keymap order is not necessarily in index order.

Will need to discuss where exactly we want to restore focus (in roughly the same place? or should it prefer the closest contextually matching row regardless of where that maybe in the collection?)

rspbot · 2025-05-30T22:33:05Z

Build successful! 🎉

LFDanLu · 2025-05-30T22:40:35Z

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

@@ -59,7 +59,7 @@ export class GridKeyboardDelegate<T, C extends GridCollection<T>> implements Key
  }

  private isDisabled(item: Node<unknown>) {
-    return this.disabledBehavior === 'all' && (item.props?.isDisabled || this.disabledKeys.has(item.key));
+    return this.disabledBehavior === 'all' && (item.props?.isDisabled || this.disabledKeys.has(item.key) || (item.type === 'loader' && !item.props.isLoading));


more places where we have to account for the fact that the loading sentinel is in the collection but not actually loading...

LFDanLu · 2025-05-30T22:42:16Z

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

@@ -175,7 +176,7 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
  );

  if (isVirtualized) {
-    gridProps['aria-rowcount'] = state.collection.size;
+    gridProps['aria-rowcount'] = getItemCount(state.collection, (node) => node.type === 'item' || (node.type === 'loader' && node.props.isLoading));


again, bit unfortunate that the collection's size is not longer reflective of the number of rows since we don't actually want to count the loading sentinel if it isn't actually loading. Shouldn't be that much more expensive hopefully since getItemCount does some caching

LFDanLu · 2025-05-30T22:49:24Z

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

+      // at the proper estimated location. If the node.type is "section" then we don't do this shortcut since we have to
+      // build the sections to see how tall they are.
+      if ((node.type === 'item' || node.type === 'loader') && y > this.requestedRect.maxY) {


This is still basically how it worked previously,

react-spectrum/packages/@react-stately/layout/src/ListLayout.ts

Line 275 in 9786f69

if (node.type === 'item' && y > this.requestedRect.maxY) {

We only used this shortcut when processing items, if there are sections then iterating through the collection will only return sections since we don't support sections and sectionless items at the same time

snowystinger · 2025-06-02T01:41:46Z

packages/@react-aria/gridlist/src/useGridList.ts

@@ -167,7 +168,7 @@ export function useGridList<T>(props: AriaGridListOptions<T>, state: ListState<T
  );

  if (isVirtualized) {
-    gridProps['aria-rowcount'] = state.collection.size;
+    gridProps['aria-rowcount'] = getItemCount(state.collection, (node) => node.type === 'item' || (node.type === 'loader' && node.props.isLoading));


line 154 i think has to check this too since the loader is "tabbable"
also line 166?

oh that right, didn't noticed since useSelectableCollection actually still ensures we have the right tabindex in this case. Relates to the last comment I made below, ugh

Hrm, this opened up a can of worms since I had incorrectly assumed that all the other collection.size checks wouldn't be a problem since they all are empty collection checks but we do need the same kind of logic there for correctness. Rather than propagate this getItemCount in all those places perhaps we should update the collection so that collection.size does a similar check to above? Or maybe we should consider trying to pull the loader out of the collection? Poking around, but I'm leaning towards updating collection's .size

@devongovett any initial opinions on this? We had discussed previously that checks like

react-spectrum/packages/@react-stately/layout/src/ListLayout.ts

Line 258 in 081172e

let isEmptyOrLoading = collection?.size === 0 || (collection.size === 1 && collection.getItem(collection.getFirstKey()!)!.type === 'loader');

were a bit weird and debated whether or not the loaders should be in the collection

snowystinger · 2025-06-02T01:44:31Z

packages/@react-stately/collections/src/getItemCount.ts

        counter++;
      }
+
+      // TODO: New collections vs old collection is different here. New collections for table will only return


still a TODO here? or are things working and you just want alternatives?

The latter, feels ok for now but a bit wary about how the loading spinners being included in the collections now means we can't lean on collection.size anymore

taken from #8326

* Update tree and listlayout to handle multi loaders * adapting other stories to new loader api and adding useAsync example story * add tests * fix story for correctness, should only need to provide a dependecy at the top most collection * restoring focus back to the tree if the user was focused on the loader * fixing estimated loader position if sections exist taken from #8326 * skip test for now

…m into focus_loading_spinners

rspbot · 2025-06-05T23:06:49Z

Build successful! 🎉

rspbot · 2025-06-05T23:06:51Z

## API Changes

react-aria-components

/react-aria-components:UNSTABLE_GridListLoadingSentinel

 UNSTABLE_GridListLoadingSentinel <T extends {}> {
   children?: ReactNode
-  className?: string
+  className?: string | ((GridListLoadingSentinelRenderProps & {
+    defaultClassName: string | undefined
+})) => string
   isLoading?: boolean
   onLoadMore?: () => any
   scrollOffset?: number = 1
-  style?: CSSProperties
+  style?: CSSProperties | ((GridListLoadingSentinelRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
 }

/react-aria-components:UNSTABLE_ListBoxLoadingSentinel

 UNSTABLE_ListBoxLoadingSentinel <T extends {}> {
   children?: ReactNode
-  className?: string
+  className?: string | ((ListBoxLoadingSentinelRenderProps & {
+    defaultClassName: string | undefined
+})) => string
   isLoading?: boolean
   onLoadMore?: () => any
   scrollOffset?: number = 1
-  style?: CSSProperties
+  style?: CSSProperties | ((ListBoxLoadingSentinelRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
 }

/react-aria-components:UNSTABLE_TableLoadingSentinel

 UNSTABLE_TableLoadingSentinel <T extends {}> {
   children?: ReactNode
-  className?: string
+  className?: string | ((TableLoadingSentinelRenderProps & {
+    defaultClassName: string | undefined
+})) => string
   isLoading?: boolean
   onLoadMore?: () => any
   scrollOffset?: number = 1
-  style?: CSSProperties
+  style?: CSSProperties | ((TableLoadingSentinelRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
 }

/react-aria-components:UNSTABLE_TreeLoadingIndicator

-UNSTABLE_TreeLoadingIndicator <T extends {}> {
-  children?: ReactNode | ((UNSTABLE_TreeLoadingIndicatorRenderProps & {
-    defaultChildren: ReactNode | undefined
-})) => ReactNode
-  className?: string | ((UNSTABLE_TreeLoadingIndicatorRenderProps & {
-    defaultClassName: string | undefined
-})) => string
-  style?: CSSProperties | ((UNSTABLE_TreeLoadingIndicatorRenderProps & {
-    defaultStyle: CSSProperties
-})) => CSSProperties | undefined
-}

/react-aria-components:UNSTABLE_TreeLoadingSentinel

+UNSTABLE_TreeLoadingSentinel <T extends {}> {
+  children?: ReactNode | ((TreeLoadingSentinelRenderProps & {
+    defaultChildren: ReactNode | undefined
+})) => ReactNode
+  className?: string | ((TreeLoadingSentinelRenderProps & {
+    defaultClassName: string | undefined
+})) => string
+  isLoading?: boolean
+  onLoadMore?: () => any
+  scrollOffset?: number = 1
+  style?: CSSProperties | ((TreeLoadingSentinelRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
+}

@react-stately/collections

/@react-stately/collections:getItemCount

 getItemCount <T> {
   collection: Collection<Node<T>>
+  isValidItem: (Node<T>) => boolean
   returnVal: undefined
 }

LFDanLu · 2025-06-10T17:36:07Z

packages/@react-spectrum/s2/src/Picker.tsx

Note: will need to handle the case where the user is using Left/Right to change the selected key when the listbox is closed and focus is on the Picker. If the loading spinner is present, it will just show an empty option, should prevent the loader from being selectable in this case

LFDanLu · 2025-06-17T21:59:54Z

closing for now, will require a larger refactor than expected since ideally we'd have each node contain information about it's own focusability/selectability

LFDanLu added 4 commits May 30, 2025 10:55

Support focusing the table load more spinner

e964eea

making gridlist loading spinner keyboard focusable

2eecd6e

progress making listbox spinners focusable

4651c91

fix item counts by accounting for loading spinner state

1beaefc

LFDanLu commented May 30, 2025

View reviewed changes

LFDanLu added 2 commits May 30, 2025 13:58

fix focus nearest row when loader is focused and disappears behavior

66a087c

debug combobox virtual focus with loader

fe0aaa2

update s2 loading spinner focus styles

054934d

fix estimated y position for loaders in listlayout if sections exist

ced8859

LFDanLu marked this pull request as ready for review May 30, 2025 22:27

LFDanLu commented May 30, 2025

View reviewed changes

This comment was marked as off-topic.

Sign in to view

snowystinger reviewed Jun 2, 2025

View reviewed changes

LFDanLu added a commit that referenced this pull request Jun 2, 2025

fixing estimated loader position if sections exist

5dbf204

taken from #8326

Base automatically changed from multi_loader_support to main June 4, 2025 17:27

LFDanLu changed the base branch from main to multi_loader_support June 4, 2025 22:20

LFDanLu added the in sprint label Jun 5, 2025

LFDanLu added 2 commits June 5, 2025 10:04

Merge branch 'multi_loader_support' of github.com:adobe/react-spectru…

ed25d17

…m into focus_loading_spinners

Merge branch 'multi_loader_support' of github.com:adobe/react-spectru…

7c62b2d

…m into focus_loading_spinners

LFDanLu commented Jun 10, 2025

View reviewed changes

LFDanLu closed this Jun 17, 2025

feat: Support keyboard focusing loading spinners in collection components #8326

feat: Support keyboard focusing loading spinners in collection components #8326

Uh oh!

Conversation

LFDanLu commented May 30, 2025

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

rspbot commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented May 30, 2025

Uh oh!

rspbot commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented Jun 5, 2025

Uh oh!

rspbot commented Jun 5, 2025

react-aria-components

/react-aria-components:UNSTABLE_GridListLoadingSentinel

/react-aria-components:UNSTABLE_ListBoxLoadingSentinel

/react-aria-components:UNSTABLE_TableLoadingSentinel

/react-aria-components:UNSTABLE_TreeLoadingIndicator

/react-aria-components:UNSTABLE_TreeLoadingSentinel

@react-stately/collections

/@react-stately/collections:getItemCount

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu commented Jun 17, 2025

Uh oh!

Uh oh!

LFDanLu Jun 2, 2025 •

edited

Loading