feat(Touchable, Buttons): Improve touchable and buttons accessibility #1070
Conversation
|
Size stats
|
|
Accessibility report ℹ️ You can run this locally by executing |
| @@ -0,0 +1,9 @@ | |||
| export const redirect = (url: string, external = false, loadOnTop = false): void => { | |||
There was a problem hiding this comment.
moved from touchable to facilitate testing
…mprove-buttons-a11y
|
Deploy preview for mistica-web ready! ✅ Preview Built with commit 27a935f. |
| @@ -377,7 +384,7 @@ const Button = React.forwardRef<TouchableElement, ButtonProps & {type: ButtonTyp | |||
| EndIcon: props.EndIcon, | |||
| }), | |||
| disabled: props.disabled || showSpinner || isFormSending, | |||
| role: 'button', | |||
| role: props.role, | |||
There was a problem hiding this comment.
Shouldn't we default to button in case role was not provided?
There was a problem hiding this comment.
the default is provided by the dom element (button for <button> and link for <a>)
| maybe: true; | ||
| to?: string | Location; | ||
| fullPageOnWebView?: boolean; | ||
| replace?: boolean; | ||
| href?: undefined; | ||
| onPress?: undefined; | ||
| /** with "to", onNavigate will be executed in parallel to the navigation */ |
There was a problem hiding this comment.
As far as I can see, it's not in parallel, right? You wait for the promise to be resolved, and after that you navigate
There was a problem hiding this comment.
what you comment is true in the "href", not for the "to".
The "to" prop will use a <Link> element which produces a client side navigation, so it is ok to perform the onNavigate actions in parallel (this was already happening in current implementation)
| @@ -44,18 +64,13 @@ test('<a> is rendered when using "to" prop', () => { | |||
| </ThemeContextProvider> | |||
| ); | |||
|
|
|||
| const anchor = screen.getByRole('button', {name: 'test'}); | |||
| const anchor = screen.getByRole('link', {name: 'test'}); | |||
There was a problem hiding this comment.
I know this is an expected change, but it will probably break many tests in webapp
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe we could have a TouchableProps<T> generic type to reuse all the onPress/to/href + friend props in any touchable component (Buttons, IconButton, Cards, Rows, etc).
I've created a ticket: https://jira.tid.es/browse/WEB-1817
|
# [15.1.0](v15.0.0...v15.1.0) (2024-04-08) ### Bug Fixes * **IconButton:** prevent interactive area from affecting button layout ([#1069](#1069)) ([f377aac](f377aac)) ### Features * **Accordion,Callout,Cards,EmptyState,Header,Hero,Row,NavigationBar:** added titleAs prop to allow configuring heading level ([#1067](#1067)) ([814c297](814c297)) * **RowList, BoxedRowList, Inline:** support list a11y role ([#1068](#1068)) ([7e2fe37](7e2fe37)), closes [/github.com/Telefonica/mistica-web/blob/master/src/stack.tsx#L64](https:/Telefonica/mistica-web/blob/master/src/stack.tsx/issues/L64) * **skin:** o2 new brand ([#968](#968)) ([56e3945](56e3945)) * **Switch:** add minimum interactive area in touchable devices ([#1063](#1063)) ([fb202e7](fb202e7)) * **Touchable, Buttons:** Improve touchable and buttons accessibility ([#1070](#1070)) ([8d93c71](8d93c71))
|
🎉 This PR is included in version 15.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
https://jira.tid.es/browse/WEB-1794
This PR adds the
onNavigateandroleprops to:TouchableButtonPrimary,ButtonSecondary, etcButtonLinkTextLinkThere are other touchable-like components like
IconButton,Row,Card,Chip... We can add these features when required but I think the scope of this PR is Ok for now