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

Rename Table's initialSortedRow to initialSortedColumn #2491

Merged
5 changes: 5 additions & 0 deletions .changeset/slimy-carpets-tease.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sumup/circuit-ui': minor
---

Renamed the Table's `initialSortedRow` prop to `initialSortedColumn` to better express its purpose. The `initialSortedRow` is deprecated and will be removed in the next major release.
30 changes: 27 additions & 3 deletions packages/circuit-ui/components/Table/Table.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,30 @@ describe('Table', () => {
});
});

it('should sort a column in ascending order when initial sort direction and initial sorted row is provided', () => {
it('should sort a column in ascending order when initial sort direction and initial sorted column is provided', () => {
render(
<Table
rows={rows}
headers={headers}
rowHeaders={false}
initialSortedColumn={1}
initialSortDirection={'ascending'}
/>,
);

const cellEls = screen.getAllByRole('cell');

const sortedRow = ['a', 'c', 'b'];

rows.forEach((_row, index) => {
const cellIndex = rowLength * index;
expect(cellEls[cellIndex]).toHaveTextContent(sortedRow[index]);
});
});

it('should sort a column in ascending order when initial sort direction and initial sorted row is provided and console.warn about it', () => {
const warnMock = vi.spyOn(console, 'warn');

render(
<Table
rows={rows}
Expand All @@ -132,6 +155,7 @@ describe('Table', () => {
const cellIndex = rowLength * index;
expect(cellEls[cellIndex]).toHaveTextContent(sortedRow[index]);
});
expect(warnMock).toHaveBeenCalled();
});

it('should sort a column in descending order', async () => {
Expand All @@ -151,13 +175,13 @@ describe('Table', () => {
});
});

it('should sort a column in descending order when initial sort direction and initial sorted row is provided', () => {
it('should sort a column in descending order when initial sort direction and initial sorted column is provided', () => {
render(
<Table
rows={rows}
headers={headers}
rowHeaders={false}
initialSortedRow={1}
initialSortedColumn={1}
initialSortDirection={'descending'}
/>,
);
Expand Down
48 changes: 30 additions & 18 deletions packages/circuit-ui/components/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Component, createRef, HTMLAttributes, UIEvent } from 'react';
import { isNil } from '../../util/type-check.js';
import { throttle } from '../../util/helpers.js';
import { clsx } from '../../styles/clsx.js';
import { deprecate } from '../../util/logger.js';

import TableHead from './components/TableHead/index.js';
import TableBody from './components/TableBody/index.js';
Expand Down Expand Up @@ -68,9 +69,15 @@ export interface TableProps extends HTMLAttributes<HTMLDivElement> {
*/
initialSortDirection?: 'ascending' | 'descending';
/**
* Specifies the row index which `initialSortDirection` will be applied to
* @deprecated
*
* Use the `initialSortedColumn` prop instead.
*/
initialSortedRow?: number;
/**
* Specifies the column index which `initialSortDirection` will be applied to
*/
initialSortedColumn?: number;
/**
* Click handler for the row
* The signature is (index)
Expand All @@ -83,7 +90,7 @@ export interface TableProps extends HTMLAttributes<HTMLDivElement> {
}

type TableState = {
sortedRow?: number;
sortedColumn?: number;
rows?: Row[];
sortHover?: number;
sortDirection?: Direction;
Expand All @@ -97,12 +104,19 @@ type TableState = {
class Table extends Component<TableProps, TableState> {
constructor(props: TableProps) {
super(props);
if (process.env.NODE_ENV !== 'production' && this.props.initialSortedRow) {
deprecate(
'Table',
'The `initialSortedRow` prop has been deprecated. Use the `initialSortedColumn` prop instead.',
);
}
this.state = {
sortedRow: this.props.initialSortedRow,
sortedColumn:
this.props.initialSortedColumn || this.props.initialSortedRow,
rows: this.getInitialRows(
this.props.rows,
this.props.initialSortDirection,
this.props.initialSortedRow,
this.props.initialSortedColumn || this.props.initialSortedRow,
),
sortHover: undefined,
sortDirection: this.props.initialSortDirection,
Expand All @@ -122,10 +136,10 @@ class Table extends Component<TableProps, TableState> {
componentDidUpdate(prevProps: TableProps): void {
if (this.props.rows !== prevProps.rows) {
// Preserve existing sorting
if (this.state.sortedRow && this.state.sortDirection) {
if (this.state.sortedColumn && this.state.sortDirection) {
const sortedRows = this.getSortedRows(
this.state.sortDirection,
this.state.sortedRow,
this.state.sortedColumn,
);
this.setState({ rows: sortedRows });
return;
Expand Down Expand Up @@ -177,8 +191,8 @@ class Table extends Component<TableProps, TableState> {
onSortLeave = (): void => this.setState({ sortHover: undefined });

onSortBy = (i: number): void => {
const { sortedRow, sortDirection } = this.state;
const isActive = i === sortedRow;
const { sortedColumn, sortDirection } = this.state;
const isActive = i === sortedColumn;
const nextDirection = getSortDirection(isActive, sortDirection);
const sortedRows = this.getSortedRows(nextDirection, i);

Expand All @@ -188,13 +202,11 @@ class Table extends Component<TableProps, TableState> {
getInitialRows = (
rows: Row[],
initialSortDirection?: Direction | undefined,
initialSortedRow?: number | undefined,
): Row[] => {
if (initialSortedRow && initialSortDirection) {
return this.getSortedRows(initialSortDirection, initialSortedRow);
}
return rows;
};
initialSortedColumn?: number | undefined,
): Row[] =>
initialSortedColumn && initialSortDirection
? this.getSortedRows(initialSortDirection, initialSortedColumn)
: rows;

getSortedRows = (sortDirection: Direction, sortedRow: number): Row[] => {
const { rows, onSortBy } = this.props;
Expand All @@ -205,7 +217,7 @@ class Table extends Component<TableProps, TableState> {

updateSort = (i: number, nextDirection: Direction, sortedRows: Row[]): void =>
this.setState({
sortedRow: i,
sortedColumn: i,
sortDirection: nextDirection,
rows: sortedRows,
});
Expand All @@ -231,7 +243,7 @@ class Table extends Component<TableProps, TableState> {
const {
sortDirection,
sortHover,
sortedRow,
sortedColumn,
scrollTop,
tableBodyHeight,
rows,
Expand Down Expand Up @@ -267,7 +279,7 @@ class Table extends Component<TableProps, TableState> {
condensed={condensed}
scrollable={scrollable}
sortDirection={sortDirection}
sortedRow={sortedRow}
sortedColumn={sortedColumn}
onSortBy={this.onSortBy}
onSortEnter={this.onSortEnter}
onSortLeave={this.onSortLeave}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ type TableHeadProps = ScrollableOptions & {
*/
sortDirection?: Direction;
/**
* The current sorted row index
* The current sorted column index
*/
sortedRow?: number;
sortedColumn?: number;
/**
* sortEnter handler
*/
Expand All @@ -87,7 +87,7 @@ export function TableHead({
top,
onSortBy,
sortDirection,
sortedRow,
sortedColumn,
onSortEnter,
onSortLeave,
}: TableHeadProps) {
Expand All @@ -105,11 +105,11 @@ export function TableHead({
const cellProps = mapCellProps(header);
const { sortable, sortLabel } = cellProps;
const sortParams = getSortParams({
rowIndex: i,
columnIndex: i,
sortable,
sortDirection,
sortLabel,
sortedRow,
sortedColumn,
});
return (
<Fragment key={`table-header-${i}`}>
Expand Down
12 changes: 6 additions & 6 deletions packages/circuit-ui/components/Table/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ describe('Table utils', () => {
describe('getSortParams', () => {
it('should return sort params with a string sortLabel', () => {
const actual = utils.getSortParams({
rowIndex: 1,
columnIndex: 1,
sortable: true,
sortLabel: 'Sort',
});
Expand All @@ -201,7 +201,7 @@ describe('Table utils', () => {
return `Sort in ${order} order`;
};
const actual = utils.getSortParams({
rowIndex: 1,
columnIndex: 1,
sortable: true,
sortLabel,
});
Expand All @@ -220,8 +220,8 @@ describe('Table utils', () => {
return `Sort in ${order} order`;
};
const actual = utils.getSortParams({
rowIndex: 1,
sortedRow: 1,
columnIndex: 1,
sortedColumn: 1,
sortable: true,
sortDirection: 'descending',
sortLabel,
Expand All @@ -237,14 +237,14 @@ describe('Table utils', () => {
});

it('should return sortable:false if sortable is falsy', () => {
const actual = utils.getSortParams({ rowIndex: 0 });
const actual = utils.getSortParams({ columnIndex: 0 });
const expected = { sortable: false };

expect(actual).toEqual(expected);
});

it('should return sortable:false if sortLabel is falsy', () => {
const actual = utils.getSortParams({ rowIndex: 0, sortable: true });
const actual = utils.getSortParams({ columnIndex: 0, sortable: true });
const expected = { sortable: false };

expect(actual).toEqual(expected);
Expand Down
10 changes: 5 additions & 5 deletions packages/circuit-ui/components/Table/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,22 @@ export const descendingSort =
};

export const getSortParams = ({
rowIndex,
columnIndex,
sortable,
sortDirection,
sortLabel,
sortedRow,
sortedColumn,
}: {
rowIndex: number;
columnIndex: number;
sortable?: boolean;
sortDirection?: Direction;
sortLabel?: string | (({ direction }: { direction?: Direction }) => string);
sortedRow?: number;
sortedColumn?: number;
}): SortParams => {
if (!sortable || !sortLabel) {
return { sortable: false };
}
const isSorted = sortedRow === rowIndex;
const isSorted = sortedColumn === columnIndex;
return {
sortable: true,
sortLabel: isFunction(sortLabel)
Expand Down
Loading