Skip to content

Commit

Permalink
Add optional sortByColumnType param to fix number sorting in TableWid…
Browse files Browse the repository at this point in the history
…get (#593)
  • Loading branch information
eamador authored Feb 6, 2023
1 parent f1f347d commit dc4754c
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Not released

- Add optional sortByColumnType param to fix number sorting in TableWidget [#593](https://github.com/CartoDB/carto-react/pull/593)
- Requests to CARTO APIs V3 will be authorized through the Authorization header instead of using a query param [#592](https://github.com/CartoDB/carto-react/pull/592)
- Provide unified CLIENT_ID for metrics [#591](https://github.com/CartoDB/carto-react/pull/591)

Expand Down
6 changes: 4 additions & 2 deletions packages/react-widgets/src/models/TableModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ export function getTable(props) {
}

function fromLocal(props) {
const { source, rowsPerPage, page, sortBy, sortDirection } = props;
// Injecting sortByColumnType externally from metadata gives better results. It allows to avoid deriving type from row value itself (with potential null values)
const { source, rowsPerPage, page, sortBy, sortDirection, sortByColumnType } = props;

return executeTask(source.id, Methods.FEATURES_RAW, {
filters: source.filters,
filtersLogicalOperator: source.filtersLogicalOperator,
limit: rowsPerPage,
page,
sortBy,
sortByDirection: sortDirection
sortByDirection: sortDirection,
sortByColumnType
});
}
12 changes: 10 additions & 2 deletions packages/react-widgets/src/widgets/TableWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function TableWidget({
const [page, setPage] = useState(0);

const [sortBy, setSortBy] = useState(undefined);
const [sortByColumnType, setSortByColumnType] = useState(undefined);
const [sortDirection, setSortDirection] = useState('asc');

const {
Expand All @@ -60,7 +61,8 @@ function TableWidget({
rowsPerPage,
page,
sortBy,
sortDirection
sortDirection,
sortByColumnType
},
global,
onError
Expand All @@ -82,6 +84,12 @@ function TableWidget({
if (onPageSizeChange) onPageSizeChange(newRowsPerPage);
};

const handleSortBy = (newSortBy) => {
setSortBy(newSortBy);
const column = columns.find((c) => c.field === newSortBy);
setSortByColumnType(column?.type);
};

return (
<WrapperWidgetUI title={title} {...wrapperProps} isLoading={isLoading}>
<WidgetWithAlert
Expand All @@ -104,7 +112,7 @@ function TableWidget({
sorting
sortBy={sortBy}
sortDirection={sortDirection}
onSetSortBy={setSortBy}
onSetSortBy={handleSortBy}
onSetSortDirection={setSortDirection}
height={height}
dense={dense}
Expand Down
29 changes: 18 additions & 11 deletions packages/react-workers/src/utils/sorting.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ import { firstBy } from 'thenby';
* @param {object} [sortOptions]
* @param {string | string[] | object[]} [sortOptions.sortBy] - One or more columns to sort by
* @param {string} [sortOptions.sortByDirection] - Direction by the columns will be sorted
* @param {('number'|'string'|'date')} [sortOptions.sortByColumnType] - Column type
*/
export function applySorting(features, { sortBy, sortByDirection = 'asc' } = {}) {
export function applySorting(
features,
{ sortBy, sortByDirection = 'asc', sortByColumnType = 'string' } = {}
) {
// If sortBy is undefined, pass all features
if (sortBy === undefined) {
return features;
Expand All @@ -21,20 +25,20 @@ export function applySorting(features, { sortBy, sortByDirection = 'asc' } = {})
if (!isValidSortBy) {
throw new Error('Sorting options are bad formatted');
}

const sortFn = createSortFn({
sortBy,
sortByDirection
sortByDirection,
columnDataType: sortByColumnType
});

return features.sort(sortFn);
}

// Aux
function createSortFn({ sortBy, sortByDirection }) {
function createSortFn({ sortBy, sortByDirection, columnDataType = 'string' }) {
const [firstSortOption, ...othersSortOptions] = normalizeSortByOptions({
sortBy,
sortByDirection
sortByDirection,
columnDataType
});

let sortFn = firstBy(...firstSortOption);
Expand All @@ -45,26 +49,29 @@ function createSortFn({ sortBy, sortByDirection }) {
return sortFn;
}

function normalizeSortByOptions({ sortBy, sortByDirection }) {
function normalizeSortByOptions({ sortBy, sortByDirection, columnDataType }) {
const numberFormat = columnDataType === 'number' && { cmp: (a, b) => a - b };
if (!Array.isArray(sortBy)) {
sortBy = [sortBy];
}

return sortBy.map((sortByEl) => {
// sortByEl is 'column'
if (typeof sortByEl === 'string') {
return [sortByEl, sortByDirection];
return [sortByEl, { direction: sortByDirection, ...numberFormat }];
}

if (Array.isArray(sortByEl)) {
// sortBy is ['column']
if (sortByEl[1] === undefined) {
return [sortByEl, sortByDirection];
return [sortByEl, { direction: sortByDirection, ...numberFormat }];
}

// sortBy is ['column', { ... }]
if (typeof sortByEl[1] === 'object') {
return [sortByEl[0], { direction: sortByDirection, ...sortByEl[1] }];
const othersSortOptions = numberFormat
? { ...numberFormat, ...sortByEl[1] }
: sortByEl[1];
return [sortByEl[0], { direction: sortByDirection, ...othersSortOptions }];
}
}
return sortByEl;
Expand Down
6 changes: 4 additions & 2 deletions packages/react-workers/src/workers/features.worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ function getRawFeatures({
limit = 10,
page = 0,
sortBy,
sortByDirection = 'asc'
sortByDirection = 'asc',
sortByColumnType
}) {
let data = [];
let numberPages = 0;
Expand All @@ -277,7 +278,8 @@ function getRawFeatures({
if (currentFeatures) {
data = applySorting(getFilteredFeatures(filters, filtersLogicalOperator), {
sortBy,
sortByDirection
sortByDirection,
sortByColumnType
});

totalCount = data.length;
Expand Down

0 comments on commit dc4754c

Please sign in to comment.