[EuiSelectable] Add a mixed state to `EuiSelectableListItem` that renders a new icon and SR-only text by 1Copenut · Pull Request #6774 · elastic/eui

1Copenut · 2023-05-12T18:07:35Z

Summary

EUI team was requested to add a "mixed" state to the EuiSelectable component so teams could communicate a "selected for some" status for the options object passed into the component. This mixed state is not selectable by users, but can be updated to checked, excluded, or unchecked entirely.

I left three granular commits in the history instead of squashing, so reviewers can follow along (if so inclined) to see how I arrived at this solution. I took a long look at the MDN guidance for aria-checked to inform the screen reader text.

This PR closes #6771.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

1Copenut · 2023-05-12T18:15:16Z

I need to add the documentation updates and finish testing across browsers. Will do that before EOD today and hopefully move this out of draft.

1Copenut · 2023-05-12T21:59:45Z

jenkins test this

src/components/selectable/selectable_list/selectable_list_item.tsx

src/components/selectable/selectable_option.tsx

cee-chen

minor copy suggestions

src/components/selectable/selectable_option.tsx

src/components/selectable/selectable_list/selectable_list_item.tsx

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

src/components/selectable/selectable_list/__snapshots__/selectable_list_item.test.tsx.snap

src/components/selectable/selectable.a11y.tsx

src/components/search_bar/filters/field_value_selection_filter.tsx

src/components/selectable/selectable_list/selectable_list_item.tsx

src-docs/src/views/selectable/selectable_example.js

kibanamachine · 2023-05-16T21:49:23Z

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

ryankeairns

LGTM
Small suggestion - we could add a button to reset the example. Its just as easy to reload the page, but might be nice as a reader clicks on items, checking states, and trying to understand the point... as I just did :)

upcoming_changelogs/6774.md

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

src/components/selectable/selectable_list/selectable_list_item.tsx

kibanamachine · 2023-05-17T18:13:12Z

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

cee-chen · 2023-05-17T18:49:45Z

src/components/selectable/selectable_list/selectable_list_item.tsx

+            <EuiI18n
+              token="euiSelectableListItem.includedOption"
+              default="Checked option."
+            />


Suggested change

<EuiI18n

token="euiSelectableListItem.includedOption"

default="Checked option."

/>

<EuiI18n

token="euiSelectableListItem.checkedOption"

default="Checked option."

/>

Since we're making the string the exact same as line 224 down below we should just go ahead and make these the same i18n token/string. Our i18n translators skip having to re-translate the same string that way.

Actually, we're repeating a bunch of i18n strings across the board down below. I'd prefer to dry those out into const's I think... any objections if I push up a clean up?

I refactored this in 7ee93d4 and found several bugs/missing or confusing SR text that was fixed as a result of cleaning up the branches (namely, mixed and non allowExclusions options did not have the correct SR text).

I think this should be fixed now - @1Copenut, do you want to do a final pass on all EuiSelectable

The permutations that generate different screen reader text that need to be tested:

Mixed

Excluded

Searchable

Vanilla

+ `aria-disabled` and `aria-checked` is valid - do not return undefined if disabled + `aria-checked` mixed is invalid for certain roles - return false if so

- add describe block tests for `role` and `aria-checked` - add describe blocks for `checked` - switch to RTL render

- DRY out repeated i18n tokens/strings - Make branching easier to follow (hopefully...) - Fix some incorrect branching logic leading to missing SR text (namely non-exclusion mixed) - Improve SR reading by using a period instead of a hyphen (at least for FF+VO) - Add more unit tests to snapshot branching output + rename various vars to be slightly clearer

- Remove `allowExclusions` from example - while they work together, it's unlikely consumers will use both, and we should focus on just mixed options for this demo to isolate concerns - Document when consumers would want to use this state - Add button that allows resetting mixed state display - Fix snippet

cee-chen

This LGTM at this point from a code & docs POV, but the screen reader text may need more robust testing. Feel free to tweak the branching if you find any issues with it!

kibanamachine · 2023-05-18T06:17:00Z

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

src-docs/src/views/selectable/selectable_example.js

kibanamachine · 2023-05-18T15:33:02Z

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

1Copenut · 2023-05-18T20:40:01Z

I passed through the recommended states using four screen reader combinations:

Safari + VO (MacOS)
Firefox + VO (MacOS)
Chrome + NVDA (Win10)
Firefox + NVDA (Win10)

All performed as expected with no regression that I could tell from previously coded version. Moving this to close after merging.

## Summary `@elastic/eui@80.0.0` ⏩ `@elastic/eui@81.0.0` --- ## [`81.0.0`](https://github.com/elastic/eui/tree/v81.0.0) - Added ability to set `options.checked` to "mixed" in `EuiSelectable` ([#6774](elastic/eui#6774)) **Bug fixes** - Portalled components (e.g. `EuiPopover`, `EuiModal`, `EuiFlyout`) will correctly inherit text color from its nearest `EuiThemeProvider` parent. `<EuiText color="default">` is no longer needed. ([#6775](elastic/eui#6775)) **Breaking changes** - `EuiSelectable` no longer renders a `data-test-selected` attribute on its list items. Use the `aria-checked` property instead ([#6774](elastic/eui#6774)) - Nested `EuiThemeProvider`s now render a wrapping `<span>` element in order to correctly set the inherited text `color` of all descendants. `<EuiText color="default">` is no longer needed. ([#6775](elastic/eui#6775)) --------- Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Constance Chen <constance.chen@elastic.co>

1Copenut added 3 commits May 12, 2023 13:01

Swapped icon assignment flow to use switch/case.

3b89959

Added logic for mixed option state. Added SR text.

f6f16fd

Updated a11y language for exclusions. Added snapshot and a11y tests.

41dd821

1Copenut added feature request labels May 12, 2023

1Copenut self-assigned this May 12, 2023

Added changelog and two updated snapshots.

64b81a7

1Copenut requested review from cee-chen and ryankeairns May 12, 2023 18:14

Updating docs for checked: 'mixed' value.

e97fce1

1Copenut commented May 15, 2023

View reviewed changes

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

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

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

1Copenut marked this pull request as ready for review May 15, 2023 18:27

1Copenut added 2 commits May 15, 2023 15:54

Fixed linting errors in src-docs.

86fc493

Updated unit test snapshot.

5fd45c1

cee-chen reviewed May 16, 2023

View reviewed changes

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

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

cee-chen reviewed May 16, 2023

View reviewed changes

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

cee-chen suggested changes May 16, 2023

View reviewed changes

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

1Copenut and others added 3 commits May 16, 2023 08:40

Update src/components/selectable/selectable_option.tsx

f0f08f3

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

Update src/components/selectable/selectable_option.tsx

8a0446f

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

Refactoring allowExceptions logic to switch case.

92620f5

cee-chen reviewed May 16, 2023

View reviewed changes

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

cee-chen reviewed May 16, 2023

View reviewed changes

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

cee-chen reviewed May 16, 2023

View reviewed changes

src/components/search_bar/filters/field_value_selection_filter.tsx Outdated Show resolved Hide resolved

cee-chen reviewed May 16, 2023

View reviewed changes

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

cee-chen reviewed May 16, 2023

View reviewed changes

src-docs/src/views/selectable/selectable_example.js Outdated Show resolved Hide resolved

1Copenut added 2 commits May 16, 2023 16:22

Updated tests, documentation to be more granular.

a9326ac

Added note about aria-checked mixed with MDN links.

7f53fbb

1Copenut requested a review from cee-chen May 16, 2023 21:28

1Copenut added the breaking change PRs with breaking changes. (Don't delete - used for automation) label May 16, 2023

ryankeairns approved these changes May 16, 2023

View reviewed changes

cee-chen reviewed May 17, 2023

View reviewed changes

upcoming_changelogs/6774.md Show resolved Hide resolved

Update upcoming_changelogs/6774.md

ec742eb

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

cee-chen reviewed May 17, 2023

View reviewed changes

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

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

cee-chen reviewed May 17, 2023

View reviewed changes

cee-chen added 7 commits May 17, 2023 13:12

Merge branch 'main' into feature/euiSelectableListItem-mixed-option

a6d4678

Refactor aria-checked to work for all eligible roles

5f2ddb0

+ `aria-disabled` and `aria-checked` is valid - do not return undefined if disabled + `aria-checked` mixed is invalid for certain roles - return false if so

Update unit tests

ca82b1c

- add describe block tests for `role` and `aria-checked` - add describe blocks for `checked` - switch to RTL render

[pr feedback] minor type cleanup

0e16f6d

fix snippet

d3ad723

cee-chen approved these changes May 18, 2023

View reviewed changes

Escaped quotes for lint error.

d54c2ae

cee-chen reviewed May 18, 2023

View reviewed changes

src-docs/src/views/selectable/selectable_example.js Outdated Show resolved Hide resolved

code editor valid quotes

07c456d

1Copenut merged commit f8bc384 into elastic:main May 18, 2023

1Copenut deleted the feature/euiSelectableListItem-mixed-option branch May 18, 2023 20:40

1Copenut mentioned this pull request May 23, 2023

Upgrade EUI to v81.0.0 elastic/kibana#158330

Merged

Conversation

1Copenut commented May 12, 2023 • edited by cee-chen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Uh oh!

1Copenut commented May 12, 2023

Uh oh!

1Copenut commented May 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented May 16, 2023

Uh oh!

ryankeairns left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented May 17, 2023

Uh oh!

cee-chen May 17, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen May 17, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen May 18, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented May 18, 2023

Uh oh!

Uh oh!

kibanamachine commented May 18, 2023

Uh oh!

1Copenut commented May 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1Copenut commented May 12, 2023 •

edited by cee-chen

Loading