Skip to content

Commit 190e7e2

Browse files
authored
chore: revert focus loading indicators and table scroll events for patch (#8376)
* Revert "Revert "Revert "feat: focus loading indicator in rac tree (#8270)" (#8336)" (#8351)" This reverts commit ea7f3e6. * Revert "feat: Allow scroll events to be added on certain table components (#8150)" This reverts commit 4150364.
1 parent 90f8432 commit 190e7e2

File tree

7 files changed

+30
-52
lines changed

7 files changed

+30
-52
lines changed

packages/@react-aria/selection/src/ListKeyboardDelegate.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export class ListKeyboardDelegate<T> implements KeyboardDelegate {
7878
let nextKey = key;
7979
while (nextKey != null) {
8080
let item = this.collection.getItem(nextKey);
81-
if (item?.type === 'loader' || (item?.type === 'item' && !this.isDisabled(item))) {
81+
if (item?.type === 'item' && !this.isDisabled(item)) {
8282
return nextKey;
8383
}
8484

packages/react-aria-components/example/index.css

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ html {
2727
}
2828
}
2929

30-
.tree-loader,
3130
.tree-item {
3231
padding: 4px 5px;
3332
outline: none;

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ export interface TableHeaderRenderProps {
540540
isHovered: boolean
541541
}
542542

543-
export interface TableHeaderProps<T> extends StyleRenderProps<TableHeaderRenderProps>, HoverEvents, Pick<React.HTMLAttributes<HTMLTableSectionElement>, 'onScroll'> {
543+
export interface TableHeaderProps<T> extends StyleRenderProps<TableHeaderRenderProps>, HoverEvents {
544544
/** A list of table columns. */
545545
columns?: Iterable<T>,
546546
/** 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(
589589
<THead
590590
{...mergeProps(filterDOMProps(props as any), rowGroupProps, hoverProps)}
591591
{...renderProps}
592-
onScroll={props.onScroll}
593592
ref={ref}
594593
data-hovered={isHovered || undefined}>
595594
{headerRows}
@@ -916,7 +915,7 @@ export interface TableBodyRenderProps {
916915
isDropTarget: boolean
917916
}
918917

919-
export interface TableBodyProps<T> extends Omit<CollectionProps<T>, 'disabledKeys'>, StyleRenderProps<TableBodyRenderProps>, Pick<React.HTMLAttributes<HTMLTableSectionElement>, 'onScroll'> {
918+
export interface TableBodyProps<T> extends Omit<CollectionProps<T>, 'disabledKeys'>, StyleRenderProps<TableBodyRenderProps> {
920919
/** Provides content to display when there are no rows in the table. */
921920
renderEmptyState?: (props: TableBodyRenderProps) => ReactNode
922921
}
@@ -980,7 +979,6 @@ export const TableBody = /*#__PURE__*/ createBranchComponent('tablebody', <T ext
980979
<TBody
981980
{...mergeProps(filterDOMProps(props as any), rowGroupProps)}
982981
{...renderProps}
983-
onScroll={props.onScroll}
984982
ref={ref}
985983
data-empty={isEmpty || undefined}>
986984
{isDroppable && <RootDropIndicator />}

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

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ export const TreeItem = /*#__PURE__*/ createBranchComponent('item', <T extends o
695695
);
696696
});
697697

698-
export interface UNSTABLE_TreeLoadingIndicatorRenderProps extends Pick<TreeItemRenderProps, 'isFocused' | 'isFocusVisible'> {
698+
export interface UNSTABLE_TreeLoadingIndicatorRenderProps {
699699
/**
700700
* What level the tree item has within the tree.
701701
* @selector [data-level]
@@ -707,44 +707,36 @@ export interface TreeLoaderProps extends RenderProps<UNSTABLE_TreeLoadingIndicat
707707

708708
export const UNSTABLE_TreeLoadingIndicator = createLeafComponent('loader', function TreeLoader<T extends object>(props: TreeLoaderProps, ref: ForwardedRef<HTMLDivElement>, item: Node<T>) {
709709
let state = useContext(TreeStateContext)!;
710-
ref = useObjectRef<HTMLDivElement>(ref);
711-
let {rowProps, gridCellProps, ...states} = useTreeItem({node: item}, state, ref);
710+
// This loader row is is non-interactable, but we want the same aria props calculated as a typical row
711+
// @ts-ignore
712+
let {rowProps} = useTreeItem({node: item}, state, ref);
712713
let level = rowProps['aria-level'] || 1;
713714

714715
let ariaProps = {
715-
role: 'row',
716716
'aria-level': rowProps['aria-level'],
717717
'aria-posinset': rowProps['aria-posinset'],
718-
'aria-setsize': rowProps['aria-setsize'],
719-
tabIndex: rowProps.tabIndex
718+
'aria-setsize': rowProps['aria-setsize']
720719
};
721720

722-
let {isFocusVisible, focusProps} = useFocusRing();
723-
724721
let renderProps = useRenderProps({
725722
...props,
726723
id: undefined,
727724
children: item.rendered,
728725
defaultClassName: 'react-aria-TreeLoader',
729726
values: {
730-
level,
731-
isFocused: states.isFocused,
732-
isFocusVisible
727+
level
733728
}
734729
});
735730

736731
return (
737732
<>
738733
<div
734+
role="row"
739735
ref={ref}
740-
{...mergeProps(filterDOMProps(props as any), ariaProps, focusProps)}
736+
{...mergeProps(filterDOMProps(props as any), ariaProps)}
741737
{...renderProps}
742-
data-key={rowProps['data-key']}
743-
data-collection={rowProps['data-collection']}
744-
data-focused={states.isFocused || undefined}
745-
data-focus-visible={isFocusVisible || undefined}
746738
data-level={level}>
747-
<div {...gridCellProps}>
739+
<div role="gridcell" aria-colindex={1}>
748740
{renderProps.children}
749741
</div>
750742
</div>

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,7 @@ let rows = [
322322

323323
const MyTreeLoader = () => {
324324
return (
325-
<UNSTABLE_TreeLoadingIndicator
326-
className={({isFocused, isFocusVisible}) => classNames(styles, 'tree-loader', {
327-
focused: isFocused,
328-
'focus-visible': isFocusVisible
329-
})}>
325+
<UNSTABLE_TreeLoadingIndicator>
330326
{({level}) => {
331327
let message = `Level ${level} loading spinner`;
332328
if (level === 1) {

packages/react-aria-components/test/Table.test.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -300,14 +300,12 @@ describe('Table', () => {
300300
}
301301
});
302302

303-
it('should support DOM props', async () => {
304-
const onScrollHeader = jest.fn();
305-
const onScrollBody = jest.fn();
303+
it('should support DOM props', () => {
306304
let {getByRole, getAllByRole} = renderTable({
307305
tableProps: {'data-testid': 'table'},
308-
tableHeaderProps: {'data-testid': 'table-header', onScroll: onScrollHeader},
306+
tableHeaderProps: {'data-testid': 'table-header'},
309307
columnProps: {'data-testid': 'column'},
310-
tableBodyProps: {'data-testid': 'table-body', onScroll: onScrollBody},
308+
tableBodyProps: {'data-testid': 'table-body'},
311309
rowProps: {'data-testid': 'row'},
312310
cellProps: {'data-testid': 'cell'}
313311
});
@@ -334,12 +332,6 @@ describe('Table', () => {
334332
for (let cell of getAllByRole('gridcell')) {
335333
expect(cell).toHaveAttribute('data-testid', 'cell');
336334
}
337-
338-
// trigger scrolls
339-
fireEvent.scroll(rowGroups[0]);
340-
fireEvent.scroll(rowGroups[1]);
341-
expect(onScrollHeader).toBeCalledTimes(1);
342-
expect(onScrollBody).toBeCalledTimes(1);
343335
});
344336

345337
it('should render checkboxes for selection', async () => {

packages/react-aria-components/test/Tree.test.tsx

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,7 @@ describe('Tree', () => {
11491149
expect(cell).toHaveAttribute('aria-colindex', '1');
11501150
});
11511151

1152-
it('should focus the load more row when using ArrowDown/ArrowUp', async () => {
1152+
it('should not focus the load more row when using ArrowDown/ArrowUp', async () => {
11531153
let {getAllByRole} = render(<LoadingMoreTree isLoading />);
11541154

11551155
let rows = getAllByRole('row');
@@ -1158,18 +1158,19 @@ describe('Tree', () => {
11581158

11591159
await user.tab();
11601160
expect(document.activeElement).toBe(rows[0]);
1161-
for (let i = 1; i < 8; i++) {
1161+
for (let i = 0; i < 5; i++) {
11621162
await user.keyboard('{ArrowDown}');
1163-
expect(document.activeElement).toBe(rows[i]);
11641163
}
1164+
expect(document.activeElement).toBe(rows[5]);
11651165

1166-
for (let i = 6; i >= 0; i--) {
1167-
await user.keyboard('{ArrowUp}');
1168-
expect(document.activeElement).toBe(rows[i]);
1169-
}
1166+
await user.keyboard('{ArrowDown}');
1167+
expect(document.activeElement).toBe(rows[7]);
1168+
1169+
await user.keyboard('{ArrowUp}');
1170+
expect(document.activeElement).toBe(rows[5]);
11701171
});
11711172

1172-
it('should focus the load more row when using End', async () => {
1173+
it('should not focus the load more row when using End', async () => {
11731174
let {getAllByRole} = render(<LoadingMoreTree isLoading />);
11741175

11751176
let rows = getAllByRole('row');
@@ -1179,14 +1180,14 @@ describe('Tree', () => {
11791180
await user.tab();
11801181
expect(document.activeElement).toBe(rows[0]);
11811182
await user.keyboard('{End}');
1182-
expect(document.activeElement).toBe(rows[21]);
1183+
expect(document.activeElement).toBe(rows[20]);
11831184

11841185
// Check that it didn't shift the focusedkey to the loader key even if DOM focus didn't shift to the loader
11851186
await user.keyboard('{ArrowUp}');
1186-
expect(document.activeElement).toBe(rows[20]);
1187+
expect(document.activeElement).toBe(rows[19]);
11871188
});
11881189

1189-
it('should focus the load more row when using PageDown', async () => {
1190+
it('should not focus the load more row when using PageDown', async () => {
11901191
let {getAllByRole} = render(<LoadingMoreTree isLoading />);
11911192

11921193
let rows = getAllByRole('row');
@@ -1196,11 +1197,11 @@ describe('Tree', () => {
11961197
await user.tab();
11971198
expect(document.activeElement).toBe(rows[0]);
11981199
await user.keyboard('{PageDown}');
1199-
expect(document.activeElement).toBe(rows[21]);
1200+
expect(document.activeElement).toBe(rows[20]);
12001201

12011202
// Check that it didn't shift the focusedkey to the loader key even if DOM focus didn't shift to the loader
12021203
await user.keyboard('{ArrowUp}');
1203-
expect(document.activeElement).toBe(rows[20]);
1204+
expect(document.activeElement).toBe(rows[19]);
12041205
});
12051206

12061207
it('should not render no results state and the loader at the same time', () => {

0 commit comments

Comments
 (0)