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

Added SR-only help text to EuiPopover and dependent components. #6589

Merged
merged 20 commits into from
Mar 7, 2023
Merged

Added SR-only help text to EuiPopover and dependent components. #6589

merged 20 commits into from
Mar 7, 2023

Conversation

1Copenut
Copy link
Contributor

@1Copenut 1Copenut commented Feb 9, 2023 •
edited
Loading

Summary

This PR closes #5813. It adds screen reader helper text to several components in two broad ways:

  1. Screen reader instructions are announced when an EuiFlyout is opened. These instructions take into account if the flyout has ownFocus={true| or ownFocus={false}.
  2. Instructions are announced as aria-describedby text for input-centric components. This was the best way to ensure the instructions were read back consistently.
  • Added SR help text to the following components:
    • EuiPopover
    • EuiSelectableTemplateSitewide
    • EuiSuggest
    • EuiSelectableSearch
    • EuiRange/EuiDualRange with input popover
    • EuiAutoRefresh
    • EuiContextMenu
    • EuiSearchBar
  • Completed preliminary testing with Safari and Firefox, VO screen reader.
  • Updated snapshot tests for SR instructions.

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
  • Checked VoiceOver screen reader with Safari and Firefox
  • 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

@1Copenut
WIP. Updating EuiPopover for focusable content instructions.
5cb12af
@1Copenut
Merge branch 'main' into feature/screen-reader-dialogue
4a5f6a4
@1Copenut
Added SR-only help text to EuiPopover and dependent components.
7232812 
* Added SR help text to EuiPopover, EuiRange, EuiDualRange.
* Completed preliminary testing with Safari and Firefox, VO screen reader.
* Updated snapshot tests for SR instructions.
@1Copenut 1Copenut self-assigned this Feb 9, 2023
1Copenut
1Copenut commented Feb 9, 2023
View reviewed changes
Copy link
Contributor Author

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

Left two review comments for the largest change and reasoning behind another.

src/components/selectable/selectable.tsx Show resolved Hide resolved
src/components/selectable/selectable_list/selectable_list.tsx Show resolved Hide resolved
@1Copenut 1Copenut marked this pull request as ready for review February 9, 2023 19:44
@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

breehall
breehall approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@breehall breehall left a comment

Choose a reason for hiding this comment

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

Good work Trevor! I used the PR preview in combination with Apple Voice Over to check these changes. I don't typically use Voice Over, but good gravy does it have a lot to say! 😂

cee-chen
cee-chen requested changes Feb 13, 2023
View reviewed changes
Copy link
Member

@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.

Making #6589 (comment) a blocking change request

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@daveyholler daveyholler mentioned this pull request Mar 6, 2023
Closed
@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

cee-chen
cee-chen reviewed Mar 6, 2023
View reviewed changes
src/components/selectable/selectable.test.tsx
@@ -495,7 +495,7 @@ describe('EuiSelectable', () => {
</EuiSelectable>
);

expect(component.find('p').text()).toEqual(
expect(component.find('p#generated-id_instructions').text()).toEqual(
Copy link
Member

@cee-chen cee-chen Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is fine for now / not a change request, but FYI, at some point we want to stop replacing the randomly generated ID strings with a static generated-id string. (#4408)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I'll keep that in mind as I write or refactor tests going forward. Thank you for the link and FYI.

@kibanamachine
Copy link

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

@1Copenut @cee-chen
Update src/components/selectable/selectable.tsx
d780a2e 
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
@1Copenut 1Copenut enabled auto-merge (squash) March 6, 2023 22:03
@kibanamachine
Copy link

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

@1Copenut
Copy link
Contributor Author

1Copenut commented Mar 6, 2023

jenkins retest this please

@1Copenut 1Copenut disabled auto-merge March 6, 2023 22:37
@kibanamachine
Copy link

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

cee-chen
cee-chen approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@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.

:shipit:

@1Copenut 1Copenut merged commit 358617d into elastic:main Mar 7, 2023
@1Copenut 1Copenut deleted the feature/screen-reader-dialogue branch March 7, 2023 14:04
This was referenced Mar 20, 2023
Closed
Merged
1Copenut added a commit to elastic/kibana that referenced this pull request Mar 23, 2023
@1Copenut @mistic
Upgrade EUI to v76.3.0 (#153341)
6d55dd9 
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([#6643](elastic/eui#6643))

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

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Mar 24, 2023
@1Copenut @mistic @jennypavlova
Upgrade EUI to v76.3.0 (elastic#153341)
72cdbde 
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([elastic#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([elastic#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([elastic#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([elastic#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([elastic#6643](elastic/eui#6643))

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

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([elastic#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([elastic#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([elastic#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([elastic#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([elastic#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@breehall breehall breehall approved these changes

@miukimiu miukimiu Awaiting requested review from miukimiu

Assignees

@1Copenut 1Copenut

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

Successfully merging this pull request may close these issues.

Add keyboard usage instructions to screen reader popover dialogue
4 participants
@1Copenut @kibanamachine @cee-chen @breehall