-
Notifications
You must be signed in to change notification settings - Fork 185
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(Alert): Move dismiss button before actions to focus on it first #8266
base: master
Are you sure you want to change the base?
Conversation
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. |
size-limit report 📦
|
e2e tests |
👀 Docs deployed
Commit 7115297 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8266 +/- ##
==========================================
- Coverage 95.58% 95.54% -0.05%
==========================================
Files 404 404
Lines 11612 11641 +29
Branches 3857 3859 +2
==========================================
+ Hits 11099 11122 +23
- Misses 513 519 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
> & | ||
React.AriaAttributes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чисто для того, чтобы у пользователей была возможность добавлять aria аттрибуты на кнопки действия.
Alert
#3042Описание
В задаче #3042 указаны две явные проблемы:
фокус должен попадать на заголовок, а он не попадает
Исходя из рекоммендаций W3C спецификации и других ресурсов ([1], [2]) выходит, что лучше всего иммено фокус на интерактивном элементе, редко когда встречается совет фокусироваться на начале текста, и то, только если текста много.
Наш
Alert
требование фокуса выполняет. В это PR я лишь передвинул кнопку закрытия алерта выше в дереве, чтобы при её наличии фокус был на ней, и пользователь не мог случайно согласиться на какое-то действие.Пожелание из [Enhancement][a11y] Доступность
Alert
#3042 от том, что нужен фокус на заголовке обусловлен тем, что нужно, чтобы скринридер прочитал текст вAlert
при его появлении.NVDA
иVoiceOver
читают текст, так как мы используем внутри связь алерта с заголовком и содержимым чере aria-labelledby и area-describedby.Кнопки выполняют одинаковые действия, но называются по разному.
Долго думал как бы спрятать или предоставить польозователям библиотеки возможность прятать лишние дублирующие кнопки, но после общения со специалистом по доступности понял, что проблема не в дублировании, а в том, что кнопки называются по разному и могут сбивать с толку, если выполняют одно и то же действие.
Решением тут вижу только исправление примера в Storybook и доке и указание в доке о том, что если кнопка закрытия алерта и кнопка действия по сути выполняют одно и то же действие, то имена у них в дереве доступности должны быть одинаковыми.
Решение: если кнопка действия и кнопка закрытия выполняют одинаковые действия их имена в дереве доступности должный быть одинаковыми. В частности кнопка закрытия через свойство
dismissLabel
должно повторять имя одной из кнопок действия.Были мысли проверять есть ли action у кнопки с режимом 'cancel`, но пришел к выводу, что тут можно только хуже сделать и усложнить.
Примера должно быть достаточно. Варианты использования могут быть разными и не зависеть от action mode.
Release notes
Исправления
Кнопка закрытия, при наличии, первой получает фокус при открытии алерта
В документацию по доступности компонента добавлен пункт по поводу имен кнопок с одинаковыми действиями