Repository: gravity-ui/uikit (ts)

Issue:
get rid of onclicks on div in the List component
It would be great to increase the accessibility of components from the keyboard and screen readers, for this you can get rid of onclicks on non-interactive elements

Pull request:
feat(List): improve a11y
Closes #686 

Patch:
diff --git a/src/components/List/List.scss b/src/components/List/List.scss
index 563c3689..7156d11d 100644
--- a/src/components/List/List.scss
+++ b/src/components/List/List.scss
@@ -53,6 +53,10 @@ $block: '.#{variables.$ns}list';
 
         &_selected {
             background: var(--g-color-base-selection);
+
+            &:hover {
+                background: var(--g-color-base-selection-hover);
+            }
         }
 
         &_sort-handle-align_right {
diff --git a/src/components/List/List.tsx b/src/components/List/List.tsx
index 79cbf999..390768d4 100644
--- a/src/components/List/List.tsx
+++ b/src/components/List/List.tsx
@@ -11,6 +11,7 @@ import {SelectLoadingIndicator} from '../Select/components/SelectList/SelectLoad
 import {TextInput} from '../controls';
 import {MobileContext} from '../mobile';
 import {block} from '../utils/cn';
+import {getUniqId} from '../utils/common';
 
 import {ListItem, SimpleContainer, defaultRenderItem} from './components';
 import {listNavigationIgnoredKeys} from './constants';
@@ -82,8 +83,9 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
     loadingItem = {value: '__LIST_ITEM_LOADING__', disabled: true} as unknown as ListItemData<
         T & {value: string}
     >;
+    uniqId = getUniqId();
 
-    componentDidUpdate(prevProps: ListProps<T>) {
+    componentDidUpdate(prevProps: ListProps<T>, prevState: ListState<T>) {
         if (this.props.items !== prevProps.items) {
             const filter = this.getFilter();
             const internalFiltering = filter && !this.props.onFilterUpdate;
@@ -98,6 +100,10 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
         if (this.props.activeItemIndex !== prevProps.activeItemIndex) {
             this.activateItem(this.props.activeItemIndex);
         }
+
+        if (this.props.onChangeActive && this.state.activeItem !== prevState.activeItem) {
+            this.props.onChangeActive(this.state.activeItem);
+        }
     }
 
     componentWillUnmount() {
@@ -105,13 +111,21 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
     }
 
     render() {
-        const {emptyPlaceholder, virtualized, className, itemsClassName, qa} = this.props;
+        const {
+            emptyPlaceholder,
+            virtualized,
+            className,
+            itemsClassName,
+            qa,
+            role = 'list',
+        } = this.props;
 
         const {items} = this.state;
 
         return (
             <MobileContext.Consumer>
                 {({mobile}) => (
+                    // The event handler should only be used to capture bubbled events
                     // eslint-disable-next-line jsx-a11y/no-static-element-interactions
                     <div
                         className={b({mobile}, className)}
@@ -126,6 +140,7 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
                             className={b('items', {virtualized}, itemsClassName)}
                             style={this.getItemsStyle()}
                             onMouseLeave={this.onMouseLeave}
+                            role={role}
                         >
                             {this.renderItems()}
                             {items.length === 0 && Boolean(emptyPlaceholder) && (
@@ -224,12 +239,15 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
     };
 
     private renderItem = ({index, style}: {index: number; style?: React.CSSProperties}) => {
-        const {sortHandleAlign} = this.props;
+        const {sortHandleAlign, role} = this.props;
         const {items, activeItem} = this.state;
         const item = this.getItemsWithLoading()[index];
         const sortable = this.props.sortable && items.length > 1 && !this.getFilter();
         const active = index === activeItem || index === this.props.activeItemIndex;
         const Item = sortable ? SortableListItem : ListItem;
+        const selected = Array.isArray(this.props.selectedItemIndex)
+            ? this.props.selectedItemIndex.includes(index)
+            : index === this.props.selectedItemIndex;
 
         return (
             <Item
@@ -243,9 +261,11 @@ export class List<T = unknown> extends React.Component<ListProps<T>, ListState<T
                 renderItem={this.renderItemContent}
                 itemClassName={this.props.itemClassName}
                 active={active}
-                selected={index === this.props.selectedItemIndex}
+                selected={selected}
                 onActivate={this.onItemActivate}
                 onClick={this.props.onItemClick}
+                role={role === 'listbox' ? 'option' : 'listitem'}
+                listId={this.props.id ?? this.uniqId}
             />
         );
     };
diff --git a/src/components/List/README.md b/src/components/List/README.md
index bf682045..28b6a448 100644
--- a/src/components/List/README.md
+++ b/src/components/List/README.md
@@ -251,6 +251,9 @@ Likewise, you can forward `onFocus` and `onBlur` if you need to repeat the behav
 | onItemClick       | Item click handler. `(item: any, index: number, fromKeyboard?: bool) => void`                                                                                                                          | `Function`        |         |
 | deactivateOnLeave | If the flag is set, an item's selection is deactivated once the cursor leaves the item or the list loses its focus. If not set, the last selected item will always be selected.                        | `Boolean`         | true    |
 | activeItemIndex   | If a value is set, an item with this index is rendered as active ~~until the curse is lifted~~.                                                                                                        | `Number`          |         |
-| selectedItemIndex | If a value is set, an item with this index is rendered as selected (the background color is from `--g-color-base-selection`).                                                                          | `Number`          |         |
+| selectedItemIndex | If a value is set, an item with this index is rendered as selected (the background color is from `--g-color-base-selection`).                                                                          | `Number/Array`    |         |
 | itemClassName     | Custom class name to be added to an item container                                                                                                                                                     | `String`          |         |
 | itemsClassName    | Custom class name to be added to an item list                                                                                                                                                          | `String`          |         |
+| role              | HTML `role` attribute                                                                                                                                                                                  | `String`          | list    |
+| id                | HTML `id` attribute                                                                                                                                                                                    | `string`          |         |
+| onChangeActive    | Fires when the index of an option in the listbox, visually indicated as having keyboard focus, is changed. `(index?: number) => void`                                                                  | `Function`        |         |
diff --git a/src/components/List/components/ListItem.tsx b/src/components/List/components/ListItem.tsx
index a1bd8d90..cf7541b2 100644
--- a/src/components/List/components/ListItem.tsx
+++ b/src/components/List/components/ListItem.tsx
@@ -17,13 +17,22 @@ export class ListItem<T = unknown> extends React.Component<ListItemProps<T>> {
     ref = React.createRef<HTMLDivElement>();
 
     render() {
-        const {item, style, sortable, sortHandleAlign, itemClassName, selected, active} =
-            this.props;
+        const {
+            item,
+            style,
+            sortable,
+            sortHandleAlign,
+            itemClassName,
+            selected,
+            active,
+            role = 'listitem',
+        } = this.props;
 
         return (
             // eslint-disable-next-line jsx-a11y/click-events-have-key-events
             <div
-                role="listitem"
+                role={role}
+                aria-selected={selected}
                 data-qa={active ? ListQa.ACTIVE_ITEM : undefined}
                 className={b(
                     'item',
@@ -42,6 +51,7 @@ export class ListItem<T = unknown> extends React.Component<ListItemProps<T>> {
                 onMouseEnter={this.onMouseEnter}
                 onMouseLeave={this.onMouseLeave}
                 ref={this.ref}
+                id={`${this.props.listId}-item-${this.props.itemIndex}`}
             >
                 {this.renderSortIcon()}
                 {this.renderContent()}
diff --git a/src/components/List/types.ts b/src/components/List/types.ts
index 0889cd4b..cd03e07b 100644
--- a/src/components/List/types.ts
+++ b/src/components/List/types.ts
@@ -19,7 +19,7 @@ export type ListProps<T = unknown> = QAProps & {
     filterPlaceholder?: string;
     filter?: string;
     activeItemIndex?: number;
-    selectedItemIndex?: number;
+    selectedItemIndex?: number | number[];
     itemHeight?: number | ((item: ListItemData<T>, itemIndex: number) => number);
     itemsHeight?: number | ((items: T[]) => number);
     virtualized?: boolean;
@@ -39,8 +39,11 @@ export type ListProps<T = unknown> = QAProps & {
     onFilterEnd?: ({items}: {items: ListItemData<T>[]}) => void;
     onSortEnd?: (params: ListSortParams) => void;
     autoFocus?: boolean;
+    role?: React.AriaRole;
     loading?: boolean;
     onLoadMore?: () => void;
+    onChangeActive?: (index?: number) => void;
+    id?: string;
 };
 
 export type ListItemProps<T> = {
@@ -55,4 +58,6 @@ export type ListItemProps<T> = {
     onActivate: (index?: number) => void;
     renderItem?: ListProps<T>['renderItem'];
     onClick?: ListProps<T>['onItemClick'];
+    role?: React.AriaRole;
+    listId?: string;
 };
diff --git a/src/components/Select/README.md b/src/components/Select/README.md
index 9b1bd9f5..b132aa9f 100644
--- a/src/components/Select/README.md
+++ b/src/components/Select/README.md
@@ -36,6 +36,7 @@
 | onBlur                          | `function`                              | `-`             | Handler that is called when the element loses focus.                                                                                                                                                                                                                                                                               |
 | loading                         | `boolean`                               | `-`             | Add the loading item to the end of the options list. Works like persistant loading indicator while the options list is empty.                                                                                                                                                                                                      |
 | onLoadMore                      | `function`                              | `-`             | Fires when loading indicator gets visible.                                                                                                                                                                                                                                                                                         |
+| id                              | `string`                                | `-`             | HTML `id` attribute                                                                                                                                                                                                                                                                                                                |
 
 ---
 
diff --git a/src/components/Select/Select.tsx b/src/components/Select/Select.tsx
index a14a78c4..c852f89c 100644
--- a/src/components/Select/Select.tsx
+++ b/src/components/Select/Select.tsx
@@ -7,6 +7,7 @@ import type {CnMods} from '../utils/cn';
 import {useFocusWithin} from '../utils/interactions';
 import {useForkRef} from '../utils/useForkRef';
 import {useSelect} from '../utils/useSelect';
+import {useUniqId} from '../utils/useUniqId';
 
 import {EmptyOptions, SelectControl, SelectFilter, SelectList, SelectPopup} from './components';
 import {DEFAULT_VIRTUALIZATION_THRESHOLD, selectBlock} from './constants';
@@ -76,6 +77,7 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
         disablePortal,
         hasClear = false,
         onClose,
+        id,
     } = props;
     const [mobile] = useMobile();
     const [{filter}, dispatch] = React.useReducer(reducer, initialState);
@@ -86,7 +88,15 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
     const filterRef = React.useRef<SelectFilterRef>(null);
     const listRef = React.useRef<List<FlattenOption>>(null);
     const handleControlRef = useForkRef(ref, controlRef);
-    const {value, open, toggleOpen, handleSelection, handleClearValue} = useSelect({
+    const {
+        value,
+        open,
+        activeIndex,
+        toggleOpen,
+        handleSelection,
+        handleClearValue,
+        setActiveIndex,
+    } = useSelect({
         onUpdate,
         value: propsValue,
         defaultValue,
@@ -96,6 +106,8 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
         onClose,
         onOpenChange,
     });
+    const uniqId = useUniqId();
+    const selectId = id ?? uniqId;
     const options = props.options || getOptionsFromChildren(props.children);
     const flattenOptions = getFlattenOptions(options);
     const filteredFlattenOptions = filterable
@@ -242,6 +254,9 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
                 onKeyDown={handleControlKeyDown}
                 renderControl={renderControl}
                 value={value}
+                popupId={`select-popup-${selectId}`}
+                selectId={`select-${selectId}`}
+                activeIndex={activeIndex}
             />
             <SelectPopup
                 ref={controlWrapRef}
@@ -253,6 +268,7 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
                 disablePortal={disablePortal}
                 virtualized={virtualized}
                 mobile={mobile}
+                id={`select-popup-${selectId}`}
             >
                 {filterable && (
                     <SelectFilter
@@ -281,6 +297,8 @@ export const Select = React.forwardRef<HTMLButtonElement, SelectProps>(function
                         getOptionGroupHeight={getOptionGroupHeight}
                         loading={props.loading}
                         onLoadMore={props.onLoadMore}
+                        selectId={`select-${selectId}`}
+                        onChangeActive={setActiveIndex}
                     />
                 ) : (
                     <EmptyOptions filter={filter} renderEmptyOptions={renderEmptyOptions} />
diff --git a/src/components/Select/components/SelectControl/SelectControl.tsx b/src/components/Select/components/SelectControl/SelectControl.tsx
index e117f687..dbbb2d73 100644
--- a/src/components/Select/components/SelectControl/SelectControl.tsx
+++ b/src/components/Select/components/SelectControl/SelectControl.tsx
@@ -55,6 +55,9 @@ export const SelectControl = React.forwardRef<HTMLButtonElement, ControlProps>((
         disabled,
         value,
         hasClear,
+        popupId,
+        selectId,
+        activeIndex,
     } = props;
     const showOptionsText = Boolean(selectedOptionsContent);
     const showPlaceholder = Boolean(placeholder && !showOptionsText);
@@ -118,6 +121,9 @@ export const SelectControl = React.forwardRef<HTMLButtonElement, ControlProps>((
                 ref,
                 open: Boolean(open),
                 renderClear: (arg) => renderClearIcon(arg),
+                popupId,
+                selectId,
+                activeIndex,
             },
             {value},
         );
@@ -128,9 +134,16 @@ export const SelectControl = React.forwardRef<HTMLButtonElement, ControlProps>((
             <div className={selectControlBlock(controlMods)} role="group">
                 <button
                     ref={ref}
+                    role="combobox"
+                    aria-controls={popupId}
                     className={selectControlButtonBlock(buttonMods, className)}
                     aria-haspopup="listbox"
                     aria-expanded={open}
+                    aria-activedescendant={
+                        activeIndex === undefined
+                            ? undefined
+                            : `${selectId}-list-item-${activeIndex}`
+                    }
                     name={name}
                     disabled={disabled}
                     onClick={toggleOpen}
diff --git a/src/components/Select/components/SelectList/SelectList.scss b/src/components/Select/components/SelectList/SelectList.scss
index 5a436ef5..385d2314 100644
--- a/src/components/Select/components/SelectList/SelectList.scss
+++ b/src/components/Select/components/SelectList/SelectList.scss
@@ -4,6 +4,7 @@
 
 $block: '.#{variables.$ns-new}select-list';
 $popupBlock: '.#{variables.$ns}popup';
+$listBlock: '.#{variables.$ns}list';
 $xl-right-padding: '12px';
 
 #{$block} {
@@ -83,6 +84,20 @@ $xl-right-padding: '12px';
         width: 100%;
     }
 
+    &__item#{$listBlock} {
+        &__item_selected {
+            background: none;
+
+            &:hover {
+                background: var(--g-color-base-simple-hover);
+            }
+        }
+
+        &__item_active {
+            background: var(--g-color-base-simple-hover);
+        }
+    }
+
     &__option {
         box-sizing: border-box;
         display: flex;
diff --git a/src/components/Select/components/SelectList/SelectList.tsx b/src/components/Select/components/SelectList/SelectList.tsx
index 46723169..fbc28935 100644
--- a/src/components/Select/components/SelectList/SelectList.tsx
+++ b/src/components/Select/components/SelectList/SelectList.tsx
@@ -26,6 +26,8 @@ type SelectListProps = {
     virtualized?: boolean;
     loading?: boolean;
     onLoadMore?: () => void;
+    selectId: string;
+    onChangeActive: (index?: number) => void;
 };
 
 const loadingOption = {value: '__SELECT_LIST_ITEM_LOADING__', disabled: true};
@@ -45,12 +47,25 @@ export const SelectList = React.forwardRef<List<FlattenOption>, SelectListProps>
         mobile,
         loading,
         onLoadMore,
+        selectId,
+        onChangeActive,
     } = props;
     const items = React.useMemo(
         () => (loading ? [...flattenOptions, loadingOption] : flattenOptions),
         [flattenOptions, loading],
     );
 
+    const selectedIndexes = React.useMemo(
+        () =>
+            flattenOptions.reduce<number[]>((acc, option, index) => {
+                if ('value' in option && value.includes(option.value)) {
+                    acc.push(index);
+                }
+                return acc;
+            }, []),
+        [flattenOptions, value],
+    );
+
     const optionsHeight = getOptionsHeight({
         options: items,
         getOptionHeight,
@@ -127,6 +142,10 @@ export const SelectList = React.forwardRef<List<FlattenOption>, SelectListProps>
             virtualized={virtualized}
             renderItem={renderItem}
             onItemClick={onOptionClick}
+            selectedItemIndex={selectedIndexes}
+            id={`${selectId}-list`}
+            role="listbox"
+            onChangeActive={onChangeActive}
         />
     );
 });
diff --git a/src/components/Select/components/SelectPopup/SelectPopup.tsx b/src/components/Select/components/SelectPopup/SelectPopup.tsx
index f9cb42a1..ebfcfd32 100644
--- a/src/components/Select/components/SelectPopup/SelectPopup.tsx
+++ b/src/components/Select/components/SelectPopup/SelectPopup.tsx
@@ -24,6 +24,7 @@ export const SelectPopup = React.forwardRef<HTMLDivElement, SelectPopupProps>(
             disablePortal,
             virtualized,
             mobile,
+            id,
         },
         ref,
     ) =>
@@ -49,6 +50,7 @@ export const SelectPopup = React.forwardRef<HTMLDivElement, SelectPopupProps>(
                 restoreFocus
                 restoreFocusRef={controlRef}
                 modifiers={getModifiers({width, disablePortal, virtualized})}
+                id={id}
             >
                 {children}
             </Popup>
diff --git a/src/components/Select/components/SelectPopup/types.ts b/src/components/Select/components/SelectPopup/types.ts
index 62743c3e..a8ed6795 100644
--- a/src/components/Select/components/SelectPopup/types.ts
+++ b/src/components/Select/components/SelectPopup/types.ts
@@ -12,4 +12,5 @@ export type SelectPopupProps = {
     className?: string;
     disablePortal?: boolean;
     virtualized?: boolean;
+    id?: string;
 };
diff --git a/src/components/Select/types.ts b/src/components/Select/types.ts
index d402582b..e0ef0d81 100644
--- a/src/components/Select/types.ts
+++ b/src/components/Select/types.ts
@@ -16,6 +16,9 @@ export type SelectRenderControlProps = {
     ref: React.Ref<HTMLElement>;
     open: boolean;
     renderClear?: (args: SelectRenderClearArgs) => React.ReactNode;
+    popupId: string;
+    selectId: string;
+    activeIndex?: number;
 };
 export type SelectRenderControlOptions = {
     value: SelectProps['value'];
@@ -90,6 +93,7 @@ export type SelectProps<T = any> = QAProps &
             | React.ReactElement<SelectOption<T>, typeof Option>[]
             | React.ReactElement<SelectOptionGroup<T>, typeof OptionGroup>
             | React.ReactElement<SelectOptionGroup<T>, typeof OptionGroup>[];
+        id?: string;
     };
 
 export type SelectOption<T = any> = QAProps &
diff --git a/src/components/utils/useSelect/README.md b/src/components/utils/useSelect/README.md
index 6ca96991..135083a1 100644
--- a/src/components/utils/useSelect/README.md
+++ b/src/components/utils/useSelect/README.md
@@ -23,3 +23,5 @@ React hook used to handle items selection in list
 | open            | `boolean`  | List container visibility state                             |
 | setOpen         | `function` | (deprecated) use toggleOpen. Sets value for `open` property |
 | toggleOpen      | `function` | Inver the value of `open`                                   |
+| activeIndex     | `number`   | Index of active option                                      |
+| setActiveIndex  | `function` | Sets value for `activeIndex` property                       |
diff --git a/src/components/utils/useSelect/useSelect.ts b/src/components/utils/useSelect/useSelect.ts
index af651ce5..ba9a20dd 100644
--- a/src/components/utils/useSelect/useSelect.ts
+++ b/src/components/utils/useSelect/useSelect.ts
@@ -6,6 +6,7 @@ import {useOpenState} from './useOpenState';
 export const useSelect = <T extends unknown>(props: UseSelectProps) => {
     const {value: valueProps, defaultValue = [], multiple, onUpdate} = props;
     const [innerValue, setInnerValue] = React.useState(defaultValue);
+    const [activeIndex, setActiveIndex] = React.useState<number>();
     const value = valueProps || innerValue;
     const uncontrolled = !valueProps;
     const {toggleOpen, ...openState} = useOpenState(props);
@@ -60,6 +61,7 @@ export const useSelect = <T extends unknown>(props: UseSelectProps) => {
 
     return {
         value,
+        activeIndex,
         handleSelection,
         handleClearValue,
         /**
@@ -67,6 +69,7 @@ export const useSelect = <T extends unknown>(props: UseSelectProps) => {
          */
         setOpen: toggleOpen,
         toggleOpen,
+        setActiveIndex,
         ...openState,
     };
 };


Test patch:
diff --git a/src/components/Select/__tests__/Select.filter.test.tsx b/src/components/Select/__tests__/Select.filter.test.tsx
index df0924ed..2fa1be18 100644
--- a/src/components/Select/__tests__/Select.filter.test.tsx
+++ b/src/components/Select/__tests__/Select.filter.test.tsx
@@ -50,13 +50,13 @@ describe('Select filter', () => {
         expect(getByPlaceholderText(FILTER_PLACEHOLDER)).toHaveFocus();
         await user.keyboard('1');
         // 1, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 21, 31
-        expect(getAllByRole('listitem').length).toBe(13);
+        expect(getAllByRole('option').length).toBe(13);
         await user.keyboard('1');
         // 11
-        expect(getAllByRole('listitem').length).toBe(1);
+        expect(getAllByRole('option').length).toBe(1);
         await user.keyboard('1');
         // empty
-        expect(queryAllByRole('listitem').length).toBe(0);
+        expect(queryAllByRole('option').length).toBe(0);
         expect(onFilterChange).toBeCalledTimes(3);
     });
 
@@ -70,7 +70,7 @@ describe('Select filter', () => {
         const selectControl = getByTestId(TEST_QA);
         await user.click(selectControl);
         await user.keyboard('z');
-        expect(queryAllByRole('listitem').length).toBe(0);
+        expect(queryAllByRole('option').length).toBe(0);
         getByTestId(EMPTY_OPTIONS_QA);
     });
 
@@ -85,7 +85,7 @@ describe('Select filter', () => {
         await user.click(selectControl);
         await user.keyboard('[a][b][c][1][2]');
         // filter shouldn`t work due to initialized filterOption
-        expect(queryAllByRole('listitem').length).toBe(40);
+        expect(queryAllByRole('option').length).toBe(40);
     });
 
     test('should filter options even if filter text is empty', async () => {
@@ -100,6 +100,6 @@ describe('Select filter', () => {
         await user.click(selectControl);
         expect(filterOption).toHaveBeenCalled();
         // 10, 20, 30, 40
-        expect(queryAllByRole('listitem').length).toBe(4);
+        expect(queryAllByRole('option').length).toBe(4);
     });
 });