[Datagrid] Fix rowHeightOption: auto datagrid calculation issue #8251
+5
−1
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #8245
The useUnconstrainedHeight hook used by datagrid has I think a subtle bug in it around auto height rows, in cases where the row height is 'auto' for all the rows in the table, and when datagrid is aware of them early in the multiple renders it uses to settle on a height, the returned value of this hook, an integer representing the height of the datagrid if it were rendered completely in full is returned as a number based on just the header row height, as described in the issue above. The reason for this, I believe, is because RowHeightUtils.getRowHeightOption will return not always a number, but sometimes the string 'auto' as well. That causes the if at https://github.com/elastic/eui/blob/main/packages/eui/src/components/datagrid/utils/grid_height_width.ts#L137 to be true, and as this is the first time we are attempting to determine the height of this row, the heightsCache of the RowHeightUtils class (these really desperately need to be converted to hooks, and a lot of these problems will disappear) returns 0 for all rows when calling getRowHeight, but we increment the knownRowCount anyway, and the result is this bug. The fix in this PR seems to be the most minimal way to fix this, however I think refactoring the class based utils that revolve around EuiDataGridRowHeightOption/EuiDataGridRowHeightsOptions would be best long term, as now multiple unneeded re-renders are forced from callbacks to force checks like useUnconstrainedHeight to be correct, which is bad for performance, but also calculations that happen outside of the React lifecycle, like with the RowHeightUtils class, unexpectedly do not happen at all in some cases, leading to bugs like this. A well done custom hooks approach could solve both problems at once. I have a branch somewhere with EuiDataGridCell converted from a class based component to a functional one to enable this I think.
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
@default
if default values are missing) and playground toggles