Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
48 changes: 48 additions & 0 deletions src/components/datagrid/data_grid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
25 changes: 18 additions & 7 deletions src/components/datagrid/data_grid_schema.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down