[EuiDataGrid] bug fixes

[chandlerprall]

Summary

[EuiDataGrid] Column action menu can freeze Chrome browser

Fixes #4641 by only responding to key events when they are triggered on an element in the grid's DOM, ignoring events React bubbles through portals.

[EuiDataGrid] Clicking a column header twice makes it impossible to close it via outside click

Fixes #4588 by moving all datagrid header cell contents into the popover's anchor button

[EuiDataGrid] Performance issue when row count changes while pagination is enabled

Fixes #4532 (and another bug) by resetting the grid's height to its container size instead of unsetting+rerender+recalculate the container size. The second bug was in that recalculate step, or more correctly that it wasn't triggered at all.

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 the any docs examples
  - [ ] Added or updated jest tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/

[thompsongl]

[EuiDataGrid] Column action menu can freeze Chrome browser

Confirmed this is fixed. The issue states using the arrow keys for list navigation, but that doesn't actually work. Tabbing does, however.

[EuiDataGrid] Clicking a column header twice makes it impossible to close it via outside click

Confirmed this is fixed. Agree that this will be useful in other instances of EuiPopover.

[EuiDataGrid] Performance issue when row count changes while pagination is enabled

Encountered the issue below. Toggling fullscreen does not reset height to the correct size.

[chandlerprall]

Encountered the issue below. Toggling fullscreen does not reset height to the correct size.

@thompsongl pushed a fix to ignore container size when fullscreen

[thompsongl]

pushed a fix to ignore container size when fullscreen

🧐 I don't think it got pushed

[thompsongl]

LGTM
Latest commit resolves the fullscreen issue while fixing the height change bug.

Don't forget CL entries

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/

[chandlerprall]

After discussing with Caroline over zoom, I've changed the solution for the second issue (#4588) to avoid adding a new prop to EuiPopover and instead moved all of the header contents into the popover anchor.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/

[thompsongl]

moved all of the header contents into the popover anchor

Solves the bug, which is the main thing. Subjectively, I think it's odd that the popover arrow is centered/not aligned with the arrow icon. Is this something the two of you discussed?

[cchaos]

Yes we did discuss, and for me, it was a tradeoff between a slight design aesthetic and further complicating and already complicated component. It also makes a little more sense to point to the middle of the cell since the whole cell IS the button. It think the main thing now is that since the button is only a small portion of the height of the cell, the popover's top position is too high:

Whereas, if it were lowered a bit, it would align better:

[cchaos]

This also brings up another issue I have brought up before which is the actual position of that caret being right aligned. The problem comes when you no longer have column dividers. The carets are then closer to the next column's text than the current one's making it very confusing as to if that's the sort indicator or what.

@chandlerprall While I was also grabbing that screenshot, one thing I did notice in your PR is that the popover also doesn't close when you resize the column.

[chandlerprall]

It think the main thing now is that since the button is only a small portion of the height of the cell, the popover's top position is too high:

Pushed a small offset

This also brings up another issue I have brought up before which is the actual position of that caret being right aligned. The problem comes when you no longer have column dividers

There's an issue for this somewhere, tho I couldn't identify it with a quick glance at our open issues

While I was also grabbing that screenshot, one thing I did notice in your PR is that the popover also doesn't close when you resize the column.

This is a side effect from preventing resizing from stealing any focus. Focus can only be prevented during the resize handle's mousedown event, which in turn disables the click event which would have closed the popover. Since the intent is for the handle to not grab focus, keeping the popover open at least feels rational? It is still closable by clicking anywhere else.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/

[thompsongl]

keeping the popover open at least feels rational

We may be able to improve upon the current behavior, but I'm good merging this with the critical bugs resolved.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_4706/