[Feature Branch] EuiSelectable accessibility #3169

myasonik · 2020-03-25T21:34:11Z

Closes #1789 and closes #2200

PRs

[Feature / Selectable A11y] Improve basic use case a11y #3070
- swap react-virtualize for react-window and react-virtualized-auto-sizer
- improve basic use case a11y
- update prop names on EuiSelectableOptionsList (breaking change 1)
[Feature / Selectable A11y] making EuiSelectable[searchable] more accessible #3234
- improves accessibility of searchable use case
- update prop names on EuiSelectableSearch and EuiSelectableList (breaking change 2 and 3)
Remove react-virtualized from ComboBox #3173
- swap react-virtualize for react-window in EuiComboBox to completetly remove react-virtualized
Misc cleanup after resurecting #3570
- resurrection! (Fixed some bugs, tweaked some docs, added it to a11y tests)
More aria attributes for Selectable #3583
- Adds aria-setsize and aria-posinset attributes to options
- Adds aria-describedby for messages
- Adds setActiveOptionIndex prop to EuiSelectableList (breaking change 4)
[EuiSelectable] Adding more keyboard shortcuts and screen reader instructions #3620
- Adds Home, End and Space shortcuts
- Adds aria-label instructions for exclusions
[EuiSelectable] Focus option after search value change #3670
- Fixed bugs with setting focus

Breaking changes

virtualizedProps on EuiSelectableOptionsList renamed to windowProps which is now passed into react-window instead of react-virtualized
Removed rootId and added makeOptionId and listId to EuiSelectableList
Added listId to EuiSelectableSearch
Added setActiveOptionIndex to EuiSelectableList
options passed into EuiSelectable cannot have an id
Requires an onChange to be passed into EuiSelectableSearch

Future TODOs

Support other Selectable-ish uses (e.g., combobox, filtergroup, maybe super select) (can either done by extending EuiSelectable or refactoring the other components to use Selectable under the hood)
Handle being wrapped in EuiFormRow ([EuiFormRow] With difficult components #3218)
Improve exclusion accessibility ([EuiSelectable] Discoverability of exclusions #3625)
Improve mobile focus support
Polish screen reader experience

Checklist

Check against all themes for compatibility in both light and dark modes
- Just checked focus states, don't think there's much more to check
Checked in mobile
- Has some oddities with focus states, but everything works
Checked in Firefox, Safari, Chrome/Edge
Props have proper autodocs
~~Added documentation examples~~
- Nothing changed from the consumer side other than props which the autodocs take care of
Added or updated jest tests
- Updated tests to pass (mostly prop changes) though didn't add new ones. (Granted, tests were/are pretty weak right now.)
Checked for breaking changes and labeled appropriately
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

* swap react-virtualize for react-window and react-virtualized-auto-sizer * improve basic use case a11y * update prop names

…e-a11y

kibanamachine · 2020-03-25T21:54:08Z

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

kibanamachine · 2020-03-25T22:27:26Z

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

…e-a11y

kibanamachine · 2020-04-01T02:59:07Z

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

…essible (#3234) * Moves many aria-* props from the List<ul> and onto the <input /> or surrounding <div> * [Breaking] EuiSelectableSearch has 2 new required props and EuiSelectableList has 1

kibanamachine · 2020-04-15T20:22:12Z

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

kibanamachine · 2020-05-12T18:36:30Z

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

…e-a11y

kibanamachine · 2020-06-05T15:55:48Z

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

kibanamachine · 2020-06-09T17:20:34Z

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

Co-authored-by: Greg Thompson <[email protected]>

kibanamachine · 2020-06-15T18:56:30Z

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

…ructions (#3620)

kibanamachine · 2020-06-18T19:51:45Z

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

…e-a11y

kibanamachine · 2020-06-18T21:34:45Z

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

cchaos

Keyboard shortcuts are awesome! With all the work that has gone into the EuiSelectable component itself, is the hope that we can just re-use this in EuiComboBox instead of re-doing all the work in there? I do see you made some edits to combobox but it doesn't have the full features like the keyboard shortcuts.

The only thing I felt odd in the functionality was when the list was filtered down from the search box, and an option is selected it loses focus where as it would normally stay focused.

package.json

CHANGELOG.md

cchaos · 2020-06-26T20:07:37Z

To comment on of your todos:

New rendering type (popover) to support search use case and to begin to bridge gap into supporting combobox/selectable/etc

I'd really rather not tie this component to other specific components. I like the flexibility of being able to place the list and the search box where they need to be. We can absolutely ensure that our examples work appropriately with screen-readers, but I don't think it should be part of the component.

Co-authored-by: Caroline Horn <[email protected]>

kibanamachine · 2020-06-29T17:09:42Z

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

myasonik · 2020-06-29T17:13:46Z

@cchaos I rephrased that TODO to be vaguer. We can either bring the combobox/filtergroup/etc patterns into Selectable or rework those components to use Selectable under the hood. Either way works, I just wrote down my preference/what I thought would be less work.

I like the flexibility of being able to place the list and the search box where they need to be.

Is this something we have today? Because that's not something I see on the demo page and isn't something I've tested.

cchaos · 2020-06-29T18:20:21Z

Is this something we have today?

Kind of but not quite. It's more that they both live in a popover. Or like the Flyout example has the input in the header and the list in the body. But this is how I setup the search in the K8 POC (need to get it back up and running) but you can see the code here: https://github.com/cchaos/kibana-8-nav/blob/master/src/components/kibana/chrome/search/search.tsx

myasonik · 2020-06-29T20:09:15Z

Ah... I didn't think through the possibility of passing in not the list in there... I haven't tested that but I think it should "just work"... If it doesn't, it shouldn't be much work to fix, and shouldn't be a breaking change either, so I'd still like to merge this as is as it's already quite large and getting old.

Also, a note for anyone doing any testing on this, the support is a little mixed on Mac so I defaulted to the best practices/spec suggestions often. Searchable Selectables work best in VO+Safari, but list Selectables work best in VO+Chrome. This is another one of those components I'll want to test in Windows at some point soon before doing more work on polishing.

cchaos · 2020-06-29T20:19:48Z

Yep, we can followup with that particular use case which will most likely be on the consuming side (pattern). Can you update the checklist with items you've completed?

myasonik · 2020-06-30T22:25:13Z

Just wanted to talk about the screen reader experience a little more because listboxes and comboboxes (what Selectable is called in ARIA) are tricky components.

Testing this PR in VoiceOver may seem like it yields some mixed results:

Safari and Chromium experiences diverge
Tabbing does less in this PR than what's on the main branch

The reason for both of those is because it more closely aligns to what's in the aria spec. The spec enables more screen reader magic which is intended to make a complex component more approachable and standardized (though it does come with the potential downside of another layer where bugs may show up and being less approachable to those with less screen reader experience).

Because we have limited capacity to do real world testing, I think we should be more closely aligning to the spec. When I have time, I have it on my roadmap to come back to this component to do further Windows testing but I think that's out of scope for this PR. (With more testing, we can polish the experience past "this meets spec expectations" to "this works in user environments.")

My full testing notes comparing what's in EUI today and this PR

EuiSelectable today

In VO+Chrome/Edge:
- In the list version:
  - Announces you're on a listbox
  - Tabbing does not read items, shift+tabbing does
  - Arrowing does nothing, need to shift+tab+tab to start moving again
  - No way to tab into virtualized options (tabs out of the list)
  - Options always appear not selected
- In the searchable version:
  - In the input field, it doesn't announce you're in a combobox or that there are options
  - Arrowing does nothing
  - No way to tab into virtualized options (tabs out of the list)
  - Options always appear not selected
  - Tabbing doesn't read items, shift+tabbing does
VO+Safari is mostly similar but does not announce how many options are either

EuiSelectable after this PR

In VO+Chrome/Edge:
- In the list version:
  - Arrows work as expected (Reads " <is/is not selected> ")
  - Tabbing works as expected (one tab stop for the whole list, the list is not an actual tab stop, the first selected - or first if none selected - option is)
  - Selecting/deselcting is announced
- In the searchable version:
  - Tab stop on input announced you're in a combobox and announced VO instructions to read options
  - Can navigate the list using cmd+option+arrow and cmd+option+space to select/deselect but arrow keys are silent (unsure what's expected here from the arrow keys, but VO controls still working is at least good). Using VO navigation and selection/deselection, everything seems to be announced as expected.
In VO+Safari:
- In the list version:
  - Works similarly to the searchable version of VO+Chrome/Edge (i.e., you need to use VO's controls, not our controls, but using VO's controls everything works)
- In the searchable version:
  - Works similarly to the VO+Chrome/Edge though using arrow keys will activate VO controls
  - Selection seems confusing because selection doesn't always seem to activate the one I just read but the last one I interacted with so I'm not sure if I'm missing a VO command I need to learn or if something is broken.

kibanamachine · 2020-07-06T16:35:25Z

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

myasonik · 2020-07-06T16:54:49Z

@cchaos @chandlerprall This is ready for a final review again!

kibanamachine · 2020-07-06T17:18:18Z

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

chandlerprall

Code changes look good. Tested selectable's example in Chrome, FF, Safari, and Android Chrome without issue.

elizabetdev

Tested locally and reviewed the code. LGTM!

src/components/selectable/selectable_list/_selectable_list.scss

kibanamachine · 2020-07-07T19:40:34Z

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

[Feature / Selectable A11y] Improve basic use case a11y (#3070)

fdc9a6b

* swap react-virtualize for react-window and react-virtualized-auto-sizer * improve basic use case a11y * update prop names

myasonik changed the title ~~[Feature / Selectable A11y] Improve basic use case a11y (#3070)~~ [Feature Branch] EuiSelectable Mar 25, 2020

Merge remote-tracking branch 'upstream/master' into feature/selectabl…

d90176e

…e-a11y

Merge remote-tracking branch 'upstream/master' into feature/selectabl…

eb62d10

…e-a11y

cchaos changed the title ~~[Feature Branch] EuiSelectable~~ [Feature Branch] EuiSelectable acessibility Apr 3, 2020

cchaos changed the title ~~[Feature Branch] EuiSelectable acessibility~~ [Feature Branch] EuiSelectable accessibility Apr 3, 2020

thompsongl added the breaking change label Apr 15, 2020

[Feature / Selectable A11y] making EuiSelectable[searchable] more acc…

a0ef625

…essible (#3234) * Moves many aria-* props from the List<ul> and onto the <input /> or surrounding <div> * [Breaking] EuiSelectableSearch has 2 new required props and EuiSelectableList has 1

myasonik added the accessibility label Apr 15, 2020

Remove react-virtualized from ComboBox (#3173)

5860e96

Merge remote-tracking branch 'upstream/master' into feature/selectabl…

7cc0c2c

…e-a11y

myasonik mentioned this pull request Jun 5, 2020

Misc cleanup after resurecting #3570

Merged

4 tasks

Misc cleanup after resurecting (#3570)

3eb02e4

More aria attributes for Selectable (#3583)

0e168f2

Co-authored-by: Greg Thompson <[email protected]>

[EuiSelectable] Adding more keyboard shortcuts and screen reader inst…

a6ba2b5

…ructions (#3620)

Merge remote-tracking branch 'upstream/master' into feature/selectabl…

81e9136

…e-a11y

myasonik marked this pull request as ready for review June 18, 2020 21:05

changelog update

b8ba6e8

cchaos reviewed Jun 26, 2020

View reviewed changes

package.json Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

Rephrasing changelog

e9b8b21

Co-authored-by: Caroline Horn <[email protected]>

cchaos requested a review from chandlerprall June 29, 2020 18:21

myasonik mentioned this pull request Jun 29, 2020

[EuiSelectable] Focus option after search value change #3670

Merged

[EuiSelectable] Focus option after search value change (#3670)

2431824

myasonik requested a review from cchaos July 6, 2020 16:48

Michail Yasonik added 2 commits July 6, 2020 12:50

Merge branch 'master' into feature/selectable-a11y

c3b4d71

Updating changelog with new breaking changes

75dd3a4

chandlerprall requested a review from elizabetdev July 7, 2020 16:20

chandlerprall approved these changes Jul 7, 2020

View reviewed changes

elizabetdev approved these changes Jul 7, 2020

View reviewed changes

src/components/selectable/selectable_list/_selectable_list.scss Outdated Show resolved Hide resolved

fixing focus state for list

913cf3e

chandlerprall mentioned this pull request Jul 7, 2020

EuiResizableContainer having nested instances (vertical and horizontal) doesn't work #3698

Closed

myasonik merged commit 296e917 into master Jul 7, 2020

myasonik deleted the feature/selectable-a11y branch July 7, 2020 19:54

myasonik mentioned this pull request Jul 22, 2020

[EuiSelectable+EuiComboBox] recent regresion fixes #3784

Merged

5 tasks

thompsongl mentioned this pull request Jul 23, 2020

Upgrade EUI to v27.2.0 elastic/kibana#72993

Closed

thompsongl mentioned this pull request Jul 31, 2020

Upgrade EUI to v27.4.0 elastic/kibana#74004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature Branch] EuiSelectable accessibility #3169

[Feature Branch] EuiSelectable accessibility #3169

myasonik commented Mar 25, 2020 •

edited

Loading

kibanamachine commented Mar 25, 2020

kibanamachine commented Mar 25, 2020

kibanamachine commented Apr 1, 2020

kibanamachine commented Apr 15, 2020

kibanamachine commented May 12, 2020

kibanamachine commented Jun 5, 2020

kibanamachine commented Jun 9, 2020

kibanamachine commented Jun 15, 2020

kibanamachine commented Jun 18, 2020

kibanamachine commented Jun 18, 2020

cchaos left a comment

cchaos commented Jun 26, 2020

kibanamachine commented Jun 29, 2020

myasonik commented Jun 29, 2020 •

edited

Loading

cchaos commented Jun 29, 2020

myasonik commented Jun 29, 2020

cchaos commented Jun 29, 2020

myasonik commented Jun 30, 2020

kibanamachine commented Jul 6, 2020

myasonik commented Jul 6, 2020

kibanamachine commented Jul 6, 2020

chandlerprall left a comment

elizabetdev left a comment

kibanamachine commented Jul 7, 2020

[Feature Branch] EuiSelectable accessibility #3169

[Feature Branch] EuiSelectable accessibility #3169

Conversation

myasonik commented Mar 25, 2020 • edited Loading

PRs

Breaking changes

Future TODOs

Checklist

kibanamachine commented Mar 25, 2020

kibanamachine commented Mar 25, 2020

kibanamachine commented Apr 1, 2020

kibanamachine commented Apr 15, 2020

kibanamachine commented May 12, 2020

kibanamachine commented Jun 5, 2020

kibanamachine commented Jun 9, 2020

kibanamachine commented Jun 15, 2020

kibanamachine commented Jun 18, 2020

kibanamachine commented Jun 18, 2020

cchaos left a comment

Choose a reason for hiding this comment

cchaos commented Jun 26, 2020

kibanamachine commented Jun 29, 2020

myasonik commented Jun 29, 2020 • edited Loading

cchaos commented Jun 29, 2020

myasonik commented Jun 29, 2020

cchaos commented Jun 29, 2020

myasonik commented Jun 30, 2020

kibanamachine commented Jul 6, 2020

myasonik commented Jul 6, 2020

kibanamachine commented Jul 6, 2020

chandlerprall left a comment

Choose a reason for hiding this comment

elizabetdev left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 7, 2020

myasonik commented Mar 25, 2020 •

edited

Loading

myasonik commented Jun 29, 2020 •

edited

Loading