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

Revisit implementation of accessibilityRole="button" #1899

Closed
necolas opened this issue Feb 7, 2021 · 9 comments
Closed

Revisit implementation of accessibilityRole="button" #1899

necolas opened this issue Feb 7, 2021 · 9 comments
Labels
has: pr Subject of a pull request project:react-native-web Issue associated with react-native-web

Comments

@necolas
Copy link
Owner

necolas commented Feb 7, 2021

Is your feature request related to a problem? Please describe.

Revisit mapping accessibilityRole="button" to button element.

The current approach uses ARIA roles to achieve similar results due to historical problems in Firefox with flexbox children of a button. But this was fixed in Firefox 63 and the current ESR version is 78, so we may be able to migrate away from this special case and map "button" to an element like we do for several other roles. (FB web still supports FF versions prior to 63 but no one performs visual tests that far back so I don't think we need to be too concerned about it.)

This shouldn't produce any savings in bytes since the code used to implement ARIA-backed buttons is also used by the unstable createElement API and for other ARIA roles that should have similar behavior to buttons (i.e., respond to user interaction).

@doumart

This comment has been minimized.

@intergalacticspacehighway
Copy link
Contributor

@necolas What should be the case for <View accessibilityRole="button" />?
Should this renderd as <div role="button" /> or <button />.
Got this as a failing test case while implementing the above change.

@georgii-ivanov
Copy link

@intergalacticspacehighway what's the problem with <View accessibilityRole="button" /> cuz at this point you'll get button tag and you can put another View/Text element into it as HTML allows you to put something else besides text node.

I think that would be a useful improvement to HTML semantics. Also probably useful for SEO (not sure about that tho)

@jgornick
Copy link

We ran into this issue because our a11y rules require semantic HTML rather than using a11y attributes (e.g. aria-*). In our case we needed to produce a <button> when using the <Button> component. However, we learned that once that goes through <TouchableOpacity> to <View> to createElement and finally propsToAccessibilityComponent, we noticed that it wasn't mapping a role of button to a <button> a11y component.

So, we started with @intergalacticspacehighway's PR #1944 implementation, but because not everyone shares our same a11y rules and may not want to map roles to semantic HTML, we short-circuited the <Button> component rendering to use createElement('button', ...). That way others could still use a <View> (<div>) with an a11y role of button.

It brought up a question of whether react-native-web should be prescriptive in converting a11y roles to semantic HTML? There is great value in using semantic HTML, so I understand why it does today. However, that might not fit everyone's need. It would be interesting to see if there is a way for components to opt-in/out of the conversion from a11y role to semantic HTML.

After all this being said and based on current react-native-web behavior to map a11y roles to semantic HTML, I think @intergalacticspacehighway's PR #1944 does what's needed to support our direct need to mapping a11y role button to <button>.

@necolas
Copy link
Owner Author

necolas commented Apr 13, 2021

Your accessibility rule requiring a semantic tag doesn't surface a functional problem for users.

@jgornick
Copy link

Your accessibility rule requiring a semantic tag doesn't surface a functional problem for users.

Great point.

With that said, then does it make sense to map the button a11y role to the button tag? If so, does #1944 address that change?

Thanks @necolas!

@antoniojps
Copy link

What's stopping #1944 from being merged?

According to mozilla best practises regarding this issue.

Where possible, it is recommended to use native HTML buttons (, , , and ) rather than the button role, as native HTML buttons are supported by all user agents and assistive technology and provide keyboard and focus requirements by default, without need for additional customization.

@p4bl1t0
Copy link

p4bl1t0 commented Jun 11, 2021

Also this is stopping the availibity of using features like this: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share

Because they need "Must be triggered some kind of "user activation""

@necolas
Copy link
Owner Author

necolas commented Jun 11, 2021

Also this is stopping the availibity of using features like this: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share

No it isn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: pr Subject of a pull request project:react-native-web Issue associated with react-native-web
Projects
None yet
Development

No branches or pull requests

7 participants