Skip to content

Conversation

@mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Nov 1, 2025

Summary

This PR refactors EuiDataGrid columns headers to remove the need for using aria-hidden on the actions button as this causes axe core warnings that require Kibana to skip tests: Warning: Do not use aria-hidden="true" on focusable elements.

The aria-hidden was originally added to prevent the nested action button from being read with the content for the case when the cell has nested content and no displayAsText is passed (which is used as text-only label for the content).

Background information

The previous state of the component would result in the following possible output for screen readers, depending on what information is passed to the column:

Note

ℹ️ After the update scenario 3 is removed. The actions button will never be read on focusing a nested cell. It's only read when focusing the actions button directly when entering a nested cell.

  • scenario 1: column with custom content and displayAsText
    • reads the displayAsText
image
  • scenario 2: column with custom content and without displayAsText
    • reads the name and the custom content (e.g. info tooltip aria-label)
image
  • scenario 3: column with custom content, without displayAsText and without aria-hidden on the actions button
    reads the name, the custom content (e.g. info tooltip aria-label) and the actions button aria-label
image

Changes

  • uses aria-labelledby to link the cell content to be read on focus of the cell wrapper to prevent the action button from being read
  • updates the collapsing of the actions button from width=0 to transform as width=0 results in the element not being perceivable for screen readers (it worked before because the toggling of aria-hidden on the button would force an accessibility tree update)
Screen.Recording.2025-10-31.at.14.45.03.mov

Why are we making this change?

:accessibility: Accessibility: Follow accessibility guidelines and not use aria-hidden on focusable elements
🧰 Maintenance/Code health: Prevent having to skip FTR tests in Kibana due to axe core warnings.

Screenshots #

variant before after
with nested content
Screen.Recording.2025-11-01.at.11.26.46.mov
Screen.Recording.2025-10-31.at.14.45.03.mov
without nested content
Screen.Recording.2025-11-01.at.11.27.46.mov
Screen.Recording.2025-11-01.at.11.28.46.mov

Impact to users

🟢 No updates are required on consumer side.

ℹ️ The DOM changes might require snapshot updates.

QA

🔗 EuiDataGrid storybook

  • verify that the action button is never read by screen readers on focusing a nested header cell
  • verify the action button is focusable, being read and actionable with screen readers
  • verify that there is no functional regression between production and this PR

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in both MacOS and Windows high contrast modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
    • If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

- this removes the need to use aria-hidden on the actions button which violates a11y guidelines
…instead of width=0

- width=0 otherwise results in screen readers not finding the DOM element anymore as it's not available and the a11y tree not updated anymore due to the remove aria-hidden
@mgadewoll mgadewoll self-assigned this Nov 1, 2025
@mgadewoll mgadewoll requested a review from alexwizp November 1, 2025 10:44
max-inline-size: 24px;
${header.hideActions} & {
${logicalCSS('width', 0)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transition to width=0 would result in screen readers not being able to detect the element, even after the transition back to full width. Likely the CSS transition doesn't trigger an update on the a11y tree, meaning the element is initially not present and stays that way.

The proposed workaround uses transform instead and uses negative margins to ensure the element collapses within the flex container to ensure parity with the existing show/hide behavior for header cells.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool idea! 👍🏻

@mgadewoll mgadewoll force-pushed the datagrid/a11y-refactor-need-for-aria-hidden-on-actions-button branch from e622987 to f6988e3 Compare November 1, 2025 12:36
@mgadewoll mgadewoll marked this pull request as ready for review November 1, 2025 13:13
@mgadewoll mgadewoll requested a review from a team as a code owner November 1, 2025 13:13
Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

The changes look correct! Thank you.

Copy link
Contributor

@weronikaolejniczak weronikaolejniczak left a comment

Choose a reason for hiding this comment

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

Overall, looks good 👏🏻 I just left a couple of nits. Thanks for refactoring the code.

Testing

Seems to be working as expected 👍🏻

Win + Edge + NVDA

Kapture.2025-11-03.at.11.10.02.mp4

MacOS + Safari + VoiceOver

Kapture.2025-11-03.at.11.12.19.mp4

@mgadewoll mgadewoll force-pushed the datagrid/a11y-refactor-need-for-aria-hidden-on-actions-button branch from 8a9775a to e69ac90 Compare November 4, 2025 12:21
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak left a comment

Choose a reason for hiding this comment

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

LGTM! 🟢 Thanks for applying my suggestions, @mgadewoll!

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 4, 2025

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll mgadewoll merged commit f3a8428 into elastic:main Nov 5, 2025
7 checks passed
alexwizp added a commit to elastic/kibana that referenced this pull request Nov 10, 2025
…y/apps/dashboard_panel_options·ts - Dashboard panel options a11y tests dashboard panel - clone panel (#241259)

Closes: #147667

**Depends on**
elastic/eui#9166

Flaky tests have passed ✅
robester0403 pushed a commit to robester0403/kibana that referenced this pull request Nov 10, 2025
…y/apps/dashboard_panel_options·ts - Dashboard panel options a11y tests dashboard panel - clone panel (elastic#241259)

Closes: elastic#147667

**Depends on**
elastic/eui#9166

Flaky tests have passed ✅
alexwizp added a commit to elastic/kibana that referenced this pull request Nov 10, 2025
Closes: #153597

**Depends on**
elastic/eui#9166

Flaky tests have passed ✅
eokoneyo pushed a commit to eokoneyo/kibana that referenced this pull request Dec 2, 2025
…y/apps/dashboard_panel_options·ts - Dashboard panel options a11y tests dashboard panel - clone panel (elastic#241259)

Closes: elastic#147667

**Depends on**
elastic/eui#9166

Flaky tests have passed ✅
eokoneyo pushed a commit to eokoneyo/kibana that referenced this pull request Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants