[EuiSelectable] List enhancements and a11y #5581

thompsongl · 2022-01-31T19:58:19Z

Summary

Supporting changes required in #5157 and #5387 related to EuiSelectable and its subcomponents

Adds paddingSize option to EuiSelectable to allow for "flush" list items (to be used by EuiSuggest)
Adds errorMessage option to EuiSelectable ([EuiFilterGroup][FieldValueSelectionFilter] Use EuiSelectable #5387 (comment))
Improved screen reader announcements for list interactions
Uses aria-checked instead of aria-selected for active items

Checklist

kibanamachine · 2022-01-31T20:42:27Z

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

kibanamachine · 2022-01-31T22:47:20Z

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

1Copenut

LGTM. I tested with Axe, keyboard navigation and JAWS. The screen reader experience felt good and it announced changes as expected.

cee-chen · 2022-02-01T21:15:19Z

src/components/selectable/selectable.tsx

      case keys.ENTER:
+      case keys.SPACE:
+        if (event.key === keys.SPACE && this.props.searchable) return;


I still think this needs reworking to change the event area of this keyDown handler to the list only and not include the search box. That way we don't need this this.props.searchable check and the spacebar can always toggle options on/off. Thoughts?

No keyboard events actually derive from the list. All keyboard interaction, including that which appears to be on the list, is handled from the search box. Focus, etc. are all faux states on the list items, and you'll notice that the cursor remains in the search input at all times.

space is the more standard select/deselect key (according to Zuhair #5157 (comment)) which is why it was added. This does present challenges to searchable instances because space is also a valid character. For this case, we've added the SR announcement that enter will toggle selections, which is less standard but gives enough information to hopefully prevent using space for selection purposes.

All keyboard interaction, including that which appears to be on the list, is handled from the search box.

That doesn't have to be the case however. The onKeyDown handler is being passed to the container div when it (or a separate handler) could easily be passed only to the list instead.

I agree that space is a more standard select/deselect key. I want it to work period over the selectable list even when a searchbox is present. I'll provide a diff/commit POC to cherry pick, I strongly feel this is a keyboard/accessibility enhancement we should provide while we're here

Here's my brief spike: thompsongl/eui@3733-selectable...constancecchen:3733-selectable

It works well with for keyboard usage but I'm not 100% that the screen reader experience is 1:1. I plan on picking Trevor's brain on this tomorrow.

After rereading the 1.2 spec, I'm thinking that we might be better served to remove space selection and continue with enter as the selection mechanism.
As is, we conform to the spec for both searchbox and list interaction when using arrow and enter, and space is never mentioned. Similarly, the react-aria implementation confirms the current approach.

If we change to something along the lines of your spike, we'd probably need to remove faux active option state because the indicators no longer match the keyboard expectation. Given the following:

enter makes no selection

space makes no selection and results in an empty list (because no options start with empty space)

down moves the active option indicator to Titan instead of Dione

cc @1Copenut for validation. We can sync if it's easier to discuss in real time.

After our sync call, we've decided to leave potentially splitting up EuiSelectable (listbox) and EuiSearchableSelectable (combobox) as an internal tech debt refactor for another day. In the meanwhile we'll just need to leave a ton of inline code comments+links to the WAI-ARIA specs/examples so that we don't end up having this exact convo again in another 4 months

For the spacebar UX, I also suggested changing the "Return/Enter" icon on the right of the EuiSelectable item to the word "Space" for when the component is a listbox vs a combobox, to give keyboard sighted users a visual cue that the toggle button is different when the searchable prop is present.

For the spacebar UX, I also suggested changing the "Return/Enter" icon on the right of the EuiSelectable item to the word "Space" for when the component is a listbox vs a combobox

After some thought, I think we should open an issue to create a new icon for spaceKey and use that icon when available.

Makes sense. Do we have the option to put a title prop on that icon to indicate the words "Enter" and "Space"? 🤔 So could we use the empty icon for now, or maybe the minus icon with a transform on it?

Although thinking that though, keyboard users likely won't be able to mouseover to see the title tooltip in any case... so... that was probably a dumb idea 💀

I'm going to open an issue for this so we can get designer input as well. The returnKey icon is still valid for the listbox case and any enhancement there isn't relevant for the EuiSuggest PR.

src/components/selectable/selectable.tsx

kibanamachine · 2022-02-01T21:55:31Z

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

cee-chen · 2022-02-01T22:35:00Z

@1Copenut VO+Mac isn't reading out any notifications for me - either the visual text or "0 results" - when a search filter box returns no results:

Can you repro? Is this intended behavior?

1Copenut · 2022-02-01T22:54:12Z

@constancecchen & @thompsongl

VO+Mac isn't reading out any notifications for me - either the visual text or "0 results" - when a search filter box returns no results:

Can you repro? Is this intended behavior?

I can reproduce the issue. Watching the DOM update in Chrome, it looks like our EuiScreenReaderOnly component is being removed when a status message is added. We will want that to be announced. Constance has an idea that we can discuss tomorrow.

kibanamachine · 2022-02-02T16:35:41Z

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

thompsongl · 2022-02-02T18:57:30Z

VO+Mac isn't reading out any notifications for me - either the visual text or "0 results"

Resolved in 5ef8ab4, which will also aid loading and error states.

kibanamachine · 2022-02-02T19:32:27Z

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

kibanamachine · 2022-02-02T20:11:39Z

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

kibanamachine · 2022-02-02T21:47:04Z

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

kibanamachine · 2022-02-03T17:42:52Z

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

cee-chen · 2022-02-04T05:37:46Z

Apologies, had a really gnarly day with the Typescript PR, will look at this first thing Friday morning!

1Copenut · 2022-02-04T16:25:54Z

@thompsongl & @constancecchen I retested with VO on Safari latest this morning. The error message and no results found message are announcing reliably and the refactor has improved the experience for single select too. Much appreciated!

kibanamachine · 2022-02-04T17:01:10Z

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

src/components/selectable/selectable.tsx

src/components/selectable/selectable_list/selectable_list_item.tsx

kibanamachine · 2022-02-04T19:30:52Z

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

src/components/selectable/selectable_list/selectable_list_item.tsx

….tsx Co-authored-by: Constance <constancecchen@users.noreply.github.com>

cee-chen

All of my remaining comments are super optional! Thanks so much for your patience with me on this Greg!

kibanamachine · 2022-02-04T21:31:34Z

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

thompsongl added 4 commits January 31, 2022 13:52

euiselectablelistiem

3a2ef10

euiselectablelist

ba0611c

euiselectable

1b92402

CL

c158a3d

thompsongl mentioned this pull request Jan 31, 2022

[EuiSuggest] Use EuiSelectable #5157

Merged

9 tasks

thompsongl added 3 commits January 31, 2022 15:56

errorMessage

2485181

more docs

76ec098

CL

7dbb597

thompsongl requested review from cee-chen and 1Copenut January 31, 2022 23:03

thompsongl marked this pull request as ready for review January 31, 2022 23:04

1Copenut approved these changes Feb 1, 2022

View reviewed changes

Merge branch 'main' into 3733-selectable

47163a6

cee-chen reviewed Feb 1, 2022

View reviewed changes

src/components/selectable/selectable.tsx Show resolved Hide resolved

Merge branch 'main' into 3733-selectable

8532931

thompsongl added 2 commits February 2, 2022 12:47

selection method comment

f1bb538

fix empty results announcement

5ef8ab4

add spec comment

3d101f3

thompsongl requested a review from cee-chen February 2, 2022 19:33

Merge branch 'main' into 3733-selectable

75a9fde

Merge branch 'main' into 3733-selectable

397f741

thompsongl mentioned this pull request Feb 3, 2022

[EuiSelectable] Use space icon in non-searchable instnces #5598

Closed

Merge branch 'main' into 3733-selectable

33618aa

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable.tsx Show resolved Hide resolved

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable_list/selectable_list_item.tsx Outdated Show resolved Hide resolved

don't reset message id

1064cd5

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable_list/selectable_list_item.tsx Outdated Show resolved Hide resolved

thompsongl added 3 commits February 4, 2022 12:54

remove extra spans

fb7c4a5

more searchable logic

0b59874

more searchable comments

b227c72

This comment was marked as resolved.

Sign in to view

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable_list/selectable_list_item.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable_list/selectable_list_item.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Feb 4, 2022

View reviewed changes

src/components/selectable/selectable_list/selectable_list_item.tsx Show resolved Hide resolved

thompsongl and others added 2 commits February 4, 2022 14:16

Update src/components/selectable/selectable_list/selectable_list_item…

f22cd79

….tsx Co-authored-by: Constance <constancecchen@users.noreply.github.com>

clean up

d4c3194

cee-chen approved these changes Feb 4, 2022

View reviewed changes

add role comment

da945c0

thompsongl merged commit 27681eb into elastic:main Feb 4, 2022

breehall mentioned this pull request Feb 8, 2022

Upgrade EUI to v48.1.1 elastic/kibana#125023

Merged

cchaos mentioned this pull request Mar 2, 2022

[EuiSelectableListItem] Revert layout changes and add textWrap option #5679

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiSelectable] List enhancements and a11y #5581

[EuiSelectable] List enhancements and a11y #5581

thompsongl commented Jan 31, 2022 •

edited

Loading

kibanamachine commented Jan 31, 2022

kibanamachine commented Jan 31, 2022

1Copenut left a comment

cee-chen Feb 1, 2022 •

edited

Loading

thompsongl Feb 1, 2022 •

edited

Loading

cee-chen Feb 1, 2022

cee-chen Feb 1, 2022 •

edited

Loading

thompsongl Feb 2, 2022 •

edited

Loading

cee-chen Feb 2, 2022 •

edited

Loading

thompsongl Feb 2, 2022

cee-chen Feb 2, 2022 •

edited

Loading

thompsongl Feb 3, 2022

thompsongl Feb 3, 2022

kibanamachine commented Feb 1, 2022

cee-chen commented Feb 1, 2022

1Copenut commented Feb 1, 2022 •

edited

Loading

kibanamachine commented Feb 2, 2022

thompsongl commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 3, 2022

cee-chen commented Feb 4, 2022

1Copenut commented Feb 4, 2022

kibanamachine commented Feb 4, 2022

This comment was marked as resolved.

kibanamachine commented Feb 4, 2022

cee-chen left a comment

kibanamachine commented Feb 4, 2022

[EuiSelectable] List enhancements and a11y #5581

[EuiSelectable] List enhancements and a11y #5581

Conversation

thompsongl commented Jan 31, 2022 • edited Loading

Summary

Checklist

kibanamachine commented Jan 31, 2022

kibanamachine commented Jan 31, 2022

1Copenut left a comment

Choose a reason for hiding this comment

cee-chen Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

thompsongl Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

cee-chen Feb 1, 2022

Choose a reason for hiding this comment

cee-chen Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

thompsongl Feb 2, 2022 • edited Loading

Choose a reason for hiding this comment

cee-chen Feb 2, 2022 • edited Loading

Choose a reason for hiding this comment

thompsongl Feb 2, 2022

Choose a reason for hiding this comment

cee-chen Feb 2, 2022 • edited Loading

Choose a reason for hiding this comment

thompsongl Feb 3, 2022

Choose a reason for hiding this comment

thompsongl Feb 3, 2022

Choose a reason for hiding this comment

kibanamachine commented Feb 1, 2022

cee-chen commented Feb 1, 2022

1Copenut commented Feb 1, 2022 • edited Loading

kibanamachine commented Feb 2, 2022

thompsongl commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 2, 2022

kibanamachine commented Feb 3, 2022

cee-chen commented Feb 4, 2022

1Copenut commented Feb 4, 2022

kibanamachine commented Feb 4, 2022

This comment was marked as resolved.

kibanamachine commented Feb 4, 2022

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 4, 2022

thompsongl commented Jan 31, 2022 •

edited

Loading

cee-chen Feb 1, 2022 •

edited

Loading

thompsongl Feb 1, 2022 •

edited

Loading

cee-chen Feb 1, 2022 •

edited

Loading

thompsongl Feb 2, 2022 •

edited

Loading

cee-chen Feb 2, 2022 •

edited

Loading

cee-chen Feb 2, 2022 •

edited

Loading

1Copenut commented Feb 1, 2022 •

edited

Loading