eslint-plugin-jsx-a11y
eslint-plugin-jsx-a11y copied to clipboard
[New] no-disabled rule
I didn't see a contribution guideline. I still need to test this out, but just thought I'd submit a PR with the proposed changes.
According to MDN:
The disabled Boolean attribute provides CSS styles and semantics and removes the ability to click or focus while not disabling hover. By removing the ability to focus and removing it from the accessibility tree, it makes it invisible to assistive technology users. For good user experience, you want to make sure everyone can access all the visible content, no matter how they access the web. It is important to be aware that using the disabled attribute can harm usability.
Sources:
- https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-disabled
- https://www.w3.org/TR/wai-aria-practices/#kbd_disabled_controls
- https://medium.com/@izzmo/anti-pattern-using-disabled-or-read-only-for-form-input-e2c24c669f5b
- https://css-tricks.com/making-disabled-buttons-more-inclusive/#aa-the-difference-between-disabled-and-aria-disabled
- https://adrianroselli.com/2021/01/multi-function-button.html#Active
Codecov Report
Merging #830 (b95ebd2) into main (566011b) will increase coverage by
0.00%
. The diff coverage is100.00%
.
:exclamation: Current head b95ebd2 differs from pull request most recent head 23d6ef9. Consider uploading reports for the commit 23d6ef9 to get more accurate results
@@ Coverage Diff @@
## main #830 +/- ##
=======================================
Coverage 99.22% 99.22%
=======================================
Files 98 99 +1
Lines 1419 1428 +9
Branches 479 481 +2
=======================================
+ Hits 1408 1417 +9
Misses 11 11
Impacted Files | Coverage Δ | |
---|---|---|
src/index.js | 100.00% <ø> (ø) |
|
src/rules/no-disabled.js | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 566011b...23d6ef9. Read the comment docs.
Can you elaborate on this? There's many perfectly reasonable cases where it's better to have an input be disabled.
@ljharb
Can you elaborate on this? There's many perfectly reasonable cases where it's better to have an input be disabled.
I am simply going off MDN on this:
The disabled Boolean attribute provides CSS styles and semantics and removes the ability to click or focus while not disabling hover. By removing the ability to focus and removing it from the accessibility tree, it makes it invisible to assistive technology users. For good user experience, you want to make sure everyone can access all the visible content, no matter how they access the web. It is important to be aware that using the disabled attribute can harm usability.
I did read that, and that is the first time in my entire career I've heard that advice - and it seems a VERY strong position.
Are there other references, a11y guidelines, ecosystem adoption, that confirm the opinion on MDN?
@ljharb I can certainly do additional digging, but do you think there is a place for this plugin where maybe we warn others of this? I just learned of it today and I would prefer to give this concern exposure.
I absolutely think that this PR is the correct place/way to handle this, assuming the concern is as urgent and valid as MDN implies.
W3's best practices lists a few specific cases where disabled
should be avoided.
Removing focusability from disabled elements can offer users both advantages and disadvantages. Allowing keyboard users to skip disabled elements usually reduces the number of key presses required to complete a task. However, preventing focus from moving to disabled elements can hide their presence from screen reader users who "see" by moving the focus.
Authors are encouraged to adopt a consistent set of conventions for the focusability of disabled elements. The examples in this guide adopt the following conventions, which both reflect common practice and attempt to balance competing concerns.
For elements that are in the tab sequence when enabled, remove them from the tab sequence when disabled. For the following composite widget elements, keep them focusable when disabled:
- Options in a Listbox
- Menu items in a Menu or menu bar
- Tab elements in a set of Tabs
- Tree items in a Tree View
That suggests that it'd be better to forbid "disabled" by default only on those four kinds of elements (altho an option to forbid it "everywhere" is fine). Thoughts?
To me, two key take-aways from WAI-Aria's section:
-
Authors are encouraged to adopt a consistent set of conventions for the focusability of disabled elements.
-
[...] make them focusable if discoverability is a concern.
The second bullet refers to elements in a toolbar, but it seems like it could apply to any element. Eg I've seen it recommended that submit-buttons that are "disabled" until all required fields are completed should be focusable to ensure users can find it even before they complete the fields.
I'm not sure what the eslint rule should be though.. Maybe it is reasonable to only default to the four types of composite widgets mentioned in the WAI-Aria section. But I'm not sure that it's intended to be a complete list.
W3 states,
Authors are encouraged to adopt a consistent set of conventions for the focusability of disabled elements.
I think that means disabled
is up for interpretation, but whatever it is, it should be adhered to. I believe assistive technology users should know about form elements, whether they are disabled or not. (Assuming this content is visible) Same goes for buttons that may be disabled due to permission. (Tabs, buttons, etc.) They ought to know. No?
To me a question I'd like answered is if the input
isn’t hidden
for visual users (when disabled); why hide it for non-visual users?
**Edit: (when disabled)
There’s a lot of things visually on a page that are hidden for non-visual users - borders, aspects of how a button looks, background colors and images - it doesn’t automatically make sense for something to be in the a11y tree just because it’s visible.
That said, i can imagine a good chunk of the places I’ve used the disabled attribute in the past are things whose presence would be useful for a non-visual user to know about. To be honest, this really feels like a bug/flaw in the way the a11y tree is built - ie, that it shouldn’t be omitting these elements. To be forced to use an aria attribute seems pretty absurd; their purpose is to provide additional info, or to hack around non-semantic html.
The attribute was brought up in a code review session today.
I have a list of actions I'd like the user to take to onboard. However, they are not able to manually click on these checkboxes, but are there to serve as a visual cue if the user completed the actions. In this case, I chose to use disabled
but that would hide the completion status from the screen-reader, so I chose to use aria-disabled
instead.
The choice is meaningful and important; but it really does seem like the default should be to include them, and the aria override should be to hide them.
That suggests that it'd be better to forbid "disabled" by default only on those four kinds of elements (altho an option to forbid it "everywhere" is fine). Thoughts?
To be honest, this really feels like a bug/flaw in the way the a11y tree is built - ie, that it shouldn’t be omitting these elements.
Great conversation here. This came up at work the other day and Matt King also recommended not using the disabled
attribute because, as folks have noted here, it knocks the elements out of the AX Tree. Instead, authors should use aria-disabled
, which conveys the disabled-ness of the element, but leaves the element in the AX Tree so it can be perceived. @courtyenn notes this approach above, as well.
I'm in favor of this rule. Let me review the code. I think we should get it in.
@jessebeach is there any effort to fix this in browsers? semantic non-aria attributes are supposed to be the best approach, and this seems like the odd one out.
is there any effort to fix this in browsers? semantic non-aria attributes are supposed to be the best approach, and this seems like the odd one out.
Agreed. I haven't heard any rumblings of changing this behavior and my sense here is that this horse is long out of the barn, never to return. I think they just got the default behavior wrong here, probably because this attribute predates or is contemporaneous with assistive tech cursor traversal.
@courtyenn I think what @ljharb is insinuating, is that this rule is encouraging counter-intuitive behavior. It runs contrary to the general heuristic "Use semantic HTML first; ARIA second or not at all". So the explanation for this rule should acknowledge that this behavior runs counter to the general advice: https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/pull/830/files#diff-1cf5c60cfe95228a04407905406f8fc98a66b9b0d0bc27f474e23a8976f8b206R3
@courtyenn it looks like the JS error in the tests is caused by the changes you introduced. Could you have a look?