[EuiCollapsibleNavBeta] Consider adding aria-hidden="true" to the icon button for screen reader UX #7353 #8236
Conversation
|
💚 CLA has been signed |
|
👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually? |
|
@Kukki8 thank you so much for your contribution! I'll review your PR soon 💚 |
There was a problem hiding this comment.
@Kukki8 we can remove this file 😄 We only need the one that's named the same as the PR number: 8236.md.
There was a problem hiding this comment.
@weronikaolejniczak Got it! I'll remove it.
| - Added aria-hidden={buttonIsInteractive ? 'true' : 'false'} tag to the accordion_trigger.tsx file, as advised in the issue description. | ||
|
|
||
| **Accessibility** | ||
|
|
||
| - Improved the accessibility experience of the accordion buttons so screen readers do not detect it twice. | ||
|
|
||
| - Summary : aria-hidden={buttonIsInteractive ? 'true' : 'false'} was added to src/components/accordion/accordion_trigger/accordion_trigger.tsx in object EuiAccordionTrigger in const arrow. |
There was a problem hiding this comment.
The top block is reserved for features. You can read more about the file structure here.
We write all accessibility improvements in the "Accessibility" section. One improvement is a one bullet point.
Here's my suggestion:
| - Added aria-hidden={buttonIsInteractive ? 'true' : 'false'} tag to the accordion_trigger.tsx file, as advised in the issue description. | |
| **Accessibility** | |
| - Improved the accessibility experience of the accordion buttons so screen readers do not detect it twice. | |
| - Summary : aria-hidden={buttonIsInteractive ? 'true' : 'false'} was added to src/components/accordion/accordion_trigger/accordion_trigger.tsx in object EuiAccordionTrigger in const arrow. | |
| **Accessibility** | |
| - Improved the `EuiAccordionTrigger`'s screen reader UX. Passed `aria-hidden` to the `EuiAccordionArrow` to avoid duplicated announcements by screen readers. |
Does this sound good to you @Kukki8? 😄
There was a problem hiding this comment.
@weronikaolejniczak Yes, sounds good!. I'm still learning, so I'm fully open to any suggestions.
There was a problem hiding this comment.
@Kukki8 this snapshot file is necessary for the packages/eui/src/components/collapsible_nav/collapsible_nav.test.tsx test suite, can you bring it back? 😄 You can easily do it with: yarn workspace @elastic/eui test-unit collapsible_nav.
There was a problem hiding this comment.
@weronikaolejniczak Sure, I'll try that! 😄
Summary
This PR provides:
aria-hidden={buttonIsInteractive ? 'true' : 'false'}tag to theaccordion_trigger.tsxfile, as advised in the issue description.This PR doesn't contain:
closes #7353
Implementation
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
Checked in both light and dark modesChecked in mobileEdge, and FirefoxAdded documentationProps have proper autodocs (using@defaultif default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesUpdated visual regression testsIf applicable, added the breaking change issue label (and filled out the breaking change 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)