[EuiDataGrid] Sorting columns after focusing a header cell throws console errors #4384
Comments
|
I'm curious why the header's popover remains open when interacting with the column position popover, but that appears unrelated - confirmed this focus error happens even if the header cell's popover is closed before re-ordering columns. |
13 tasks
|
tested it and it's still an issue |
41 tasks
cee-chen
added a commit
to cee-chen/eui
that referenced
this issue
Sep 21, 2021
Setting isCellEntered to true already makes that same call, so *should* have that same effect as long as we're consistent about changing logic via isCellEntered This also fixes elastic#4384, which was a focus-lock issue caused focusin/focusout events firing multiple times. Now with the setIsCellEntered change, focus does not fire repeatedly
6 tasks
|
@kertal Hey hey! If you have time, would you mind quickly QA testing https://eui.elastic.co/pr_5209/#/tabular-content/data-grid to confirm that the issue is fixed in my PR? 🙏 |
|
ACK .. testing |
cee-chen
pushed a commit
that referenced
this issue
Sep 27, 2021
…refactors (#5209) * [Fix] Fix console error when dragging to reorder columns * [Fix] DRY out enableInteractives call Setting isCellEntered to true already makes that same call, so *should* have that same effect as long as we're consistent about changing logic via isCellEntered This also fixes #4384, which was a focus-lock issue caused focusin/focusout events firing multiple times. Now with the setIsCellEntered change, focus does not fire repeatedly * [Refactor] Remove unnecessary useEffect dependencies - now that we're not calling enableInteractives and focusInteractives directly but letting isCellEntered handle that for us, there's no need to include them + setIsCellEntered isn't necessary either/doesn't trigger the lint rule * [Refactor] Improve focus unit tests - Separate focus tests for isFocused logic vs focusin / focusout events * [Refactor] Move interactive fns to separate/external fns vs. useCallback - Removes the need for useCallback and setting the utility fns as dependencies, simplifying code - refactor for loops to forEach's - remove setIsCellEntered(true) in focusInteractables, since it now only gets called when isCellEntered is true * [Refactor] Combine enable and focus interactives action - they don't get called separately, so it makes sense to optimize the call and not make an extra tabbables call (+ reduces unnecessary branching) * [Refactor] More complete multiple interactive children unit tests - covers last uncovered branch in file - move to bottom of the file rather than top, since after talking to Chandler this is an edge case that only applies to control header cells + remove `data-euigrid-tab-managed` attr - tests should pass without it * [Refactor] Misc cleanup - convert functions to arrow functions - use shorter headerNode var instead of ref * [Fix] Remove F2 key event - it's had no effect since we switched header cells to use a popover for actions * Add changelog entry * Update changelog
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When you sort columns after you've selected a header cell, there are lot's of errors in console. Seems there is a problem with focus in this case:
The only reliable way to reproduce it was using Safari, but it also happened in Chrome (Reported by @majagrubic)
elastic/kibana#67259 (comment)
Steps to reproduce
NameheaderNameandEmail Adresse- should take a whileThe text was updated successfully, but these errors were encountered: