Skip to content

feat: nested collection support #7379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
83ab27d
feat: nested collection support
nwidynski Nov 15, 2024
642d6ae
feat: add support for custom node keys
nwidynski Dec 13, 2024
9f24163
Merge branch 'main' into document_prefix
nwidynski Dec 13, 2024
10a3290
fix: key generation for static scenario
nwidynski Dec 13, 2024
ab327ca
fix: lint error
nwidynski Dec 13, 2024
e78c176
chore: remove key normalization
nwidynski Dec 13, 2024
4e3d68e
fix: static node key scopes & table tests
nwidynski Dec 14, 2024
8e9dfb8
fix: grid id mismatch
nwidynski Dec 18, 2024
060673c
fix: reordering in virtualized gridlist
nwidynski Dec 19, 2024
90acf6c
fix: drag and drop
nwidynski Jan 6, 2025
aab8a0c
feat: node key util & grid/table support
nwidynski Jan 7, 2025
c814b48
chore: upgrade pkg-lock and leverage combined scope in builder
nwidynski Jan 7, 2025
aad0e44
Merge branch 'main' into document_prefix
nwidynski Jan 7, 2025
466e4f0
fix: duplicate import
nwidynski Jan 7, 2025
aede6fc
chore: merge upstream
nwidynski Jan 23, 2025
7cbb3ad
fix: table rendering in docs
nwidynski Jan 23, 2025
f90758f
chore: fix build
nwidynski Jan 23, 2025
e04da95
Merge branch 'main' into document_prefix
nwidynski Feb 6, 2025
d25b5ca
Merge branch 'main' of github.com:adobe/react-spectrum into document_…
devongovett Feb 15, 2025
c0c33a8
contain changes to selection package
devongovett Feb 15, 2025
0f6a1f2
revert idScope change
devongovett Feb 20, 2025
22964a3
Merge branch 'main' of github.com:adobe/react-spectrum into document_…
devongovett Feb 20, 2025
2ce6172
Merge branch 'main' of github.com:adobe/react-spectrum into document_…
devongovett Feb 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions packages/@react-aria/gridlist/src/useGridList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ export interface AriaGridListProps<T> extends GridListProps<T>, DOMProps, AriaLa
export interface AriaGridListOptions<T> extends Omit<AriaGridListProps<T>, 'children'> {
/** Whether the list uses virtual scrolling. */
isVirtualized?: boolean,
/**
* Whether typeahead navigation is disabled.
* @default false
*/
disallowTypeAhead?: boolean,
/**
* An optional keyboard delegate implementation for type to select,
* to override the default.
Expand Down Expand Up @@ -95,6 +100,7 @@ export function useGridList<T>(props: AriaGridListOptions<T>, state: ListState<T
keyboardDelegate,
layoutDelegate,
onAction,
disallowTypeAhead,
linkBehavior = 'action',
keyboardNavigationBehavior = 'arrow'
} = props;
Expand All @@ -103,21 +109,24 @@ export function useGridList<T>(props: AriaGridListOptions<T>, state: ListState<T
console.warn('An aria-label or aria-labelledby prop is required for accessibility.');
}

let id = useId(props.id);
listMap.set(state, {id, onAction, linkBehavior, keyboardNavigationBehavior});

let {listProps} = useSelectableList({
selectionManager: state.selectionManager,
collection: state.collection,
disabledKeys: state.disabledKeys,
ref,
idScope: id,
keyboardDelegate,
layoutDelegate,
isVirtualized,
selectOnFocus: state.selectionManager.selectionBehavior === 'replace',
shouldFocusWrap: props.shouldFocusWrap,
linkBehavior
linkBehavior,
disallowTypeAhead
});

let id = useId(props.id);
listMap.set(state, {id, onAction, linkBehavior, keyboardNavigationBehavior});

let descriptionProps = useHighlightSelectionDescription({
selectionManager: state.selectionManager,
Expand Down
7 changes: 4 additions & 3 deletions packages/@react-aria/gridlist/src/useGridListItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,11 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt
onFocus,
// 'aria-label': [(node.textValue || undefined), rowAnnouncement].filter(Boolean).join(', '),
'aria-label': node.textValue || undefined,
'aria-selected': state.selectionManager.canSelectItem(node.key) ? state.selectionManager.isSelected(node.key) : undefined,
'aria-disabled': state.selectionManager.isDisabled(node.key) || undefined,
'aria-selected': itemStates.allowsSelection ? itemStates.isSelected : undefined,
'aria-disabled': itemStates.isDisabled || undefined,
'aria-labelledby': descriptionId && node.textValue ? `${getRowId(state, node.key)} ${descriptionId}` : undefined,
id: getRowId(state, node.key)
id: getRowId(state, node.key),
'data-key': getRowId(state, node.key)
});

if (isVirtualized) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-aria/selection/src/DOMLayoutDelegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class DOMLayoutDelegate implements LayoutDelegate {
if (!container) {
return null;
}
let item = key != null ? container.querySelector(`[data-key="${CSS.escape(key.toString())}"]`) : null;
let item = key != null ? container.querySelector(`[data-key$="${CSS.escape(key.toString())}"]`) : null;
if (!item) {
return null;
}
Expand Down
12 changes: 9 additions & 3 deletions packages/@react-aria/selection/src/useSelectableCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export interface AriaSelectableCollectionOptions {
* The ref attached to the element representing the collection.
*/
ref: RefObject<HTMLElement | null>,
/** A scope to prepend to all child node keys to ensure they are unique. */
idScope?: Key,
/**
* Whether the collection or one of its items should be automatically focused upon render.
* @default false
Expand Down Expand Up @@ -104,6 +106,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
selectionManager: manager,
keyboardDelegate: delegate,
ref,
idScope,
autoFocus = false,
shouldFocusWrap = false,
disallowEmptySelection = false,
Expand Down Expand Up @@ -140,7 +143,8 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
manager.setFocusedKey(key, childFocus);
});

let item = scrollRef.current?.querySelector(`[data-key="${CSS.escape(key.toString())}"]`);
let nodeKey = idScope ? `${idScope}-${key.toString()}` : key.toString();
let item = scrollRef.current?.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`);
let itemProps = manager.getItemProps(key);
if (item) {
router.open(item, e, itemProps.href, itemProps.routerOptions);
Expand Down Expand Up @@ -369,7 +373,8 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions

if (manager.focusedKey != null && scrollRef.current) {
// Refocus and scroll the focused item into view if it exists within the scrollable region.
let element = scrollRef.current.querySelector(`[data-key="${CSS.escape(manager.focusedKey.toString())}"]`) as HTMLElement;
let nodeKey = idScope ? `${idScope}-${manager.focusedKey.toString()}` : manager.focusedKey.toString();
let element = scrollRef.current.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`) as HTMLElement;
if (element) {
// This prevents a flash of focus on the first/last element in the collection, or the collection itself.
if (!element.contains(document.activeElement)) {
Expand Down Expand Up @@ -461,7 +466,8 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
useEffect(() => {
if (manager.isFocused && manager.focusedKey != null && (manager.focusedKey !== lastFocusedKey.current || autoFocusRef.current) && scrollRef.current && ref.current) {
let modality = getInteractionModality();
let element = ref.current.querySelector(`[data-key="${CSS.escape(manager.focusedKey.toString())}"]`) as HTMLElement;
let nodeKey = idScope ? `${idScope}-${manager.focusedKey.toString()}` : manager.focusedKey.toString();
let element = ref.current.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`) as HTMLElement;
if (!element) {
// If item element wasn't found, return early (don't update autoFocusRef and lastFocusedKey).
// The collection may initially be empty (e.g. virtualizer), so wait until the element exists.
Expand Down
9 changes: 7 additions & 2 deletions packages/react-aria-components/src/GridList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ export interface GridListRenderProps {
}

export interface GridListProps<T> extends Omit<AriaGridListProps<T>, 'children'>, CollectionProps<T>, StyleRenderProps<GridListRenderProps>, SlotProps, ScrollableProps<HTMLDivElement> {
/**
* Whether typeahead navigation is disabled.
* @default false
*/
disallowTypeAhead?: boolean,
/** How multiple selection should behave in the collection. */
selectionBehavior?: SelectionBehavior,
/** The drag and drop hooks returned by `useDragAndDrop` used to enable drag and drop behavior for the GridList. */
Expand Down Expand Up @@ -120,7 +125,7 @@ function GridListInner<T extends object>({props, collection, gridListRef: ref}:
})
), [collection, ref, layout, disabledKeys, disabledBehavior, layoutDelegate, collator, direction]);

let {gridProps} = useGridList({
let {gridProps: {id, ...gridProps}} = useGridList({
...props,
keyboardDelegate,
// Only tab navigation is supported in grid layout.
Expand Down Expand Up @@ -218,7 +223,7 @@ function GridListInner<T extends object>({props, collection, gridListRef: ref}:
<div
{...filterDOMProps(props)}
{...renderProps}
{...mergeProps(gridProps, focusProps, droppableCollection?.collectionProps, emptyStatePropOverrides)}
{...mergeProps(gridProps, focusProps, droppableCollection?.collectionProps, emptyStatePropOverrides, {id})}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened here?

Copy link
Contributor Author

@nwidynski nwidynski Feb 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses two issues. The first is #7655, which we scheduled as a follow-up to this PR but has since landed. Second is that drag & drop hooks generate their own id without a defaultId, overriding any custom ids here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm does the latter case still occur? I think mergeProps should merge the ids

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. @LFDanLu and I just agreed it would be better to hand the id down to gridlists call to useDroppableCollection to avoid implicitly relying on mergeProps #7379 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will just push that as follow-up once i get into the office tomorrow 👍

ref={ref}
slot={props.slot || undefined}
onScroll={props.onScroll}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-aria-components/src/ListBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function ListBoxInner<T extends object>({state, props, listBoxRef}: ListBoxInner
})
), [collection, collator, listBoxRef, disabledBehavior, disabledKeys, orientation, direction, props.keyboardDelegate, layout, layoutDelegate]);

let {listBoxProps} = useListBox({
let {listBoxProps: {id, ...listBoxProps}} = useListBox({
...props,
shouldSelectOnPressUp: isListDraggable || props.shouldSelectOnPressUp,
keyboardDelegate,
Expand Down Expand Up @@ -230,7 +230,7 @@ function ListBoxInner<T extends object>({state, props, listBoxRef}: ListBoxInner
<FocusScope>
<div
{...filterDOMProps(props)}
{...mergeProps(listBoxProps, focusProps, droppableCollection?.collectionProps)}
{...mergeProps(listBoxProps, focusProps, droppableCollection?.collectionProps, {id})}
{...renderProps}
ref={listBoxRef}
slot={props.slot || undefined}
Expand Down
8 changes: 5 additions & 3 deletions packages/react-aria-components/src/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl

let {isVirtualized, layoutDelegate, dropTargetDelegate: ctxDropTargetDelegate, CollectionRoot} = useContext(CollectionRendererContext);
let {dragAndDropHooks} = props;
let {gridProps} = useTable({
let {gridProps: {id, ...gridProps}} = useTable({
...props,
layoutDelegate,
isVirtualized
Expand Down Expand Up @@ -471,7 +471,7 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl
<ElementType
{...filterDOMProps(props)}
{...renderProps}
{...mergeProps(gridProps, focusProps, droppableCollection?.collectionProps)}
{...mergeProps(gridProps, focusProps, droppableCollection?.collectionProps, {id})}
style={style}
ref={ref}
slot={props.slot || undefined}
Expand Down Expand Up @@ -1137,9 +1137,11 @@ export const Row = /*#__PURE__*/ createBranchComponent(
throw new Error('No id detected for the Row element. The Row element requires a id to be provided to it when the cells are rendered dynamically.');
}

let scope = props.columns ? props.id : undefined;
let dependencies = [props.value].concat(props.dependencies);

return (
<Collection dependencies={dependencies} items={props.columns} idScope={props.id}>
<Collection dependencies={dependencies} items={props.columns} idScope={scope}>
{props.children}
</Collection>
);
Expand Down
16 changes: 14 additions & 2 deletions packages/react-aria-components/stories/GridList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export function TagGroupInsideGridList() {
<GridList
className={styles.menu}
aria-label="Grid list with tag group"
keyboardNavigationBehavior="tab"
style={{
width: 300,
height: 300
Expand All @@ -194,8 +195,19 @@ export function TagGroupInsideGridList() {
</TagList>
</TagGroup>
</MyGridListItem>
<MyGridListItem>1,2 <Button>Actions</Button></MyGridListItem>
<MyGridListItem>1,3 <Button>Actions</Button></MyGridListItem>
<MyGridListItem>
1,2 <Button>Actions</Button>
</MyGridListItem>
<MyGridListItem>
1,3
<TagGroup aria-label="Tag group">
<TagList style={{display: 'flex', gap: 10}}>
<Tag key="1">Tag 1</Tag>
<Tag key="2">Tag 2</Tag>
<Tag key="3">Tag 3</Tag>
</TagList>
</TagGroup>
</MyGridListItem>
</GridList>
);
}
2 changes: 1 addition & 1 deletion packages/react-aria-components/test/Table.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function MyRow({id, columns, children, ...otherProps}) {
let {selectionBehavior, allowsDragging} = useTableOptions();

return (
<Row id={id} {...otherProps}>
<Row id={id} {...otherProps} columns={columns}>
{allowsDragging && (
<Cell>
<Button slot="drag">≡</Button>
Expand Down