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] Change header cell popover menu trigger element to be the verticalBoxes icon button #7660

Closed
Tracked by #182611
tkajtoch opened this issue Apr 8, 2024 · 7 comments · Fixed by #7898
Closed
Tracked by #182611
Assignees

Comments

@tkajtoch
Copy link
Member

tkajtoch commented Apr 8, 2024

Summary

This request came up from the discussion in #7469.

There are cases where customers want to add interactive components like EuiTooltips to EuiDataGrid header cells to provide more context and display help text. This is currently impossible to achieve while keeping the header cells accessible. We've decided it would be worth changing the element we listen for click events on from the whole header cell to just the vertical boxes icon (with some padding added to reduce cursor/touch precision required).

Acceptance criteria

  • EuiDataGrid's header cell popover menus should open only when clicking on the verticalBoxes icon button and not the whole header cell
  • The updated header cell rendering implementation should be accessible
  • The verticalBoxes icon button should have a data-test-subj attribute for easy referencing in automated tests
@JasonStoltz
Copy link
Member

Some things to consider here:

  • Is the click target for the menu currently large enough for accessibility?
  • If we implement drag and drop for header cells, how does that interaction fit together with additional click targets in the header cell?

@ninoslavmiskovic
Copy link

We want to e.g. add ECS descriptions as tooltops for fields in the table in Discover.

E.g. for service.name

Skærmbillede 2024-06-07 kl  13 18 56

So this could be useful :)

@davismcphee
Copy link
Contributor

Hey team, just wanted to check in on this issue and ask if it's still something that could be prioritized in the near term? We'd like to implement the ECS field descriptions functionality @ninoslavmiskovic mentioned above in Discover, but currently the a11y issue is an obstacle.

@JasonStoltz
Copy link
Member

@davismcphee Hey Davis, we weren't planning to work on this until next quarter. We'll see if we can work it into our schedule for the next week or two.

@davismcphee
Copy link
Contributor

Thanks for considering it!

@mgadewoll
Copy link
Contributor

After having had a first look at this I think there are still some Accessibility/Usability questions to align on and I think we should align with @1Copenut to verify what expected output and functionality we need to ensure.

  1. How will screen reader users know there are interactive nested elements and how many?
  • We should add context information for screen reader users that there are nested items. We could try using semantic groups to utilize standard semantics
  • We should be able to leverage the already used focus trap functionality to indicate cells with interactive content
  1. How do we expect the keyboard navigation to function for nested interactive cell items?
  • We already use focus traps for cells with interactive content, we should be able to reuse this which then adds indication that a user can enter a cell to navigate its content (e.g. already in use for row data cells with interactive content)

@mgadewoll
Copy link
Contributor

ℹ After aligning with @1Copenut I updated the POC solution and asked for a full accessibility check next to verify our changes work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants