Material-ui: [ButtonGroup] Support Child Button variant overwritting

Created on 7 Aug 2019  路  9Comments  路  Source: mui-org/material-ui

  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 馃

When creating a ButtonGroup I actually want to implement the group, but have the possibility to overwrite the Button variant in the child.

Current Behavior 馃槸

Child overwriting isn't supported - ButtonGroup sets the variant to "outlined" by default

Summary 馃挕

When creating a ButtonGroup I actually want to implement the group, but have the possibility to overwrite the Button variant in the child.

Motivation 馃敠

I have 3 buttons in the group and I use a component reducer to check which one of them is currently "selected / active" - but I've checked the component source and since the ButtonGroup sets the variant I'm not able to overwrite the variant of one of the child buttons.

The idea is to have one set to "contained" to use it as am indicator which one is currently selected.

In the end I would / could create a PR.

ButtonGroup enhancement good first issue

Most helpful comment

@nvwebd Do you want to submit a pull request? :)

Let's not change the current demo though, if that's okay.

All 9 comments

@nvwebd This sounds like a similar concern to: #16876.

@oliviertassinari well kind of - this would only change the color but the difference that would be cool to change is the variant - since there's a big difference between those 2 values.

What I'm trying to achieve is basically have 1 of the "contained" and the other two "outlined".

What I'm trying to achieve is basically have 1 of the "contained" and the other two "outlined".

I can imagine a use case for it. Have you tried experimenting around?

This sounds like a similar concern to: #16876.

@nvwebd The fix would be the same too.

@oliviertassinari yeah I tried a bit - will have to check if I can simply add the styles directly to it and overwrite the styles to have it filled - will report. If not then I'll update the component itself and create a PR.

@mbrookes sure? I checked the component implementation and it seems to me that only the color would be changed. What I wanted is to use the variant prop to have the styles applied / the button to be filled.

The proposal of @mbrookes seems to work correctly:

diff --git a/docs/src/pages/components/buttons/SplitButton.js b/docs/src/pages/components/buttons/SplitButton.js
index 7ddb74ace..31b7ce9fc 100644
--- a/docs/src/pages/components/buttons/SplitButton.js
+++ b/docs/src/pages/components/buttons/SplitButton.js
@@ -41,7 +41,7 @@ export default function SplitButton() {
   return (
     <Grid container>
       <Grid item xs={12} align="center">
-        <ButtonGroup variant="contained" color="primary" ref={anchorRef} aria-label="split button">
+        <ButtonGroup color="primary" ref={anchorRef} aria-label="split button">
           <Button onClick={handleClick}>{options[selectedIndex]}</Button>
           <Button
             color="primary"
diff --git a/packages/material-ui/src/ButtonGroup/ButtonGroup.js b/packages/material-ui/src/ButtonGroup/ButtonGroup.js
index 0798ea1aa..752ea8057 100644
--- a/packages/material-ui/src/ButtonGroup/ButtonGroup.js
+++ b/packages/material-ui/src/ButtonGroup/ButtonGroup.js
@@ -135,7 +135,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(props, ref) {
           disableRipple,
           fullWidth,
           size: child.props.size || size,
-          variant,
+          variant: child.props.variant || variant,
         });
       })}
     </Component>

Capture d鈥檈虂cran 2019-08-08 a虁 16 15 17
@nvwebd Do you want to submit a pull request? :)

@nvwebd Do you want to submit a pull request? :)

Let's not change the current demo though, if that's okay.

Will add the PR then ;)

PR created. Hope I followed correctly :) https://github.com/mui-org/material-ui/pull/16946

Was this page helpful?
0 / 5 - 0 ratings

Related issues

NonameSLdev picture NonameSLdev  路  56Comments

aranw picture aranw  路  95Comments

kybarg picture kybarg  路  164Comments

amcasey picture amcasey  路  70Comments

iceafish picture iceafish  路  62Comments