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

[Fab] Add disabled class to FAB button #34245

Merged

Conversation

meenarama
Copy link
Contributor

@meenarama meenarama commented Sep 9, 2022

Issue:

Fixes #34227

Similar Issue in the past:

[25072] (#25072)

What does it do?

Fixes disabled class not getting added to button element.

Why is it needed?
disabled class not working in Fab button

@mui-bot
Copy link

mui-bot commented Sep 9, 2022

Details of bundle changes

Generated by 🚫 dangerJS against d911e6f

@ZeeshanTamboli ZeeshanTamboli changed the title [Fab] Add disabled class to FAB button #34227 [Fab] Add disabled class to FAB button Sep 10, 2022
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work component: Fab The React component. labels Sep 10, 2022
@michaldudak
Copy link
Member

The Mui-disabled class now gets applied twice on <Fab disabled />

@meenarama meenarama force-pushed the issue/34227-add-disabled-class-to-fab-button branch from e08d0fc to 3782714 Compare September 13, 2022 21:13
@meenarama
Copy link
Contributor Author

meenarama commented Sep 13, 2022

@michaldudak, I did the same thing as in Icon Button, but I fixed multiple occurrences of Mui-disabled class. I guess Icon Button has the same behavior, FYI

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed changes which changes the logic to be similar to what we have in the Button component. The test that you've added helped with ensuring that the logic is correct :)

Thanks for raising this up.

@meenarama
Copy link
Contributor Author

@mnajdova , thanks for this implementation, I tested this, looks good. can this be merged?

@mnajdova
Copy link
Member

@meenarama will merge it today, I've updated with the latest master, let's wait for the green CI :)

@mnajdova mnajdova merged commit ddbde9c into mui:master Sep 30, 2022
@oliviertassinari oliviertassinari added the component: button This is the name of the generic UI component, not the React module! label Sep 30, 2022
@meenarama
Copy link
Contributor Author

@mnajdova , could you let me know which release, this will be in?

@mnajdova
Copy link
Member

It's released already @meenarama v5.10.8

@meenarama
Copy link
Contributor Author

@mnajdova , Yup, saw that, thx!

alexfauquette pushed a commit to alexfauquette/material-ui that referenced this pull request Oct 14, 2022
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module! component: Fab The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fab] button disabled class (classes.disabled) is not working.
6 participants