Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[New] no-disabled rule #830
base: main
Are you sure you want to change the base?
[New] no-disabled rule #830
Changes from 5 commits
c95e07b
aa41427
1d89958
b2cfb4b
b95ebd2
167c235
a556263
23d6ef9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The disabled attribute makes the element imperceivable to assistive technology agents, like screen readers."
Just a small nit that an
attribute
is a HTML feature, whereas aprop
is a DOM concept. In this case, we are detecting the presence of an attribute, not a prop.Also, we should avoid being screen-reader-centric. There are many assistive technology agents. Screen readers are common and probably what a developer is familiar with, so it helps to mention them. We just want to be sure we leave room for the full spectrum of agents that we need to support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall that we have support issues with
includes
and older versions of Node. Could you rewrite this as afor
loop?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb do you have an issue with
includes
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a much larger issue with loops :-) we support down to eslint 3, which requires node 4+, which lacks
.includes
. However, we can and should use thearray-includes
package for this, which will prefer the native method when available.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I will endeavor to commit this detail to memory for the next time =D