[EuiInMemoryTable] Error appears in console related to prop sorting.sort.direction
#4138
Conversation
…sort.direction` Closes elastic#4137
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
@chandlerprall Could you review it, please? |
| @@ -410,7 +410,8 @@ export class EuiInMemoryTable<T> extends Component< | |||
| // map back to `name` if this is the case | |||
| for (let i = 0; i < this.props.columns.length; i++) { | |||
| const column = this.props.columns[i]; | |||
| if ((column as EuiTableFieldDataColumnType<T>).field === sortName) { | |||
| const columnField = (column as EuiTableFieldDataColumnType<T>).field; | |||
There was a problem hiding this comment.
Thank you for the PR submission! This does solve the problem, but I'm a little worried about future changes to this code accidentally undoing the fix. Please include two modifications to help future-proof this code:
-
Instead of always pulling
columnField, please use a type guard instead. A quick test if the column contains thefieldvalue would suffice to determine if a field isEuiTableFieldDataColumnType. Using a type guard here helps the code self-document what it is doing. -
Add a unit test to in_memory_table.test.tsx under the
behaviorsection. There's an existingpaginationtest that you can copy the format of, but if you're unsure how to set it up please ask! The test need only change the number of displayed rows, we have a utility that automatically fails a test if a proptype check fails.
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4138/ |
…sort.direction` (elastic#4138) * [EuiInMemoryTable] Error appears in console related to prop `sorting.sort.direction` Closes elastic#4137 * Add descrition to CHANGELOG * Replace columnField with type guard for field and create a unit test for that case Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
Closes #4137
Summary
The error is caused by setting a value to
sortNamewhencolumnFieldis compared tosortName. So when both are undefined,sortNameis set to the last column name value, that is why check forcolumnFieldpresence is needed.Checklist