[ButtonGroup] diableRipple not working

[dimitropoulos]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Current behavior 😯

see https://codesandbox.io/s/splitbutton-demo-material-ui-forked-uvbgo3 for an example.

The guidance (https://mui.com/material-ui/getting-started/faq/#how-can-i-disable-the-ripple-effect-globally) for disabling ripple globally does not affect the ButtonGroup.

Expected behavior 🤔

The ButtonGroup component in the example does not have a ripple.

Steps to reproduce 🕹

Steps:

1. Follow the guidance in https://mui.com/material-ui/getting-started/faq/#how-can-i-disable-the-ripple-effect-globally
2. Create a ButtonGroup component.

Context 🔦

Trying to accomplish disabling the ripple effect globally.

Your environment 🌎

no env

[Moizsohail]

When using ButtonGroup. you also need to add this in the createThemeObject

MuiButtonGroup: {
    defaultProps: {
      disableRipple: true
    }
  },

[dimitropoulos]

@Moizsohail thanks, and I did do that in my code, but the bug here is that it's intended that MUI can turn off ripple globally as instructed in the docs ("How can I disable the ripple effect globally?") and that instruction doesn't work only in this one situation (because of a presumed bug with MuiButtonGroup). While it's helpful to have a workaround, of course, the bug remains (at least insofar as the design goal is concerned).

[Moizsohail]

I thought about it but I don't think it's a bug. Basically the default disableProp being passed from the ButtonGroup takes preference over the overrides from the globalContext. Let's see what the maintainers have to say about it. @oliviertassinari There can be two possible resolutions.

1. Remove the default prop.
2. Update the doc by adding a small hint.

I am happy to work on it though.

[dimitropoulos]

both 1 and 2 would be good, but it sounds like 1 would be better.

It sounds like we understand each other. I know what you mean, but from a user experience standpoint (not a code standpoint) it's 100% a bug. It says "do this to globally disable the ripple" and when you do the thing it says it doesn't globally disable the ripple.

If nothing else, maybe the default of the MuiButtonGroup can inherit from MuiButtonBase somehow? While updating the snippet to include a section for MuiButtonGroup, it'd be great if it could stay simple to do like it is now on the docs. As MUI expands, it'd be great to have all components look to MuiButtonBase for this functionality (the way Tabs do now).

[oliviertassinari]

I wouldn't be surprised if this problem hasn't been discussed multiple times on GitHub issues in the past. I think to have a deep dive into the issue history to find them before moving in any direction. e.g. https://github.com/mui/material-ui/issues/29740#issuecomment-989805271

[dimitropoulos]

Totally.

So what are the next steps towards fixing this? By "fix" I mean that MUI is changed in such a way that the instruction on the docs remains as-is and (if followed) the ripple will actually be disabled globally (i.e. including MuiButtonGroup).

[Moizsohail]

@oliviertassinari By your response https://github.com/mui/material-ui/issues/29740#issuecomment-989805271, what I understand is that this not the intended behavior.

I am interested in working on fixing this issue.

[michaldudak]

@oliviertassinari This is a different issue than the one discussed in https://github.com/mui/material-ui/issues/29740#issuecomment-989805271. In the case of LoadingButton, we were discussing inheriting props. In this case, the behavior of the Button is changed depending on whether it's standalone or within a ButtonGroup.

I don't think the ButtonGroup should set defaults for props like disableElevation or disableRipple if they are not explicitly defined.

[dimitropoulos]

@michaldudak if you don't think it should be set by default, how do you suggest, then, that a user would disable rippling globally? this is a pretty common thing that people want to turn off since it's quite material-design-specific and plenty of people aren't using material design.

[michaldudak]

What I meant was if a Button Group doesn't have props like disableElevation explicitly set, it should not initialize them with false, but rather allow the child buttons to set it themselves. This way you would be able to override the defaults of MuiButton and have them working everywhere, no matter if a Button is a part of a Button Group or standalone. I'm afraid this would be a breaking change, though, so we may include such a change in the next major version.