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

Add select eslint-plugin-jsx-a11y rules to lint config. #175

Merged
merged 1 commit into from
Jul 25, 2016
Merged

Add select eslint-plugin-jsx-a11y rules to lint config. #175

merged 1 commit into from
Jul 25, 2016

Conversation

beefancohen
Copy link
Contributor

This project is a really good use case to apply some accessibility rules to the lint config. For beginners getting started with React and/or web in general, we can also teach & enforce basic accessibility rules in the web at author time.

For now, I’ve just applied to the rules that are listed in eslint-config-airbnb because these are most real-world tested, and we can continue to add as you see fit.

I know from Twitter that you're wary about adding more dependencies, but very happy to help if there are any questions/issues/concerns about the plugin!

This project is a really good use case to apply some accessibility
rules to the lint config. For beginners getting started with React, we
can also teach/enforce basic accessibility rules in the web at author
time.

For now, I’ve just applied to the rules that are listed in
`eslint-config-airbnb` because these are most real-world tested, and we
can continue to add.

Happy to help if there are any questions/issues/concerns about the
plugin!
@ghost ghost added the CLA Signed label Jul 25, 2016
@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Another constraint is we don’t want to overwhelm developers with lint warnings, or they’ll just start ignoring them. These rules appear sensible to me and I’d be up to merge this.

I’d like a 👍 from @lacker on this.

@gaearon gaearon added this to the 0.2.0 milestone Jul 25, 2016
@ghost ghost added the CLA Signed label Jul 25, 2016
@mxstbr
Copy link
Contributor

mxstbr commented Jul 25, 2016

This looks very solid to me, great addition!

@gaearon gaearon merged commit 90d49f8 into facebook:master Jul 25, 2016
@lacker
Copy link
Contributor

lacker commented Jul 25, 2016

This looks good to me. I can't think of any reason offhand for images to not have an alt, although honestly I know I skip alts a lot, so let's merge it and see.

@beefancohen
Copy link
Contributor Author

Great, thanks!

FYI - don't need alt when role=presentation

<img src="foo" alt="" />
<img src="foo" role="presentation" />

@gaearon gaearon mentioned this pull request Jul 27, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants