Add support for custom collection renderers (e.g. virtualization) #5912

devongovett · 2024-02-20T19:41:05Z

This adds support for custom collection renderers to React Aria Components, which enables features like virtualized scrolling where a subset of the collection is rendered to the DOM. Other features like breadcrumb and action group collapsing can also be built as collection renderers.

A collection renderer is a function that is provided to a component via context, which means it wraps around a collection component. For example, to make a ListBox virtualized, you would wrap it in a <Virtualizer>:

<Virtualizer>
  <ListBox>
    <ListBoxItem>Foo</ListBoxItem>
  </ListBox>
</Virtualizer>

The Virtualizer component provides the CollectionRendererContext to the ListBox, which uses it to render the collection to DOM nodes. By default, CollectionRendererContext is set to a function that renders all items to the DOM (the same way as currently). This function accepts two arguments: the collection itself, and the parent node whose items should be rendered (e.g. in the case of sections). A virtualizer would then filter the list of items to include only visible items.

To enable this in a generic way, collection nodes now know how to render themselves to the DOM. This means that a CollectionRenderer does not need to know how to render specific nodes, it delegates back to the nodes themselves. This also means that custom implementations of collection components such as ListBoxItem can be created and work within an existing ListBox – the item components are no longer coupled to the parent components.

This works by creating two new APIs for building collection components:

createLeafComponent – creates a collection component that doesn't accept child nodes. You must specify a node type (e.g. item), and a function to render that node.
createBranchComponent – creates a collection component that expects children. It also accepts a node type and render function, but also accepts a function that returns the collection children (defaulting to useCollectionChildren).

The plan would be to eventually expose these APIs publicly so custom collection components can be built, but I'm not totally happy with these APIs yet. Feels like there could be something that might be able to combine them a bit more. Suggestions welcome.

rspbot · 2024-02-20T19:49:29Z

Build successful! 🎉

devongovett · 2024-02-20T19:54:48Z

packages/react-aria-components/src/Menu.tsx

-
-function SubmenuTriggerInner(props) {
-  let {item, parentMenuRef} = props;
+export const SubmenuTrigger =  /*#__PURE__*/ createBranchComponent('submenutrigger', (props: SubmenuTriggerProps, ref: ForwardedRef<HTMLDivElement>, item) => {


SubmenuTrigger changed so that the state/hooks for submenus live at the trigger level and get passed down to the MenuItem via context rather than having two separate MenuItem implementations. This works due to ref merging by useContextProps.

devongovett · 2024-02-20T19:55:32Z

packages/@react-aria/menu/src/useSubmenuTrigger.ts

@@ -117,7 +115,7 @@ export function UNSTABLE_useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, st

  let submenuProps = {
    id: overlayId,
-    'aria-label': node.textValue,
+    'aria-labelledby': submenuTriggerId,


This avoids needing to pass in the menu item node. In RAC's case, the node is the submenutrigger node instead.

majornista · 2024-02-20T20:06:30Z

With virtualization, rendered option items should include aria-posinset and aria-setsize.

Also, when the heading for a group is scrolled out of view, the group will no longer have an accessibility name, which differs from the behavior in React-Spectrum ListBox.

packages/react-aria-components/stories/ListBox.stories.tsx

LFDanLu

Just some initial things found from testing, will edit the comment as I continue my review

LFDanLu · 2024-02-20T22:59:56Z

packages/react-aria-components/stories/ListBox.stories.tsx

(Haven't looked at the internals yet, will revisit then. Jotting down issues found during a testing):

Noticed that PageUp/PageDown only advance focus by one item after the first initial "page" jump

Home/End don't seem to cause scroll position to move the the first/last item

focus is lost to the body of the page when you scroll the focused item out of view

yeah in a real implementation the keyboard delegate would need to be overridden on the listbox and get set to the layout. I can probably do that in the story.

devongovett · 2024-02-20T23:09:55Z

For reviewers, keep in mind that the demo virtualizer is not a full implementation yet, it's just a simple demo in the story. The PR is mainly about setting up the infrastructure to support custom collection renderers. More work will be needed for a full virtualizer implementation. But feel free to post the issues you run into!

LFDanLu · 2024-02-20T23:05:49Z

packages/react-aria-components/stories/ListBox.stories.tsx

+      layout={layout}
+      style={{height: 'inherit'}}
+      collection={collection}
+      shouldUseVirtualFocus>


Should this really have shouldUseVirtualFocus?

I used that so that we don't apply a tabIndex to the virtualizer element. In RSP, the virtualizer is the outer most element (the one with role="listbox") so this made sense. But here, the virtualizer is rendered inside the listbox (it has role="presentation"), so it should not also have a tabIndex. Really virtualizer is duplicating the logic already handled by the listbox/selection hooks, so it isn't really necessary.

LFDanLu · 2024-02-20T23:44:44Z

packages/react-aria-components/src/Menu.tsx

-      }
-    }
-  });
+  let renderer = useContext(CollectionRendererContext);


Since this is grabbing from the CollectionRendererContext, what happens if you have nested components but the inner component shouldn't be virtualized? For instance, maybe a virtualized Table with non-virtualized Menus within?

could clear the context in "leaves"?

this is a good point. another idea would be to change the API to provide collection renderers as a prop instead of context. The component would then have to propagate it to children via its own context, but that wouldn't apply to other components. I sort of liked the idea that virtualizer could just be a wrapper though. Not sure...

packages/react-aria-components/src/Table.tsx

LFDanLu · 2024-02-21T00:29:13Z

packages/react-aria-components/src/Collection.tsx

+  return <>{useCollectionChildren(props)}</>;
+}
+
+export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>, node: Node<T>) => JSX.Element) {


You mentioned you weren't happy about the api here for these, did you have any other specific concerns other than possibly combining them more? To me, using these for custom collections already feels like a pretty advanced use case that needs a deeper understanding of how the RAC collections work so not sure how to simplify them more tbh

Thanks for the input, we'd still like to eliminate the useShallowComponent since that name doesn't make a ton of sense. We'd discussed something like this in order to make a meaningfully named option, though haven't quite gotten the types worked out for it.

export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>, node: Node<T>) => JSX.Element, allowsOutsideCollection?: false); export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>, node?: Node<T>) => JSX.Element, allowsOutsideCollection: true); export function createLeafComponent<T extends object, P extends object, E extends Element>(type: string, render: (props: P, ref: ForwardedRef<E>, node?: Node<T>) => JSX.Element, allowsOutsideCollection: boolean = false) {

Got it, I understand the desire to remove createShallowComponent() 👍.

Personally i am not a big fan of hiding things behind boolean flags, since they are hard to read and discover, but I also don’t have a better suggestion in mind at current.

PS: I must have accidentally deleted my previous comment, so excuse the messed up timeline.

Yep, we're not totally enamored with it, either. But we think it's better than createShallowComponent, it's descriptive and shows it's only allowed for a leaf. If anyone has a better idea, we're all ears.

No worries about the deleted comment.

packages/react-aria-components/src/Breadcrumbs.tsx

packages/react-aria-components/src/GridList.tsx

snowystinger · 2024-02-21T00:31:15Z

packages/react-aria-components/src/Menu.tsx

-}
-
-function MenuSection<T>({section, className, style, parentMenuRef, ...otherProps}: MenuSectionProps<T>) {
+function MenuSection<T>(section: Node<T>, props: SectionProps<T>, ref: ForwardedRef<HTMLElement>) {


nitpick to be consistent with argument order, the createBranch etc all do
props, ref, node
whereas this is node, props, ref

ah yeah I changed this at the last minute, good point

can we align these? or does this just need to be resolved now?

…n-renderers

rspbot · 2024-02-28T20:34:13Z

Build successful! 🎉

nwidynski · 2024-03-31T14:37:29Z

packages/react-aria-components/src/Collection.tsx

@@ -77,6 +78,7 @@ export class NodeValue<T> implements Node<T> {
    node.firstChildKey = this.firstChildKey;
    node.lastChildKey = this.lastChildKey;
    node.props = this.props;
+    node.render = this.render;
    return node;


nit: This whole block could be replaced.

Suggested change

return node;

return Object.assign(node, this);

…n-renderers # Conflicts: # packages/@react-aria/menu/src/useSubmenuTrigger.ts # packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx # packages/@react-spectrum/menu/src/SubmenuTrigger.tsx # packages/@react-spectrum/menu/test/SubMenuTrigger.test.tsx # packages/react-aria-components/src/Breadcrumbs.tsx # packages/react-aria-components/src/Menu.tsx # packages/react-aria-components/src/Table.tsx # packages/react-aria-components/stories/ListBox.stories.tsx

…n-renderers

packages/@react-aria/menu/src/useSubmenuTrigger.ts

rspbot · 2024-05-08T23:29:20Z

Build successful! 🎉

snowystinger

Couple questions from just looking at the code, I'll go test it out now

packages/react-aria-components/src/Collection.tsx

packages/@react-aria/menu/src/useSubmenuTrigger.ts

packages/react-aria-components/src/Collection.tsx

snowystinger · 2024-05-09T01:15:17Z

packages/react-aria-components/src/Collection.tsx

+  });
+};
+
+export const CollectionRendererContext = createContext<CollectionRenderer>(useDefaultCollectionRenderer);


is the renderer all we'll ever need back from this?
or do we want to make it an object in case we decide we want to send more information through this context?
I don't have an example of anything yet

snowystinger · 2024-05-09T01:33:09Z

packages/react-aria-components/src/Menu.tsx

-}
-
-function MenuSection<T>({section, className, style, parentMenuRef, ...otherProps}: MenuSectionProps<T>) {
+function MenuSection<T>(section: Node<T>, props: SectionProps<T>, ref: ForwardedRef<HTMLElement>) {


can we align these? or does this just need to be resolved now?

snowystinger · 2024-05-09T02:17:05Z

Initial testing looks good!

When the virtualizer updates the visible views, it triggers a re-render of the parent component (e.g. section) whose children changed. But we don't need to re-render the wrapper element, just the children. By splitting this into a separate component, we avoid re-rendering the parent when only its children changed.

rspbot · 2024-05-10T00:35:30Z

Build successful! 🎉

…n-renderers # Conflicts: # packages/react-aria-components/src/Breadcrumbs.tsx

devongovett · 2024-05-22T20:24:17Z

I realized I will need to do some significant refactoring to Virtualizer to support RAC, which is somewhat separate from this work to support custom collection renderers. I would like to land this change initially and follow up with that additional work next. Though there might be some additional changes to the collection renderer API, this is already useful on its own for other non-virtualizer cases (e.g. collapsing breadcrumbs).

rspbot · 2024-05-22T20:46:55Z

Build successful! 🎉

rspbot · 2024-05-22T21:02:37Z

Build successful! 🎉

rspbot · 2024-05-24T17:30:16Z

Build successful! 🎉

LFDanLu

LGTM, did a quick sweep and the collection components seem to be working as expected. Also verified locally that leaf components properly error if rendered outside a collection except for Header. Definitely like how this has simplified the internals of many of the RAC components now that the collection nodes know how to render themselves

LFDanLu · 2024-05-24T17:30:27Z

packages/@react-spectrum/tree/src/TreeView.tsx

+  /** A list of child tree item objects used when dynamically rendering the tree item children. */
+  childItems?: Iterable<T>


For posterity in case we need to find the related ticket/discussion: https://github.com/orgs/adobe/projects/19/views/4?filterQuery=rac+colle&pane=issue&itemId=24923448

We will want to possibly change TreeView to a more RAC Tree like api instead of mimicking the same api of other current RSP collection components (e.g. using TreeItemContent instead of <TreeViewItem childItems={item.childItems} ...>) which will let us get rid of this prop

…n-renderers # Conflicts: # packages/react-aria-components/src/Breadcrumbs.tsx

rspbot · 2024-05-25T05:23:08Z

Build successful! 🎉

rspbot · 2024-05-25T05:23:09Z

## 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: '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' }
undefined already in set

@react-aria/menu

AriaSubmenuTriggerProps

 AriaSubmenuTriggerProps {
   delay?: number = 200
   isDisabled?: boolean
-  node: RSNode<unknown>
   parentMenuRef: RefObject<HTMLElement>
   submenuRef: RefObject<HTMLElement>
   type?: 'dialog' | 'menu'
 }

it changed:

useSubmenuTrigger

@react-spectrum/tree

TreeViewItem

-TreeViewItem {
-  TreeViewItem?: boolean
-}
+

SpectrumTreeViewItemProps

-SpectrumTreeViewItemProps {
-  children: ReactNode
-  hasChildItems?: boolean
-}
+

undefined

-
+TreeViewItem<T extends {}> {
+  TreeViewItem?: boolean
+}

undefined

-
+SpectrumTreeViewItemProps<T extends {} = {}> {
+  childItems?: Iterable<{}>
+  children: ReactNode
+  hasChildItems?: boolean
+}

@react-stately/layout

ListLayout

 ListLayout<T> {
   allowDisabledKeyFocus: boolean
   buildChild: (Node<T>, number, number) => LayoutNode
   buildCollection: () => Array<LayoutNode>
+  buildHeader: (Node<T>, number, number) => LayoutNode
   buildItem: (Node<T>, number, number) => LayoutNode
   buildNode: (Node<T>, number, number) => LayoutNode
   buildSection: (Node<T>, number, number) => LayoutNode
   collection: Collection<Node<T>>
   disabledKeys: Set<Key>
   getContentSize: () => void
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
   getFinalLayoutInfo: (LayoutInfo) => void
   getFirstKey: () => Key | null
   getInitialLayoutInfo: (LayoutInfo) => void
   getKeyAbove: (Key) => Key | null
   getKeyBelow: (Key) => Key | null
   getKeyForSearch: (string, Key) => Key | null
   getKeyPageAbove: (Key) => Key | null
   getKeyPageBelow: (Key) => Key | null
   getLastKey: () => Key | null
   getLayoutInfo: (Key) => void
   getVisibleLayoutInfos: (Rect) => void
   isLoading: boolean
   isValid: (Node<T>, number) => void
   isVisible: (LayoutNode, Rect) => void
   updateItemSize: (Key, Size) => void
   updateLayoutNode: (Key, LayoutInfo, LayoutInfo) => void
   validate: (InvalidationContext<Node<T>, unknown>) => void
 }

Add support for custom collection renderers (e.g. virtualization)

11e152d

devongovett commented Feb 20, 2024

View reviewed changes

reidbarber reviewed Feb 20, 2024

View reviewed changes

packages/react-aria-components/stories/ListBox.stories.tsx Show resolved Hide resolved

packages/react-aria-components/stories/ListBox.stories.tsx Show resolved Hide resolved

LFDanLu reviewed Feb 20, 2024

View reviewed changes

LFDanLu reviewed Feb 21, 2024

View reviewed changes

snowystinger reviewed Feb 21, 2024

View reviewed changes

Merge branch 'main' of github.com:adobe/react-spectrum into collectio…

3b483bd

…n-renderers

LFDanLu mentioned this pull request Mar 1, 2024

[RAC] Expose collection components and utils to enable custom collections #5954

Open

nwidynski reviewed Apr 10, 2024

View reviewed changes

snowystinger mentioned this pull request Apr 22, 2024

Passing nested row into table causes getCollectionNode error #2746

Open

devongovett self-assigned this May 7, 2024

devongovett added 3 commits May 8, 2024 14:57

Remove createShallowComponent and clear context at leaves

a31e0a0

Merge branch 'main' of github.com:adobe/react-spectrum into collectio…

b606a45

…n-renderers

lint

3227437

devongovett commented May 8, 2024

View reviewed changes

packages/@react-aria/menu/src/useSubmenuTrigger.ts Show resolved Hide resolved

snowystinger reviewed May 9, 2024

View reviewed changes

snowystinger previously approved these changes May 16, 2024

View reviewed changes

devongovett added 2 commits May 22, 2024 11:39

Merge branch 'main' of github.com:adobe/react-spectrum into collectio…

57c12f2

…n-renderers # Conflicts: # packages/react-aria-components/src/Breadcrumbs.tsx

add node back to useSubmenuTrigger to prevent breaking change

d7e3b10

devongovett dismissed snowystinger’s stale review via d7e3b10 May 22, 2024 20:21

fix lint

0b6a2d3

devongovett added 2 commits May 22, 2024 13:25

remove createShallowComponent from docs generator

da871e0

change argument order for rob

39db25f

snowystinger previously approved these changes May 22, 2024

View reviewed changes

Merge branch 'main' into collection-renderers

74827d3

LFDanLu previously approved these changes May 24, 2024

View reviewed changes

Merge branch 'main' of github.com:adobe/react-spectrum into collectio…

13475a1

…n-renderers # Conflicts: # packages/react-aria-components/src/Breadcrumbs.tsx

devongovett dismissed stale reviews from LFDanLu and snowystinger via 13475a1 May 25, 2024 05:08

devongovett merged commit 2bda4c9 into main May 25, 2024
26 checks passed

devongovett deleted the collection-renderers branch May 25, 2024 05:24

devongovett mentioned this pull request Jun 8, 2024

Add Virtualizer to React Aria Components #6518

Merged

		/** A list of child tree item objects used when dynamically rendering the tree item children. */
		childItems?: Iterable<T>

Add support for custom collection renderers (e.g. virtualization) #5912

Add support for custom collection renderers (e.g. virtualization) #5912

Conversation

devongovett commented Feb 20, 2024 • edited Loading

rspbot commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majornista commented Feb 20, 2024 • edited Loading

LFDanLu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devongovett commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Feb 28, 2024

Choose a reason for hiding this comment

rspbot commented May 8, 2024

snowystinger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snowystinger commented May 9, 2024

rspbot commented May 10, 2024

devongovett commented May 22, 2024

rspbot commented May 22, 2024

rspbot commented May 22, 2024

rspbot commented May 24, 2024

LFDanLu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented May 25, 2024

rspbot commented May 25, 2024

@react-aria/menu

AriaSubmenuTriggerProps

@react-spectrum/tree

TreeViewItem

SpectrumTreeViewItemProps

undefined

undefined

@react-stately/layout

ListLayout

devongovett commented Feb 20, 2024 •

edited

Loading

majornista commented Feb 20, 2024 •

edited

Loading