feat(material/button): add the ability to interact with disabled buttons #28242
Conversation
crisbeto
added
P2
|
Deployed dev-app for 5c26124 to: https://ng-dev-previews-comp--pr-angular-components-28242-dev-1mxl5pxh.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
crisbeto
requested review from
devversion,
josephperrott,
andrewseguin and
mmalerba
as code owners
crisbeto
force-pushed
the
disabled-interactive-button
branch
7 times, most recently
from
4ebe2ce to
3a54dc4
Compare
There was a problem hiding this comment.
This looks pretty reasonable to me.
Asked a few questions.
Question about describe/it is off-topic. Wondering what you're thinking about structuring unit tests. Changing the structure isn't a blocker for me on this PR :). I wanted to start a conversation about this because I'm having a problem in another PR for Tree where there are so many unit tests that it's hard to understand what's the unit tests are actually doing and what's being covered and what isn't being covered by tests.
Smoke tested on VoiceOver.
- macOS 14.1.1 (23B81)
- Chrome Version 119.0.6045.199 (Official Build) (arm64)
- Firefox 120.0 (64-bit)
- Safari Version 17.1 (19616.2.9.11.7)
| /** Object that can be used to configure the default options for the button component. */ | ||
| export interface MatButtonConfig { | ||
| /** Whether disabled buttons should be interactive. */ | ||
| disabledInteractive?: boolean; |
There was a problem hiding this comment.
I see that the unit tests cover disabledInteractive via InputBinding. I don't see the unit testing covering disabledInteractive via MatButtonConfig. Do you think there's value in covering the DI workflow in unit tests?
|
|
||
| **Note:** using the `disabledInteractive` input can result in buttons that previously prevented | ||
| actions to no longer do so, for example a submit button in a form. When using this input, you should | ||
| guard against such cases in your component. |
There was a problem hiding this comment.
Can we provide an actionable suggestion, e.g. setting tabindex=0 (not sure if thats right)?
There was a problem hiding this comment.
I don't think that we can provide a general advice since it depends on what the button does.
crisbeto
force-pushed
the
disabled-interactive-button
branch
from
3a54dc4 to
7510df0
Compare
Native disabled buttons don't allow focus and prevent the button from dispatching events. In some cases this can be problematic, because it prevents the app from showing to the user why the button is disabled. These changes introduce a new opt-in input that will style buttons as disabled and set `aria-disabled="true"`, but not set the native `disabled` attribute, allowing them to be interactive.
crisbeto
force-pushed
the
disabled-interactive-button
branch
from
7510df0 to
5c26124
Compare
crisbeto
added
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Native disabled buttons don't allow focus and prevent the button from dispatching events. In some cases this can be problematic, because it prevents the app from showing to the user why the button is disabled.
These changes introduce a new opt-in input that will style buttons as disabled and set
aria-disabled="true", but not set the nativedisabledattribute, allowing them to be interactive.Note for reviewers: there's an example of the feature in the button demo, but you have to scroll down and check the "Allow disabled button interactivity" checkbox. Afterwards you should see tooltips when hovering over the disabled buttons.