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
[ButtonGroup] Should not retain divider color when it is disabled and variant is text
#36967
[ButtonGroup] Should not retain divider color when it is disabled and variant is text
#36967
Conversation
Netlify deploy previewhttps://deploy-preview-36967--material-ui.netlify.app/ Bundle size report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to apply a disabled border color using the disabled class as shown below instead of not applying a border color at all.
material-ui/packages/mui-material/src/ButtonGroup/ButtonGroup.js
Lines 140 to 153 in 4fc9750
...(ownerState.variant === 'contained' && | |
ownerState.orientation === 'horizontal' && { | |
borderRight: `1px solid ${(theme.vars || theme).palette.grey[400]}`, | |
[`&.${buttonGroupClasses.disabled}`]: { | |
borderRight: `1px solid ${(theme.vars || theme).palette.action.disabled}`, | |
}, | |
}), | |
...(ownerState.variant === 'contained' && | |
ownerState.orientation === 'vertical' && { | |
borderBottom: `1px solid ${(theme.vars || theme).palette.grey[400]}`, | |
[`&.${buttonGroupClasses.disabled}`]: { | |
borderBottom: `1px solid ${(theme.vars || theme).palette.action.disabled}`, | |
}, | |
}), |
The above code snippet is for the contained variant. The same is to be applied for the text variant for both horizontal and vertical orientations.
Also please add visual regression test. You can follow the README instructions on how to add it.
text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed the remaining changes because the issue is opened from a long time. @DavidBoyer11 Thanks for the PR.
… variant is `text` (mui#36967) Co-authored-by: FeCurtain <david_boyer@mac.com> Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>
Fixes #36735