Eslint-plugin-jsx-a11y: Add rule for empty buttons

[afercia]

Would be great to have a check for empty buttons ❤️
I'm seeing lots of empty buttons with just a glyph or a background image these days.

Fails:
<button></button>

Passes:
<button>text</button>
<button aria-label="text"></button>

I guess it would be a bit more complicated to check also for input types submit, reset, button with an empty value, but it would be great :)

[ljharb]

button imo shouldn't pass unless it has an explicit type, since the default is "submit" but everyone thinks it's "button" (not that that's an a11y concern directly, ofc).

Certainly I think a button with no visible contents, no matter the tag name, should fail an a11y check.

[afercia]

I agree and always recommend to use type="button" (which also simplifies the scripting part since you don't have to prevent the default action). I was focusing on the button content though ;)

[evcohen]

Should we consolidate all of the x-has-content rules into a single rule? This will check for button, a, heading (h1, h2, etc) for now. A tradeoff here is that if any of the elements has different logic than the others it would have to live in the same file (which doesn't really scale). However, I can't think of a case where this would really happen

[jessebeach]

This is one of those cases that might need to be solved with a render layer in place. For instance:

<button><i class="edit" /></button>

Lacks an accessible label, but

<button><i class="edit" aria-label="Edit" /></button>

has an accessible label. Both have content. We could further check that the children have content or an aria-label, but then we're into the game of reproducing the accessible label algorithm. And this code

<button>{this.props.children}</button>

is just completely inscrutable. My current opinion is that this just isn't the type of problem we can flag with linting.

Maybe you'd get great mileage out of a rule like this in your particular codebase. I'm never opposed to reviewing PRs 😄

[ljharb]

(However, in both cases those lack an explicit type, which a different linter rule should exist to prevent :-p)

[evcohen]

@jessebeach don't these concerns also exist with all of our *-has-content rules?? checking for aria-label, aria-labelledby or if a button has accessible children is still a good start, which is what I am currently doing for object-has-alt here

[evcohen]

(However, in both cases those lack an explicit type, which a different linter rule should exist to prevent :-p)

@ljharb haha does this rule belong in this project??

[jessebeach]

don't these concerns also exist with all of our *-has-content rules??

Indeed. We're trying to solve the label issue through a consistent prop combo (label, labelIsHidden) rather than linting. But again, these rules might certainly be useful in other circumstances.

[ljharb]

@evcohen i don't think there's a great place for that linter rule; we already have it in airbnb's internal plugin tho :-p

[jzaefferer]

@ljharb how about https://www.npmjs.com/package/eslint-plugin-react for the type=button rule? That also deals with JSX.

[ljharb]

@jzaefferer yes that would be excellent! If you'd like to make a PR, I'd love to review it.

[jzaefferer]

Well, you said you already have it in an internal plugin, is there something to extract or contribute?

[ljharb]

touché, that's a good point. I'll take a look in the next couple weeks and see what I can do.

[ljharb]

whoops, turns out we don't have it built yet, it's just on a list :-)

@evcohen what do you think about adding the rule to this project? Specifically because a <button> without a type defaults to submit, and most people probably aren't thinking about it in terms of progressive enhancement.

[evcohen]

@ljharb hm, this doesn't seem to have a great home. I wouldn't say explicitly setting the button type has a11y concerns, but it feels like this project is more concerned with HTML semantics than eslint-plugin-react. We have an anchor-is-valid rule, maybe a button-is-valid 🤔

[ljharb]

I'm fine with button-is-valid. I agree there's not a clear home, but I think it's important the rule exists somewhere, and eslint's broken peer dep setup for plugins means that adding a new plugin to a shared config like airbnb's is prohibitively costly :-/

[ljharb]

Rule about the type added to eslint-plugin-react in https://github.com/yannickcr/eslint-plugin-react/pull/1525

[n1313]

What is the most current recommendation to enforce an empty <button> to have aria-label attribute?

[jessebeach]

The new rule control-has-associated-label will cover this case.

@afercia what do you think?