From fdc9a6b0e34e39b079bda5d960b5f5371aa04892 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 25 Mar 2020 16:32:45 -0500 Subject: [PATCH 01/11] [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 --- package.json | 4 + src/components/selectable/selectable.tsx | 42 ++-- .../selectable_list.test.tsx.snap | 168 ++------------- .../selectable_list_item.test.tsx.snap | 50 +++-- .../selectable_list/_selectable_list.scss | 4 + .../_selectable_list_item.scss | 21 +- .../selectable_list/selectable_list.tsx | 195 ++++++++++++------ .../selectable_list/selectable_list_item.tsx | 21 +- .../selectable_search/selectable_search.tsx | 25 ++- yarn.lock | 55 +++-- 10 files changed, 271 insertions(+), 314 deletions(-) diff --git a/package.json b/package.json index 22d631f7cfe..fb9b7019365 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,8 @@ "@types/react-beautiful-dnd": "^10.1.0", "@types/react-input-autosize": "^2.0.2", "@types/react-virtualized": "^9.18.7", + "@types/react-virtualized-auto-sizer": "^1.0.0", + "@types/react-window": "^1.8.1", "chroma-js": "^2.0.4", "classnames": "^2.2.5", "highlight.js": "^9.12.0", @@ -67,6 +69,8 @@ "react-input-autosize": "^2.2.2", "react-is": "~16.3.0", "react-virtualized": "^9.21.2", + "react-virtualized-auto-sizer": "^1.0.2", + "react-window": "^1.8.5", "resize-observer-polyfill": "^1.5.0", "tabbable": "^3.0.0", "uuid": "^3.1.0" diff --git a/src/components/selectable/selectable.tsx b/src/components/selectable/selectable.tsx index 00073a95f6c..e9ad28f45c2 100644 --- a/src/components/selectable/selectable.tsx +++ b/src/components/selectable/selectable.tsx @@ -5,6 +5,7 @@ import React, { createRef, Fragment, ReactElement, + KeyboardEvent, } from 'react'; import classNames from 'classnames'; import { CommonProps, ExclusiveUnion } from '../common'; @@ -14,13 +15,9 @@ import { EuiSelectableList } from './selectable_list'; import { EuiLoadingChart } from '../loading'; import { getMatchingOptions } from './matching_options'; import { comboBoxKeyCodes } from '../../services'; -import { TAB } from '../../services/key_codes'; import { EuiI18n } from '../i18n'; import { EuiSelectableOption } from './selectable_option'; -import { - EuiSelectableOptionsListProps, - EuiSelectableSingleOptionProps, -} from './selectable_list/selectable_list'; +import { EuiSelectableOptionsListProps } from './selectable_list/selectable_list'; type RequiredEuiSelectableOptionsListProps = Omit< EuiSelectableOptionsListProps, @@ -80,7 +77,7 @@ export type EuiSelectableProps = Omit< * `true`: only allows one selection * `always`: can and must have only one selection */ - singleSelection?: EuiSelectableSingleOptionProps; + singleSelection?: EuiSelectableOptionsListProps['singleSelection']; /** * Allows marking options as `checked='off'` as well as `'on'` */ @@ -174,7 +171,19 @@ export class EuiSelectable extends Component< return this.state.activeOptionIndex != null; }; - onKeyDown = (e: any) => { + onFocus = () => { + const firstSelected = this.state.visibleOptions.findIndex( + option => option.checked && !option.disabled + ); + + if (firstSelected > -1) { + this.setState({ activeOptionIndex: firstSelected }); + } else { + this.incrementActiveOptionIndex(1); + } + }; + + onKeyDown = (e: KeyboardEvent) => { const optionsList = this.optionsListRef.current; switch (e.keyCode) { @@ -199,15 +208,6 @@ export class EuiSelectable extends Component< } break; - case TAB: - // Disallow tabbing when the user is navigating the options. - // TODO: Can we force the tab to the next sibling element? - if (this.hasActiveOption()) { - e.preventDefault(); - e.stopPropagation(); - } - break; - default: if (this.props.onKeyDown) { this.props.onKeyDown(e); @@ -239,10 +239,12 @@ export class EuiSelectable extends Component< } } - // Group titles are included in option list but are not selectable - // Skip group title options + // Group titles and disabled options are included in option list but are not selectable const direction = amount > 0 ? 1 : -1; - while (visibleOptions[nextActiveOptionIndex].isGroupLabel) { + while ( + visibleOptions[nextActiveOptionIndex].isGroupLabel || + visibleOptions[nextActiveOptionIndex].disabled + ) { nextActiveOptionIndex = nextActiveOptionIndex + direction; if (nextActiveOptionIndex < 0) { @@ -377,6 +379,7 @@ export class EuiSelectable extends Component< renderOption={renderOption} height={height} allowExclusions={allowExclusions} + searchable={searchable} {...listProps} /> ); @@ -386,6 +389,7 @@ export class EuiSelectable extends Component< className={classes} onKeyDown={this.onKeyDown} onBlur={this.onContainerBlur} + onFocus={this.onFocus} {...rest}> {children && children(list, search)} diff --git a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap index dbb151532e0..04f3bf1411a 100644 --- a/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap +++ b/src/components/selectable/selectable_list/__snapshots__/selectable_list.test.tsx.snap @@ -8,17 +8,7 @@ exports[`EuiSelectableListItem is rendered 1`] = ` >
-
-
+ />
`; @@ -28,17 +18,7 @@ exports[`EuiSelectableListItem props activeOptionIndex 1`] = ` >
-
-
+ />
`; @@ -48,17 +28,7 @@ exports[`EuiSelectableListItem props allowExclusions 1`] = ` >
-
-
+ />
`; @@ -68,17 +38,7 @@ exports[`EuiSelectableListItem props bordered 1`] = ` >
-
-
+ />
`; @@ -88,17 +48,7 @@ exports[`EuiSelectableListItem props height is forced 1`] = ` >
-
-
+ />
`; @@ -108,17 +58,7 @@ exports[`EuiSelectableListItem props height is full 1`] = ` >
-
-
+ />
`; @@ -128,17 +68,7 @@ exports[`EuiSelectableListItem props renderOption 1`] = ` >
-
-
+ />
`; @@ -148,17 +78,7 @@ exports[`EuiSelectableListItem props rowHeight 1`] = ` >
-
-
+ />
`; @@ -168,17 +88,7 @@ exports[`EuiSelectableListItem props searchValue 1`] = ` >
-
-
+ />
`; @@ -188,17 +98,7 @@ exports[`EuiSelectableListItem props searchValue 2`] = ` >
-
-
+ />
`; @@ -208,17 +108,7 @@ exports[`EuiSelectableListItem props showIcons can be turned off 1`] = ` >
-
-
+ />
`; @@ -228,17 +118,7 @@ exports[`EuiSelectableListItem props singleSelection can be forced so that at le >
-
-
+ />
`; @@ -248,17 +128,7 @@ exports[`EuiSelectableListItem props singleSelection can be turned on 1`] = ` >
-
-
+ />
`; @@ -268,16 +138,6 @@ exports[`EuiSelectableListItem props visibleOptions 1`] = ` >
-
-
+ />
`; diff --git a/src/components/selectable/selectable_list/__snapshots__/selectable_list_item.test.tsx.snap b/src/components/selectable/selectable_list/__snapshots__/selectable_list_item.test.tsx.snap index 16b977c8c09..e471912970d 100644 --- a/src/components/selectable/selectable_list/__snapshots__/selectable_list_item.test.tsx.snap +++ b/src/components/selectable/selectable_list/__snapshots__/selectable_list_item.test.tsx.snap @@ -1,12 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EuiSelectableListItem is rendered 1`] = ` - + `; exports[`EuiSelectableListItem props append 1`] = ` - + `; exports[`EuiSelectableListItem props checked is off 1`] = ` - + `; exports[`EuiSelectableListItem props checked is on 1`] = ` - + `; exports[`EuiSelectableListItem props disabled 1`] = ` - + `; exports[`EuiSelectableListItem props isFocused 1`] = ` - + `; exports[`EuiSelectableListItem props prepend 1`] = ` - + `; exports[`EuiSelectableListItem props showIcons can be turned off 1`] = ` - + `; diff --git a/src/components/selectable/selectable_list/_selectable_list.scss b/src/components/selectable/selectable_list/_selectable_list.scss index fff87543bd9..3a1aa008f45 100644 --- a/src/components/selectable/selectable_list/_selectable_list.scss +++ b/src/components/selectable/selectable_list/_selectable_list.scss @@ -12,6 +12,10 @@ .euiSelectableList__list { @include euiOverflowShadow; @include euiScrollBar; + + &:focus-within { + @include euiFocusRing; // TODO @myasonik why don't this work? + } } .euiSelectableList__groupLabel { diff --git a/src/components/selectable/selectable_list/_selectable_list_item.scss b/src/components/selectable/selectable_list/_selectable_list_item.scss index c7f42bd29ca..cd2bbd28c2d 100644 --- a/src/components/selectable/selectable_list/_selectable_list_item.scss +++ b/src/components/selectable/selectable_list/_selectable_list_item.scss @@ -5,32 +5,25 @@ width: 100%; text-align: left; color: $euiTextColor; + cursor: pointer; &:not(:last-of-type) { border-bottom: $euiSelectableListItemBorder; } - &:hover, - &:focus { + &-isFocused:not([aria-disabled='true']), + &:hover:not([aria-disabled='true']) { + color: $euiColorPrimary; + background-color: $euiFocusBackgroundColor; + .euiSelectableListItem__text { text-decoration: underline; } } - &:focus, - &-isFocused { - cursor: pointer; - color: $euiColorPrimary; - background-color: $euiFocusBackgroundColor; - } - - &[disabled] { + &[aria-disabled='true'] { color: $euiColorMediumShade; cursor: not-allowed; - - &:hover { - text-decoration: none; - } } } diff --git a/src/components/selectable/selectable_list/selectable_list.tsx b/src/components/selectable/selectable_list/selectable_list.tsx index bb5440c295b..2e7bf1485dc 100644 --- a/src/components/selectable/selectable_list/selectable_list.tsx +++ b/src/components/selectable/selectable_list/selectable_list.tsx @@ -1,8 +1,6 @@ -import React, { Component, HTMLAttributes, ReactNode } from 'react'; +import React, { Component, HTMLAttributes, ReactNode, memo } from 'react'; import classNames from 'classnames'; import { CommonProps } from '../../common'; -// eslint-disable-next-line import/named -import { List, AutoSizer, ListProps } from 'react-virtualized'; import { htmlIdGenerator } from '../../../services'; import { EuiSelectableListItem, @@ -10,8 +8,13 @@ import { } from './selectable_list_item'; import { EuiHighlight } from '../../highlight'; import { EuiSelectableOption } from '../selectable_option'; - -export type EuiSelectableSingleOptionProps = 'always' | boolean; +import AutoSizer from 'react-virtualized-auto-sizer'; +import { + FixedSizeList, + ListProps, + ListChildComponentProps, + areEqual, +} from 'react-window'; // Consumer Configurable Props via `EuiSelectable.listProps` export type EuiSelectableOptionsListProps = CommonProps & @@ -30,11 +33,11 @@ export type EuiSelectableOptionsListProps = CommonProps & * Show the check/cross selection indicator icons */ showIcons?: boolean; - singleSelection?: EuiSelectableSingleOptionProps; + singleSelection?: 'always' | boolean; /** - * Any props to send specifically to the react-virtualized `List` + * Any props to send specifically to the react-window `FixedSizeList` */ - virtualizedProps?: ListProps; + windowProps?: ListProps; /** * Adds a border around the list to indicate the bounds; * Useful when the list scrolls, otherwise use your own container @@ -79,6 +82,7 @@ export type EuiSelectableListProps = EuiSelectableOptionsListProps & { */ allowExclusions?: boolean; rootId?: (appendix?: string) => string; + searchable?: boolean; }; export class EuiSelectableList extends Component { @@ -88,11 +92,116 @@ export class EuiSelectableList extends Component { }; rootId = this.props.rootId || htmlIdGenerator(); + listRef: FixedSizeList | null = null; + listBoxRef: HTMLUListElement | null = null; + + makeOptionId(index: number | undefined) { + if (typeof index === 'undefined') { + return ''; + } + + return this.rootId(`_option-${index}`); + } + + setListRef = (ref: FixedSizeList | null) => { + this.listRef = ref; + + if (ref && this.props.activeOptionIndex) { + ref.scrollToItem(this.props.activeOptionIndex, 'auto'); + } + }; + + setListBoxRef = (ref: HTMLUListElement | null) => { + this.listBoxRef = ref; + + if (ref) { + ref.setAttribute('id', this.rootId('listbox')); + ref.setAttribute('role', 'listBox'); + + if (this.props.searchable !== true) { + ref.setAttribute('tabindex', '0'); + } + + if ( + this.props.singleSelection !== 'always' && + this.props.singleSelection !== true + ) { + ref.setAttribute('aria-multiselectable', 'true'); + } + } + }; + + componentDidUpdate() { + const { activeOptionIndex } = this.props; + + if (this.listBoxRef) { + this.listBoxRef.setAttribute( + 'aria-activedescendant', + `${this.makeOptionId(activeOptionIndex)}` + ); + } + + if (this.listRef && typeof this.props.activeOptionIndex !== 'undefined') { + this.listRef.scrollToItem(this.props.activeOptionIndex, 'auto'); + } + } constructor(props: EuiSelectableListProps) { super(props); } + ListRow = memo(({ data, index, style }: ListChildComponentProps) => { + const option = data[index]; + const { + label, + isGroupLabel, + checked, + disabled, + prepend, + append, + ref, + key, + ...optionRest + } = option; + + if (isGroupLabel) { + return ( +
  • }> + {prepend} + {label} + {append} +
    • + ); + } + + return ( + this.onAddOrRemoveOption(option)} + ref={ref ? ref.bind(null, index) : undefined} + isFocused={this.props.activeOptionIndex === index} + title={label} + showIcons={this.props.showIcons} + checked={checked} + disabled={disabled} + prepend={prepend} + append={append} + {...optionRest as EuiSelectableListItemProps}> + {this.props.renderOption ? ( + this.props.renderOption(option, this.props.searchValue) + ) : ( + {label} + )} + + ); + }, areEqual); + render() { const { className, @@ -101,7 +210,7 @@ export class EuiSelectableList extends Component { onOptionClick, renderOption, height: forcedHeight, - virtualizedProps, + windowProps, rowHeight, activeOptionIndex, rootId, @@ -110,6 +219,7 @@ export class EuiSelectableList extends Component { visibleOptions, allowExclusions, bordered, + searchable, ...rest } = this.props; @@ -149,66 +259,19 @@ export class EuiSelectableList extends Component {
    {({ width, height }) => ( - { - const option = optionArray[index]; - const { - label, - isGroupLabel, - checked, - disabled, - prepend, - append, - ref, - key, - ...optionRest - } = option; - if (isGroupLabel) { - return ( -
    }> - {prepend} - {label} - {append} -
    - ); - } - return ( - this.onAddOrRemoveOption(option)} - ref={ref ? ref.bind(null, index) : undefined} - isFocused={activeOptionIndex === index} - title={label} - showIcons={showIcons} - checked={checked} - disabled={disabled} - prepend={prepend} - append={append} - {...optionRest as EuiSelectableListItemProps}> - {renderOption ? ( - renderOption(option, searchValue) - ) : ( - {label} - )} - - ); - }} - /> + itemCount={optionArray.length} + itemData={optionArray} + itemSize={rowHeight} + innerElementType="ul" + innerRef={this.setListBoxRef} + {...windowProps}> + {this.ListRow} + )}
    diff --git a/src/components/selectable/selectable_list/selectable_list_item.tsx b/src/components/selectable/selectable_list/selectable_list_item.tsx index efb76779ef7..c186c812bd4 100644 --- a/src/components/selectable/selectable_list/selectable_list_item.tsx +++ b/src/components/selectable/selectable_list/selectable_list_item.tsx @@ -1,4 +1,4 @@ -import React, { Component, ButtonHTMLAttributes } from 'react'; +import React, { Component, LiHTMLAttributes } from 'react'; import classNames from 'classnames'; import { CommonProps } from '../../common'; import { EuiIcon, IconType, IconColor } from '../../icon'; @@ -15,9 +15,7 @@ function resolveIconAndColor( : { icon: 'cross', color: 'text' }; } -export type EuiSelectableListItemProps = ButtonHTMLAttributes< - HTMLButtonElement -> & +export type EuiSelectableListItemProps = LiHTMLAttributes & CommonProps & { children?: React.ReactNode; /** @@ -70,10 +68,10 @@ export class EuiSelectableListItem extends Component< className ); - let buttonIcon: React.ReactNode; + let optionIcon: React.ReactNode; if (showIcons) { const { icon, color } = resolveIconAndColor(checked); - buttonIcon = ( + optionIcon = ( - {buttonIcon} + {optionIcon} {prependNode} {children} {appendNode} - + ); } } diff --git a/src/components/selectable/selectable_search/selectable_search.tsx b/src/components/selectable/selectable_search/selectable_search.tsx index 39559fc7625..c378d6a57b4 100644 --- a/src/components/selectable/selectable_search/selectable_search.tsx +++ b/src/components/selectable/selectable_search/selectable_search.tsx @@ -4,6 +4,7 @@ import { CommonProps } from '../../common'; import { EuiFieldSearch, EuiFieldSearchProps } from '../../form'; import { getMatchingOptions } from '../matching_options'; import { EuiSelectableOption } from '../selectable_option'; +import { EuiI18n } from '../../i18n'; export type EuiSelectableSearchProps = Omit< InputHTMLAttributes & EuiFieldSearchProps, @@ -67,15 +68,21 @@ export class EuiSelectableSearch extends Component< const classes = classNames('euiSelectableSearch', className); return ( - + + {(placeholderName: string) => ( + + )} + ); } } diff --git a/yarn.lock b/yarn.lock index 0ae4cfaaf25..e55d9630a76 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1304,14 +1304,28 @@ dependencies: "@types/react" "*" +"@types/react-virtualized-auto-sizer@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.0.tgz#fc32f30a8dab527b5816f3a757e1e1d040c8f272" + integrity sha512-NMErdIdSnm2j/7IqMteRiRvRulpjoELnXWUwdbucYCz84xG9PHcoOrr7QfXwB/ku7wd6egiKFrzt/+QK4Imeeg== + dependencies: + "@types/react" "*" + "@types/react-virtualized@^9.18.7": - version "9.21.5" - resolved "https://registry.yarnpkg.com/@types/react-virtualized/-/react-virtualized-9.21.5.tgz#8b849c7c3c92c7b28b29bc75602c067a912310c6" - integrity sha512-oCoGJzkW90YQkvXwvtkCBDN0TTYvaQs217TJDOh+VipzJ9iiHD/NpD0ILvB844+ewf3/4xYOI5Oj5kj5m6J/4w== + version "9.21.8" + resolved "https://registry.yarnpkg.com/@types/react-virtualized/-/react-virtualized-9.21.8.tgz#dc0150a75fd6e42f33729886463ece04d03367ea" + integrity sha512-7fZoA0Azd2jLIE9XC37fMZgMqaJe3o3pfzGjvrzphoKjBCdT4oNl6wikvo4dDMESDnpkZ8DvVTc7aSe4DW86Ew== dependencies: "@types/prop-types" "*" "@types/react" "*" +"@types/react-window@^1.8.1": + version "1.8.1" + resolved "https://registry.yarnpkg.com/@types/react-window/-/react-window-1.8.1.tgz#6e1ceab2e6f2f78dbf1f774ee0e00f1bb0364bb3" + integrity sha512-V3k1O5cbfZIRa0VVbQ81Ekq/7w42CK1SuiB9U1oPMTxv270D9qUn7rHb3sZoqMkIJFfB1NZxaH7NRDlk+ToDsg== + dependencies: + "@types/react" "*" + "@types/react@*", "@types/react@^16.9.23": version "16.9.23" resolved "https://registry.yarnpkg.com/@types/react/-/react-16.9.23.tgz#1a66c6d468ba11a8943ad958a8cb3e737568271c" @@ -3358,9 +3372,9 @@ cloneable-readable@^1.0.0: through2 "^2.0.1" clsx@^1.0.1: - version "1.0.4" - resolved "https://registry.yarnpkg.com/clsx/-/clsx-1.0.4.tgz#0c0171f6d5cb2fe83848463c15fcc26b4df8c2ec" - integrity sha512-1mQ557MIZTrL/140j+JVdRM6e31/OA4vTYxXgqIIZlndyfjHpyawKZia1Im05Vp9BWmImkcNrNtFYQMyFcgJDg== + version "1.1.0" + resolved "https://registry.yarnpkg.com/clsx/-/clsx-1.1.0.tgz#62937c6adfea771247c34b54d320fb99624f5702" + integrity sha512-3avwM37fSK5oP6M5rQ9CNe99lwxhXDOeSWVPAOYF6OazUTgZCMb0yWlJpmdD74REy1gkEaFiub2ULv4fq9GUhA== co@^4.6.0: version "4.6.0" @@ -4214,16 +4228,11 @@ cssom@0.3.x, "cssom@>= 0.3.2 < 0.4.0": dependencies: cssom "0.3.x" -csstype@^2.2.0: +csstype@^2.2.0, csstype@^2.6.7: version "2.6.9" resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.9.tgz#05141d0cd557a56b8891394c1911c40c8a98d098" integrity sha512-xz39Sb4+OaTsULgUERcCk+TJj8ylkL4aSVDQiX/ksxbELSqwkgt4d4RD7fovIdgJGSuNYqwZEiVjYY5l0ask+Q== -csstype@^2.6.7: - version "2.6.7" - resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.7.tgz#20b0024c20b6718f4eda3853a1f5a1cce7f5e4a5" - integrity sha512-9Mcn9sFbGBAdmimWb2gLVDtFJzeKtDGIr76TUqmjZrw9LFXBMSU70lcs+C0/7fyCd6iBDqmksUcCOUIkisPHsQ== - currently-unhandled@^0.4.1: version "0.4.1" resolved "https://registry.yarnpkg.com/currently-unhandled/-/currently-unhandled-0.4.1.tgz#988df33feab191ef799a61369dd76c17adf957ea" @@ -9145,14 +9154,14 @@ loglevel@^1.4.1: resolved "https://registry.yarnpkg.com/loglevel/-/loglevel-1.6.1.tgz#e0fc95133b6ef276cdc8887cdaf24aa6f156f8fa" integrity sha1-4PyVEztu8nbNyIh82vJKpvFW+Po= -loose-envify@^1.0.0, loose-envify@^1.3.1, loose-envify@^1.4.0: +loose-envify@^1.0.0, loose-envify@^1.3.0, loose-envify@^1.3.1, loose-envify@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/loose-envify/-/loose-envify-1.4.0.tgz#71ee51fa7be4caec1a63839f7e682d8132d30caf" integrity sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q== dependencies: js-tokens "^3.0.0 || ^4.0.0" -loose-envify@^1.1.0, loose-envify@^1.2.0, loose-envify@^1.3.0: +loose-envify@^1.1.0, loose-envify@^1.2.0: version "1.3.1" resolved "https://registry.yarnpkg.com/loose-envify/-/loose-envify-1.3.1.tgz#d1a8ad33fa9ce0e713d65fdd0ac8b748d478c848" integrity sha1-0aitM/qc4OcT1l/dCsi3SNR4yEg= @@ -9372,6 +9381,11 @@ mem@^4.0.0: mimic-fn "^1.0.0" p-is-promise "^1.1.0" +"memoize-one@>=3.1.1 <6": + version "5.1.1" + resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.1.1.tgz#047b6e3199b508eaec03504de71229b8eb1d75c0" + integrity sha512-HKeeBpWvqiVJD57ZUAsJNm71eHTykffzcLZVYWiVfQeI1rJtuEaS7hQiEpWfVVk18donPwJEcFKIkCmPJNOhHA== + memoize-one@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.0.0.tgz#d55007dffefb8de7546659a1722a5d42e128286e" @@ -12303,6 +12317,11 @@ react-test-renderer@^16.2.0: object-assign "^4.1.1" prop-types "^15.6.0" +react-virtualized-auto-sizer@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.2.tgz#a61dd4f756458bbf63bd895a92379f9b70f803bd" + integrity sha512-MYXhTY1BZpdJFjUovvYHVBmkq79szK/k7V3MO+36gJkWGkrXKtyr4vCPtpphaTLRAdDNoYEYFZWE8LjN+PIHNg== + react-virtualized@^9.21.2: version "9.21.2" resolved "https://registry.yarnpkg.com/react-virtualized/-/react-virtualized-9.21.2.tgz#02e6df65c1e020c8dbf574ec4ce971652afca84e" @@ -12315,6 +12334,14 @@ react-virtualized@^9.21.2: prop-types "^15.6.0" react-lifecycles-compat "^3.0.4" +react-window@^1.8.5: + version "1.8.5" + resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.8.5.tgz#a56b39307e79979721021f5d06a67742ecca52d1" + integrity sha512-HeTwlNa37AFa8MDZFZOKcNEkuF2YflA0hpGPiTT9vR7OawEt+GZbfM6wqkBahD3D3pUjIabQYzsnY/BSJbgq6Q== + dependencies: + "@babel/runtime" "^7.0.0" + memoize-one ">=3.1.1 <6" + react@^16.12.0: version "16.12.0" resolved "https://registry.yarnpkg.com/react/-/react-16.12.0.tgz#0c0a9c6a142429e3614834d5a778e18aa78a0b83" From a0ef6250ff736f9fddaae7412f121d6c89097107 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 15 Apr 2020 15:00:10 -0500 Subject: [PATCH 02/11] [Feature / Selectable A11y] making EuiSelectable[searchable] more accessible (#3234) * Moves many aria-* props from the List
      and onto the or surrounding
      * [Breaking] EuiSelectableSearch has 2 new required props and EuiSelectableList has 1 --- src-docs/src/views/selectable/data.ts | 1 - src-docs/src/views/selectable/selectable.tsx | 1 + .../selectable/selectable_custom_render.js | 1 + .../views/selectable/selectable_example.js | 16 +- .../views/selectable/selectable_exclusion.tsx | 1 + .../views/selectable/selectable_messages.tsx | 6 +- .../views/selectable/selectable_popover.js | 2 + .../views/selectable/selectable_search.tsx | 1 + .../views/selectable/selectable_single.tsx | 1 + .../__snapshots__/selectable.test.tsx.snap | 1 - src/components/selectable/selectable.test.tsx | 5 + src/components/selectable/selectable.tsx | 142 +++++++++++++++--- .../selectable_list.test.tsx.snap | 40 +++-- .../selectable_list/selectable_list.test.tsx | 40 ++--- .../selectable_list/selectable_list.tsx | 48 +++--- .../selectable/selectable_option.tsx | 6 +- .../selectable_search.test.tsx.snap | 16 +- .../selectable_search.test.tsx | 4 +- .../selectable_search/selectable_search.tsx | 50 +++--- 19 files changed, 267 insertions(+), 115 deletions(-) diff --git a/src-docs/src/views/selectable/data.ts b/src-docs/src/views/selectable/data.ts index 85b0bda4ad9..67f425d3394 100644 --- a/src-docs/src/views/selectable/data.ts +++ b/src-docs/src/views/selectable/data.ts @@ -15,7 +15,6 @@ export const Options: EuiSelectableOption[] = [ }, { label: 'Dione', - id: 'id_dione', }, { label: 'Iapetus', diff --git a/src-docs/src/views/selectable/selectable.tsx b/src-docs/src/views/selectable/selectable.tsx index 29993c2bb17..75977051b5b 100644 --- a/src-docs/src/views/selectable/selectable.tsx +++ b/src-docs/src/views/selectable/selectable.tsx @@ -8,6 +8,7 @@ export default () => { return ( setOptions(newOptions)}> diff --git a/src-docs/src/views/selectable/selectable_custom_render.js b/src-docs/src/views/selectable/selectable_custom_render.js index db103ce7f90..230a1ea86a7 100644 --- a/src-docs/src/views/selectable/selectable_custom_render.js +++ b/src-docs/src/views/selectable/selectable_custom_render.js @@ -84,6 +84,7 @@ export default class extends Component { , snippet: ` this.onChange(options)} listProps={{ bordered: true }}> @@ -141,6 +142,7 @@ export const SelectableExample = { props: { EuiSelectable }, demo: , snippet: `, snippet: ` - {list => list} - + aria-label="Single selection example" + options={options} + onChange={this.onChange} + singleSelection={true} + listProps={{ bordered: true }}> + {list => list} + `, }, { @@ -255,6 +258,7 @@ export const SelectableExample = { props: { EuiSelectable }, demo: , snippet: ` this.onChange(options)}> diff --git a/src-docs/src/views/selectable/selectable_exclusion.tsx b/src-docs/src/views/selectable/selectable_exclusion.tsx index 1a4c87d27b1..48614971c55 100644 --- a/src-docs/src/views/selectable/selectable_exclusion.tsx +++ b/src-docs/src/views/selectable/selectable_exclusion.tsx @@ -8,6 +8,7 @@ export default () => { return ( setOptions(newOptions)}> diff --git a/src-docs/src/views/selectable/selectable_messages.tsx b/src-docs/src/views/selectable/selectable_messages.tsx index 95ec006c00a..26da0f56b65 100644 --- a/src-docs/src/views/selectable/selectable_messages.tsx +++ b/src-docs/src/views/selectable/selectable_messages.tsx @@ -30,7 +30,11 @@ export default () => { checked={isLoading} /> - + {list => (useCustomMessage && !isLoading ? customMessage : list)} diff --git a/src-docs/src/views/selectable/selectable_popover.js b/src-docs/src/views/selectable/selectable_popover.js index a87f2df99cc..0f4d2822a4e 100644 --- a/src-docs/src/views/selectable/selectable_popover.js +++ b/src-docs/src/views/selectable/selectable_popover.js @@ -122,6 +122,7 @@ export default class extends Component { onClose={this.closeFlyout} aria-labelledby="flyoutTitle"> {}} style={{ width: 300 }} diff --git a/src-docs/src/views/selectable/selectable_search.tsx b/src-docs/src/views/selectable/selectable_search.tsx index 4566e56ab71..6649a7ac9cd 100644 --- a/src-docs/src/views/selectable/selectable_search.tsx +++ b/src-docs/src/views/selectable/selectable_search.tsx @@ -9,6 +9,7 @@ export default () => { return ( { return ( setOptions(newOptions)} singleSelection={true} diff --git a/src/components/selectable/__snapshots__/selectable.test.tsx.snap b/src/components/selectable/__snapshots__/selectable.test.tsx.snap index ec2b5e77f42..7bbd9e21b00 100644 --- a/src/components/selectable/__snapshots__/selectable.test.tsx.snap +++ b/src/components/selectable/__snapshots__/selectable.test.tsx.snap @@ -2,7 +2,6 @@ exports[`EuiSelectable is rendered 1`] = `
      diff --git a/src/components/selectable/selectable.test.tsx b/src/components/selectable/selectable.test.tsx index 0a886af1047..9a16c3f7889 100644 --- a/src/components/selectable/selectable.test.tsx +++ b/src/components/selectable/selectable.test.tsx @@ -19,6 +19,11 @@ const options: EuiSelectableOption[] = [ }, ]; +// Mock the htmlIdGenerator to generate predictable ids for snapshot tests +jest.mock('../../services/accessibility/html_id_generator', () => ({ + htmlIdGenerator: () => () => 'htmlId', +})); + describe('EuiSelectable', () => { test('is rendered', () => { const component = render( diff --git a/src/components/selectable/selectable.tsx b/src/components/selectable/selectable.tsx index e9ad28f45c2..c752a528e6d 100644 --- a/src/components/selectable/selectable.tsx +++ b/src/components/selectable/selectable.tsx @@ -14,10 +14,11 @@ import { EuiSelectableMessage } from './selectable_message'; import { EuiSelectableList } from './selectable_list'; import { EuiLoadingChart } from '../loading'; import { getMatchingOptions } from './matching_options'; -import { comboBoxKeyCodes } from '../../services'; +import { comboBoxKeyCodes, htmlIdGenerator } from '../../services'; import { EuiI18n } from '../i18n'; import { EuiSelectableOption } from './selectable_option'; import { EuiSelectableOptionsListProps } from './selectable_list/selectable_list'; +import { EuiSelectableSearchProps } from './selectable_search/selectable_search'; type RequiredEuiSelectableOptionsListProps = Omit< EuiSelectableOptionsListProps, @@ -43,7 +44,7 @@ type EuiSelectableSearchableProps = ExclusiveUnion< /** * Passes props down to the `EuiFieldSearch` */ - searchProps?: {}; + searchProps?: Partial; } >; @@ -120,7 +121,7 @@ export class EuiSelectable extends Component< }; private optionsListRef = createRef(); - + rootId = htmlIdGenerator(); constructor(props: EuiSelectableProps) { super(props); @@ -302,11 +303,31 @@ export class EuiSelectable extends Component< renderOption, height, allowExclusions, + 'aria-label': ariaLabel, + 'aria-describedby': ariaDescribedby, ...rest } = this.props; const { searchValue, visibleOptions, activeOptionIndex } = this.state; + // Some messy destructuring here to remove aria-label/describedby from searchProps and listProps + // Made messier by some TS requirements + // The aria attributes are then used in getAccessibleName() to place them where they need to go + const unknownAccessibleName = { + 'aria-label': undefined, + 'aria-describedby': undefined, + }; + const { + 'aria-label': searchAriaLabel, + 'aria-describedby': searchAriaDescribedby, + ...cleanedSearchProps + } = searchProps || unknownAccessibleName; + const { + 'aria-label': listAriaLabel, + 'aria-describedby': listAriaDescribedby, + ...cleanedListProps + } = listProps || unknownAccessibleName; + let messageContent; if (isLoading) { @@ -351,37 +372,110 @@ export class EuiSelectable extends Component< className ); + const listId = this.rootId('listbox'); + const makeOptionId = (index: number | undefined) => { + if (typeof index === 'undefined') { + return ''; + } + + return `${listId}_option-${index}`; + }; + + /** + * There are lots of ways to add an accessible name + * Usually we want the same name for the input and the listbox (which is added by aria-label/describedby) + * But you can always override it using searchProps or listProps + * This finds the correct name to use + * + * TODO: This doesn't handle being labelled (