Skip to content

Commit

Permalink
Rename Table's initialSortedRow to initialSortedColumn (#2491)
Browse files Browse the repository at this point in the history
Co-authored-by: Connor Bär <[email protected]>
Co-authored-by: Connor Bär <[email protected]>
  • Loading branch information
3 people authored Apr 15, 2024
1 parent 3c2763c commit 35ee26a
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 37 deletions.
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

0 comments on commit 35ee26a

Please sign in to comment.