diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx index fad9e8245df22..3cb58ce6d06c9 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx @@ -116,10 +116,10 @@ describe('FilterableTable sorting - RTL', () => { expect(gridCells[1]).toHaveTextContent('Bravo'); expect(gridCells[2]).toHaveTextContent('Alpha'); - // Third click to sort ascending again + // Third click to clear sorting userEvent.click(stringColumn); - expect(gridCells[0]).toHaveTextContent('Alpha'); - expect(gridCells[1]).toHaveTextContent('Bravo'); + expect(gridCells[0]).toHaveTextContent('Bravo'); + expect(gridCells[1]).toHaveTextContent('Alpha'); expect(gridCells[2]).toHaveTextContent('Charlie'); }); @@ -151,10 +151,10 @@ describe('FilterableTable sorting - RTL', () => { expect(gridCells[1]).toHaveTextContent('10'); expect(gridCells[2]).toHaveTextContent('0'); - // Third click to sort ascending again + // Third click to clear sorting userEvent.click(integerColumn); - expect(gridCells[0]).toHaveTextContent('0'); - expect(gridCells[1]).toHaveTextContent('10'); + expect(gridCells[0]).toHaveTextContent('10'); + expect(gridCells[1]).toHaveTextContent('0'); expect(gridCells[2]).toHaveTextContent('100'); }); @@ -186,10 +186,10 @@ describe('FilterableTable sorting - RTL', () => { expect(gridCells[1]).toHaveTextContent('45.67'); expect(gridCells[2]).toHaveTextContent('1.23'); - // Third click to sort ascending again + // Third click to clear sorting userEvent.click(floatColumn); - expect(gridCells[0]).toHaveTextContent('1.23'); - expect(gridCells[1]).toHaveTextContent('45.67'); + expect(gridCells[0]).toHaveTextContent('45.67'); + expect(gridCells[1]).toHaveTextContent('1.23'); expect(gridCells[2]).toHaveTextContent('89.0000001'); }); @@ -257,17 +257,17 @@ describe('FilterableTable sorting - RTL', () => { expect(gridCells[9]).toHaveTextContent('26260.210000000003'); expect(gridCells[10]).toHaveTextContent('24078.610000000004'); - // Third click to sort ascending again + // Third click to clear sorting userEvent.click(mixedFloatColumn); - expect(gridCells[0]).toHaveTextContent('24078.610000000004'); - expect(gridCells[1]).toHaveTextContent('26260.210000000003'); - expect(gridCells[2]).toHaveTextContent('28550.59'); - expect(gridCells[3]).toHaveTextContent('48710.92'); - expect(gridCells[4]).toHaveTextContent('72212.86'); - expect(gridCells[5]).toHaveTextContent('98089.08000000002'); - expect(gridCells[6]).toHaveTextContent('144729.96000000002'); - expect(gridCells[7]).toHaveTextContent('145776.56'); - expect(gridCells[8]).toHaveTextContent('152718.97999999998'); + expect(gridCells[0]).toHaveTextContent('48710.92'); + expect(gridCells[1]).toHaveTextContent('145776.56'); + expect(gridCells[2]).toHaveTextContent('72212.86'); + expect(gridCells[3]).toHaveTextContent('144729.96000000002'); + expect(gridCells[4]).toHaveTextContent('26260.210000000003'); + expect(gridCells[5]).toHaveTextContent('152718.97999999998'); + expect(gridCells[6]).toHaveTextContent('28550.59'); + expect(gridCells[7]).toHaveTextContent('24078.610000000004'); + expect(gridCells[8]).toHaveTextContent('98089.08000000002'); expect(gridCells[9]).toHaveTextContent('3439718.0300000007'); expect(gridCells[10]).toHaveTextContent('4528047.219999993'); }); @@ -320,14 +320,14 @@ describe('FilterableTable sorting - RTL', () => { expect(gridCells[5]).toHaveTextContent('2021-01-02'); expect(gridCells[6]).toHaveTextContent('2021-01-01'); - // Third click to sort ascending again + // Third click to clear sorting userEvent.click(dsColumn); expect(gridCells[0]).toHaveTextContent('2021-01-01'); - expect(gridCells[1]).toHaveTextContent('2021-01-02'); - expect(gridCells[2]).toHaveTextContent('2021-01-03'); - expect(gridCells[3]).toHaveTextContent('2021-10-01'); + expect(gridCells[1]).toHaveTextContent('2022-01-01'); + expect(gridCells[2]).toHaveTextContent('2021-01-02'); + expect(gridCells[3]).toHaveTextContent('2021-01-03'); expect(gridCells[4]).toHaveTextContent('2021-12-01'); - expect(gridCells[5]).toHaveTextContent('2022-01-01'); + expect(gridCells[5]).toHaveTextContent('2021-10-01'); expect(gridCells[6]).toHaveTextContent('2022-01-02'); }); }); diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx index 29a40d320d387..23d68f96850c9 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.tsx @@ -114,8 +114,9 @@ interface FilterableTableProps { interface FilterableTableState { sortBy?: string; - sortDirection: SortDirectionType; + sortDirection?: SortDirectionType; fitted: boolean; + displayedList: Datum[]; } export default class FilterableTable extends PureComponent< @@ -175,8 +176,8 @@ export default class FilterableTable extends PureComponent< this.totalTableHeight = props.height; this.state = { - sortDirection: SortDirection.ASC, fitted: false, + displayedList: [...this.list], }; this.container = React.createRef(); @@ -191,7 +192,7 @@ export default class FilterableTable extends PureComponent< } getWidthsForColumns() { - const PADDING = 40; // accounts for cell padding and width of sorting icon + const PADDING = 50; // accounts for cell padding and width of sorting icon const widthsByColumnKey = {}; const cellContent = ([] as string[]).concat( ...this.props.orderedColumnKeys.map(key => { @@ -295,7 +296,31 @@ export default class FilterableTable extends PureComponent< sortBy: string; sortDirection: SortDirectionType; }) { - this.setState({ sortBy, sortDirection }); + let updatedState: FilterableTableState; + + const shouldClearSort = + this.state.sortDirection === SortDirection.DESC && + this.state.sortBy === sortBy; + + if (shouldClearSort) { + updatedState = { + ...this.state, + sortBy: undefined, + sortDirection: undefined, + displayedList: [...this.list], + }; + } else { + updatedState = { + ...this.state, + sortBy, + sortDirection, + displayedList: [...this.list].sort( + this.sortResults(sortBy, sortDirection === SortDirection.DESC), + ), + }; + } + + this.setState(updatedState); } fitTableToWidthIfNeeded() { @@ -362,6 +387,17 @@ export default class FilterableTable extends PureComponent< }; } + sortGrid = (label: string) => { + this.sort({ + sortBy: label, + sortDirection: + this.state.sortDirection === SortDirection.DESC || + this.state.sortBy !== label + ? SortDirection.ASC + : SortDirection.DESC, + }); + }; + renderTableHeader({ dataKey, label, @@ -425,8 +461,14 @@ export default class FilterableTable extends PureComponent< : style.top, }} className={`${className} grid-cell grid-header-cell`} + role="columnheader" + tabIndex={columnIndex} + onClick={() => this.sortGrid(label)} > -