Skip to content

Commit

Permalink
fix(ActionSheet)Make ActionSheet accessible (#6955)
Browse files Browse the repository at this point in the history
- Избавляемся от логики FocusTrap связянную с фокусом на первом интерактивном элементе внутри FocusTrap при условии что элемент был открыт с клавиатуры. Эта логика опирается на хук useKeyboardInputTracker, который слушает события document и устанавливает флаг keyboardInput если поймано событие нажатия клавиатуры, или сбрасывает флаг keyboardInput, если событие было от мышки.
Без этой логики мы всегда будем фокусировать на элементе внутри FocusTrap, но это ничего не ломает. Визуально это не видно. Стоит заметить, что когда FocusTrap опирался на флаг keyboardInput, флаг учитывался только при монтировании компонента, и фокус попадал только если FocusTrap появился от нажатия с клавиатуры, в то же время независимо от keyboardInput при закрытии FocusTrap фокус всегда возвращался на элемент имевший фокус до открытия FocusTrap.

- Убираем логику сбрасывающую фокус с активного элемента в PopoutRoot при открытии ActionSheet, так как это ломает фокус после закрытия ActionSheet, фокус не возвращается на элемент, который был активный до открытия ActionSheet.


Оказалось, что фокус FocusTrap в ActionSheet не работает из NVDA, потому что NVDA вместо keydown события при нажатии Enter генерирует mousedown, а события нажатия стрелочек вообще не долетают до document, что ломает FocusTrap.
В итоге получается, что если пользователь полностью полагается на NVDA, и ни разу не нажал TAB с самой загрузки страницы и пользовался лишь виртуальным буфером скринридера, то фокус в FocusTrap не попадёт и пользователь не сможет взаимодействовать с контентом внутри FocusTrap. Это касается всех компонентов, которые используют FocusTrap: Popover, Alert, ActionSheet, ...
Модалки в то же время работают, потому что у них, не смотря на FocusTrap, своя логика фокуса при появлении и исчезновении.

В задаче указан пример где не доступен список опций. Оказалось, что он реализован не с помощью ActionSheet, а с помощью Popover, где внутрь переданы компоненты типа CellButton. Не смотря на это данный PR также сделает его доступнее, потому что FocusTrap также используется в Popover и имеет такие же проблемы как и ActionSheet.
Но, кроме этого, в реализации ActionSheet на основе Popover необходимо добавить role="dialog". Просто на Popover повесить role="dialog" в рамках библиотеки мы не можем. Хотя почему не можем. Можем, это всегда может сбросить конечный пользователь, если это требуется или перебить другим значением role, а по умолчанию пусть будет role=dialog, скринридерами он лучше всего поддерживается и наличие FocusTrap подразумевает, что пользователь скринридера должен удобно с контентом Popover взаимодействовать.
Изменения


Требования доступности

ActionSheet

Необходимо, чтобы у кнопки, открывающей ActionSheet был аттрибут aria-expanded, а у ActionSheet role="dialog" и aria-modal="true".
role="dialog" нужен, чтобы не дать виртуальному буферу скринридера выйти за пределы ActionSheet. Без этого легко потерятся, особенно если ActionSheet и кнопка по которой он открывается находятся в разных частях DOM дерева.
aria-modal="true" говорит, что нельзя взаимодействовать с другими частями страницы пока не закрыт ActionSheet.

role="dialog" и aria-modal="true" мы выставляем сами, их легко перебить, если нужно, а aria-expanded неободимо выставлять пользователю библиотеки.

Popover

Если Popover по какой-то причине рендерится в портале, далеко в DOM дереве от кнопки по которой он был вызван, и имеет интерактивные элементы, то стоит ему также добавить role="dialog". aria-expanded мы выставляем самостоятельна на элементе, который открыл Popover.
  • Loading branch information
mendrew authored Jun 13, 2024
1 parent 02d78d7 commit 3e4b97e
Show file tree
Hide file tree
Showing 18 changed files with 138 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ export const Base: Story = {
<SplitLayout center popout={popout}>
<SplitCol width="100%" maxWidth="560px" stretchedOnMobile autoSpaced>
<Placeholder stretched>
<Button getRootRef={baseToggleRef} onClick={() => setVisible(true)}>
<Button
getRootRef={baseToggleRef}
onClick={() => setVisible(true)}
aria-expanded={visible}
>
Открыть
</Button>
</Placeholder>
Expand Down
29 changes: 25 additions & 4 deletions packages/vkui/src/components/ActionSheet/ActionSheet.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ const ActionSheetDesktop = ({ onClose = jest.fn(), ...restProps }: Partial<Actio
return (
<ConfigProvider platform="vkcom">
<AdaptivityProvider viewWidth={ViewWidth.DESKTOP} hasPointer>
<ActionSheet toggleRef={toggleRef} onClose={onClose} {...restProps} />
<ActionSheet
aria-label="menu list"
toggleRef={toggleRef}
onClose={onClose}
{...restProps}
/>
</AdaptivityProvider>
</ConfigProvider>
);
Expand All @@ -35,7 +40,7 @@ const ActionSheetMobile = ({ onClose = jest.fn(), ...restProps }: Partial<Action
}, []);
return (
<AdaptivityProvider viewWidth={ViewWidth.MOBILE} hasPointer={false}>
<ActionSheet toggleRef={toggleRef} onClose={onClose} {...restProps} />
<ActionSheet aria-label="menu list" toggleRef={toggleRef} onClose={onClose} {...restProps} />
</AdaptivityProvider>
);
};
Expand All @@ -45,15 +50,31 @@ const ActionSheetMenu = ({ onClose = jest.fn(), ...restProps }: Partial<ActionSh
React.useLayoutEffect(() => {
setToggleRef(screen.getByTestId('target'));
}, []);
return <ActionSheet mode="menu" toggleRef={toggleRef} onClose={onClose} {...restProps} />;
return (
<ActionSheet
aria-label="menu list"
mode="menu"
toggleRef={toggleRef}
onClose={onClose}
{...restProps}
/>
);
};

const ActionSheetSheet = ({ onClose = jest.fn(), ...restProps }: Partial<ActionSheetProps>) => {
const [toggleRef, setToggleRef] = React.useState<HTMLElement | null>(null);
React.useLayoutEffect(() => {
setToggleRef(screen.getByTestId('target'));
}, []);
return <ActionSheet mode="sheet" toggleRef={toggleRef} onClose={onClose} {...restProps} />;
return (
<ActionSheet
aria-label="menu list"
mode="sheet"
toggleRef={toggleRef}
onClose={onClose}
{...restProps}
/>
);
};

describe(ActionSheet, () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/vkui/src/components/ActionSheet/ActionSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export const ActionSheet = ({
<DropdownComponent
closing={Boolean(closingBy)}
timeout={timeout}
role="dialog"
aria-modal="true"
{...dropdownProps}
onClose={onClose}
className={mode === 'menu' ? className : undefined}
Expand Down
74 changes: 61 additions & 13 deletions packages/vkui/src/components/ActionSheet/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,27 @@ ActionSheet – имитация [нативного компонента](https
> - Согласно гайдлайнам Apple, в `ActionSheet` должен быть элемент для закрытия, для этого предусмотрен атрибут `iosCloseItem`. По умолчанию будет использоваться `ActionSheetDefaultIosCloseItem`.
> Для Android версии он не нужен.
## Цифровая доступность (a11y)

Старайтесь сопровождать элемент текстовым описанием для корректной работы скринридеров. Для этого
необходимо вручную передавать некоторые параметры, такие как `aria-label`.

- У всплывающего элемента уже заданы атрибуты `role="dialog"` и `aria-modal="true"`, потому что по умолчанию он как раз появляется как диалог и, пока он не закроется, нельзя взаимодействовать с другими элементами на странице. Эти аттрибуты можно перебить явно передавая их компоненту.
- У целевого элемента обязательно должен быть выставлен атрибут `aria-expanded`.

```jsx { "props": { "layout": false, "adaptivity": true } }
const [popout, setPopout] = useState(null);
const onClose = () => setPopout(null);
const [openedPopoutName, setOpenedPopoutName] = useState(null);

const openActionSheet = (name, popout) => {
setPopout(popout);
setOpenedPopoutName(name);
};
const onClose = () => {
setPopout(null);
setOpenedPopoutName(null);
};

const [filter, setFilter] = useState('best');
const onChange = (e) => setFilter(e.target.value);
const baseTargetRef = React.useRef(null);
Expand All @@ -24,7 +42,8 @@ const titleTargetRef = React.useRef(null);
const baseTopTargetRef = React.useRef(null);

const openBase = () =>
setPopout(
openActionSheet(
'base',
<ActionSheet onClose={onClose} toggleRef={baseTargetRef}>
<ActionSheetItem>Сохранить в закладках</ActionSheetItem>
<ActionSheetItem>Закрепить запись</ActionSheetItem>
Expand All @@ -35,7 +54,8 @@ const openBase = () =>
);

const openIcons = () =>
setPopout(
openActionSheet(
'icons',
<ActionSheet onClose={onClose} toggleRef={iconsTargetRef}>
<ActionSheetItem
before={
Expand Down Expand Up @@ -83,7 +103,8 @@ const openIcons = () =>
);

const openSubtitle = () =>
setPopout(
openActionSheet(
'subtitle',
<ActionSheet onClose={onClose} toggleRef={subtitleTargetRef}>
<ActionSheetItem
before={
Expand Down Expand Up @@ -120,7 +141,8 @@ const openSubtitle = () =>
);

const openSelectable = () =>
setPopout(
openActionSheet(
'selectable',
<ActionSheet onClose={onClose} toggleRef={selectableTargetRef}>
<ActionSheetItem
onChange={onChange}
Expand Down Expand Up @@ -171,7 +193,8 @@ const openSelectable = () =>
);

const openTitle = () =>
setPopout(
openActionSheet(
'title',
<ActionSheet
onClose={onClose}
header="Вы действительно хотите удалить это видео из Ваших видео?"
Expand All @@ -182,7 +205,8 @@ const openTitle = () =>
);

const openBaseTop = () =>
setPopout(
openActionSheet(
'baseTop',
<ActionSheet onClose={onClose} toggleRef={baseTopTargetRef} placement="top-end">
<ActionSheetItem>Сохранить в закладках</ActionSheetItem>
<ActionSheetItem>Закрепить запись</ActionSheetItem>
Expand All @@ -200,22 +224,46 @@ React.useEffect(openBase, []);
<Panel id="panel">
<PanelHeader>ActionSheet</PanelHeader>
<Group>
<CellButton getRootRef={baseTargetRef} onClick={openBase}>
<CellButton
getRootRef={baseTargetRef}
onClick={openBase}
aria-expanded={'base' === openedPopoutName}
>
Базовый список
</CellButton>
<CellButton getRootRef={iconsTargetRef} onClick={openIcons}>
<CellButton
getRootRef={iconsTargetRef}
onClick={openIcons}
aria-expanded={'icons' === openedPopoutName}
>
Список с иконками
</CellButton>
<CellButton getRootRef={subtitleTargetRef} onClick={openSubtitle}>
<CellButton
getRootRef={subtitleTargetRef}
onClick={openSubtitle}
aria-expanded={'subtitle' === openedPopoutName}
>
С подзаголовком
</CellButton>
<CellButton getRootRef={selectableTargetRef} onClick={openSelectable}>
<CellButton
getRootRef={selectableTargetRef}
onClick={openSelectable}
aria-expanded={'selectable' === openedPopoutName}
>
Выделяемые
</CellButton>
<CellButton getRootRef={titleTargetRef} onClick={openTitle}>
<CellButton
getRootRef={titleTargetRef}
onClick={openTitle}
aria-expanded={'title' === openedPopoutName}
>
C заголовком
</CellButton>
<CellButton getRootRef={baseTopTargetRef} onClick={openBaseTop}>
<CellButton
getRootRef={baseTopTargetRef}
onClick={openBaseTop}
aria-expanded={'baseTop' === openedPopoutName}
>
Базовый список, открывается наверх на десктопах
</CellButton>
</Group>
Expand Down
5 changes: 4 additions & 1 deletion packages/vkui/src/components/Alert/Alert.e2e-playground.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const baseRender = (props: AlertProps) => (
header="Подтвердите действие"
text="Вы уверены, что хотите лишить пользователя права на модерацию контента?"
style={{ position: 'relative' }}
autoFocus={false}
{...props}
/>
);
Expand Down Expand Up @@ -89,7 +90,9 @@ export const AlertLongWordPlayground = (props: ComponentPlaygroundProps) => {
]}
AppWrapper={AppWrapper}
>
{(props: AlertProps) => <Alert {...props} style={{ position: 'relative' }} />}
{(props: AlertProps) => (
<Alert {...props} autoFocus={false} style={{ position: 'relative' }} />
)}
</ComponentPlayground>
);
};
4 changes: 2 additions & 2 deletions packages/vkui/src/components/FocusTrap/FocusTrap.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ describe(FocusTrap, () => {
expect(screen.getByTestId('sheet')).toBeInTheDocument();
});

it('does not focus first element by default', async () => {
it('focuses first element by default', async () => {
render(<ActionSheetTest />);
await mountActionSheetViaClick();

expect(screen.getByTestId('first')).not.toHaveFocus();
expect(screen.getByTestId('first')).toHaveFocus();
});

it('always calls passed onClose on ESCAPE press', async () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/vkui/src/components/FocusTrap/FocusTrap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
} from '../../lib/dom';
import { useIsomorphicLayoutEffect } from '../../lib/useIsomorphicLayoutEffect';
import { HasComponent, HasRootRef } from '../../types';
import { AppRootContext } from '../AppRoot/AppRootContext';

const FOCUSABLE_ELEMENTS: string = FOCUSABLE_ELEMENTS_LIST.join();
export interface FocusTrapProps<T extends HTMLElement = HTMLElement>
Expand Down Expand Up @@ -37,7 +36,6 @@ export const FocusTrap = <T extends HTMLElement = HTMLElement>({
}: FocusTrapProps<T>) => {
const ref = useExternRef<T>(getRootRef);

const { keyboardInput } = React.useContext(AppRootContext);
const focusableNodesRef = React.useRef<HTMLElement[]>([]);

useIsomorphicLayoutEffect(
Expand Down Expand Up @@ -67,9 +65,10 @@ export const FocusTrap = <T extends HTMLElement = HTMLElement>({

useIsomorphicLayoutEffect(
function tryToAutoFocusToFirstNode() {
if (!ref.current || !autoFocus || !keyboardInput) {
if (!ref.current || !autoFocus) {
return;
}

const autoFocusToFirstNode = () => {
if (!ref.current || !focusableNodesRef.current.length) {
return;
Expand All @@ -84,7 +83,7 @@ export const FocusTrap = <T extends HTMLElement = HTMLElement>({
clearTimeout(timeoutId);
};
},
[autoFocus, timeout, keyboardInput],
[autoFocus, timeout],
);

useIsomorphicLayoutEffect(
Expand Down
1 change: 1 addition & 0 deletions packages/vkui/src/components/ModalRoot/ModalRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ class ModalRootTouchComponent extends React.Component<
isPage && modalState.expandable && 'vkuiInternalModalRoot__modal--expandable',
isPage && modalState.collapsed && 'vkuiInternalModalRoot__modal--collapsed',
)}
autoFocus={false}
restoreFocus={false}
>
{Modal}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export const ModalRootDesktop = ({

return (
<FocusTrap
autoFocus={false}
restoreFocus={false}
onClose={onExit}
timeout={timeout}
Expand Down
7 changes: 0 additions & 7 deletions packages/vkui/src/components/PopoutRoot/PopoutRoot.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from 'react';
import { classNames } from '@vkontakte/vkjs';
import { blurActiveElement, useDOM } from '../../lib/dom';
import { HTMLAttributesWithRootRef } from '../../types';
import { AppRootPortal } from '../AppRoot/AppRootPortal';
import { RootComponent } from '../RootComponent/RootComponent';
Expand Down Expand Up @@ -35,12 +34,6 @@ export interface PopoutRootProps extends HTMLAttributesWithRootRef<HTMLDivElemen
* @private
*/
export const PopoutRoot = ({ popout, modal, children, ...restProps }: PopoutRootProps) => {
const { document } = useDOM();

React.useEffect(() => {
popout && blurActiveElement(document);
}, [document, popout]);

return (
<RootComponent {...restProps} baseClassName={styles['PopoutRoot']}>
{children}
Expand Down
4 changes: 2 additions & 2 deletions packages/vkui/src/components/Popover/Popover.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const Example: Story = {
noStyling
trigger="click"
id="menupopup"
role="menu"
role="dialog"
aria-labelledby="menubutton"
content={({ onClose }) => (
<Group>
Expand All @@ -93,7 +93,7 @@ export const Example: Story = {
</Group>
)}
>
<Button id="menubutton" aria-controls="menupopup" aria-haspopup="true" mode="outline">
<Button id="menubutton" aria-controls="menupopup" mode="outline">
Нажми на меня
</Button>
</Popover>
Expand Down
10 changes: 5 additions & 5 deletions packages/vkui/src/components/Popover/Popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import styles from './Popover.module.css';

describe(Popover, () => {
baselineComponent((props) => (
<Popover defaultShown {...props}>
<div>Test</div>
<Popover defaultShown {...props} aria-label="меню базового компонента">
<button aria-label="greetings dialog">Test</button>
</Popover>
));

Expand All @@ -33,11 +33,11 @@ describe(Popover, () => {
<Popover
shown={shown}
id="menu"
role="menu"
role="dialog"
aria-labelledby="target"
content={<div role="menuitem">1</div>}
content={<button>1</button>}
>
<div id="target" aria-haspopup="true" aria-controls="menu" data-testid="target">
<div id="target" aria-controls="menu" data-testid="target">
Target
</div>
</Popover>
Expand Down
2 changes: 1 addition & 1 deletion packages/vkui/src/components/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export const Popover = ({
noStyling = false,
zIndex = 'var(--vkui--z_index_popout)',
// a11y
role,
role = 'dialog',
...restPopoverProps
}: PopoverProps) => {
const [arrowRef, setArrowRef] = React.useState<HTMLDivElement | null>(null);
Expand Down
Loading

0 comments on commit 3e4b97e

Please sign in to comment.