Clickable table row about eslint-plugin-jsx-a11y HOT 18 CLOSED

@niksajanjic, check out the ARIA grid role and its child roles: row, gridcell. It'll confer table semantics, allow you to use divs and you'll get around the strict parent/child rules of HTML table. I think you'll find it all around more pleasant to work with and you don't have to deal with table layout quirks either!

http://w3c.github.io/aria-practices/#grid

Thank you for taking accessibility into account!

from eslint-plugin-jsx-a11y.

There are only 2 ways of making tr clickable through HTML and still not break specifications, but both are bad and usually not used in practice. By specification tr can only have td or th as immediate children and tr can only be a direct child of table, tbody, thead, tfoot. First way of making whole row clickable would be like this:

<table>
  <tbody>
    <tr>
      <td><a href="/user/1">John Smith</a></td>
      <td><a href="/user/1">123 Fake St</a></td>
      <td><a href="/user/1">90210</a></td>
    </tr>
    <tr>
      <td><a href="/user/2">Peter Nguyen</a></td>
      <td><a href="/user/2">456 Elm Ave</a></td>
      <td><a href="/user/2">90210</a></td>
    </tr>
  </tbody>
</table>

As it can be seen it's bad, you add useless repetitive anchor tags in every cell and you also have to style your cells differently and add more useless CSS to make it work.

Second option would be to not use table at all. Instead you could be able to do table with div's and then style it to look like table through CSS which would allow us to mimic row using divs and wrapping them in anchor tag. Negative side of it is that you can't mimic colspan and rowspan attributes through CSS so you are forced to hack things out again. Also, if your content really should be in a table you are breaking semantics just to allow this rule to go away.

I'm sorry but I prefer to believe this is a problem of this module and not HTML nor CSS.

from eslint-plugin-jsx-a11y.

Additional problem is turning off this specific rule inside JSX. This rule is generally good as it is, but in specific lines in code I want to turn it off, but trying to do that inside JSX is sometimes painful. Look at this example:

renderTableBody() {
    const { users } = this.props;

    /* eslint-disable jsx-a11y/no-static-element-interactions */
    return users.get('list').map((item, index) =>
      <tr
        className={css(styles.row)}
        key={index}
        onClick={this.clickHandler(item.get('id'))}
      >
        <td className={css(styles.cell)}>{item.get('firstname') || 'N/A'}</td>
        <td onClick={this.doSomething} className={css(styles.cell)}>{item.get('lastname') || 'N/A'}</td>
        <td className={css(styles.cell)}>{item.get('email')}</td>
        <td className={css(styles.cell)}>{item.get('bio') || 'N/A'}</td>
        <td className={css(styles.cell)}>{item.get('confirmed').toString()}</td>
      </tr>
    );
    /* eslint-enable jsx-a11y/no-static-element-interactions */
  }

If I turn off this rule outside JSX, than all JSX is omitted of this rule and I have a problem because now it's not erroring out for td being clickable which shouldn't be. If I put it inside JSX I have additional code and it still doesn't work properly:

  renderTableBody() {
    const { users } = this.props;

    return users.get('list').map((item, index) => {
      /* eslint-disable jsx-a11y/no-static-element-interactions */
      return (
        <tr
          className={css(styles.row)}
          key={index}
          onClick={this.clickHandler(item.get('id'))}
        >
          {/* eslint-enable jsx-a11y/no-static-element-interactions */}
          <td className={css(styles.cell)}>{item.get('firstname') || 'N/A'}</td>
          <td onClick={this.doSomething} className={css(styles.cell)}>{item.get('lastname') || 'N/A'}</td>
          <td className={css(styles.cell)}>{item.get('email')}</td>
          <td className={css(styles.cell)}>{item.get('bio') || 'N/A'}</td>
          <td className={css(styles.cell)}>{item.get('confirmed').toString()}</td>
        </tr>
      );
    });
  }

Now it will error out for td which is what I expect, but then again I get another error out from another eslint library:

Unexpected block statement surrounding arrow body                                    arrow-body-style

I'm forced to turn this rule off for this block code and that is very unpractical.

from eslint-plugin-jsx-a11y.

I like how you say "both are bad and usually not used in practice" and then say "this is a problem of this module and not HTML" in the same post :-)

Repeating the anchor tags, or not using a table, is the proper solution.

from eslint-plugin-jsx-a11y.

Here is a possible solution to deal with clickable table row

from eslint-plugin-jsx-a11y.

yes - this is valid and its an opinionated rule. you can turn it off, but i would try wrapping the the <tr> element in a link instead, though. let me know if that works out for you!

from eslint-plugin-jsx-a11y.

from eslint-plugin-jsx-a11y.

@emyk the tr contents should be the link, and it should take up 100% of its space.

from eslint-plugin-jsx-a11y.

@ljharb A <tr> must have a combination of <td> or <th> elements; otherwise, it is not valid HTML. Sure, you could have a table cell with a 100% width/height link, but that does not scale well for multiple columns. You would have to create a link container within each cell and naturally adjust any styles being applied to table cells. Not very pretty.

I think it would be nice if we could define exceptions, such as a <tr>, because some elements are extremely rigid in what their contents can be.

from eslint-plugin-jsx-a11y.

@ljharb In SPA applications it's usual use case to have row clickable and to go to edit page for that specific row data. HTML has nothing against me using click handlers on td tags, but this rule does. Therefore I don't think it's HTML problem.

Doing anchors in each cell for tables with 10+ columns and 30+ rows is bug prone, especially when whole table is dynamically generated by data returned from server. QA needs to check if every single cell in every table has link, then it has to check that each cell in each row does lead to same route instead of just checking a row for click handler route.

As for not using a table, as I already explained, tabular data needs to be inside table or it breaks semantics so that is not even an option.

Last thing, I don't write specifications and I don't even care if I'm right or not about HTML. I already explained why those 2 solutions make no sense and I stick with my opinion that this rule has an issue which it obviously does. It's really simple to close issue if it's not meant to be fixed, no hard feelings about it, but the issue still stays as it is, one thing is for sure, HTML specification will not change and this module potentially can.

from eslint-plugin-jsx-a11y.

@ljharb One more example why this rule has issue. If I have some parent element which is clickable and some of it's children can also be clickable. I can't nest anchors and buttons into each other to satisfy this rule, therefore, I have linting errors again.

from eslint-plugin-jsx-a11y.

@niksajanjic hey! thanks for contributing. the rule is what it is and it does not require you to follow it. This is the reason that eslint allows you to configure and also temporarily disable rules for certain use cases where it doesn't make sense.

This rule/plugin was created to help author your application with accessibility in mind. If you want to create something that does not comply with good a11y standards, then that's your prerogative at the risk of potentially losing/frustrating many users who rely on this technology. IMO, your design has more issues than the rule 😃 !

from eslint-plugin-jsx-a11y.

I understand that clickable table row that contains clickable elements inside is an accessibility issue. I'm aware that clickable elements should be separated and not contained in each other. I understand that kind of design is flawed but it will still happen a lot in real world. Sometimes, developer doesn't have control over design/flow or client doesn't care too much about a11y.

I'm not proposing rule change, nor saying that it's broken, just possibility to fix explained issue. That could be possible if we would be able to customize the rule inside .eslintrc. According to documentation here:

https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-static-element-interactions.md

If I'm not mistaking, this rule can't be configured. For example, I would like to be able to ignore this rule check in case my element has ARIA role role="button" just like creator of this issue proposed. This would prevent the need to write comments inside code to disable/enable rule in multiple places. Any other solution is also welcome.

from eslint-plugin-jsx-a11y.

That's why in the real world, if you can't conform to the ideal of an eslint rule, you disable the rule.

If you don't want to disable it inline in multiple places (which is best), you should disable it for your entire project.

from eslint-plugin-jsx-a11y.

@jessebeach Thanks for the info. I'm currently disabling rule inline or using anchors in each cell if my table doesn't have borders or border spacing, but I'll take a look at this and see if there's better solution.

from eslint-plugin-jsx-a11y.

@niksajanjic,

WRT emulating CSS that uses colspan/rowspan when making a table-like structure from divs, aria 1.1 has
aria-colspan
and aria-rowspan
for the the ARIA grid and table roles. So, you could use attribute selectors along with these properties and improve accessibility at the same time.

For search results, you may find the
layout grid examples
especially helpful. Look particularly at example 3.

ARIA grids offer a lot of accessibility benefit over tables containing links, especially for keyboard users.

One feature that is not yet in those examples are adjustments to the visual design to help sighted keyboard users understand that you arrow to links inside the table instead of tab. We plan to make a custom focus ring that helps people understand. Another option that can be used in some applications is to provide a help popup, AKA nux, that appears the first time a person presses tab from within the grid that explains what the custom focus ring means.

from eslint-plugin-jsx-a11y.

@niksajanjic, feel free to link me to a pen or gist. I'm happy to review the code.

from eslint-plugin-jsx-a11y.

Soft-closing. Please re-open if you feel this issue has not been addressed.

from eslint-plugin-jsx-a11y.