Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Select): make Select accessible for screen readers #6087

Merged
merged 63 commits into from
Nov 23, 2023

Conversation

mendrew
Copy link
Contributor

@mendrew mendrew commented Nov 9, 2023


  • Unit-тесты
  • e2e-тесты
  • a11y-ревью (стенд https://pspn9h.csb.app/. Артём уже аппрувил, но я попросил ещё раз на всякий случай ещё раз аппрувнул)
  • Документация фичи

Описание

Добавляем aria-аттрибуты CustomSelect'у, следуя паттерну Combobox.
Как для обычного режима селекта, так и для searchable режима, когда в инпут можно вводить текст для фильтрации опций.

Изменения

  • добавлены aria-аттрибуты для CustomSelect, следуя паттерну Combobox
  • исправлена работа onBlur/onFocus пропов. Начиная с react17 react больше не реагирует на dispatch кастомных событий blur/focus и мы должны создавать события focusin focusout.
  • вместо попеременного использования Input и SelectMimicry мы теперь всегда используем внутренний компонент CustomSelectInput. Это позволило избавиться от множества проблем и блокеров для поддержки доступности.
    • ушла проблема с потерей фокуса при выборе опции в режиме searchable
    • при фокусе на селекте в режиме searchable сразу доступен ввод с клавиатуры для поиска опций, причём скрин ридер правильно зачитывает информацию о селект и выбранной опции если она есть, либо плейсхолдер.
    • стал из коробки работать фокус на селекте при клике на связанный с селектом лэйбл.
    • для связи с лэйблом достаточно просто задать id селекта в htmlFor аттрибуте лэйбла. Без инпута для этого надо было бы ещё и прокидывать связь через aria-labelledby.
  • инпут имеет свойство autoComplete="off", потому что хром начинает предлагать свои варианты для заполнения инпута и мешает взаимодействовать с открытым списком.
  • новый компонент (CustomSelectInput) был добавлен ввиду того, что невозможно было переиспользовать имеющийся Input или SelectMimicry без внесений лишних изменений. Тем не менее CustomSelectInput продолжает внутри себя использовать FormField и заимствует часть свойство, имеющихся у Input и SelectMimicry.
    В данный момент я не собираюсь переделывать SelectMimicry в рамках этого PR, но не против попробовать сделать это в будущем.
  • после тестирования селекта специалистом по доступности, было принято решение убрать обёртку компонентом label из CustomSelect и NativeSelect, потому что это заставляет скринридер зачитывать текст дважды при навигации с помощью стрелок, если у селекта выбранна опция. Не всегда, лишь в некоторых версиях Хром, но поведение стабильное.
  • тем не менее мы оставили поведения подобное label с помощью дополнительного кода в CustomSelect. В NativeSelect дополнительного кода не требуется, потому что нативный селект растянут и лежит поверх визуальных элементов и гарантирует фокус на селекте при клике по любой области внутри компонента NativeSelect.
  • исправлено прыгающее поведение фокуса на опции при навигации с клавиатуры, если на одну из опций наведён курсор.
    onMouseDown: handleOptionDown,
    // Используем `onMouseMove` вместо `onMouseEnter/onMouseOver`.
    // Потому что если при навигации с клавиатуры курсор наведён на
    // список, то при первом автоматическом скролле списка вызывается событие MouseOver/MouseEnter
    // обработчик которого фокусирует опцию под курсором, хотя при навигация с клавиатуры пользователь мог уйти дальше по списку, это путает.
    // Причём координаты события меняются на пару пикселей по сравнению с прошлым вызовом,
    // а значит нельзя на них опираться, чтобы запретить обработку такого события.
    // C mousemove такой проблемы нет, что позволяет реализовать поведение при наведении с клавиатуры и при наведении мышью идентично нативному селекту.
    onMouseMove: focusOptionOnMouseMove,
  • в документацию по Select/CustomSelect добавлены разделы про доступность с рекомендацией использования label и связывания label с селект.
  • обновлены примеры документации, FormItem лэйблы теперь связаны с селектами, для демонстрации предпочтительного использования.

@github-actions github-actions bot added ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча v5 Автоматизация: PR продублируется в ветку v5 labels Nov 9, 2023
@mendrew mendrew removed the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 9, 2023
Copy link
Contributor

github-actions bot commented Nov 9, 2023

size-limit report 📦

Path Size
JS 344.68 KB (+0.66% 🔺)
JS (gzip) 105.27 KB (+0.64% 🔺)
JS (brotli) 87.05 KB (+0.69% 🔺)
JS import Div (tree shaking) 2.71 KB (0%)
CSS 259.25 KB (+1.09% 🔺)
CSS (gzip) 33.87 KB (+0.72% 🔺)
CSS (brotli) 27.53 KB (+0.86% 🔺)

Copy link
Contributor

github-actions bot commented Nov 9, 2023

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented Nov 9, 2023

👀 Docs deployed

Commit 952e1ec

@mendrew mendrew force-pushed the mendrew/fix/CustomSelect/3547/a11y-for-custom-select branch from eaea297 to 28897ff Compare November 9, 2023 12:16
@github-actions github-actions bot added the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 9, 2023
Copy link

codesandbox-ci bot commented Nov 9, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 952e1ec:

Sandbox Source
VKUI TypeScript Configuration

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (9d47c34) 79.13% compared to head (952e1ec) 79.17%.

Files Patch % Lines
.../vkui/src/components/CustomSelect/CustomSelect.tsx 77.38% 19 Missing ⚠️
.../src/components/CustomSelect/CustomSelectInput.tsx 96.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6087      +/-   ##
==========================================
+ Coverage   79.13%   79.17%   +0.04%     
==========================================
  Files         312      313       +1     
  Lines        9957    10018      +61     
  Branches     3332     3352      +20     
==========================================
+ Hits         7879     7932      +53     
- Misses       2078     2086       +8     
Flag Coverage Δ
unittests 79.17% <83.60%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mendrew mendrew removed the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 9, 2023
@github-actions github-actions bot added the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 10, 2023
@mendrew mendrew force-pushed the mendrew/fix/CustomSelect/3547/a11y-for-custom-select branch from 4ff2e40 to ca60b21 Compare November 21, 2023 17:37
@mendrew mendrew removed the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 21, 2023
@mendrew mendrew marked this pull request as ready for review November 21, 2023 17:47
@mendrew mendrew requested a review from a team as a code owner November 21, 2023 17:47
@github-actions github-actions bot added the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 21, 2023
@mendrew mendrew removed the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 21, 2023
@github-actions github-actions bot added the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 22, 2023
Copy link
Contributor

@eugpoloz eugpoloz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Андрей, ты 🚀 Огромная работа проделана!

@mendrew mendrew requested review from eugpoloz and a team November 22, 2023 12:01
Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️‍🔥❤️‍🔥❤️‍🔥

Group aria-attributes
Improve focus on SelectMimicry in searchable mode
Improve FromItem to use together with Select/CustomSelect

Update doc for FormItem to recommend to use more attributes to
correctly connect FormItem with Select

Use a11y attributes in CustomSelect examples

Update Select and NativeSelect examples to include a11y

Update links in the FormItem doc

Add a11y doc to Select and CustomSelect
@mendrew mendrew force-pushed the mendrew/fix/CustomSelect/3547/a11y-for-custom-select branch from 1d04e7d to fcf39dc Compare November 23, 2023 12:04
@mendrew
Copy link
Contributor Author

mendrew commented Nov 23, 2023

Спасибо за ревью! 👍
@eugpoloz изменения после твоего последнего ревью: 500b902...952e1ec

@mendrew
Copy link
Contributor Author

mendrew commented Nov 23, 2023

Спасибо за ревью! 💪
@inomdzhon изменения после твоего последнего ревью: d416279...952e1ec

@mendrew
Copy link
Contributor Author

mendrew commented Nov 23, 2023

Спасибо за ревью! 💪 ✍️
@SevereCloud постарался тут собрать все коммиты по твоим комментариям: fcf39dc...952e1ec

@mendrew mendrew requested review from SevereCloud, inomdzhon and eugpoloz and removed request for eugpoloz November 23, 2023 13:02
@mendrew mendrew removed the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 23, 2023
@mendrew mendrew merged commit ec3a56d into master Nov 23, 2023
@mendrew mendrew deleted the mendrew/fix/CustomSelect/3547/a11y-for-custom-select branch November 23, 2023 15:08
vkcom-publisher pushed a commit that referenced this pull request Nov 23, 2023
…ders (#6087)

- close #3547

Описание
Добавляем `aria`-аттрибуты `CustomSelect`'у, следуя паттерну [Combobox](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/).
Как для обычного режима селекта, так и для `searchable` режима, когда в инпут можно вводить текст для фильтрации опций.

 Изменения
- добавлены aria-аттрибуты для CustomSelect, следуя паттерну [Combobox](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/)
- исправлена работа `onBlur/onFocus` пропов. Начиная с react17 react больше не реагирует на dispatch кастомных событий `blur/focus` и мы должны создавать события `focusin` `focusout`.
- вместо попеременного использования `Input` и `SelectMimicry` мы теперь всегда  используем внутренний компонент `CustomSelectInput`. Это позволило избавиться от множества проблем и блокеров для поддержки доступности.
  - ушла проблема с потерей фокуса при выборе опции в режиме `searchable`
  - при фокусе на селекте в режиме `searchable` сразу доступен ввод с клавиатуры для поиска опций, причём скрин ридер правильно зачитывает информацию о селект и выбранной опции если она есть, либо плейсхолдер.
  - стал из коробки работать фокус на селекте при клике на связанный с селектом лэйбл.
  - для связи с лэйблом достаточно просто задать `id` селекта в `htmlFor` аттрибуте лэйбла. Без инпута для этого надо было бы ещё и прокидывать связь через `aria-labelledby`.
- инпут имеет свойство `autoComplete="off"`, потому что хром начинает предлагать свои варианты для заполнения инпута и мешает взаимодействовать с открытым списком.
- новый компонент (`CustomSelectInput`) был добавлен ввиду того, что невозможно было переиспользовать имеющийся `Input` или `SelectMimicry` без внесений лишних изменений. Тем не менее `CustomSelectInput` продолжает внутри себя использовать `FormField` и заимствует часть свойство, имеющихся у `Input` и `SelectMimicry`.
  В данный момент я не собираюсь переделывать `SelectMimicry` в рамках этого PR, но не против попробовать сделать это в будущем.
- после тестирования селекта специалистом по доступности, было принято решение убрать обёртку компонентом `label` из `CustomSelect` и `NativeSelect`, потому что это заставляет скринридер зачитывать текст дважды при навигации с помощью стрелок, если у селекта выбранна опция. Не всегда, лишь в некоторых версиях Хром, но поведение стабильное.
- тем не менее мы оставили поведения подобное `label` с помощью дополнительного кода в CustomSelect. В NativeSelect дополнительного кода не требуется, потому что нативный селект растянут и лежит поверх визуальных элементов и гарантирует фокус на селекте при клике по любой области внутри компонента NativeSelect.
- исправлено прыгающее поведение фокуса на опции при навигации с клавиатуры, если на одну из опций наведён курсор.
https://github.com/VKCOM/VKUI/blob/9bae73ff784eaf7f6de096e8b7f56485f2e9cb50/packages/vkui/src/components/CustomSelect/CustomSelect.tsx#L677-L685
- в документацию по `Select/CustomSelect` добавлены разделы про доступность с рекомендацией использования label и связывания label с селект.
- обновлены примеры документации, FormItem лэйблы теперь связаны с селектами, для демонстрации предпочтительного использования.
andrey-medvedev-vk added a commit that referenced this pull request Oct 24, 2024
…7816)

На touch устройстве не работает фокус на input при клике в зоне инпута ближе к стрелочке. Фокус есть на обертке, но сам инпут фокуса не имеет и клавиатура не появляется, как при клике на сам инпут, ближе к левому краю.
Воспроизводится в симуляторе Iphone и на реальном устройстве.

Дело в том, что не во всей видимой области инпута событие клика принимает input, и чтобы это победить мы программно вызываем фокус на инпуте.

Изначально, в #6087, а конкретно в 0f45bb9 это было реализовано с помощью отложенного вызова фокуса для кнопки очистки, а потом функция с отложенным фокусом перекочевала и на остальной новый код.

Объяснялось это тем, что без таймаута фокус просто не работал при клике на clear button.
Финальный код был сложнее, чем тот, когда этот отложенный фокус был добавлен, и он уже мог работать без отложенного вызова фокуса.

Протестировал в браузере и в симуляторе Iphone.
vkcom-publisher pushed a commit that referenced this pull request Oct 24, 2024
)

На touch устройстве не работает фокус на input при клике в зоне инпута ближе к стрелочке. Фокус есть на обертке, но сам инпут фокуса не имеет и клавиатура не появляется, как при клике на сам инпут, ближе к левому краю.
Воспроизводится в симуляторе Iphone и на реальном устройстве.

Дело в том, что не во всей видимой области инпута событие клика принимает input, и чтобы это победить мы программно вызываем фокус на инпуте.

Изначально, в #6087, а конкретно в 0f45bb9 это было реализовано с помощью отложенного вызова фокуса для кнопки очистки, а потом функция с отложенным фокусом перекочевала и на остальной новый код.

Объяснялось это тем, что без таймаута фокус просто не работал при клике на clear button.
Финальный код был сложнее, чем тот, когда этот отложенный фокус был добавлен, и он уже мог работать без отложенного вызова фокуса.

Протестировал в браузере и в симуляторе Iphone.
andrey-medvedev-vk added a commit that referenced this pull request Oct 24, 2024
…7816)

На touch устройстве не работает фокус на input при клике в зоне инпута ближе к стрелочке. Фокус есть на обертке, но сам инпут фокуса не имеет и клавиатура не появляется, как при клике на сам инпут, ближе к левому краю.
Воспроизводится в симуляторе Iphone и на реальном устройстве.

Дело в том, что не во всей видимой области инпута событие клика принимает input, и чтобы это победить мы программно вызываем фокус на инпуте.

Изначально, в #6087, а конкретно в 0f45bb9 это было реализовано с помощью отложенного вызова фокуса для кнопки очистки, а потом функция с отложенным фокусом перекочевала и на остальной новый код.

Объяснялось это тем, что без таймаута фокус просто не работал при клике на clear button.
Финальный код был сложнее, чем тот, когда этот отложенный фокус был добавлен, и он уже мог работать без отложенного вызова фокуса.

Протестировал в браузере и в симуляторе Iphone.
andrey-medvedev-vk added a commit that referenced this pull request Oct 24, 2024
…7816) (#7825)

На touch устройстве не работает фокус на input при клике в зоне инпута ближе к стрелочке. Фокус есть на обертке, но сам инпут фокуса не имеет и клавиатура не появляется, как при клике на сам инпут, ближе к левому краю.
Воспроизводится в симуляторе Iphone и на реальном устройстве.

Дело в том, что не во всей видимой области инпута событие клика принимает input, и чтобы это победить мы программно вызываем фокус на инпуте.

Изначально, в #6087, а конкретно в 0f45bb9 это было реализовано с помощью отложенного вызова фокуса для кнопки очистки, а потом функция с отложенным фокусом перекочевала и на остальной новый код.

Объяснялось это тем, что без таймаута фокус просто не работал при клике на clear button.
Финальный код был сложнее, чем тот, когда этот отложенный фокус был добавлен, и он уже мог работать без отложенного вызова фокуса.

Протестировал в браузере и в симуляторе Iphone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v5 Автоматизация: PR продублируется в ветку v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug][a11y][Select]: Скринридер игнорирует Select
4 participants