Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiDataGrid] Enhance column header actions popover to be keyboard navigable with up/down arrow keys #6017

Merged

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 30, 2022

Summary

EuiDataGrid changes

closes #4642

Look ma, no mouse!

Implementation should still respect native Tab behavior as it works off of document.activeElement.

screencap

EuiPopover changes

  • I fixed a prop being incorrectly passed to the underlying DOM (4bff0b8)
  • I added a popoverScreenReaderText prop to allow adding custom keyboard navigation instructions for SR users (689dd44):

Checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • Added or updated jest and cypress tests
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Props have proper autodocs and playground toggles
- [ ] Added documentation
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Updated the Figma library counterpart

@cee-chen cee-chen requested a review from chandlerprall June 30, 2022 19:16
@cee-chen cee-chen force-pushed the datagrid/column-header-actions-arrow-keys branch from 010fd29 to dd38910 Compare June 30, 2022 19:17
@cee-chen cee-chen requested a review from 1Copenut June 30, 2022 19:19
@cee-chen cee-chen marked this pull request as ready for review June 30, 2022 19:21
@cee-chen
Copy link
Contributor Author

cee-chen commented Jun 30, 2022

Also wanted to mention - I did spike out converting EuiListGroup to an EuiContextMenuPanel, which comes with up/down arrow navigation OOTB, but unfortunately the way we've built the object configuration of our cell header actions is super specific to EuiListGroupItem props (see: column_actions.tsx). There's also a few props such as color which is unique to EuiListGroupItem that we wouldn't get for free from our context menus for example. That's not to say the conversion isn't possible, but I avoided it for now for the sake of not either introducing a breaking change or a lot of annoying-to-grok transmogging code. Also worth noting we would need to add a new xs size for EuiContextMenu, which is easy enough, but is something we would need to run by the designers to ensure it's OK for the component.

TL;DR I found it easier to write a bunch of index if statements lol 🤪

@kibanamachine
Copy link

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

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really liking this enhancement. I left one copy suggestion / question, but have no items specifically to change. LGTM!

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; tested in the PR's published docs. ❤️ those tests

@cee-chen cee-chen merged commit 36df658 into elastic:main Jul 5, 2022
@cee-chen cee-chen deleted the datagrid/column-header-actions-arrow-keys branch July 5, 2022 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiDataGrid] Navigate column actions with arrow keys
4 participants