yoast-components icon indicating copy to clipboard operation
yoast-components copied to clipboard

jsx-a11y can't check styled components

Open afercia opened this issue 7 years ago • 3 comments

Not sure this issue is actionable but it's worth considering. At least, everyone should be aware of it.

jsx-a11y checks for common accessibility issues and does a good job when it understands what a component is made of. For example, it checks for click events attached to non-interactive elements (e.g. a <div>) and warns against that. Of course, it's able to do that when it understands there's a <div>. When it encounters a styled <div> renamed to, for example, <Container> it doesn't understand what the underlying HTML element is.

A clear example is the following bit of code introduced in #714:

https://github.com/Yoast/yoast-components/blob/22569438ecc82da8e568d1b2fd3e2e5921886630/composites/Plugin/Shared/components/ButtonSection.js#L33-L35

where <Container> is a <div> element with a click event, but there are no jsx-a11y warnings or errors. Change that to a native <div>, run grunt eslint and you will get the error.

Not sure there's a way to address this issue. At the very least, we should be aware of it and not rely on jsx-a11y when styled components come into play.

afercia avatar Sep 07 '18 08:09 afercia

For reference: https://github.com/evcohen/eslint-plugin-jsx-a11y/issues/466

afercia avatar Sep 07 '18 08:09 afercia

Some (most?) of the jsx-a11y rules can take a list of Components to check, see for example: https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/label-has-associated-control.md

We should check if that works with Styled Components too. However, Configuring and maintaining the rules would be a challenge.

afercia avatar Sep 13 '18 08:09 afercia

Hey, I made a pretty cool plugin for this (hopefully). It found 6,567 errors in one of my repos that were undiscovered using airbnb/jsx-a11y rules. Please give it a star so we can get more people interested, using it, and contributing to make a more accessible internet for all.

Here’s the github repo it covers all 4 styled components/emotion syntaxes

Existing a11y linting has become significantly less useful (personally, 99% of the components I need linted are styled.). Each scenario below would display the error

Visible, non-interactive elements with click handlers must have at least one keyboard listener.`
const Div = styled.div``;
<Div onClick={() => null}/>
const Div = styled.div.attrs({ onClick: () => null})``;
<Div />
const Div = styled.div.attrs({ onClick: () => null})``;
const StyledDiv = styled(Div)``;
<StyledDiv />
const ButtonAsDiv = styled.button``;
<ButtonAsDiv as="div" onClick={() => null} />

It’s not perfect (no linters are), but it’s super useful, and with a little time and community support, it could be even better.

brendanmorrell avatar Mar 09 '20 02:03 brendanmorrell