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

Jump to bottom

[A11y] [SearchBar] [Filters] Improve accessibility of the Filters toggle button #8091

Merged
merged 15 commits into from
Nov 26, 2024
Merged

[A11y] [SearchBar] [Filters] Improve accessibility of the Filters toggle button #8091

merged 15 commits into from
Nov 26, 2024

Conversation

rbrtj
Copy link
Contributor

@rbrtj rbrtj commented Oct 21, 2024

Summary

Related to:
#8088

Added additional text inside the filter button so that the screen reader reads <filter label> Select instead of just <filter label>. Updated snapshots to ensure all tests pass.

@rbrtj rbrtj requested a review from a team as a code owner October 21, 2024 09:27
@rbrtj rbrtj self-assigned this Oct 21, 2024
@cee-chen
Copy link
Contributor

@alexwizp Any chance we could get you to test-drive this on https://eui.elastic.co/pr_8091/storybook/?path=/story/forms-euisearchbar-euisearchbar--playground and get your a11y expertise on the changes before we dive into code review?

@JasonStoltz
Copy link
Member

Hey @rbrtj. Thanks for the contribution. Just FYI that this is on our radar we just haven't had a chance to review and test it yet.

@mgadewoll
Copy link
Contributor

mgadewoll commented Nov 22, 2024 •
edited
Loading

@rbrtj I finally got to check your PR, excuse the longer waiting time!

When checking the component, I narrowed down two main issues:

  1. the toggle button could be more descriptive, instead of the output {LABEL}, button we can insert additional text to be read for clarification, e.g. like Tag selection, button
  • This is what you implemented and your implementation works overall!
    I do think we should add the additional label at the usage level though, instead of on the EuiFilterButton. EuiFilterButton already supports aria attributes to be passed, as the props are spread to the button. We don't need to add additional props. Instead we can use aria-label on the implementation of EuiFilterButton in FieldValueSelectionFilter to pass the expected screen reader label.
  1. When clicking the selection button it opens a dialog. This dialog will be focused initially which is unexpected. Instead the first focusable (and logically meaningful) content should be focused instead. (docs)
    This currently means a user lands on the dialog and needs to use Tab to focus the listbox to be able to select anything. Instead we should focus the listbox once it's available to ensure users can use the arrow keys to select options immediately, similar to a native select element.

How this would look like:

NVDA

Screen.Recording.2024-11-22.at.16.10.31.mov

JAWS

Screen.Recording.2024-11-22.at.16.14.24.mov

@rbrtj If you don't mind, I would update your PR to address the two points mentioned above.

@mgadewoll mgadewoll requested a review from cee-chen November 25, 2024 08:54
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @rbrtj

@rbrtj
Copy link
Contributor Author

rbrtj commented Nov 26, 2024

Looks great!
Thanks for the details and for updating my PR 😄

cee-chen
cee-chen approved these changes Nov 26, 2024
View reviewed changes
Copy link
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.

Awesome work on this Lene, I always appreciate your a11y expertise and screen reader testing!

@mgadewoll mgadewoll merged commit c6cf094 into elastic:main Nov 26, 2024
5 checks passed
@tkajtoch tkajtoch mentioned this pull request Nov 27, 2024
Merged
@rbrtj rbrtj mentioned this pull request Nov 27, 2024
Closed
@mgadewoll mgadewoll mentioned this pull request Dec 18, 2024
Merged
mgadewoll added a commit to elastic/kibana that referenced this pull request Jan 8, 2025
@mgadewoll
[8.x] Upgrade EUI to v98.2.1-borealis.1 (#204783)
1384ced 
>[!IMPORTANT]
This PR introduces the Borealis enabled EUI package to `8.x`.
The intention is to support new features being developed that need to
live in `main` as well as `8.x`.


>[!IMPORTANT]
All `8.x` versions should continue to use the current Amsterdam theme.
To ensure this even with upcoming changes to the default theme for EUI,
we're adding `euiThemeAmsterdam` on `EuiProvider` usages manually.

## Description

This PR introduces all previous Borealis related PR changes but excludes
the specific changes to support `@elastic/eui-theme-borealis` as only
Amsterdam is supported as theme in `8.x`.

Previous PRs

- #199993
- #201049
- #202073

---

`v97.3.1`⏩`v98.2.1-borealis.1`

_[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)_

---

# `@elastic/eui`

## [`v98.2.1`](https://github.com/elastic/eui/releases/v98.2.1)

- Updated the EUI theme color values to use a full 6 char hex code
format ([#8244](elastic/eui#8244))

## [`v98.2.0`](https://github.com/elastic/eui/releases/v98.2.0)

- Added two new icons: `contrast` and `contrastHigh`
([#8216](elastic/eui#8216))
- Updated `EuiDataGrid` content to have a transparent background.
([#8220](elastic/eui#8220))

**Accessibility**

- When the tooltips components (`EuiTooltip`, `EuiIconTip`) are used
inside components that handle the Escape key (like `EuiFlyout` or
`EuiModal`), pressing the Escape key will now only close the tooltip and
not the entire wrapping component.
([#8140](elastic/eui#8140))
- Improved the accessibility of `EuiCodeBlock`s by
([#8195](elastic/eui#8195))
  - adding screen reader only labels
  - adding `role="dialog"` on in fullscreen mode
  - ensuring focus is returned on closing fullscreen mode
  
# Borealis updates
  
- [Visual Refresh] Update color token mappings
([#8211](elastic/eui#8211))
- [Visual Refresh] Introduce shared popover arrow styles to Borealis
([#8212](elastic/eui#8212))
- [Visual Refresh] Add forms.maxWidth token
([#8221](elastic/eui#8221))
- [Visual Refresh] Set darker shade for subdued text
([#8224](elastic/eui#8224))
- [Visual Refresh] Remove support for accentSecondary badges
([#8224](elastic/eui#8227))
- [Visual Refresh] Add severity vis colors
([#8247](elastic/eui#8247))

---

# `@elastic/eui`

## [`v98.1.0`](https://github.com/elastic/eui/releases/v98.1.0)

- Updated `EuiBetaBadge` with a new `warning` color variant
([#8177](elastic/eui#8177))

**Accessibility**

- Ensures `autoFocus` on `EuiSelectableList` triggers initial focus
([#8091](elastic/eui#8091))
- Improved the accessibility of `EuiSearchBarFilters` by:
([#8091](elastic/eui#8091))
  - adding a more descriptive `aria-label` to selection filter buttons
- ensuring the selection listbox is initially focused when opening a
selection popover
- Improved the accessibility experience of tabs (EuiTab, EuiTabs): tab
group is a tab stop and tabs can be traversed with arrow keys.
([#8116](elastic/eui#8116))
- Updated `EuiCodeBlock` with a new `copyAriaLabel` prop, which allows
setting a custom screen reader label on the copy button.
([#8176](elastic/eui#8176))

**CSS-in-JS conversions**

- Removed the following global Sass variables:
([#8169](elastic/eui#8169))
  - `$euiButtonMinWidth`
  - `$euiDatePickerCalendarWidth`
- Removed the following Sass animations:
([#8169](elastic/eui#8169))
  - `euiAnimFadeIn`
  - `euiGrow`
  - `focusRingAnimate`
  - `focusRingAnimateLarge`
  - `euiButtonActive`
- Removed the following Sass mixins:
([#8169](elastic/eui#8169))
  - `euiFullHeight`
  - `euiSlightShadowHover`
  - `datePickerArrow`

# Borealis updates

- [Visual Refresh] Fix euiColorFullShade Sass variable mapping
(elastic/eui#8178)

---

# `@elastic/eui`

## [`v98.0.0`](https://github.com/elastic/eui/releases/v98.0.0)

**Bug fixes**

- Fixed an `EuiDataGrid` bug where column actions where not clickable
when `EuiDataGrid` with `columnVisibility.canDragAndDropColumns` was
used inside a modal ([#8135](elastic/eui#8135))

**Breaking changes**

- Removed `EuiFormRow`'s deprecated `columnCompressedSwitch` display
prop. Use `columnCompressed` instead
([#8113](elastic/eui#8113))

# Borealis updates

**Bug fixes**

- Fixed computed border token mapping
(elastic/eui#8170)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgadewoll mgadewoll mgadewoll left review comments

@cee-chen cee-chen cee-chen approved these changes

Assignees

@rbrtj rbrtj

Labels
accessibility - screen reader
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rbrtj @cee-chen @JasonStoltz @mgadewoll @kibanamachine @elasticmachine