Skip to content
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

feat: (DNM) Test build for nested collection support #7616

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
feat: node key util & grid/table support
nwidynski committed Jan 7, 2025
commit aab8a0cefcb67338d2acfb0f16cbfa1393e2d986
1 change: 1 addition & 0 deletions packages/@react-aria/collections/src/index.ts
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@ export {CollectionBuilder, Collection, createLeafComponent, createBranchComponen
export {createHideableComponent, useIsHidden} from './Hidden';
export {useCachedChildren} from './useCachedChildren';
export {BaseCollection, CollectionNode} from './BaseCollection';
export {getNodeKey} from './utils';

export type {CollectionBuilderProps, CollectionProps} from './CollectionBuilder';
export type {CachedChildrenOptions} from './useCachedChildren';
3 changes: 2 additions & 1 deletion packages/@react-aria/collections/src/useCachedChildren.ts
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@
*/

import {cloneElement, ReactElement, ReactNode, useMemo} from 'react';
import {getNodeKey} from './utils';
import {Key} from '@react-types/shared';

export interface CachedChildrenOptions<T> {
@@ -51,7 +52,7 @@ export function useCachedChildren<T extends object>(props: CachedChildrenOptions
}

if (idScope) {
key = idScope + ':' + key;
key = getNodeKey(key, idScope);
}
// Note: only works if wrapped Item passes through id...
rendered = cloneElement(
5 changes: 5 additions & 0 deletions packages/@react-aria/collections/src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import {Key} from '@react-types/shared';

export function getNodeKey(key: Key, idScope?: Key) {
return idScope ? `${idScope}:${key}` : `${key}`;
}
1 change: 1 addition & 0 deletions packages/@react-aria/dnd/package.json
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@
},
"dependencies": {
"@internationalized/string": "^3.2.5",
"@react-aria/collections": "3.0.0-alpha.6",
"@react-aria/i18n": "^3.12.4",
"@react-aria/interactions": "^3.22.5",
"@react-aria/live-announcer": "^3.4.1",
4 changes: 2 additions & 2 deletions packages/@react-aria/dnd/src/ListDropTargetDelegate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Direction, DropTarget, DropTargetDelegate, Node, Orientation, RefObject} from '@react-types/shared';
import {getNodeKey} from '@react-aria/collections';

interface ListDropTargetDelegateOptions {
/**
@@ -106,8 +107,7 @@ export class ListDropTargetDelegate implements DropTargetDelegate {
while (low < high) {
let mid = Math.floor((low + high) / 2);
let item = items[mid];
let nodeKey = idScope ? `${idScope}-${item.key}` : item.key;
let element = elementMap.get(nodeKey.toString());
let element = elementMap.get(getNodeKey(item.key, idScope));
if (!element) {
break;
}
1 change: 1 addition & 0 deletions packages/@react-aria/grid/package.json
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@
"url": "https://github.com/adobe/react-spectrum"
},
"dependencies": {
"@react-aria/collections": "3.0.0-alpha.6",
"@react-aria/focus": "^3.19.0",
"@react-aria/i18n": "^3.12.4",
"@react-aria/interactions": "^3.22.5",
16 changes: 12 additions & 4 deletions packages/@react-aria/grid/src/useGrid.ts
Original file line number Diff line number Diff line change
@@ -26,6 +26,11 @@ import {useSelectableCollection} from '@react-aria/selection';
export interface GridProps extends DOMProps, AriaLabelingProps {
/** Whether the grid 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.
@@ -66,6 +71,7 @@ export interface GridAria {
export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<T>>, ref: RefObject<HTMLElement | null>): GridAria {
let {
isVirtualized,
disallowTypeAhead,
keyboardDelegate,
focusMode,
scrollRef,
@@ -94,17 +100,19 @@ export function useGrid<T>(props: GridProps, state: GridState<T, GridCollection<
focusMode
}), [keyboardDelegate, state.collection, state.disabledKeys, disabledBehavior, ref, direction, collator, focusMode]);

let id = useId(props.id);
gridMap.set(state, {id, keyboardDelegate: delegate, actions: {onRowAction, onCellAction}});

let {collectionProps} = useSelectableCollection({
ref,
idScope: id,
selectionManager: manager,
keyboardDelegate: delegate,
isVirtualized,
scrollRef
scrollRef,
disallowTypeAhead
});

let id = useId(props.id);
gridMap.set(state, {keyboardDelegate: delegate, actions: {onRowAction, onCellAction}});

let descriptionProps = useHighlightSelectionDescription({
selectionManager: manager,
hasItemActions: !!(onRowAction || onCellAction)
6 changes: 4 additions & 2 deletions packages/@react-aria/grid/src/useGridCell.ts
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@

import {DOMAttributes, FocusableElement, Key, RefObject} from '@react-types/shared';
import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus';
import {getNodeKey} from '@react-aria/collections';
import {getScrollParent, mergeProps, scrollIntoViewport} from '@react-aria/utils';
import {GridCollection, GridNode} from '@react-types/grid';
import {gridMap} from './utils';
@@ -60,7 +61,7 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
} = props;

let {direction} = useLocale();
let {keyboardDelegate, actions: {onCellAction}} = gridMap.get(state)!;
let {id, keyboardDelegate, actions: {onCellAction}} = gridMap.get(state)!;

// We need to track the key of the item at the time it was last focused so that we force
// focus to go to the item when the DOM node is reused for a different item in a virtualizer.
@@ -251,7 +252,8 @@ export function useGridCell<T, C extends GridCollection<T>>(props: GridCellProps
let gridCellProps: DOMAttributes = mergeProps(itemProps, {
role: 'gridcell',
onKeyDownCapture,
onFocus
onFocus,
'data-key': getNodeKey(node.key, id)
});

if (isVirtualized) {
11 changes: 6 additions & 5 deletions packages/@react-aria/grid/src/useGridRow.ts
Original file line number Diff line number Diff line change
@@ -10,8 +10,9 @@
* governing permissions and limitations under the License.
*/

import {chain} from '@react-aria/utils';
import {chain, mergeProps} from '@react-aria/utils';
import {DOMAttributes, FocusableElement, RefObject} from '@react-types/shared';
import {getNodeKey} from '@react-aria/collections';
import {GridCollection, GridNode} from '@react-types/grid';
import {gridMap} from './utils';
import {GridState} from '@react-stately/grid';
@@ -52,7 +53,7 @@ export function useGridRow<T, C extends GridCollection<T>, S extends GridState<T
onAction
} = props;

let {actions} = gridMap.get(state)!;
let {id, actions} = gridMap.get(state)!;
let onRowAction = actions.onRowAction ? () => actions.onRowAction?.(node.key) : onAction;
let {itemProps, ...states} = useSelectableItem({
selectionManager: state.selectionManager,
@@ -66,12 +67,12 @@ export function useGridRow<T, C extends GridCollection<T>, S extends GridState<T

let isSelected = state.selectionManager.isSelected(node.key);

let rowProps: DOMAttributes = {
let rowProps: DOMAttributes = mergeProps(itemProps, {
role: 'row',
'aria-selected': state.selectionManager.selectionMode !== 'none' ? isSelected : undefined,
'aria-disabled': states.isDisabled || undefined,
...itemProps
};
'data-key': getNodeKey(node.key, id)
});

if (isVirtualized) {
rowProps['aria-rowindex'] = node.index + 1; // aria-rowindex is 1 based
1 change: 1 addition & 0 deletions packages/@react-aria/grid/src/utils.ts
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@ import type {GridState} from '@react-stately/grid';
import type {Key, KeyboardDelegate} from '@react-types/shared';

interface GridMapShared {
id: string,
keyboardDelegate: KeyboardDelegate,
actions: {
onRowAction?: (key: Key) => void,
1 change: 1 addition & 0 deletions packages/@react-aria/gridlist/package.json
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@
"url": "https://github.com/adobe/react-spectrum"
},
"dependencies": {
"@react-aria/collections": "3.0.0-alpha.6",
"@react-aria/focus": "^3.19.0",
"@react-aria/grid": "^3.11.0",
"@react-aria/i18n": "^3.12.4",
5 changes: 3 additions & 2 deletions packages/@react-aria/gridlist/src/useGridListItem.ts
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@ import {chain, getScrollParent, mergeProps, scrollIntoViewport, useSlotId, useSy
import {DOMAttributes, FocusableElement, Key, RefObject, Node as RSNode} from '@react-types/shared';
import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus';
import {getLastItem} from '@react-stately/collections';
import {getNodeKey} from '@react-aria/collections';
import {getRowId, listMap} from './utils';
import {HTMLAttributes, KeyboardEvent as ReactKeyboardEvent, useRef} from 'react';
import {isFocusVisible} from '@react-aria/interactions';
@@ -67,7 +68,7 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt

// let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/gridlist');
let {direction} = useLocale();
let {onAction, linkBehavior, keyboardNavigationBehavior} = listMap.get(state)!;
let {id, onAction, linkBehavior, keyboardNavigationBehavior} = listMap.get(state)!;
let descriptionId = useSlotId();

// We need to track the key of the item at the time it was last focused so that we force
@@ -275,7 +276,7 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt
'aria-disabled': itemStates.isDisabled || undefined,
'aria-labelledby': descriptionId && node.textValue ? `${getRowId(state, node.key)} ${descriptionId}` : undefined,
id: getRowId(state, node.key),
'data-key': getRowId(state, node.key)
'data-key': getNodeKey(node.key, id)
});

if (isVirtualized) {
1 change: 1 addition & 0 deletions packages/@react-aria/selection/package.json
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@
"url": "https://github.com/adobe/react-spectrum"
},
"dependencies": {
"@react-aria/collections": "3.0.0-alpha.6",
"@react-aria/focus": "^3.19.0",
"@react-aria/i18n": "^3.12.4",
"@react-aria/interactions": "^3.22.5",
4 changes: 2 additions & 2 deletions packages/@react-aria/selection/src/DOMLayoutDelegate.ts
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
* governing permissions and limitations under the License.
*/

import {getNodeKey} from '@react-aria/collections';
import {Key, LayoutDelegate, Rect, RefObject, Size} from '@react-types/shared';

export class DOMLayoutDelegate implements LayoutDelegate {
@@ -25,8 +26,7 @@ export class DOMLayoutDelegate implements LayoutDelegate {
return null;
}
let idScope = this.ref.current?.dataset['scope'];
let nodeKey = idScope ? `${idScope}-${key}` : key;
let item = key != null ? container.querySelector(`[data-key="${CSS.escape(nodeKey.toString())}"]`) : null;
let item = key != null ? container.querySelector(`[data-key="${CSS.escape(getNodeKey(key, idScope))}"]`) : null;
if (!item) {
return null;
}
10 changes: 4 additions & 6 deletions packages/@react-aria/selection/src/useSelectableCollection.ts
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ import {flushSync} from 'react-dom';
import {FocusEvent, KeyboardEvent, useEffect, useRef} from 'react';
import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus';
import {getInteractionModality} from '@react-aria/interactions';
import {getNodeKey} from '@react-aria/collections';
import {isCtrlKeyPressed, isNonContiguousSelectionModifier} from './utils';
import {MultipleSelectionManager} from '@react-stately/selection';
import {useLocale} from '@react-aria/i18n';
@@ -143,8 +144,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
manager.setFocusedKey(key, childFocus);
});

let nodeKey = idScope ? `${idScope}-${key.toString()}` : key.toString();
let item = scrollRef.current?.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`);
let item = scrollRef.current?.querySelector(`[data-key="${CSS.escape(getNodeKey(key, idScope))}"]`);
let itemProps = manager.getItemProps(key);
if (item) {
router.open(item, e, itemProps.href, itemProps.routerOptions);
@@ -373,8 +373,7 @@ 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 nodeKey = idScope ? `${idScope}-${manager.focusedKey.toString()}` : manager.focusedKey.toString();
let element = scrollRef.current.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`) as HTMLElement;
let element = scrollRef.current.querySelector(`[data-key="${CSS.escape(getNodeKey(manager.focusedKey, idScope))}"]`) 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)) {
@@ -466,8 +465,7 @@ 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 nodeKey = idScope ? `${idScope}-${manager.focusedKey.toString()}` : manager.focusedKey.toString();
let element = ref.current.querySelector(`[data-key="${CSS.escape(nodeKey)}"]`) as HTMLElement;
let element = ref.current.querySelector(`[data-key="${CSS.escape(getNodeKey(manager.focusedKey, idScope))}"]`) 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.
22 changes: 22 additions & 0 deletions packages/react-aria-components/test/GridList.test.js
Original file line number Diff line number Diff line change
@@ -492,6 +492,28 @@ describe('GridList', () => {
expect(checkbox).toBeInTheDocument();
});

it('should support nested collections with colliding keys', async () => {
let {container} = render(
<GridList aria-label="CardView" keyboardNavigationBehavior="Tab">
<GridListItem id="1" textValue="Card">
<GridList aria-label="Previews">
<GridListItem id="1">Paco de Lucia</GridListItem>
</GridList>
</GridListItem>
</GridList>
);

let itemMap = new Map();
let items = container.querySelectorAll('[data-key]');

for (let item of items) {
if (item instanceof HTMLElement) {
expect(itemMap.has(item.dataset.key)).toBe(false);
itemMap.set(item.dataset.key, item);
}
}
});

describe('drag and drop', () => {
it('should support drag button slot', () => {
let {getAllByRole} = render(<DraggableGridList />);
41 changes: 40 additions & 1 deletion packages/react-aria-components/test/Table.test.js
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@
*/

import {act, fireEvent, installPointerEvent, mockClickDefault, pointerMap, render, triggerLongPress, within} from '@react-spectrum/test-utils-internal';
import {Button, Cell, Checkbox, Collection, Column, ColumnResizer, Dialog, DialogTrigger, DropIndicator, Label, Modal, ResizableTableContainer, Row, Table, TableBody, TableHeader, UNSTABLE_TableLayout as TableLayout, useDragAndDrop, useTableOptions, UNSTABLE_Virtualizer as Virtualizer} from '../';
import {Button, Cell, Checkbox, Collection, Column, ColumnResizer, Dialog, DialogTrigger, DropIndicator, Label, Modal, ResizableTableContainer, Row, Table, TableBody, TableHeader, UNSTABLE_TableLayout as TableLayout, Tag, TagGroup, TagList, useDragAndDrop, useTableOptions, UNSTABLE_Virtualizer as Virtualizer} from '../';
import {composeStories} from '@storybook/react';
import {DataTransfer, DragEvent} from '@react-aria/dnd/test/mocks';
import React, {useMemo, useRef, useState} from 'react';
@@ -124,6 +124,31 @@ let TestTable = ({tableProps, tableHeaderProps, columnProps, tableBodyProps, row
</Table>
);

let EditableTable = ({tableProps, tableHeaderProps, columnProps, tableBodyProps, rowProps, cellProps}) => (
<Table aria-label="Files" {...tableProps}>
<MyTableHeader {...tableHeaderProps}>
<MyColumn id="name" isRowHeader {...columnProps}>Name</MyColumn>
<MyColumn {...columnProps}>Type</MyColumn>
<MyColumn {...columnProps}>Actions</MyColumn>
</MyTableHeader>
<TableBody {...tableBodyProps}>
<MyRow id="1" textValue="Edit" {...rowProps}>
<Cell {...cellProps}>Games</Cell>
<Cell {...cellProps}>File folder</Cell>
<Cell {...cellProps}>
<TagGroup aria-label="Tag group">
<TagList>
<Tag id="1">Tag 1</Tag>
<Tag id="2">Tag 2</Tag>
<Tag id="3">Tag 3</Tag>
</TagList>
</TagGroup>
</Cell>
</MyRow>
</TableBody>
</Table>
);

let DraggableTable = (props) => {
let {dragAndDropHooks} = useDragAndDrop({
getItems: (keys) => [...keys].map((key) => ({'text/plain': key})),
@@ -895,6 +920,20 @@ describe('Table', () => {
expect(rows.map(r => r.textContent)).toEqual(['FooBar', 'Foo 7Bar 7', 'Foo 8Bar 8', 'Foo 9Bar 9', 'Foo 10Bar 10', 'Foo 11Bar 11', 'Foo 12Bar 12', 'Foo 13Bar 13', 'Foo 49Bar 49']);
});

it('should support nested collections with colliding keys', async () => {
let {container} = render(<EditableTable />);

let itemMap = new Map();
let items = container.querySelectorAll('[data-key]');

for (let item of items) {
if (item instanceof HTMLElement) {
expect(itemMap.has(item.dataset.key)).toBe(false);
itemMap.set(item.dataset.key, item);
}
}
});

describe('drag and drop', () => {
it('should support drag button slot', () => {
let {getAllByRole} = render(<DraggableTable />);