[Data Grid] made numeric sorting look at the full field value (#2603)

* Made datagrid's numeric sorting work against all digit groups in a value instead of the first float value found * changelog
elastic · Dec 9, 2019 · 53880eb · 53880eb
1 parent 2b2cf69
commit 53880eb
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 - Fixed UX/focus bug in `EuiDataGrid` when using keyboard shortcuts to paginate ([#2602](https://github.com/elastic/eui/pull/2602))
 - Fixed `EuiIcon` accessibility by adding a `title` prop and a default `aria-label` ([#2554](https://github.com/elastic/eui/pull/2554))
+- Fixed `EuiDataGrid`'s in-memory sorting of numeric columns when the cell data contains multiple digit groups ([#2603](https://github.com/elastic/eui/pull/2603))
 
 ## [`17.0.0`](https://github.com/elastic/eui/tree/v17.0.0)
 

diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx
@@ -1404,6 +1404,54 @@ Array [
           ['0', '9'],
         ]);
       });
+
+      it('sorts with all digit groups in numerical-like', () => {
+        const onSort = jest.fn(columns => {
+          component.setProps({ sorting: { columns, onSort } });
+          component.update();
+        });
+
+        const component = mount(
+          <EuiDataGrid
+            aria-label="test"
+            columns={[{ id: 'version' }]}
+            columnVisibility={{
+              visibleColumns: ['version'],
+              setVisibleColumns: () => {},
+            }}
+            rowCount={5}
+            renderCellValue={
+              ({ rowIndex }) => `1.0.${(rowIndex % 3) + rowIndex}` // computes as 0,2,4,3,5
+            }
+            inMemory={{ level: 'sorting' }}
+            sorting={{
+              columns: [],
+              onSort,
+            }}
+          />
+        );
+
+        // verify rows are unordered
+        expect(extractGridData(component)).toEqual([
+          ['version'],
+          ['1.0.0'],
+          ['1.0.2'],
+          ['1.0.4'],
+          ['1.0.3'],
+          ['1.0.5'],
+        ]);
+
+        sortByColumn(component, 'version', 'asc');
+
+        expect(extractGridData(component)).toEqual([
+          ['version'],
+          ['1.0.0'],
+          ['1.0.2'],
+          ['1.0.3'],
+          ['1.0.4'],
+          ['1.0.5'],
+        ]);
+      });
     });
 
     it('uses schema information to sort', () => {

diff --git a/src/components/datagrid/data_grid_schema.tsx b/src/components/datagrid/data_grid_schema.tsx
@@ -167,14 +167,25 @@ export const schemaDetectors: EuiDataGridSchemaDetector[] = [
       return matchLength / value.length || 0;
     },
     comparator: (a, b, direction) => {
-      const aChars = a.split('').filter(char => numericChars.has(char));
-      const aValue = parseFloat(aChars.join(''));
-
-      const bChars = b.split('').filter(char => numericChars.has(char));
-      const bValue = parseFloat(bChars.join(''));
+      // sort on all digits groups
+      const aGroups = a.split(/\D+/);
+      const bGroups = b.split(/\D+/);
+
+      const maxGroups = Math.max(aGroups.length, bGroups.length);
+      for (let i = 0; i < maxGroups; i++) {
+        // if A and B's group counts differ and they match until that difference, prefer whichever is shorter
+        if (i >= aGroups.length) return direction === 'asc' ? -1 : 1;
+        if (i >= bGroups.length) return direction === 'asc' ? 1 : -1;
+
+        const aChars = aGroups[i];
+        const bChars = bGroups[i];
+        const aValue = parseInt(aChars, 10);
+        const bValue = parseInt(bChars, 10);
+
+        if (aValue < bValue) return direction === 'asc' ? -1 : 1;
+        if (aValue > bValue) return direction === 'asc' ? 1 : -1;
+      }
 
-      if (aValue < bValue) return direction === 'asc' ? -1 : 1;
-      if (aValue > bValue) return direction === 'asc' ? 1 : -1;
       return 0;
     },
     icon: 'number',