Skip to content

Commit

Permalink
Table resizing api audit fixes (#3855)
Browse files Browse the repository at this point in the history
  • Loading branch information
LFDanLu authored Dec 15, 2022
1 parent 694a181 commit f5f2631
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 17 deletions.
9 changes: 5 additions & 4 deletions packages/@react-aria/table/src/useTableColumnResize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ export interface AriaTableColumnResizeProps<T> {
* */
onMoveEnd?: (e: MoveEndEvent) => void,
/** Called when resizing starts. */
onResizeStart: (key: Key) => void,
onResizeStart?: (widths: Map<Key, number | string>) => void,
/** Called for every resize event that results in new column sizes. */
onResize: (widths: Map<Key, number | string>) => void,
onResize?: (widths: Map<Key, number | string>) => void,
/** Called when resizing ends. */
onResizeEnd: (key: Key) => void
onResizeEnd?: (widths: Map<Key, number | string>) => void
}


Expand Down Expand Up @@ -96,8 +96,9 @@ export function useTableColumnResize<T>(props: AriaTableColumnResizeProps<T>, st

let startResize = useCallback((item) => {
if (!isResizing.current) {
lastSize.current = layoutState.onColumnResize(item.key, layoutState.getColumnWidth(item.key));
layoutState.onColumnResizeStart(item.key);
onResizeStart?.(item.key);
onResizeStart?.(lastSize.current);
}
isResizing.current = true;
}, [isResizing, onResizeStart, layoutState]);
Expand Down
9 changes: 5 additions & 4 deletions packages/@react-aria/table/stories/example-resizing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function Table(props) {
{[...headerRow.childNodes].map(column =>
column.props.isSelectionCell
? <TableSelectAllCell key={column.key} column={column} state={state} widths={widths} />
: <TableColumnHeader key={column.key} column={column} state={state} widths={widths} layoutState={layoutState} onResize={props.onResize} onResizeEnd={props.onResizeEnd} />
: <TableColumnHeader key={column.key} column={column} state={state} widths={widths} layoutState={layoutState} onResizeStart={props.onResizeStart} onResize={props.onResize} onResizeEnd={props.onResizeEnd} />
)}
</TableHeaderRow>
))}
Expand Down Expand Up @@ -125,11 +125,12 @@ export function TableHeaderRow({item, state, children, className}) {
</tr>
);
}
function Resizer({column, state, layoutState, onResize, onResizeEnd}) {
function Resizer({column, state, layoutState, onResizeStart, onResize, onResizeEnd}) {
let ref = useRef(null);
let {resizerProps, inputProps} = useTableColumnResize({
column,
label: 'Resizer',
onResizeStart,
onResize,
onResizeEnd
} as AriaTableColumnResizeProps<any>, state, layoutState, ref);
Expand Down Expand Up @@ -162,7 +163,7 @@ function Resizer({column, state, layoutState, onResize, onResizeEnd}) {
</>
);
}
export function TableColumnHeader({column, state, widths, layoutState, onResize, onResizeEnd}) {
export function TableColumnHeader({column, state, widths, layoutState, onResizeStart, onResize, onResizeEnd}) {
let ref = useRef();
let {columnHeaderProps} = useTableColumnHeader({node: column}, state, ref);
let {isFocusVisible, focusProps} = useFocusRing();
Expand Down Expand Up @@ -196,7 +197,7 @@ export function TableColumnHeader({column, state, widths, layoutState, onResize,
</div>
{
column.props.allowsResizing &&
<Resizer column={column} state={state} layoutState={layoutState} onResize={onResize} onResizeEnd={onResizeEnd} />
<Resizer column={column} state={state} layoutState={layoutState} onResizeStart={onResizeStart} onResize={onResize} onResizeEnd={onResizeEnd} />
}
</div>
</th>
Expand Down
19 changes: 19 additions & 0 deletions packages/@react-aria/table/test/tableResizingTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,25 @@ export let resizingTests = (render, rerender, Table, ControlledTable, resizeCol,
expect(onResizeEnd).toHaveBeenCalledWith(mapFromWidths(columnNames, [113, 112, '1fr', '1fr', '4fr']));
expect(getColumnWidths(tree)).toStrictEqual([113, 112, 113, 112, 450]);
});

it('onResizeStart called with expected values', function () {
let columns = [
{name: 'Name', uid: 'name', width: '1fr'},
{name: 'Type', uid: 'type', width: '1fr'},
{name: 'Height', uid: 'height'},
{name: 'Weight', uid: 'weight'},
{name: 'Level', uid: 'level', width: '4fr'}
];

let columnNames = ['Name', 'Type', 'Height', 'Weight', 'Level'];
let onResizeStart = jest.fn();

let tree = render(<ControlledTable columns={columns} onResizeStart={onResizeStart} />);
expect(getColumnWidths(tree)).toStrictEqual([113, 112, 113, 112, 450]);
resizeCol(tree, 'Height', -50);
expect(onResizeStart).toHaveBeenCalledTimes(1);
expect(onResizeStart).toHaveBeenCalledWith(mapFromWidths(columnNames, [113, 112, 113, '1fr', '4fr']));
});
});

describe('resizing table', () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/@react-spectrum/table/src/Resizer.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable jsx-a11y/role-supports-aria-props */
import {classNames} from '@react-spectrum/utils';
import {ColumnSize} from '@react-types/table';
import {FocusRing} from '@react-aria/focus';
import {GridNode} from '@react-types/grid';
// @ts-ignore
Expand All @@ -16,9 +17,9 @@ interface ResizerProps<T> {
column: GridNode<T>,
showResizer: boolean,
triggerRef: RefObject<HTMLDivElement>,
onResizeStart: (key: Key) => void,
onResize: (widths: Map<Key, number | string>) => void,
onResizeEnd: (key: Key) => void,
onResizeStart: (widths: Map<Key, ColumnSize>) => void,
onResize: (widths: Map<Key, ColumnSize>) => void,
onResizeEnd: (widths: Map<Key, ColumnSize>) => void,
onMoveResizer: (e: MoveMoveEvent) => void
}

Expand Down
13 changes: 7 additions & 6 deletions packages/@react-spectrum/table/src/TableView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ interface TableContextValue<T> {
setIsInResizeMode: (val: boolean) => void,
isEmpty: boolean,
onFocusedResizer: () => void,
onResizeStart: (key: Key) => void,
onResizeStart: (widths: Map<Key, ColumnSize>) => void,
onResize: (widths: Map<Key, ColumnSize>) => void,
onResizeEnd: (key: Key) => void,
onResizeEnd: (widths: Map<Key, ColumnSize>) => void,
onMoveResizer: (e: MoveMoveEvent) => void,
headerMenuOpen: boolean,
setHeaderMenuOpen: (val: boolean) => void
Expand All @@ -114,7 +114,7 @@ export function useVirtualizerContext() {

function TableView<T extends object>(props: SpectrumTableProps<T>, ref: DOMRef<HTMLDivElement>) {
props = useProviderProps(props);
let {isQuiet, onAction, onResizeEnd: propsOnResizeEnd} = props;
let {isQuiet, onAction, onResizeStart: propsOnResizeStart, onResizeEnd: propsOnResizeEnd} = props;
let {styleProps} = useStyleProps(props);

let [showSelectionCheckboxes, setShowSelectionCheckboxes] = useState(props.selectionStyle !== 'highlight');
Expand Down Expand Up @@ -369,9 +369,10 @@ function TableView<T extends object>(props: SpectrumTableProps<T>, ref: DOMRef<H
lastResizeInteractionModality.current = undefined;
}
};
let onResizeStart = useCallback(() => {
let onResizeStart = useCallback((widths) => {
setIsResizing(true);
}, [setIsResizing]);
propsOnResizeStart?.(widths);
}, [setIsResizing, propsOnResizeStart]);
let onResizeEnd = useCallback((widths) => {
setIsInResizeMode(false);
setIsResizing(false);
Expand Down Expand Up @@ -504,7 +505,7 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo
let resizerAtEdge = resizerPosition > Math.max(state.virtualizer.contentSize.width, state.virtualizer.visibleRect.width) - 3;
// this should be fine, every movement of the resizer causes a rerender
// scrolling can cause it to lag for a moment, but it's always updated
let resizerInVisibleRegion = resizerPosition < state.virtualizer.visibleRect.width + (isNaN(bodyRef.current?.scrollLeft) ? 0 : bodyRef.current?.scrollLeft);
let resizerInVisibleRegion = resizerPosition < state.virtualizer.visibleRect.maxX;
let shouldHardCornerResizeCorner = resizerAtEdge && resizerInVisibleRegion;

// minimize re-render caused on Resizers by memoing this
Expand Down
26 changes: 26 additions & 0 deletions packages/@react-spectrum/table/stories/Table.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,32 @@ storiesOf('TableView', module)
),
{chromatic: {disable: true}}
)
.add(
'allowsResizing, onColumnResizeStart action',
() => (
<TableView aria-label="TableView with resizable columns" width={800} height={200} onResizeStart={action('onResizeStart')}>
<TableHeader>
<Column allowsResizing defaultWidth="1fr">File Name</Column>
<Column allowsResizing defaultWidth="2fr">Type</Column>
<Column allowsResizing defaultWidth="2fr">Size</Column>
<Column allowsResizing defaultWidth="1fr">Weight</Column>
</TableHeader>
<TableBody>
<Row>
<Cell>2018 Proposal</Cell>
<Cell>PDF</Cell>
<Cell>214 KB</Cell>
<Cell>1 LB</Cell>
</Row>
<Row>
<Cell>Budget</Cell>
<Cell>XLS</Cell>
<Cell>120 KB</Cell>
<Cell>20 LB</Cell>
</Row>
</TableBody>
</TableView>
))
.add(
'allowsResizing, onColumnResize action',
() => (
Expand Down
4 changes: 4 additions & 0 deletions packages/@react-types/table/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ export interface SpectrumTableProps<T> extends TableProps<T>, SpectrumSelectionP
renderEmptyState?: () => JSX.Element,
/** Handler that is called when a user performs an action on a row. */
onAction?: (key: Key) => void,
/**
* Handler that is called when a user starts a column resize.
*/
onResizeStart?: (widths: Map<Key, ColumnSize>) => void,
/**
* Handler that is called when a user performs a column resize.
* Can be used with the width property on columns to put the column widths into
Expand Down

0 comments on commit f5f2631

Please sign in to comment.