[a11y] More role="region" cleanup
#7327
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1Copenut
approved these changes
Nov 1, 2023
There was a problem hiding this comment.
👍 Sorry this review took so long. I had to figure out a number of things, which I'll attempt to summarize here:
- Drag and drop still works great and announces the picked up, moved, and dropped actions correctly in all pairings except Safari + VoiceOver. This is not a regression; it works as it did before. Firefox + VoiceOver worked correctly, as did Firefox + NVDA and Chrome + JAWS.
- NVDA and JAWS require the user to be in Focus/Forms mode to use the Drag and Drop controls. We mention this in our
aria-describedbyso yay! to us. I had to figure out how to turn off the PC virtual cursor in JAWS to activate these controls, then re-enable PC virtual cursor when I was done manipulating the list order. I do not consider this an issue; it's more about me learning to use JAWS as it is, not as I thought it was. - The scrollable blocks work correctly and do not appear as designated landmarks or in the Landmarks menu. Scrollable blocks are an interesting case. As long as they have
tabindex="0"keyboard users can traverse them with arrow keys. Screen readers will traverse blocks that have text or nodes to evaluate using arrow keys, but the content in the block's viewport does not change. Screen readers can also traverse scrollable blocks like a keyboard user (content in viewport changes on arrow key press) by switching into Forms/Focus mode. Testing this was important to me because users with some vision loss or in low light situations may lean into a screen reader to navigate pages and still want to follow along visually. - It took me a long time to reliably figure out why NVDA and JAWS were behaving erratically. The
tl;drfor JAWS is to toggle PC virtual cursor by press ingIns + Z. When PC virtual cursor is off, you traverse exactly like a keyboard user, and our non-standard controls like DnD are usable. HP laptops have a feature to override the Functions key and disable volume/brightness/etc. by pressingEsc + Func. Then the NVDA shortcuts work correctly. I'll write these up because the answers were found obliquely in StackOverflow and Reddit answers to other questions.
- there's a whole lotta extra divs here doing a whole lotta nothin - the `aria-live="polite"` region IMO adds nothing to SR experience (our drag/drop components already announce status updates on drop), so I removed it
a scrollable region does not strictly need `role="region"`, it just needs `tabIndex={0}`
in this case our usage in our docs is also wrong because we're using it within a `<section>` for 2 reasons (IMO) - they're not significant enough to be landmarks, and a landmark within a landmark is always weird
cee-chen
force-pushed
the
more-region-cleanup
branch
from
ee1e90f
to
d077ef9
Compare
|
Preview staging links for this PR:
|
💚 Build Succeeded
History
|
Closed
jbudz
pushed a commit
to elastic/kibana
that referenced
this pull request
Dec 18, 2023
`v90.0.0`⏩`v91.0.0-backport.0`⚠️ While this upgrade pings many teams and has a large code diff, **the majority of the changes are snapshots or tests-related** and do not touch source code, so should theoretically only need a code review and not dedicated QA. The changes in EUI that required a large swathe of these updates are: - **EuiPopover** removed an extra unnecessary `<div>` wrapper on its anchors, which affected many snapshots and a few CSS overrides, which should have been updated - **EuiButtonGroup** now renders `<button>` elements instead of `<input type="radio">` elements for single selection, which affected both snapshots and E2E tests - **EuiSuperDatePicker**'s absolute date input now requires an `Enter` keypress when parsing dates (affected E2E tests) - **EuiComboBox**, when rendered with `singleSelection={{ plainText: 'true' }}`, no longer renders a pill (i.e. text). This combobox type now behaves more like an `EuiFieldText`, where the selection is rendered via input `value` instead. This affected a high amount of E2E tests (both FTR and Cypress), both in terms of updating assertions and changing selections, but should **not** significantly affect user experience - see elastic/eui#7332 for more. --- ## [`v91.0.0-backport.0`](https://github.com/elastic/eui/tree/v91.0.0-backport.0) **This is a backport release only intended for use by Kibana.** - Added `esqlVis`, `pipeBreaks`, and `pipeNoBreaks` icon glyphs. - `EuiSelectable` now allows configurable text truncation via `listProps.truncationProps` ([#7388](elastic/eui#7388)) - `EuiTextTruncate` now supports a new `calculationDelayMs` prop for working around font loading or layout shifting scenarios ([#7388](elastic/eui#7388)) **Bug fixes** - Fixed a bug with `EuiSelectable`s with custom `truncationProps`, where scrollbar widths were not being accounted for ([#7392](elastic/eui#7392)) ## [`91.0.0`](https://github.com/elastic/eui/tree/v91.0.0) - Updated the background color of `EuiPopover`s in dark mode to increase visibility & contrast against other page/panel backgrounds ([#7310](elastic/eui#7310)) - Memoized `EuiDataGrid` to prevent unneeded re-renders ([#7324](elastic/eui#7324)) - Added a configurable `role` prop to `EuiAccordion` ([#7326](elastic/eui#7326)) - Added a configurable `role` prop to `EuiGlobalToastList` ([#7328](elastic/eui#7328)) - For greater flexibility, `EuiSuperDatePicker` now allows users to paste ISO 8601, RFC 2822, and Unix timestamps in the `Absolute` tab input, in addition to timestamps in the `dateFormat` prop ([#7331](elastic/eui#7331)) - Plain text `EuiComboBox`es now behave more like a normal text field/input. Backspacing will no longer delete the entire value, and selected values can now be double clicked and copied. ([#7332](elastic/eui#7332)) - `EuiDataGrid`'s display settings popover now allows users to clear the "Lines per row" input before typing in a new number ([#7338](elastic/eui#7338)) - Improved the UX of `EuiSuperDatePicker`'s Absolute tab for users manually typing in timestamps ([#7341](elastic/eui#7341)) - Updated `EuiI18n`s with multiple `tokens` to accept dynamic `values` ([#7341](elastic/eui#7341)) **Bug fixes** - Fixed `EuiComboBox`'s `onSearchChange` callback to pass the correct `hasMatchingOptions` value ([#7334](elastic/eui#7334)) - Fixed an `EuiSelectableTemplateSitewide` bug where the `popoverButton` behavior would break if passed a non-DOM React wrapper ([#7339](elastic/eui#7339)) **Deprecations** - `EuiPopover`: deprecated `anchorClassName`. Use `className` instead ([#7311](elastic/eui#7311)) - `EuiPopover`: deprecated `buttonRef`. Use `popoverRef` instead ([#7311](elastic/eui#7311)) - `EuiPopover`: removed extra `.euiPopover__anchor` div wrapper. Target `.euiPopover` instead if necessary ([#7311](elastic/eui#7311)) - Deprecated `EuiButtonGroup`'s `name` prop. This can safely be removed. ([#7325](elastic/eui#7325)) **Breaking changes** - Removed deprecated `euiPaletteComplimentary` - use `euiPaletteComplementary` Instead ([#7333](elastic/eui#7333)) **Accessibility** - Updated `type="single"` `EuiButtonGroup`s to render standard buttons instead of radio buttons under the hood, per recent a11y recommendations ([#7325](elastic/eui#7325)) - `EuiAccordion` now defaults to a less screenreader-noisy `group` role instead of `region`. If your accordion contains significant enough content to be a document landmark role, you may re-configure it back to `region`. ([#7326](elastic/eui#7326)) - Reduced screen reader noisiness when sorting `EuiDataGrid` columns via toolbar ([#7327](elastic/eui#7327)) - `EuiGlobalToastList` now defaults to a `log` role. If your toasts will always require immediate user action, consider (with caution) using the `alert` role instead. ([#7328](elastic/eui#7328)) **CSS-in-JS conversions** - Updated `$euiFontFamily` and `$euiCodeFontFamily` to match Emotion fonts ([#7332](elastic/eui#7332)) --------- Co-authored-by: Cee Chen <[email protected]> Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Cee Chen <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
See #7316 for more discussion and sources on why not to overuse
role="region". Also see individual git commit messages for more rationale on specific changes.Note: most of the line diffs in this PR come from indentation, so please turn off whitespace changes.
QA
Sort fieldsbutton in the datagrid toolbarGeneral checklist
and cypress tests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)