Skip to content

Commit b472822

Browse files
committed
Code review comments
1 parent c5407ad commit b472822

File tree

2 files changed

+9
-15
lines changed

2 files changed

+9
-15
lines changed

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent(
591591
<THead
592592
{...mergeProps(filterDOMProps(props as any, {propNames: tableHeaderPropNames}), rowGroupProps, hoverProps)}
593593
{...renderProps}
594+
onScroll={props.onScroll}
594595
ref={ref}
595596
data-hovered={isHovered || undefined}>
596597
{headerRows}
@@ -904,9 +905,7 @@ export const ColumnResizer = forwardRef(function ColumnResizer(props: ColumnResi
904905
);
905906
});
906907

907-
const tableBodyPropNames = new Set(['onScroll']);
908-
909-
export interface TableBodyRenderProps extends Pick<React.HTMLAttributes<HTMLTableSectionElement>, 'onScroll'> {
908+
export interface TableBodyRenderProps {
910909
/**
911910
* Whether the table body has no rows and should display its empty state.
912911
* @selector [data-empty]
@@ -919,7 +918,7 @@ export interface TableBodyRenderProps extends Pick<React.HTMLAttributes<HTMLTabl
919918
isDropTarget: boolean
920919
}
921920

922-
export interface TableBodyProps<T> extends Omit<CollectionProps<T>, 'disabledKeys'>, StyleRenderProps<TableBodyRenderProps> {
921+
export interface TableBodyProps<T> extends Omit<CollectionProps<T>, 'disabledKeys'>, StyleRenderProps<TableBodyRenderProps>, Pick<React.HTMLAttributes<HTMLTableSectionElement>, 'onScroll'> {
923922
/** Provides content to display when there are no rows in the table. */
924923
renderEmptyState?: (props: TableBodyRenderProps) => ReactNode
925924
}
@@ -981,8 +980,9 @@ export const TableBody = /*#__PURE__*/ createBranchComponent('tablebody', <T ext
981980
// call useLoadMore here and walk up the DOM to the nearest scrollable element to set scrollRef
982981
return (
983982
<TBody
984-
{...mergeProps(filterDOMProps(props as any, {propNames: tableBodyPropNames}), rowGroupProps)}
983+
{...mergeProps(filterDOMProps(props as any), rowGroupProps)}
985984
{...renderProps}
985+
onScroll={props.onScroll}
986986
ref={ref}
987987
data-empty={isEmpty || undefined}>
988988
{isDroppable && <RootDropIndicator />}
@@ -995,16 +995,14 @@ export const TableBody = /*#__PURE__*/ createBranchComponent('tablebody', <T ext
995995
);
996996
});
997997

998-
const rowPropNames = new Set(['onContextMenu']);
999-
1000998
export interface RowRenderProps extends ItemRenderProps {
1001999
/** Whether the row's children have keyboard focus. */
10021000
isFocusVisibleWithin: boolean,
10031001
/** The unique id of the row. */
10041002
id?: Key
10051003
}
10061004

1007-
export interface RowProps<T> extends StyleRenderProps<RowRenderProps>, LinkDOMProps, HoverEvents, Pick<React.HTMLAttributes<HTMLTableRowElement>, 'onContextMenu'> {
1005+
export interface RowProps<T> extends StyleRenderProps<RowRenderProps>, LinkDOMProps, HoverEvents {
10081006
/** A list of columns used when dynamically rendering cells. */
10091007
columns?: Iterable<T>,
10101008
/** The cells within the row. Supports static items or a function for dynamic rendering. */
@@ -1118,7 +1116,7 @@ export const Row = /*#__PURE__*/ createBranchComponent(
11181116
</TR>
11191117
)}
11201118
<TR
1121-
{...mergeProps(filterDOMProps(props as any, {propNames: rowPropNames}), rowProps, focusProps, hoverProps, draggableItem?.dragProps, focusWithinProps)}
1119+
{...mergeProps(filterDOMProps(props as any), rowProps, focusProps, hoverProps, draggableItem?.dragProps, focusWithinProps)}
11221120
{...renderProps}
11231121
ref={ref}
11241122
data-disabled={states.isDisabled || undefined}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,16 +299,15 @@ describe('Table', () => {
299299
}
300300
});
301301

302-
it('should support DOM props', async () => {
303-
const onContextMenu = jest.fn();
302+
it('should support onScroll props', async () => {
304303
const onScrollHeader = jest.fn();
305304
const onScrollBody = jest.fn();
306305
let {getByRole, getAllByRole} = renderTable({
307306
tableProps: {'data-testid': 'table'},
308307
tableHeaderProps: {'data-testid': 'table-header', onScroll: onScrollHeader},
309308
columnProps: {'data-testid': 'column'},
310309
tableBodyProps: {'data-testid': 'table-body', onScroll: onScrollBody},
311-
rowProps: {'data-testid': 'row', onContextMenu},
310+
rowProps: {'data-testid': 'row'},
312311
cellProps: {'data-testid': 'cell'}
313312
});
314313
let table = getByRole('grid');
@@ -340,9 +339,6 @@ describe('Table', () => {
340339
fireEvent.scroll(rowGroups[1]);
341340
expect(onScrollHeader).toBeCalledTimes(1);
342341
expect(onScrollBody).toBeCalledTimes(1);
343-
const row = getAllByRole('row')[1];
344-
fireEvent.contextMenu(row);
345-
expect(onContextMenu).toBeCalledTimes(1);
346342
});
347343

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

0 commit comments

Comments
 (0)