[EuiAccordion] Update component logic to retain focus on `<button>` when clicked by 1Copenut · Pull Request #7696 · elastic/eui

1Copenut · 2024-04-17T21:45:08Z

Summary

This PR updates the EuiAccordion component to not move focus to the child component when the expand/collapse button is clicked. This came about from an accessibility audit that moving focus in this instance is in violation of SC 3.2.2: On Input (Level A).

Specifically, moving focus to the child container caused a change of context by resetting focus.

I don't believe this is a breaking change, but would ask reviewers to speak up early and loudly if they feel it might be a breaking change.

QA

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

Manual QA

I checked in all four browsers. I reviewed the EuiAccordion and EuiCollapsibleNavGroup in the local staging site, and the beta collapsible nav in Storybook. I reviewed for axe-core violations, keyboard navigation, and screen reader usability with VoiceOver in Safari and Firefox. Reviewed NVDA (Firefox, Chrome) and JAWS (Chrome). All had consistent behavior and announcement.

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
Docs site QA - N/A
Code quality checklist
- Added or updated jest and cypress tests
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)

1Copenut · 2024-04-17T21:51:06Z

src/components/accordion/accordion_children/accordion_children.tsx

      style={heightInlineStyle}
-      ref={accordionChildrenRef}
      role={role}
-      tabIndex={-1}


It felt safe to remove the tabindex here because the only time it was being called was from our parent component to set focus programmatically. Given that's what we don't want anymore, seemed the right play.

1Copenut · 2024-04-18T14:25:18Z

I mocked up a quick <details> / <summary> this morning to validate we are following standard pattern to keep focus on the trigger. Confirmed this to be accurate.

cee-chen · 2024-04-18T21:17:30Z

This looks really great, awesome work Trevor! I'm going to push up some minor test cleanups (the entire describe block doesn't really make sense now that we're removing focus handling) and then merge

cee-chen

🚀

kibanamachine · 2024-04-18T21:53:31Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7696/
Storybook: https://eui.elastic.co/pr_7696/storybook

elasticmachine · 2024-04-18T22:12:19Z

💚 Build Succeeded

Buildkite Build
Commit: 5ff2e60

History

💚 Build #1795 succeeded fba7170

cc @1Copenut

`v94.1.0-backport.0` ⏩ `v94.2.1-backport.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v94.2.1-backport.0`](https://github.com/elastic/eui/releases/v94.2.1-backport.0) **This is a backport release only intended for use by Kibana.** - Reverted the `EuiFlexGroup`/`EuiFlexItem` `component` prop feature due to Kibana typing issues ## [`v94.2.1`](https://github.com/elastic/eui/releases/v94.2.1) **Bug fixes** - Fixed an `EuiTabbedContent` edge case bug that occurred when updated with a completely different set of `tabs` ([#7713](elastic/eui#7713)) - Fixed the `@storybook/test` dependency to be listed in `devDependencies` and not `dependencies` ([#7719](elastic/eui#7719)) ## [`v94.2.0`](https://github.com/elastic/eui/releases/v94.2.0) - Updated `getDefaultEuiMarkdownPlugins()` to allow excluding the following plugins in addition to `tooltip`: ([#7676](elastic/eui#7676)) - `checkbox` - `linkValidator` - `lineBreaks` - `emoji` - Updated `EuiSelectable`'s `isPreFiltered` prop to allow passing a configuration object, which allows disabling search highlighting in addition to search filtering ([#7683](elastic/eui#7683)) - Updated `EuiFlexGroup` and `EuiFlexItem` prop types to support passing any valid React component type to the `component` prop and ensure proper type checking of the extra props forwarded to the `component`. ([#7688](elastic/eui#7688)) - Updated `EuiSearchBar` to allow the `@` special character in query string searches ([#7702](elastic/eui#7702)) - Added a new, optional `optionMatcher` prop to `EuiSelectable` and `EuiComboBox` allowing passing a custom option matcher function to these components and controlling option filtering for given search string ([#7709](elastic/eui#7709)) **Bug fixes** - Fixed an `EuiPageTemplate` bug where prop updates would not cascade down to child sections ([#7648](elastic/eui#7648)) - To cascade props down to the sidebar, `EuiPageTemplate` now explicitly requires using the `EuiPageTemplate.Sidebar` rather than `EuiPageSidebar` - Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct paddings for icon shapes set to `side: 'right'` ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore `icon`/`prepend`/`append` when `controlOnly` is set to true ([#7666](elastic/eui#7666)) - Fixed `EuiColorPicker`'s input not setting the correct right padding for the number of icons displayed ([#7666](elastic/eui#7666)) - Visual fixes for `EuiRange`s with `showInput`: ([#7678](elastic/eui#7678)) - Longer `append`/`prepend` labels no longer cause a background bug - Inputs can no longer overwhelm the actual range in width - Fixed a visual text alignment regression in `EuiTableRowCell`s with the `row` header scope ([#7681](elastic/eui#7681)) - Fixed `toolTipProps` type on `EuiSuperUpdateButton` to use `Partial<EuiToolTipProps>` ([#7692](elastic/eui#7692)) - Fixes missing prop type for `popperProps` on `EuiDatePicker` ([#7694](elastic/eui#7694)) - Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns` when moving columns to the left/right ([#7701](elastic/eui#7701)) ([#7698](elastic/eui#7698)) - Fixed `EuiSuperDatePicker` to validate date string with respect of locale on `EuiAbsoluteTab`. ([#7705](elastic/eui#7705)) - Fixed a visual bug with `EuiSuperDatePicker`'s absolute tab on small mobile screens ([#7708](elastic/eui#7708)) - Fixed i18n of empty and loading state messages for the `FieldValueSelectionFilter` component ([#7718](elastic/eui#7718)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.6.0 ([#7599](elastic/eui#7599)) - Updated `remark-rehype` to v8.1.0 ([#7601](elastic/eui#7601)) **Accessibility** - Improved `EuiBasicTable` and `EuiInMemoryTable`'s selection checkboxes to have unique aria-labels per row ([#7672](elastic/eui#7672)) - Added `aria-valuetext` attributes to `EuiRange`s with tick labels for improved screen reader UX ([#7675](elastic/eui#7675)) - Updated `EuiAccordion` to keep focus on accordion trigger instead of moving to content on click/keypress ([#7696](elastic/eui#7696)) - Added `aria-disabled` attribute to `EuiHorizontalSteps` when status is "disabled" ([#7699](elastic/eui#7699)) --------- Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

…hen clicked (elastic#7696) Co-authored-by: Cee Chen <constance.chen@elastic.co>

1Copenut added 3 commits April 17, 2024 16:32

Removed the logic for moving focus to child container.

c0473b4

Updated unit tests and snapshots.

03b4edb

Updated Cypress functional test.

2a16967

1Copenut added chore accessibility accessibility - WCAG A labels Apr 17, 2024

1Copenut self-assigned this Apr 17, 2024

1Copenut requested a review from a team as a code owner April 17, 2024 21:45

Added changelog for PR 7696.

fba7170

1Copenut requested a review from cee-chen April 17, 2024 21:48

1Copenut commented Apr 17, 2024

View reviewed changes

simplify tests and add comment block with explanation

5ff2e60

cee-chen approved these changes Apr 18, 2024

View reviewed changes

cee-chen changed the title ~~[CHORE][EuiAccordion]: Update component logic to retain focus on <button> when clicked~~ [EuiAccordion] Update component logic to retain focus on <button> when clicked Apr 18, 2024

cee-chen enabled auto-merge (squash) April 18, 2024 21:25

cee-chen merged commit 860764b into elastic:main Apr 18, 2024

cee-chen mentioned this pull request Apr 29, 2024

Upgrade EUI to v94.2.1 elastic/kibana#182023

Merged

mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request May 3, 2024

[EuiAccordion] Update component logic to retain focus on <button> w…

a51d62e

…hen clicked (elastic#7696) Co-authored-by: Cee Chen <constance.chen@elastic.co>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiAccordion] Update component logic to retain focus on `<button>` when clicked#7696

[EuiAccordion] Update component logic to retain focus on `<button>` when clicked#7696
cee-chen merged 5 commits intoelastic:mainfrom
1Copenut:chore/euiAccordion-focus

1Copenut commented Apr 17, 2024 •

edited by cee-chen

Loading

Uh oh!

1Copenut Apr 17, 2024

Uh oh!

1Copenut commented Apr 18, 2024

Uh oh!

cee-chen commented Apr 18, 2024

Uh oh!

cee-chen left a comment

Uh oh!

kibanamachine commented Apr 18, 2024

Uh oh!

elasticmachine commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

1Copenut commented Apr 17, 2024 • edited by cee-chen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

Manual QA

General checklist

Uh oh!

1Copenut Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

1Copenut commented Apr 18, 2024

Uh oh!

cee-chen commented Apr 18, 2024

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Apr 18, 2024

Uh oh!

elasticmachine commented Apr 18, 2024

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1Copenut commented Apr 17, 2024 •

edited by cee-chen

Loading