jsx-eslint/eslint-plugin-jsx-a11y

Is no-onchange still a relevant rule?

Closed this issue · 16 comments

Hey, all. Like the title says, is this still a necessary rule? onchange is a pretty ubiquitous technique. React suggests using it for <select> elements to set state. Using onblur instead of onchange in this case means a user could potentially select an option with their keyboard and submit the form without ever losing focus, and the state wouldn't be updated.

Of course, not everyone is using React and I could just ignore the rule, but I wanted to throw this question out. W3C even has a suggested technique for their Web Content Accessibility Guidelines 2.0 that shows how to use onchange with a <select> element.

If this is still relevant, would we be able to get some more current references in the docs that support this rule? Of the two current citations, one is from 2004 and the other is from 2005. Considering this is an eslint plugin, it might be nice to have documentation that's not from a time when IE6 dominated the web 😝

Also if using only onBlur without onChange then React prints out a warning in the Browser console:

Warning: Failed prop type: You provided a value prop to a form field without an onChange handler. This will render a read-only field. If the field should be mutable use defaultValue. Otherwise, set either onChange or readOnly.

So this is rule is in conflict with what React suggests. Like @brendanthomas1 says the official React select docs also say that one should use onChange:

Still getting burnt by this, @evcohen can you weigh in on this?

Working with the newest version of Svelte I now too have run into the warning.
"Fixing" it would break my functionality, but leaving it (or in my case disabling the warning) feels like cheating.

The comment sveltejs/svelte#4946 (comment) possibly points out why this ruleset is redundant and shouldn't be used any longer.

Any thoughts on this?

Okay I did some research:
According to http://www.themaninblue.com/writing/perspective/2004/10/19/

when you use the keyboard to explore the options in a select menu (using cursor keys) it triggers the onchange event handler (in IE).

However in my tests using IE 11 running

document.querySelector('#my-select').addEventListener('change', function(e) {
    console.log("change");
})

The log change only appears when I confirm my selection, as in: clicking outside or pressing enter, which would make this rule redundant.

Can't tell how older browsers handle it though. 🤔

I ran into this also just now, teaching students.

I researched a bit, and I guess this rule can be deprecated and removed now? I didn't find the behavior described in the original motivation blog posts.

Would the maintainers accept a pull request to remove no-onchange?

@jessebeach @backwardok @octatone @evcohen @ljharb

Removing it would be a breaking change, so that PR would sit open for a long time.

Either way, it’d have to be tested back to IE 6 so we at least know for which browsers the rule is relevant - not everyone can drop IE support, or even drop support of IE < 11.

sit open for a long time

Maybe it's worth it to open it, for the next major version in the next year or so?

I suppose changing the default to 'off' in the built-in configs would also require a major version bump?

Either way, it’d have to be tested back to IE 6 so we at least know for which browsers the rule is relevant

Good point, that seems like a reasonable pre-requisite.

There is no default, unless you mean in one of the included configs - turning it off there would be a patch.

Right sorry, was unclear there - I meant to remove the 'error' currently set in the recommended and strict configs. I've opened a PR for feedback here: #757

I think if this PR is acceptable, one more thing to do would be to mark the rule as deprecated in the docs for the rule (not sure about format for this - I guess somewhere near the top?).

Then I suppose this issue could be closed - the rule itself could stay around for those who want to enable it.

So the rule has been deprecated and removed from the recommended and strict configs in #757.

Since the current version of eslint-plugin-jsx-a11y is 6.4.1, then I'm assuming we're looking at a release in versions 6.4.2 or 6.5.0 - would either of those be correct @jessebeach ?

Certainly it’ll be one of those :-)

I've been reading along. I need to find the time to sit down and consider the proposal. Thank you for the thoughtful change suggestion!

@jessebeach its already been merged; but there’s certainly time to change it if you disagree :-)

I'll have a read later, but I trust you all were thoughtful.

@ljharb @jessebeach is a new version of this plugin going to be released any time soon? :)

You can temporarily bypass this error (until the fix is released), doing the following:
Add this: 'jsx-a11y/no-onchange': 'off' into the rules object in the .eslintrc.js file