Skip to content
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

Swap button attribute disabled to aria-disabled #352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dmkk01
Copy link

@Dmkk01 Dmkk01 commented Aug 12, 2022

As the title suggests, I have swapped the current disabled attribute to the aria-disabled on the button as that makes the component more inclusive for users with assistive technologies.

@illright
Copy link
Owner

Could you please provide a reference as to why that makes the component more accessible? To my knowledge, this change has the opposite effect

@Dmkk01
Copy link
Author

Dmkk01 commented Aug 12, 2022

Sure, I read this article some time ago while I was working on my other projects: https://css-tricks.com/making-disabled-buttons-more-inclusive/.

Some of the main points include:
image
image

@illright
Copy link
Owner

I see, thanks for the reference. I've noticed that a similar PR (mui/material-ui#27719) has been opened for Material-UI, a very popular component library. They have put this PR on hold because these two behaviors are kind of hard to compare — they both have a place and a use case.

That said, the current changes you've made are insufficient as the clicks will suddenly start going through for everyone. So to roll out this change, we'd have to bake in click prevention into the component itself.

That's still going to be a breaking change since some people want one behavior and others want the other. And as the article rightfully states, disabled buttons mostly suck anyway.

I think that we should keep the current behavior, since it's still possible to implement accessible disabling yourself with the current design, bypassing the disabled prop. Perhaps we should add a warning to the docs about disabled buttons, but logic-wise, I think it should stay as it is

@illright
Copy link
Owner

Alternatively, we could implement this behavior with a separate prop, something like accessiblyDisabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants