jsx-eslint / eslint-plugin-jsx-a11y

Static AST checker for a11y rules on JSX elements.
MIT License
3.42k stars 637 forks source link

<td> with role="cell" is failing no-interactive-element-to-noninteractive #999

Closed Watso196 closed 2 months ago

Watso196 commented 3 months ago

When supplying a role="cell" attribute to my table cell , I'm failing the no-interactive-element-to-noninteractive rule, which seems like an incorrect failure.

Failing case: <td role="cell"> Some text here </td>

This pattern is part of a suggested pattern to best support all screen reader users of tables, as table semantics are inconsistently reported: Adrian Roselli's article on the topic

Is there anything I can do to fix this? We still want to use the cell role to best support all users.

ljharb commented 3 months ago

hmm, I'm not seeing any usage of role on a td in that article, nor any use of the "cell" role value. What part of that article is suggesting that td role="cell" is beneficial?

Watso196 commented 3 months ago

@ljharb apologies, I pasted the wrong link here. I meant to provide this one: https://adrianroselli.com/2018/02/tables-css-display-properties-and-aria.html

To provide a summary, when using flexbox or grid styling with table elements, table semantics are overridden. Thus, we need to reapply table semantics using ARIA.

My confusion here is that no-interactive-element-to-noninteractive doesn't seem an appropriate rule to fail in this case - I wouldn't consider table cells interactive, at least not to my knowledge.

ljharb commented 2 months ago

OK, reading that article, I'm still not clear on why you'd need an explicit role when the only default role for a td that makes sense is cell.

Also:

If you are not able to test with a screen reader, maybe don’t do this. You run the risk of making an already problematic responsive table downright unusable. Further, if you are not going to test regularly as browsers and screen readers get updated, maybe don’t do this.

Watso196 commented 2 months ago

@ljharb You bring up a good point about needing to test this pattern consistently in order to use it without breaking accessibility information.

The default role for td is cell but the CSS display property of a table can change this. However, updating jsx-a11y to meet current browser conditions which require consistent monitoring and screen reader retesting probably isn't the best idea.

I appreciate you pushing back on this and giving your perspective.

ljharb commented 2 months ago

Any way we can reliably improve a11y, we should do!

It seems like either there's no action to take here, or, we should just quietly (or via an option) allow td's to have a role as long as it's cell (and in general, we should maybe also allow any explicit role that matches the default?

Watso196 commented 2 months ago

I do like the idea of allowing any explicit role that matches the element's default to pass! That would alleviate the failures we're seeing here. I can't think of any rules offhand that this would interfere with either?

There is an option for this rule specifically where we can ask for td to allow role=cell but it requires setting that option explicitly. Given that this pattern should be SR testing continually, perhaps that is the best path for this specific scenario?

ljharb commented 2 months ago

Yes, I think expecting you to explicitly enable the option is safer.

Watso196 commented 2 months ago

In that case I'm not sure there's much more to be done here. I'd be open to discussing allowing roles that match the default, but I couldn't think of any other issues that would assist in resolving outside of this one. At least not to my knowledge!

ljharb commented 2 months ago

Sounds great, thanks for the discussion!