diff --git a/CHANGELOG.md b/CHANGELOG.md index 0112479698e..5776a1db6c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ **Bug fixes** - Added `role="dialog"` to `EuiFlyout` to improve screen reader accessibility ([#916](https://github.com/elastic/eui/pull/916)) +- Default sort comparator (used by `EuiInMemoryTable`) now handles `null` and `undefined` values ([#922](https://github.com/elastic/eui/pull/922)) ## [`0.0.52`](https://github.com/elastic/eui/tree/v0.0.52) diff --git a/src-docs/src/views/tables/data_store.js b/src-docs/src/views/tables/data_store.js index a1202e1faec..ab04a6cc379 100644 --- a/src-docs/src/views/tables/data_store.js +++ b/src-docs/src/views/tables/data_store.js @@ -32,7 +32,7 @@ const createCountries = () => [ ]; const firstNames = ['Very long first name that will wrap or be truncated', 'Another very long first name which will wrap or be truncated', - 'Clinton', 'Igor', 'Karl', 'Drew', 'Honza', 'Rashid', 'Jordan', 'John']; + 'Clinton', 'Igor', undefined, 'Drew', null, 'Rashid', undefined, 'John']; const lastNames = ['Very long last name that will wrap or be truncated', 'Another very long last name which will wrap or be truncated', 'Gormley', 'Motov', 'Minarik', 'Raines', 'Král', 'Khan', 'Sissel', 'Dorlus']; diff --git a/src/services/sort/comparators.js b/src/services/sort/comparators.js index bfc5759cd9a..b45295db79e 100644 --- a/src/services/sort/comparators.js +++ b/src/services/sort/comparators.js @@ -5,6 +5,34 @@ export const Comparators = Object.freeze({ default: (direction = SortDirection.ASC) => { return (v1, v2) => { + // JavaScript's comparison of null/undefined (and some others not handled here) values always returns `false` + // (https://www.ecma-international.org/ecma-262/#sec-abstract-relational-comparison) + // resulting in cases where v1 < v2 and v1 > v2 are both false. + // This leads to unpredictable sorting results in multiple JS engine sorting algorithms + // which causes unpredictable user experiences. + // Instead: + // * 1. push undefined/null values to the end of the sorted list, _regardless of sort direction_ + // (non-sortable values always appear at the end, and sortable values are sorted at the top) + // * 2. report undefined/null values as equal + // * 3. when both values are comparable they are sorted normally + + const v1IsComparable = v1 != null; + const v2IsComparable = v2 != null; + + // * 1. + if (v1IsComparable && !v2IsComparable) { + return -1; + } + if (!v1IsComparable && v2IsComparable) { + return 1; + } + + // * 2. + if (!v1IsComparable && !v2IsComparable) { + return 0; + } + + // * 3. if (v1 === v2) { return 0; } diff --git a/src/services/sort/comparators.test.js b/src/services/sort/comparators.test.js index 78bd16455bc..5e59d187b67 100644 --- a/src/services/sort/comparators.test.js +++ b/src/services/sort/comparators.test.js @@ -46,3 +46,10 @@ describe('comparators - property', () => { }); }); +describe('default comparator', () => { + test('null/undefined values are sorted to the end', () => { + const sorted = [undefined, '7', 3, null, 5, undefined].sort(Comparators.default()); + expect(sorted).toEqual([3, 5, '7', null, undefined, undefined]) + }) +}); +