Eslint-plugin-jsx-a11y: The no-static-element-interactions rule breaks when used as part of the airbnb preset

Created on 18 Apr 2017  路  23Comments  路  Source: jsx-eslint/eslint-plugin-jsx-a11y

Originally posted on airbnb/javascript.

Take this code (based on the no-static-element-interactions example):

import React from 'react';

const fn = () => {};

const element = <div
  onClick={fn}
  onKeyPress={fn}
  role="button"
  tabIndex="0">
  Save
</div>

The above code should pass the jsx-a11y/no-static-element-interactions rule, because the element has role="button", so it's OK.

And it _does_ pass that rule, when the rule is used on its own:

rules:
  jsx-a11y/no-static-element-interactions: error

But when the rule is used as part of the airbnb present, it fails, no matter what you put in the role attribute:

extends:
  - airbnb
Visible, non-interactive elements should not have mouse or
keyboard event listeners (jsx-a11y/no-static-element-interactions)

I don't know what specifically causes the incompatibility in the airbnb preset, but hopefully someone with more knowledge will be able to find it.

Most helpful comment

@jessebeach there's no reason to wait imo; we can major-bump as many times as we want if we have to, we won't run out of numbers.

I'm never opposed to moving fast :) If folks prefer a little major-ver thrash vs a little major-ver quirk, then ya, let's cut a release. @evcohen ?

All 23 comments

Thanks for reporting! What version of this plugin are you using? Do you have a yarn.lock file that is locking this down to a version < v4? Also make sure you have the latest eslint-config-airbnb so that it picks up the peerDependency of this plugin for v4. The addition of the role attribute logic was added in version 4.

4.0.0 on npm doesn't seem to include the role attribute logic.

yarn.lock entry:

eslint-plugin-jsx-a11y@^4.0.0:
  version "4.0.0"
  resolved "https://registry.yarnpkg.com/eslint-plugin-jsx-a11y/-/eslint-plugin-jsx-a11y-4.0.0.tgz#779bb0fe7b08da564a422624911de10061e048ee"
  dependencies:
    aria-query "^0.3.0"
    ast-types-flow "0.0.7"
    damerau-levenshtein "^1.0.0"
    emoji-regex "^6.1.0"
    jsx-ast-utils "^1.0.0"
    object-assign "^4.0.1"

Contents of lib/no-static-element-interactions.js

This has been improved in the master branch. Pending a major release.

@jessebeach do you know when it might happen?

@jessebeach do you know when it might happen?

It's my top project for the quarter. So before the end of June is my prediction. @evcohen @ljharb does that seem like a reasonable timeline? I'm activating no-static-element-interactions on the FB codebase this week with no-noninteractive-element-interactions next week. Those are the rules we made the most significant changes to. I plan to go through the rest of them to see if there are opportunities to tweak; I don't foresee any major refactors though.

I am a bit surprised. I thought these changes are already in the branch and are not working as part of airbnb preset only because of some minor oversight, so I expected this issue to be fixed in a matter of days, not months. What am I missing?

@jessebeach there's no reason to wait imo; we can major-bump as many times as we want if we have to, we won't run out of numbers.

I thought these changes are already in the branch and are not working as part of airbnb preset only because of some minor oversight, so I expected this issue to be fixed in a matter of days, not months. What am I missing?

That's a great question. No one had yet modeled, in a queriable format, the relationship between HTML, ARIA and the AX semantic layer in the browser. We're forging new understanding into how HTML, inflected by attributes and ARIA, maps to an AX layer. This is why aria-query and axobject-query were introduced.

There are a few paths from HTML to the AX semantic layer.

[HTML] -> [AX]
[HTML] + [HTML ATTRIBUTES] -> [AX]
[HTML] + [ARIA] -> [AX]
[HTML] + [ARIA] + [ARIA ATTRIBUTES] -> [AX]

The current work in master provides a complete (I hope!) mapping of these paths to the semantic representation so that the plugin can make the appropriate evaluation of a violation.

@jessebeach there's no reason to wait imo; we can major-bump as many times as we want if we have to, we won't run out of numbers.

I'm never opposed to moving fast :) If folks prefer a little major-ver thrash vs a little major-ver quirk, then ya, let's cut a release. @evcohen ?

ah happy to! just want to squeeze in one more breaking change for alt-text will post a PR ASAP.

On a more general note, I think the tradeoff of releasing majors too often is that people feel left behind and burdened to integrate at a high rate. I think trying to get as many breaking changes in the roadmap into a single major bump release is important

@evcohen Ping me on Twitter when it's up.

@evcohen while i agree with that, there's a countering tradeoff of unreleased things sitting in master so long that the people who want them get frustrated waiting for them to be released :-)

imo the best solution is making everything semver-minor for as long as possible, and waiting to merge any semver-major PRs until we're ready to actually cut a major release immediately.

I ended up here because the linter-eslint Atom plugin provides direct links to the rule docs (via eslint-rule-documentation), which references master and is therefore not in line with the current version of the plugin.

IMO for that reason it might be better to keep unreleased work out of master if possible.

@OscarBarrett better than that, eslint-rule-documentation shouldn't be pointing to docs in master; it should be pointing to the docs under the appropriate git tag. Unreleased things are totally fine in master.

Any updates on when to expect this?

@evcohen Shall we cut the 5.0 today and bump to 6.0 when #220 is committed?

@jessebeach ah just saw this, #220 is updated. mind taking a second look? i think its ready to go and we can include in v5.0 early this week.

馃憤

Sorry I have version 6.2.1 but I'm experiencing this same error, got the role="button" and the event in a div, but the error still appears, and I'm also using airbnb, would that might be the issue?

@valentinaAl can you file a new issue? this one was fixed 2 years ago :-)

Yes, I'm aware, but this thread is the most precise with what I'm experiencing, I have a div with a onKeyDown event (just catch the esc key to close the div) and if I put the role="presentation", the error still appears, and with the role="button", even though is not a button, has the same response, the thing is that if I add, role button, and on click event, and a tabindex, I still have the same issue, I cannot fix the error, only deleting the onKeyDown event, but that doesn't allow me to close the div, which is the expected behavior

A new issue that links back to this thread would be most helpful in helping us fix it.

Alright I'll create it, I forgot to add that I'm using airbnb also

Was this page helpful?
0 / 5 - 0 ratings