[EuiInMemoryTable] Fix onTableChange returning the sort field's name instead of field key#5588
Conversation
- Should currently fail with field returns as `Name` instead of `name`
f625c78 to
e1cee95
Compare
| let sortColumn = findColumnByProp(columns, 'field', sortName as string); | ||
| if (sortColumn == null) { | ||
| sortColumn = findColumnByProp(columns, 'name', sortName as string); | ||
| } |
There was a problem hiding this comment.
I copied this handy 'findColumnBy' logic up above in getInitialSorting:
eui/src/components/basic_table/in_memory_table.tsx
Lines 221 to 226 in e1cee95
There was a problem hiding this comment.
Want to make this a function that's reused both places?
thompsongl
left a comment
There was a problem hiding this comment.
🙏 Thanks for the new test and additional comments!
One suggestion but the rest LGTM
Don't forget your REVERT commit 🎗️
| let sortColumn = findColumnByProp(columns, 'field', sortName as string); | ||
| if (sortColumn == null) { | ||
| sortColumn = findColumnByProp(columns, 'name', sortName as string); | ||
| } |
There was a problem hiding this comment.
Want to make this a function that's reused both places?
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5588/ |
+ fix value typing to accept names which can be ReactNodes
This reverts commit 3c1e939.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5588/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5588/ |
Summary
closes #5587
TL;DR: On sort, in memory tables'
onTableChangereturn the correctsort.fieldvalue ascolumn.field, but on pagination, it incorrectly returnssort.fieldascolumn.name.Before
Jest test failure comparison:
After
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in Chrome, Safari, Edge, and Firefox- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for any docs examples- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes