Skip to content

Commit

Permalink
fix(sqllab): fix query results sorting (#18666)
Browse files Browse the repository at this point in the history 
* add sorting for query results grid and add natural order sort

* modify tests for natural/original order state

* clean up setState

Co-authored-by: Corbin Robb <[email protected]>
  • Loading branch information
@corbinrobb
corbinrobb and Corbin Robb authored Feb 14, 2022
1 parent 48a8095 commit 5bb406b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 38 deletions.
48 changes: 24 additions & 24 deletions superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down Expand Up @@ -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');
});

Expand Down Expand Up @@ -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');
});

Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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');
});
});
64 changes: 50 additions & 14 deletions superset-frontend/src/components/FilterableTable/FilterableTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ interface FilterableTableProps {

interface FilterableTableState {
sortBy?: string;
sortDirection: SortDirectionType;
sortDirection?: SortDirectionType;
fitted: boolean;
displayedList: Datum[];
}

export default class FilterableTable extends PureComponent<
Expand Down Expand Up @@ -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();
Expand All @@ -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 => {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)}
>
<div>{label}</div>
{label}
{this.state.sortBy === label && (
<SortIndicator sortDirection={this.state.sortDirection} />
)}
</div>
</Tooltip>
);
Expand All @@ -444,7 +486,7 @@ export default class FilterableTable extends PureComponent<
style: React.CSSProperties;
}) {
const columnKey = this.props.orderedColumnKeys[columnIndex];
const cellData = this.list[rowIndex][columnKey];
const cellData = this.state.displayedList[rowIndex][columnKey];
const cellText = this.getCellContent({ cellData, columnKey });
const content =
cellData === null ? <i className="text-muted">{cellText}</i> : cellText;
Expand Down Expand Up @@ -563,19 +605,13 @@ export default class FilterableTable extends PureComponent<
rowHeight,
} = this.props;

let sortedAndFilteredList = this.list;
let sortedAndFilteredList = this.state.displayedList;
// filter list
if (filterText) {
sortedAndFilteredList = this.list.filter((row: Datum) =>
sortedAndFilteredList = sortedAndFilteredList.filter((row: Datum) =>
this.hasMatch(filterText, row),
);
}
// sort list
if (sortBy) {
sortedAndFilteredList = sortedAndFilteredList.sort(
this.sortResults(sortBy, sortDirection === SortDirection.DESC),
);
}

let { height } = this.props;
let totalTableHeight = height;
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/components/FilterableTable/FilterableTableStyles.less
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@

.grid-header-cell {
font-weight: @font-weight-bold;
cursor: pointer;
}

.ReactVirtualized__Table__headerColumn:last-of-type,
Expand Down

0 comments on commit 5bb406b

Please sign in to comment.