Skip to content

[EuiAccordion] Fixed a bug in aria-expanded for axe-core.#6694

Merged
1Copenut merged 7 commits intoelastic:mainfrom
1Copenut:bug/euiAccordion-aria-expanded-interactive
Apr 6, 2023
Merged

[EuiAccordion] Fixed a bug in aria-expanded for axe-core.#6694
1Copenut merged 7 commits intoelastic:mainfrom
1Copenut:bug/euiAccordion-aria-expanded-interactive

Conversation

@1Copenut
Copy link
Copy Markdown
Contributor

@1Copenut 1Copenut commented Apr 5, 2023

Summary

The EuiAccordion component was throwing an axe-core violation when we included interactive content in the trigger. This was due to non-button elements receiving an aria-expanded attribute. This PR adds a logic check to only render the aria-expanded attribute on buttons, and in the correct state.

  • Added logic check for EuiAccordion aria-expanded attr.
  • Adding comment for new logic reasoning.

Closes #6689

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

* Added logic check for EuiAccordion aria-expanded attr.
* Adding comment for new logic reasoning.
@1Copenut 1Copenut self-assigned this Apr 5, 2023
@1Copenut 1Copenut changed the title Fixed a bug in EuiAccordion aria-expanded for axe. [WIP: Do not merge][EuiAccordion] Fixed a bug in aria-expanded for axe-core. Apr 5, 2023
@kibanamachine
Copy link
Copy Markdown

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

@1Copenut 1Copenut added breaking change PRs with breaking changes. (Don't delete - used for automation) bug accessibility labels Apr 5, 2023
@kibanamachine
Copy link
Copy Markdown

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

@1Copenut 1Copenut changed the title [WIP: Do not merge][EuiAccordion] Fixed a bug in aria-expanded for axe-core. [EuiAccordion] Fixed a bug in aria-expanded for axe-core. Apr 6, 2023
@1Copenut 1Copenut marked this pull request as ready for review April 6, 2023 19:54
@1Copenut 1Copenut requested review from breehall and cee-chen April 6, 2023 19:54
className={buttonClasses}
aria-controls={id}
aria-expanded={isOpen}
aria-expanded={isExpandableButton ? isOpen : undefined}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Jest and Cypress tests broke in local testing while I was working through this logic, so I've got high confidence in this approach. The two snapshot test changes were using div | fieldset for the ButtonElement which is what I expected.

1Copenut and others added 4 commits April 6, 2023 15:12
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
@kibanamachine
Copy link
Copy Markdown

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

@1Copenut 1Copenut requested a review from cee-chen April 6, 2023 21:26
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🎉 Woohoo!

@1Copenut 1Copenut merged commit 35b6a27 into elastic:main Apr 6, 2023
@1Copenut 1Copenut deleted the bug/euiAccordion-aria-expanded-interactive branch April 6, 2023 21:48
jbudz added a commit to elastic/kibana that referenced this pull request Apr 18, 2023
EUI `77.0.0` ➡️ `77.1.1`

## [`77.1.0`](https://github.com/elastic/eui/tree/v77.1.0)

- Updated `EuiDatePicker` to display a warning icon and correctly set
`aria-invalid` when `isInvalid` is passed
([#6677](elastic/eui#6677))
- Updated `EuiFilePicker` to display an alert icon when `isInvalid`
([#6678](elastic/eui#6678))
- Updated `EuiTextArea` to display an alert icon when `isInvalid`
([#6679](elastic/eui#6679))
- Updated `EuiTextArea` to support the `isLoading` prop
([#6679](elastic/eui#6679))
- Updated `EuiComboBox` to display a warning icon and correctly set
`aria-invalid` when `isInvalid` is passed
([#6680](elastic/eui#6680))

**Bug fixes**

- Fixed `EuiAccordion` to not set an `aria-expanded` attribute on
non-interactive `buttonElement`s
([#6694](elastic/eui#6694))
- Fixed an `EuiPopoverFooter` bug causing nested popovers within
popovers (note: not a recommended use-case) to unintentionally override
its panel padding size inherited from context
([#6698](elastic/eui#6698))
- Fixed `EuiComboBox` to only delete the last selected item on backspace
if the input caret is present
([#6699](elastic/eui#6699))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Jon <jon@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility breaking change PRs with breaking changes. (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiAccordion][AXE-CORE]: Interactive content in trigger must not have aria-expanded attribute

3 participants