lyndsiWilliams commented on a change in pull request #17573:
URL: https://github.com/apache/superset/pull/17573#discussion_r760674672
##########
File path: superset-frontend/src/components/FilterableTable/FilterableTable.tsx
##########
@@ -323,19 +323,19 @@ export default class FilterableTable extends
PureComponent<
sortResults(sortBy: string, descending: boolean) {
return (a: Datum, b: Datum) => {
- const aValue = a[sortBy];
- const bValue = b[sortBy];
- if (aValue === bValue) {
- // equal items sort equally
- return 0;
- }
- if (aValue === null) {
- // nulls sort after anything else
- return 1;
- }
- if (bValue === null) {
- return -1;
- }
+ // Parse any floating numbers so they'll sort correctly
+ const parseFloatingNums = (value: any) => {
+ const floatValue = parseFloat(value);
+ return Number.isNaN(floatValue) ? value : parseFloat(value);
+ };
+
+ const aValue = parseFloatingNums(a[sortBy]);
+ const bValue = parseFloatingNums(b[sortBy]);
+
+ String(aValue).localeCompare(String(bValue), undefined, {
Review comment:
I did a lot of playing around with this and figured out that the sorting
works without `localeCompare` so I removed it and added testing in [`this
commit`](https://github.com/apache/superset/pull/17573/commits/8be6346d756a43203abf820c9e88d9c2f8f63b13).
While testing I found any floating point number that had more than 2 floating
points (like `12.001` or longer) is a `string`, while any other number is a
`number`. I think originally, the floating numbers weren't sorting correctly
because the types were mixed. Just using the `parseFloatingNums` function seems
to solve this so maybe we can look at using `localeCompare` in the future for
multilingual sorting, since it just seems to be causing issues in this
instance. I also didn't realize you had to explicitly pass the language you
want to support as a second argument, so I don't think I'll be able to make
this work with `localeCompare`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
