-
-
Notifications
You must be signed in to change notification settings - Fork 637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is no-onchange still a relevant rule? #398
Comments
Also if using only
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 |
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. 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:
However in my tests using document.querySelector('#my-select').addEventListener('change', function(e) {
console.log("change");
}) The log 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 @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. |
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
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 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 Since the current version of |
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? :) |
As outlined in jsx-eslint/eslint-plugin-jsx-a11y#398, the rule is no longer relevant. It goes against the official React docs and keyboard navigation in <select> controls works fine using the `onChange` prop. The issue was in browsers older than IE 11. The rule will be removed from jsx-a11y/recommended in a future release.
* Disable the jsx-a11y/no-onchange lint rule As outlined in jsx-eslint/eslint-plugin-jsx-a11y#398, the rule is no longer relevant. It goes against the official React docs and keyboard navigation in <select> controls works fine using the `onChange` prop. The issue was in browsers older than IE 11. The rule will be removed from jsx-a11y/recommended in a future release. * Remove unnecessary lint rule exceptions
It's going away jsx-eslint/eslint-plugin-jsx-a11y#398
You can temporarily bypass this error (until the fix is released), doing the following: |
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. Usingonblur
instead ofonchange
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 😝
The text was updated successfully, but these errors were encountered: