Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Added data-* and aria-* attributes #40

Merged
merged 2 commits into from
Apr 23, 2018
Merged

Conversation

rmarren1
Copy link
Contributor

* A wild-card data-* attribute
*/
'data-*': PropTypes.string,

Copy link
Member

Choose a reason for hiding this comment

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

Let's add aria-* too

@rmarren1 rmarren1 changed the title Added data-* attribute to all components Added data-* and aria-* attributes Apr 4, 2018
@ngnpope
Copy link

ngnpope commented Apr 10, 2018

Hi @rmarren1,

I've also not been able to use the role attribute with html.A():

TypeError: Unexpected keyword argument `role`

This attribute is required for use with aria-* attributes, so it would be good if it were supported.

@rmarren1
Copy link
Contributor Author

Are there any elements where role should not be allowed? It is does not appear in https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes which is why it is currently not supported, but it could be hard-coded into the generation script if it applies to all components.

@ngnpope
Copy link

ngnpope commented Apr 10, 2018

Can be applied to any element:

Other documents of interest:

Hard-coding this seems sensible if that is already being done for aria-* as part of this PR.

For background, I bumped into this because of using Bootstrap which requires data-* for some of the scripted components. Have worked around the issue in the meantime client-side by using a mutation observer and adding the attributes as the elements appear.

@rmarren1
Copy link
Contributor Author

added role

@ngnpope
Copy link

ngnpope commented Apr 10, 2018

Thanks!

@chriddyp
Copy link
Member

@rmarren1 - I'm 👍 with these changes. Is there anything else that you would like to add? Otherwise, I will merge and release.

@rmarren1
Copy link
Contributor Author

I'm good with it, go ahead!

@chriddyp chriddyp merged commit 9aa7c8f into plotly:master Apr 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants