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.

Sign up for GitHub

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

[EuiCollapsibleNavBeta] Consider adding aria-hidden="true" to the icon button for screen reader UX #7353 #8236

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+38 −418
Open

[EuiCollapsibleNavBeta] Consider adding aria-hidden="true" to the icon button for screen reader UX #7353 #8236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fbf959e
[EuiCollapsibleNavBeta] Added aria-hidden=true to arrow button (#7353)
Kukki8 Dec 5, 2024
973ddf6
[EuiCollapsibleNavBeta] Fixing pre push issue (#7353)
Kukki8 Dec 5, 2024
f95ebb0
[EuiCollapsibleNavBeta] Creating new snapshots for the pre push fix (…
Kukki8 Dec 5, 2024
7e29e04
Adding changelog for issue #7353
Kukki8 Dec 13, 2024
9d0b720
Adding changelog for main
Kukki8 Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/eui/changelogs/upcoming/2.md
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

@Kukki8 we can remove this file 😄 We only need the one that's named the same as the PR number: 8236.md.

Copy link
Author

Choose a reason for hiding this comment

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

@weronikaolejniczak Got it! I'll remove it.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- 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.
7 changes: 7 additions & 0 deletions packages/eui/changelogs/upcoming/8236.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- 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.
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
- 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? 😄

Copy link
Author

Choose a reason for hiding this comment

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

@weronikaolejniczak Yes, sounds good!. I'm still learning, so I'm fully open to any suggestions.

18 changes: 18 additions & 0 deletions packages/eui/src/components/accordion/__snapshots__/accordion.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ exports[`EuiAccordion is rendered 1`] = `
<button
aria-controls="0"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -61,6 +62,7 @@ exports[`EuiAccordion isDisabled is rendered 1`] = `
<button
aria-controls="21"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-disabled-isDisabled-euiAccordion__arrow-left-isClosed"
disabled=""
Expand Down Expand Up @@ -157,6 +159,7 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = `
<button
aria-controls="13"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-right-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -195,6 +198,7 @@ exports[`EuiAccordion props arrowProps is rendered 1`] = `
<button
aria-controls="15"
aria-expanded="false"
aria-hidden="true"
aria-label="aria-label"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow testClass1 testClass2 emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed-euiTestCss"
Expand Down Expand Up @@ -246,6 +250,7 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = `
<button
aria-controls="3"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -299,6 +304,7 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = `
<button
aria-controls="2"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -348,6 +354,7 @@ exports[`EuiAccordion props buttonElement is rendered 1`] = `
<button
aria-controls="10"
aria-expanded="false"
aria-hidden="false"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="0"
Expand Down Expand Up @@ -395,6 +402,7 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = `
<button
aria-controls="4"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -446,6 +454,7 @@ exports[`EuiAccordion props buttonProps paddingSize l 1`] = `
<button
aria-controls="7"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -496,6 +505,7 @@ exports[`EuiAccordion props buttonProps paddingSize m 1`] = `
<button
aria-controls="6"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -546,6 +556,7 @@ exports[`EuiAccordion props buttonProps paddingSize s 1`] = `
<button
aria-controls="5"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -596,6 +607,7 @@ exports[`EuiAccordion props element is rendered 1`] = `
<button
aria-controls="1"
aria-expanded="false"
aria-hidden="false"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="0"
Expand Down Expand Up @@ -643,6 +655,7 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
<button
aria-controls="11"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -699,6 +712,7 @@ exports[`EuiAccordion props forceState closed is rendered 1`] = `
<button
aria-controls="16"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -752,6 +766,7 @@ exports[`EuiAccordion props forceState open is rendered 1`] = `
<button
aria-controls="17"
aria-expanded="true"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isOpen"
tabindex="-1"
Expand Down Expand Up @@ -804,6 +819,7 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
<button
aria-controls="12"
aria-expanded="true"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isOpen"
tabindex="-1"
Expand Down Expand Up @@ -856,6 +872,7 @@ exports[`EuiAccordion props isLoading is rendered 1`] = `
<button
aria-controls="19"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down Expand Up @@ -914,6 +931,7 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = `
<button
aria-controls="20"
aria-expanded="false"
aria-hidden="true"
aria-labelledby="generated-id"
class="euiButtonIcon euiAccordion__arrow emotion-euiButtonIcon-xs-empty-text-euiAccordion__arrow-left-isClosed"
tabindex="-1"
Expand Down
1 change: 1 addition & 0 deletions packages/eui/src/components/accordion/accordion_trigger/accordion_trigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export const EuiAccordionTrigger: FunctionComponent<
aria-controls={ariaControlsId}
aria-expanded={isOpen}
aria-labelledby={buttonId}
aria-hidden={buttonIsInteractive ? 'true' : 'false'}
tabIndex={buttonIsInteractive ? -1 : 0}
/>
);
Expand Down
Loading