[Button] Implement inclusive disabled state #27719

eps1lon · 2021-08-13T08:18:05Z

Adds a prop that disables "click interactions" for buttons but enables "cursor interactions" (hover for mouse, focus for keyboard).
Preview: https://deploy-preview-27719--material-ui.netlify.app/guides/right-to-left/#opting-out-of-rtl-transformation

This is largely an implementation of https://css-tricks.com/making-disabled-buttons-more-inclusive/.
TL;DR: Disabled buttons are mostly inaccessible to keyboard users and oftentimes don't tell why they're disabled. We can show additional "why" information for disabled buttons on hover/focus.

Use case for us

Instead of hiding the codesandbox/stackblitz buttons (confusing since they are displayed for almost all demos) we can show them in a disabled state with a reason

Prior art

#26412
#23101

Implementation

Adds a new inclusiveDisabled prop. We could hijack aria-disabled but this would be a breaking change. A separate prop gives us the opportunity to experiment with it (potentially "unstable_" prefix?).

Future

For v6 we can debate whether this should be the default behavior for buttons (tooltips for disabled buttons are a common footgun).

TODO

dedicated demo
test
tabbable
Do we need focus-visible styles when inclusiveDisabled?
~~It might be disorienting to not display the cursor. But it's inconsistent with mouse-cursor i.e. hover where we don't have additional styles when hovered.~~
Yes we do. Mouse cursor styles are removed but the cursor itself is still visible. Removing focus-visible styles removes the keyboard cursor entirely though.
Form integration ([Button] Implement inclusive disabled state #27719 (comment))

mui-pr-bot · 2021-08-13T08:20:14Z

Details of bundle changes

@material-ui/core: parsed: +0.12% , gzip: +0.11%
@material-ui/lab: parsed: +0.16% , gzip: +0.17%

Generated by 🚫 dangerJS against 0f5a0c6

oliviertassinari · 2021-08-14T10:41:56Z

For the prior art, I think that we can also mention:

A previous workaround https://next.material-ui.com/components/buttons/#cursor-not-allowed that we might be able to remove now
A previous iteration on a similar prop disabledItemsFocusable. This prop is available on the Autocomplete, TreeView, and Select components, e.g. [MenuList] Allow including disabled items in keyboard navigation #19967. We could probably add it to the Tabs too. Maybe there is some prop naming harmonization to do.
Remove cursor: not-allowed styles twbs/bootstrap#22403 on how the not-allowed cursor has downsides

mbrookes · 2021-08-14T21:33:22Z

Happy to see this. The only thing I'm wondering is the name. Would accessibleDisabled (or enableAccessibleDisabled) be more apparent as to what it's for?

eps1lon · 2021-08-17T07:54:29Z

Happy to see this.

Me too 😃

The only thing I'm wondering is the name. Would accessibleDisabled (or enableAccessibleDisabled) be more apparent as to what it's for?

Yeah, the name was just a working title for me. "accessibleDisabled" sounded a bit weird at first.

The "enableAccessibleDisabled" approach sounds like it would also require disabled to be set? Though I feel like it's just too much words. I wouldn't mind a wordy prop name if you'd only set this once (via theme). But I'm not quite sure if this is what you always want (hence marking this feature as "experimental").

oliviertassinari

Is it related to https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_disabled_controls?
I have tried the inclusiveDisabled prop on a Button but I couldn't Tab on it: https://codesandbox.io/s/textbuttons-material-demo-forked-hq1m4?file=/demo.js. Is it expected?

docs/translations/translations.json

oliviertassinari · 2021-08-17T20:56:07Z

packages/material-ui/src/Button/Button.js

+    [`&.${buttonClasses.inclusiveDisabled}`]: {
+      cursor: 'not-allowed',
+    },


I would propose we use the default cursor when disabled because we use the pointer cursor when enabled. It would already differentiate the two cases.

Looking a bit closer on Windows:

On a regular <button>, the cursor is identical https://codepen.io/oliviertassinari/pen/oNWRKKq

disabled.native.windows.mp4

The not-allowed cursor looks unfriendly

disabled.windows.mp4

I would propose we use the default cursor when disabled because we use the pointer cursor when enabled

The pointer cursor is already wrong.

With your argumentation we could also use the drag cursor. You're missing why we should use the default curso.

You're missing why we should use the default cursor

The argumens for why was 1. default cursor is how the native button behave, it seems enough 2. not-allowed looks "unfriendly".

default cursor is how the native button behave

So you agree that we should also use cursor: default when the Button is not disabled because it's how the native button behaves?

not-allowed looks "unfriendly".

Don't agree there.

@eps1lon I recall a similar discussion in #17871. I personally think that the expected behavior is button with cursor: pointer on the web, and cursor: default on the native UIs.

Why? It's how the apps I use on my OS, e.g. macOS Finder, and the websites e.g. GitHub I use on a day to day have trained my brain over the years. I would find something else confusing.
Is it the right behavior? Windows explains why on a native UI cursor: default should be used.

Win32 / Design
To avoid confusion, it is imperative not to use the hand pointer for other purposes

We see the same in https://developer.apple.com/design/human-interface-guidelines/macos/user-interaction/mouse-and-trackpad/ ("The content beneath the pointer is a URL link to a webpage, document, or other item.") and https://www.w3.org/TR/css-ui-3/#cursor ("The cursor is a pointer that indicates a link."). The community, at large, seems to have decided to override these guidelines. At the end of the day, I think that it's ultimately a convention to create predictability. So I think that going against the trend on the web would be wrong. It would be better to update these convention guidelines: w3c/csswg-drafts#1936. Which they did: w3c/csswg-drafts@272dd50.

Back to argument 1. I think that it could work because we would use two different cursors between enabled and disabled.

Don't agree there.

For 2. it's fair, I think that it's a matter of feeling, no objective arguments to find :). I was personally influenced by twbs/bootstrap#22222 (comment).

Since this is a UI/UX problem, in the end, I think that we can involve @danilo-leal, to bring a new perspective, as a designer.

I actually don't have a new perspective to offer 😅 I read the CSS tricks article and learned from it, it makes a lot of sense. I also don't have any problem with the not-allowed cursor... Really liked how the experience turned out to be on this one. And happier that is making it more accessible :)

This is a hot topic very polarized. From one side we have the "web guidelines" (from Microsoft, w3c, apple). On the other side, we have the actual user expectations.

Based on Apple website, expectations seem to be more important than "guidelines". For example, the bag button, uses the pointer instead of the default one.

My thoughts are better summarised by this tweet from David K:

So cursor: pointer is for navigation and not for performing some action.
You know who knows that rule?
Approximately 3% of web developers and 0.0000001% of users.

Matching user expectations is what makes me adding a pointer cursor to normal buttons.

Then we have another point: aesthetic tastes. not-allowed cursor looks different from OS to OS macOS example. This might lead some people to override it in their apps because it's not aesthetically pleasing (a.k.a. ugly).

We already have the pointer cursor to distinguish clickable from non-clickable elements.

With all of that said... Honestly, I don't have any preference, both sides have pros/cons. But if I had to take a side probably I'd be inclined towards using cursor: default on disabled buttons to keep the consistency with the current disabled prop at MUI that does not change the cursor either.

Matching user expectations is what makes me adding a pointer cursor to normal buttons.

I think web developers tunnel vision too much on the user interface they work on. Ordinary people don't know where the OS ends and the web begins. There's only a single "computer". The OS does not use pointer for buttons . There may be exceptions but the majority of clickable surfaces in macOS, Ubuntu (gnome) and Windows 10 do not use the pointer cursor.

Then we have another point: aesthetic tastes. not-allowed cursor looks different from OS to OS macOS example. This might lead some people to override it in their apps because it's not aesthetically pleasing (a.k.a. ugly).

Doesn't that go against "matching expectations" i.e. "not-allowed cursor looks different from OS to OS macOS example." is exactly the point. People will recognize "not allowed" semantics since that's what they're used to from their computer.

We already have the pointer cursor to distinguish clickable from non-clickable elements.

Not sure how this is relevant here. But it's controversial as well.

But if I had to take a side probably I'd be inclined towards using cursor: default on disabled buttons to keep the consistency with the current disabled prop at MUI that does not change the cursor either.

This is simply due to our current implementation. You currently cannot change the cursor for <Button disabled /> since it's imlemented with pointer-events: none. So the argument for historical reasons doesn't apply here.

packages/material-ui/src/ButtonBase/ButtonBase.js

eps1lon · 2021-08-18T07:14:53Z

I have tried the inclusiveDisabled prop on a Button but I couldn't Tab on it: codesandbox.io/s/textbuttons-material-demo-forked-hq1m4?file=/demo.js. Is it expected?

Expected but not intended.

oliviertassinari · 2021-08-18T09:07:14Z

Expected but not intended.

@eps1lon ~~By "expected", do you mean that the expected behavior is to not be able to tab? I'm asking because in https://css-tricks.com/making-disabled-buttons-more-inclusive/ the button is tabbable.~~

If the expected behavior is to not make the button tabbable, only hoverable when disabled, maybe we should rename the prop to disabledHoverable? This way, we could bring disabledFocusable later on.

~~By "intended", do you mean that it's not for this PR but could/would be done in a follow-up?~~

Edit: I just saw the TODO

eps1lon · 2021-08-19T06:59:50Z

Edit: I just saw the TODO

Sorry, should have mentioned that. The PR is in a weird spot since I do want some feedback but don't intend to actively work on it right now. It can be pushed to 5.1 or even be included in a set of changes focusing on a11y (a11y statement etc).

sandrina-p

Hi, Sandrina here! 👋
I'm glad my article about disabled button motivated you folks to improve Material UI 🥰 Thank you!

sandrina-p · 2021-08-21T13:36:55Z

framer/Material-UI.framerfx/code/IconButton.tsx

@@ -6,6 +6,7 @@ import { Icon } from './Icon';

 interface Props extends MuiIconButtonProps {
  disabled: boolean;
+  inclusiveDisabled: boolean;


I agree with @mbrookes (#27719 (comment)) about the name inclusiveDisable, it's a little strange.

What do you think of disabledClickOnly? Beneftis:

autocomplete will work when start typing dis... - more people will notice the new prop.

It reflects the purpose: only the click is removed - all the other interactions are still possible.

autocomplete will work when start typing dis... - more people will notice the new prop.

That is a great point.

It reflects the purpose: only the click is removed - all the other interactions are still possible.

Yeah, I'm a bit split on this part. Because I generally don't want to name props after "how" they do something but rather what they're for. For example, I rather name a prop "error" instead of "red" even though "red" is what's actually happening.

Though I lean towards optimizing for having the prop right next to disabled when it comes to sorting. And "inclusive" is almost as bad as the generic "accessible" tag line: "inclusive" for whom?

sandrina-p · 2021-08-21T13:45:02Z

packages/material-ui/src/Button/Button.js

@@ -330,6 +334,7 @@ const Button = React.forwardRef(function Button(inProps, ref) {
      disabled={disabled}
      focusRipple={!disableFocusRipple}
      focusVisibleClassName={clsx(classes.focusVisible, focusVisibleClassName)}


[@espipj] Do we need focus-visible styles when inclusiveDisabled?

Inclusive disabled buttons still allow focus with the keyboard, therefore it needs a focus indicator. Otherwise, people might not know what's the currently focused element.

This prop can be used in more scenarios besides tooltips elements. That means we can't assume that there will always exist a visual clue from a surrounding element (eg the tooltip). For that reason, yes, add the focus indicator, even if it's redundant in some cases. Redundant is better than none. Does this make sense to you?

Yep, that's exactly the argument I would make. Though I'm always grappling with the concept of "cursor" and how it relates to mouse and keyboard inputs. We remove the hover styles for mouse cursor as well which is to me the same as focus styles for the "keyboard cursor".

But I am still leaning towards having different visuals for hover and focus styles because the mouse cursor is inherently visible. The keyboard cursor has no inherent visibility so it makes sense to always display them. Removing focus-visible styles is like hiding the mouse cursor once you hover over an element.

sandrina-p · 2021-08-21T13:51:55Z

For v6 we can debate whether this should be the default behavior for buttons (tooltips for disabled buttons are a common footgun).

I'm biased, obviously, but at my place of work, we made it the default behavior. Until now we didn't find a real-scenario expectation yet, although we thought about it. For example disabledAttribute would force the native disabled attribute to be added to the element.

eps1lon · 2021-08-23T09:46:47Z

Until now we didn't find a real-scenario expectation yet, although we thought about it. For example disabledAttribute would force the native disabled attribute to be added to the element.

Yeah that's what I'm dreadding. The size of our audience requires that we communicate the difference. And people have a certain expectation about disabled so breaking that requires some work. Your article is an important part of that story. It might just be a timing issue. Flip the default too late and we get angry tweets about how we're not "accessible". Flip it too early and people post bug reports.

So while I'm writing this I just realzied that form integration isn't solved. For example, <form><input /><Button inclusiveDisabeld type="submit" /></form> would allow form submission when pressing enter while focus is on the textbox (as well as when clicking but we can fix that).

sandrina-p · 2021-11-28T18:15:41Z

@eps1lon I agree that shipping this can have side effects. Taking into account Material UI is widely used, I'd make it optional as a first version, promote it, and see how the community reacts.

For example,
would allow form submission when pressing enter while focus is on the textbox (as well as when clicking but we can fix that).

Yeah, that's a downside I mentioned in the article, we need JS to prevent that. This also makes me realize that whatever approach we take here, it should gracefully fall back in non-JS browsers, where the native disabled attribute is used instead. (in case Material UI supports such cases)

eps1lon · 2021-11-28T18:33:31Z

(in case Material UI supports such cases)

I don't think React supports that. You'd have to render disabled on the server but then hydrate aria-disabled. That's generally not something that's supported by React without escape hatches (like mutating attributes via DOM APIs).

Yeah, that's a downside I mentioned in the article, we need JS to prevent that.

Note that this isn't trivial at all since form submits via textbox are submitted from the corresponding buttons. This gets especially tricky since form submission via textbox is a user agent thing not specified behavior. So we need to do more research in that area and setup tests to monitor user agent behavior (since that can change at any point).

In short: It's not as simple as putting aria-disabled on a button and prevent clicks if your target is a generic button supporting all types. We have to do some more research to understand form submission implications.

sandrina-p · 2021-11-28T18:38:17Z

In short: It's not as simple as putting aria-disabled on a button and prevent clicks if your target is a generic button supporting all types. We have to do some more research to understand form submission implications.

True. One more reason to add this as an enhanced/optional approach rather than being the new default behavior. That way is up to the developers to use that approach whenever they see the fit for it. And that also avoids breaking changes.

eps1lon · 2021-11-28T18:41:35Z

True. One more reason to add this as an enhanced/optional approach rather than being the new default behavior.

Sure, but we can't just add the option and expect people to use it properly. The proper usage needs to be part of the documentation and cases where it fails also need to be prominent. That's why I'm saying this needs a bit more work to be releaseable in a stable release.

sandrina-p · 2021-11-28T18:47:04Z

@eps1lon I understand that and wasn't expecting anything different than a comprehensive documentation about this new prop. :)

bytasv · 2023-01-30T08:27:39Z

Couple of thoughts after reading it all:

IMO this could be a great addition to match what large chunk of devs expect when they use disabled buttons, especially with Tooltips to indicate the reason
I'm also a bit sceptical about the name of the prop - if I wasn't reading this PR it might be difficult to understand what's this about 🤔 OFC there is a part where you have to learn it at some point and I don't know and can't offer something that could be the better name anyways 😅
There was a discussion about breaking the default behaviour and I was looking at the table from the article 👇

I was trying to think of a way if it was possible to add this as a default disabled={true} behaviour without breaking the code? 🤔

@eps1lon you mentioned that using current approach would break forms because submit button would capture enter. This could also be "tricked" by conditionally changing button's type, i.e. type={disabled ? 'button' : type} - changing from submit to button should prevent form from being submitted, no?

As for all the other things mentioned in the table:

We can prevent click handlers
We can ensure that hover state remains the same as with regular disabled
We can skip focus if we want/need to

So I'm wondering what else could potentially break there? 🤔

Alternative suggestions: what if instead of adding new prop and instead of doing this as a default disabled behaviour we would extend current disabled type to disabled?: boolean | 'inclusive'. Couple reason:

Optional: We could potentially hint users with info type message in console (which could be disabled) that they can use inclusive prop value to enable pointer events behaviour, etc..
If no breaking behaviour is reported consider changing this to default behaviour in the next major release

It very much feels to me as developer that this is the default behaviour I'd expect disabled buttons to have

eps1lon · 2023-01-31T04:52:05Z

type={disabled ? 'button' : type}

I remember some subtle bugs with HTML forms and event order when type was changed but I can't remember specifics. This is pretty sketchy so I wouldn't rely on it.

ZeeshanTamboli · 2024-04-26T13:03:05Z

This PR is inactive since a long time. I suppose we can close it.

eps1lon added new feature New feature or request accessibility a11y component: button This is the name of the generic UI component, not the React module! labels Aug 13, 2021

github-actions bot mentioned this pull request Aug 13, 2021

Update snapshots for pr/27719 eps1lon/mui-scripts-incubator#2950

Open

eps1lon force-pushed the docs/hideEditButton branch from e7d255a to b89ea22 Compare August 13, 2021 11:00

eps1lon marked this pull request as ready for review August 14, 2021 08:32

eps1lon requested review from mbrookes and oliviertassinari August 14, 2021 08:33

oliviertassinari reviewed Aug 17, 2021

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 18, 2021

eps1lon added this to the v5.1 milestone Aug 19, 2021

sandrina-p reviewed Aug 21, 2021

View reviewed changes

eps1lon changed the base branch from next to master September 14, 2021 09:29

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 28, 2021

eps1lon marked this pull request as draft September 28, 2021 08:58

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 2, 2021

oliviertassinari removed this from the v5.1 milestone Nov 10, 2021

eps1lon added 2 commits November 29, 2021 10:30

[Button] Implement inclusive disabled state

b184a4f

hideToolbar -> hideEditButton

0f5a0c6

eps1lon force-pushed the docs/hideEditButton branch from 9e037b3 to 0f5a0c6 Compare November 29, 2021 09:41

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 29, 2021

danilo-leal mentioned this pull request Apr 20, 2022

[Button] Disabled buttons should have cursor: not-allowed by default #32371

Open

2 tasks

illright mentioned this pull request Aug 12, 2022

Swap button attribute disabled to aria-disabled illright/attractions#352

Closed

michaldudak mentioned this pull request Sep 1, 2022

[Button][Checkbox] Allow receiving focus when disabled #32917

Open

2 tasks

eps1lon mentioned this pull request Jan 29, 2023

Tooltip doesn't work for <IconButton disabled> #8416

Open

1 task

siriwatknp mentioned this pull request Feb 27, 2023

Tooltip on ToggleButton in ToggleButtonGroup #36270

Closed

2 tasks

michaldudak mentioned this pull request Jan 24, 2024

[base-ui][Button] Make demos more readable, modern, and robust #40742

Closed

ZeeshanTamboli closed this Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Button] Implement inclusive disabled state #27719

[Button] Implement inclusive disabled state #27719

eps1lon commented Aug 13, 2021 •

edited

Loading

mui-pr-bot commented Aug 13, 2021 •

edited

Loading

oliviertassinari commented Aug 14, 2021 •

edited

Loading

mbrookes commented Aug 14, 2021

eps1lon commented Aug 17, 2021

oliviertassinari left a comment •

edited

Loading

oliviertassinari Aug 17, 2021 •

edited

Loading

eps1lon Aug 18, 2021 •

edited

Loading

oliviertassinari Aug 18, 2021 •

edited

Loading

eps1lon Aug 18, 2021

oliviertassinari Aug 18, 2021 •

edited

Loading

danilo-leal Aug 18, 2021

sandrina-p Aug 21, 2021 •

edited

Loading

eps1lon Aug 23, 2021

eps1lon commented Aug 18, 2021

oliviertassinari commented Aug 18, 2021 •

edited

Loading

eps1lon commented Aug 19, 2021

sandrina-p left a comment

sandrina-p Aug 21, 2021

eps1lon Aug 23, 2021

sandrina-p Aug 21, 2021

eps1lon Aug 23, 2021

sandrina-p commented Aug 21, 2021 •

edited

Loading

eps1lon commented Aug 23, 2021

sandrina-p commented Nov 28, 2021 •

edited

Loading

eps1lon commented Nov 28, 2021

sandrina-p commented Nov 28, 2021

eps1lon commented Nov 28, 2021

sandrina-p commented Nov 28, 2021

bytasv commented Jan 30, 2023

eps1lon commented Jan 31, 2023

ZeeshanTamboli commented Apr 26, 2024 •

edited

Loading

[Button] Implement inclusive disabled state #27719

[Button] Implement inclusive disabled state #27719

Conversation

eps1lon commented Aug 13, 2021 • edited Loading

Use case for us

Prior art

Implementation

Future

TODO

mui-pr-bot commented Aug 13, 2021 • edited Loading

oliviertassinari commented Aug 14, 2021 • edited Loading

mbrookes commented Aug 14, 2021

eps1lon commented Aug 17, 2021

oliviertassinari left a comment • edited Loading

Choose a reason for hiding this comment

oliviertassinari Aug 17, 2021 • edited Loading

Choose a reason for hiding this comment

eps1lon Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

eps1lon Aug 18, 2021

Choose a reason for hiding this comment

oliviertassinari Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

danilo-leal Aug 18, 2021

Choose a reason for hiding this comment

sandrina-p Aug 21, 2021 • edited Loading

Choose a reason for hiding this comment

eps1lon Aug 23, 2021

Choose a reason for hiding this comment

eps1lon commented Aug 18, 2021

oliviertassinari commented Aug 18, 2021 • edited Loading

eps1lon commented Aug 19, 2021

sandrina-p left a comment

Choose a reason for hiding this comment

sandrina-p Aug 21, 2021

Choose a reason for hiding this comment

eps1lon Aug 23, 2021

Choose a reason for hiding this comment

sandrina-p Aug 21, 2021

Choose a reason for hiding this comment

eps1lon Aug 23, 2021

Choose a reason for hiding this comment

sandrina-p commented Aug 21, 2021 • edited Loading

eps1lon commented Aug 23, 2021

sandrina-p commented Nov 28, 2021 • edited Loading

eps1lon commented Nov 28, 2021

sandrina-p commented Nov 28, 2021

eps1lon commented Nov 28, 2021

sandrina-p commented Nov 28, 2021

bytasv commented Jan 30, 2023

eps1lon commented Jan 31, 2023

ZeeshanTamboli commented Apr 26, 2024 • edited Loading

eps1lon commented Aug 13, 2021 •

edited

Loading

mui-pr-bot commented Aug 13, 2021 •

edited

Loading

oliviertassinari commented Aug 14, 2021 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

oliviertassinari Aug 17, 2021 •

edited

Loading

eps1lon Aug 18, 2021 •

edited

Loading

oliviertassinari Aug 18, 2021 •

edited

Loading

oliviertassinari Aug 18, 2021 •

edited

Loading

sandrina-p Aug 21, 2021 •

edited

Loading

oliviertassinari commented Aug 18, 2021 •

edited

Loading

sandrina-p commented Aug 21, 2021 •

edited

Loading

sandrina-p commented Nov 28, 2021 •

edited

Loading

ZeeshanTamboli commented Apr 26, 2024 •

edited

Loading