From 912d4bc9196f528c504fdb97e41579091edfefaa Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 10 Jun 2025 10:31:08 -0700 Subject: [PATCH 1/2] Revert "Revert "Revert "feat: focus loading indicator in rac tree (#8270)" (#8336)" (#8351)" This reverts commit ea7f3e6eb4170c8aa037a99d6a23c24003d2ff51. --- .../selection/src/ListKeyboardDelegate.ts | 2 +- .../react-aria-components/example/index.css | 1 - packages/react-aria-components/src/Tree.tsx | 26 +++++++----------- .../stories/Tree.stories.tsx | 6 +---- .../react-aria-components/test/Tree.test.tsx | 27 ++++++++++--------- 5 files changed, 25 insertions(+), 37 deletions(-) diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index c92e16fc8e1..f76d740f5af 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -78,7 +78,7 @@ export class ListKeyboardDelegate implements KeyboardDelegate { let nextKey = key; while (nextKey != null) { let item = this.collection.getItem(nextKey); - if (item?.type === 'loader' || (item?.type === 'item' && !this.isDisabled(item))) { + if (item?.type === 'item' && !this.isDisabled(item)) { return nextKey; } diff --git a/packages/react-aria-components/example/index.css b/packages/react-aria-components/example/index.css index 8b3803867d5..11e2637ad97 100644 --- a/packages/react-aria-components/example/index.css +++ b/packages/react-aria-components/example/index.css @@ -27,7 +27,6 @@ html { } } -.tree-loader, .tree-item { padding: 4px 5px; outline: none; diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index 01606651510..e02ec0a2b9e 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -695,7 +695,7 @@ export const TreeItem = /*#__PURE__*/ createBranchComponent('item', { +export interface UNSTABLE_TreeLoadingIndicatorRenderProps { /** * What level the tree item has within the tree. * @selector [data-level] @@ -707,44 +707,36 @@ export interface TreeLoaderProps extends RenderProps(props: TreeLoaderProps, ref: ForwardedRef, item: Node) { let state = useContext(TreeStateContext)!; - ref = useObjectRef(ref); - let {rowProps, gridCellProps, ...states} = useTreeItem({node: item}, state, ref); + // This loader row is is non-interactable, but we want the same aria props calculated as a typical row + // @ts-ignore + let {rowProps} = useTreeItem({node: item}, state, ref); let level = rowProps['aria-level'] || 1; let ariaProps = { - role: 'row', 'aria-level': rowProps['aria-level'], 'aria-posinset': rowProps['aria-posinset'], - 'aria-setsize': rowProps['aria-setsize'], - tabIndex: rowProps.tabIndex + 'aria-setsize': rowProps['aria-setsize'] }; - let {isFocusVisible, focusProps} = useFocusRing(); - let renderProps = useRenderProps({ ...props, id: undefined, children: item.rendered, defaultClassName: 'react-aria-TreeLoader', values: { - level, - isFocused: states.isFocused, - isFocusVisible + level } }); return ( <>
-
+
{renderProps.children}
diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index 790467b6d17..f9a83404b71 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -322,11 +322,7 @@ let rows = [ const MyTreeLoader = () => { return ( - classNames(styles, 'tree-loader', { - focused: isFocused, - 'focus-visible': isFocusVisible - })}> + {({level}) => { let message = `Level ${level} loading spinner`; if (level === 1) { diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 6076f97ffcf..0d7017382a9 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -1149,7 +1149,7 @@ describe('Tree', () => { expect(cell).toHaveAttribute('aria-colindex', '1'); }); - it('should focus the load more row when using ArrowDown/ArrowUp', async () => { + it('should not focus the load more row when using ArrowDown/ArrowUp', async () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); @@ -1158,18 +1158,19 @@ describe('Tree', () => { await user.tab(); expect(document.activeElement).toBe(rows[0]); - for (let i = 1; i < 8; i++) { + for (let i = 0; i < 5; i++) { await user.keyboard('{ArrowDown}'); - expect(document.activeElement).toBe(rows[i]); } + expect(document.activeElement).toBe(rows[5]); - for (let i = 6; i >= 0; i--) { - await user.keyboard('{ArrowUp}'); - expect(document.activeElement).toBe(rows[i]); - } + await user.keyboard('{ArrowDown}'); + expect(document.activeElement).toBe(rows[7]); + + await user.keyboard('{ArrowUp}'); + expect(document.activeElement).toBe(rows[5]); }); - it('should focus the load more row when using End', async () => { + it('should not focus the load more row when using End', async () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); @@ -1179,14 +1180,14 @@ describe('Tree', () => { await user.tab(); expect(document.activeElement).toBe(rows[0]); await user.keyboard('{End}'); - expect(document.activeElement).toBe(rows[21]); + expect(document.activeElement).toBe(rows[20]); // Check that it didn't shift the focusedkey to the loader key even if DOM focus didn't shift to the loader await user.keyboard('{ArrowUp}'); - expect(document.activeElement).toBe(rows[20]); + expect(document.activeElement).toBe(rows[19]); }); - it('should focus the load more row when using PageDown', async () => { + it('should not focus the load more row when using PageDown', async () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); @@ -1196,11 +1197,11 @@ describe('Tree', () => { await user.tab(); expect(document.activeElement).toBe(rows[0]); await user.keyboard('{PageDown}'); - expect(document.activeElement).toBe(rows[21]); + expect(document.activeElement).toBe(rows[20]); // Check that it didn't shift the focusedkey to the loader key even if DOM focus didn't shift to the loader await user.keyboard('{ArrowUp}'); - expect(document.activeElement).toBe(rows[20]); + expect(document.activeElement).toBe(rows[19]); }); it('should not render no results state and the loader at the same time', () => { From 65300409ee1db2c92c390e4e9fe34179e43c2a0f Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 10 Jun 2025 10:31:52 -0700 Subject: [PATCH 2/2] Revert "feat: Allow scroll events to be added on certain table components (#8150)" This reverts commit 4150364a2dfa249ca149723548c62e0d3325476f. --- packages/react-aria-components/src/Table.tsx | 6 ++---- packages/react-aria-components/test/Table.test.js | 14 +++----------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index c9968f05e35..b0559e64713 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -540,7 +540,7 @@ export interface TableHeaderRenderProps { isHovered: boolean } -export interface TableHeaderProps extends StyleRenderProps, HoverEvents, Pick, 'onScroll'> { +export interface TableHeaderProps extends StyleRenderProps, HoverEvents { /** A list of table columns. */ columns?: Iterable, /** A list of `Column(s)` or a function. If the latter, a list of columns must be provided using the `columns` prop. */ @@ -589,7 +589,6 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( {headerRows} @@ -916,7 +915,7 @@ export interface TableBodyRenderProps { isDropTarget: boolean } -export interface TableBodyProps extends Omit, 'disabledKeys'>, StyleRenderProps, Pick, 'onScroll'> { +export interface TableBodyProps extends Omit, 'disabledKeys'>, StyleRenderProps { /** Provides content to display when there are no rows in the table. */ renderEmptyState?: (props: TableBodyRenderProps) => ReactNode } @@ -980,7 +979,6 @@ export const TableBody = /*#__PURE__*/ createBranchComponent('tablebody', {isDroppable && } diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index 41d57e24fbb..01f0d6fc243 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -300,14 +300,12 @@ describe('Table', () => { } }); - it('should support DOM props', async () => { - const onScrollHeader = jest.fn(); - const onScrollBody = jest.fn(); + it('should support DOM props', () => { let {getByRole, getAllByRole} = renderTable({ tableProps: {'data-testid': 'table'}, - tableHeaderProps: {'data-testid': 'table-header', onScroll: onScrollHeader}, + tableHeaderProps: {'data-testid': 'table-header'}, columnProps: {'data-testid': 'column'}, - tableBodyProps: {'data-testid': 'table-body', onScroll: onScrollBody}, + tableBodyProps: {'data-testid': 'table-body'}, rowProps: {'data-testid': 'row'}, cellProps: {'data-testid': 'cell'} }); @@ -334,12 +332,6 @@ describe('Table', () => { for (let cell of getAllByRole('gridcell')) { expect(cell).toHaveAttribute('data-testid', 'cell'); } - - // trigger scrolls - fireEvent.scroll(rowGroups[0]); - fireEvent.scroll(rowGroups[1]); - expect(onScrollHeader).toBeCalledTimes(1); - expect(onScrollBody).toBeCalledTimes(1); }); it('should render checkboxes for selection', async () => {