Eui: [EuiDataGrid] Add ability to sort in-memory cell values without SchemaDetector

[PhaedrusTheGreek]

Today there is no straightforward way to sort in-memory cell values. For example, suppose I had a column called bytes (my source data is already in numerical format) that I wanted to display as human readable (10mb, 1gb). The only way to sort these values is to implement a SchemaDetector comparator that parses these human readable strings back to bytes.

Both the actual schema detection and the comparator make sense coupled together, since they both are related to parsing already-formatted text. But in the case where a developer already knows the format, it seems correct to omit the implementation of a detector, instead providing a standalone comparator that sorts based on the cell's original value.

Since renderCellValue is the only source of cell data, and directly outputs a React component, there seems to be no real access or concept of underlying cell data. Is there any way that renderCellValue could output both an underlying value and a rendered cell?

[chandlerprall]

Since renderCellValue is the only source of cell data, and directly outputs a React component, there seems to be no real access or concept of underlying cell data. Is there any way that renderCellValue could output both an underlying value and a rendered cell?

This certainly seems like the right approach for passing raw values into the datagrid. Likely a method similar to setting the cell's DOM props via setCellProps. The new method would need to merge the values into the existing inMemoryValues somehow, either directly or by a pass-through map lookup (e.g. check if value is in map providedValues and if not fallback to inMemoryValues).

[timroes]

_Copying from duplicate issue over_

Data grid currently has imho one fundamental technical flaw: we cannot sort on actual values, but only on the "rendered" content of a cell. That makes it at the moment nearly impossible to display and sort any kind of formatted numbers (without doing very large workarounds).

The sorting happens via the comperator function of a schema attached to a column. This will get as input the renderCellValue output, which actually needs to produce the component that's displayed within the cell. E.g. if you have a column shown a percentage value, the renderCellValue will format it as percentage and thus output a value like 0,5 % (I assumed a German locale here). This will now be put intot he comperator and there is no chance of sorting those values, without trying to string parse them back to numbres, which gets really hard with different locales or more complex cases like percentage formatting. By default it will instead just do the string comparison, which does not sort numbers anyhow meaningful.

Since the sorting has no access to the raw values atm, it's rather useless for anything except strings imho. I think we need to spend some time trying to rethink how this could work and I'd see the following two options:

Option 1 - The "proper" solution

I think one of the most proper solutions would be, to actually replace renderCellValue by a getCellValue method, that needs to return the raw value for a specific cell, and instead give the Column Schema a renderValue(value: T, rowIndex: number) method instead, that will be used to format a value within this cell. The decorator in this case would get the return value of the getCellValue and thus the raw value for sorting and formatting happens only afterwards.

Also it keeps formatting a bit better separated. Currently to have columns with different formatting, you'll often end up with a long switch in the renderCellValue to cover the different render types.

Option 2 - The "less breaking" solution

Option 1 will break quit some APIs, and potentially require quite some refactoring. I think there might be an quicker solution, though slightly less clean, that would still make sorting possible. In this case we would extend the comperator function to get access to (part of) the EuiDataGridCellValueElementProps passed into the renderCellValue, so it'll be able to retrieve the raw value itself via the similar logic renderCellValue already got it. I think rowIndex and columnId would already be enough. So the new interface would look like:

type CellInfoLite = { rowIndex: number; columnId: string; };
// or potentially instead of 
comparator?: (a: string, b: string, direction: 'asc' | 'desc', rawInfo: { a: CellInfoLite, b: CellInfoLite }) => -1 | 0 | 1;

This has the advantage we're not breaking any APIs, but therefore still not having a clear separation between value and formatting in data grid.

[timroes]

Gonna close this as a duplicate of ^ that issue (#4108). While we'll keep that as a feature request, the current recommend solutions for this are 1) don't use in-memory sorting 2) use a custom schema that can parse e.g. percentages for sorting.

The in-memory sorting feature is intentionally not a catch-all solution, and I'd hesitate to add API complexity to solve the case. That said, we should not shy away from exploring possible ways to make it better and give better coverage.

@chandlerprall I have the feeling the in-memory sorting is at the moment not a solution at all ;) The problem is that since you're not able to actually even sort on any numeric field as soon as it's formatted (which I assume should be the standard in most applications), I am failing to see the usage of in-memory sorting at all?

Creating custom parsers is also imho not really a good solution, since they'd need to be aware of locales, and this can become already really tricky, since otherwise you cannot parse strings back into numbers, since you don't know what might be a decimal point and what a thousand separator (e.g. German uses the exact opposite as English - , for decimal point and . for thousand separator).

If we decide not to address sorting properly via the in-memory sorting I wonder if there is currently any use-case at all? Otherwise I'd suggest we could simply remove the option altogether if it won't work for 90%+ of use-cases and we're not planning to fix it, and therefore would be able to keep the EUI code itself simpler since it doesn't need to support it.

[chandlerprall]

since you're not able to actually even sort on any numeric field as soon as it's formatted

The numeric columns are sorted by their digit groups, which has worked in the cases I have tested with (which I'll admit have been mostly contrived). I'd be interested in cases where a column that was determined to be numeric isn't in-memory sorting correctly.

Apart from that, I largely agree with your additional assessments. Though I think the efficacy of sorting numeric columns is important to understand before coming to a decision.

[timroes]

@chandlerprall I've created a minified sample codesandbox: https://codesandbox.io/s/eui-datagrid-grid-sorting-example-zbjs8 This sorts the val column with numerical values inside it ascending... or doesn't it ;)

The same problem appears with basically any kind of locale formatting on the numbers (which is a common requirement for presenting numbres). The problem is, the one outlined in the above comments, that as soon as any kind of formatting happened we basically sorting on the fields. I might have been a bit confusing, since even with a numeric schema it's technically still strings presented.

[chandlerprall]

You're saying 3000 isn't smaller than 5? 😊

Thanks for the quick demo, definitely shows the current numeric sorting logic is incomplete.

[timroes]

One more reason why I think this should be fixed or removed altogether: With the upcoming virtualization PR in memory sorting will still cause all cells to render to retrieve their text representation for sorting, i.e. having in memory sorting or pagination enabled virtualiation will not have much benefit (at least for the sorted columns). I think that is something we should at least document (since it was not initially obvious to me) and without knowing the implementation it's hard to know at all.