Material-ui: [ButtonGroup] Different elements to <Button> as a child issue warnings

[Manasamahesh]

update this props name to fullwidth and textcolor,
React DOM Parser is not recognizing the props values.

import React from 'react';
import ButtonGroup from '@material-ui/core/ButtonGroup';
import Button from '@material-ui/core/Button';

export default function App() {  
  return (
    <div>
      <ButtonGroup variant="contained" size="small" aria-label="small contained button group">
        <Button>One</Button>
        <Button>Two</Button>
        <Button>Three</Button>
        <div>
          this div produces 'fullWidth' error
        </div>        
      </ButtonGroup>      
    </div >
  );
}

[eps1lon]

Your issue has been closed because it does not conform to our issue requirements.

For how-to questions and other non-issues, please use Spectrum.chat or StackOverflow instead of Github issues. There is a StackOverflow tag called "material-ui" that you can use to tag your questions.

[devel0]

Here a repro for the error.

this part of code makes console to log the error about fullWidth property; of course it's not expected to use div as child of a ButtonGroup, btw the error recall something coded that react reports as non conformant.

I noticed another property disableRipple reported too.

[hellorashid]

Any updates on this? I'm getting this same issue as well

[eps1lon]

Moving cloneElement -> Context switch a bit up my priority list. It's a common footgun since the default assumption is that children is a special prop that accepts anything. Our ButtonGroup only accepts Button but does not document this.

[eps1lon]

Any updates on this? I'm getting this same issue as well

Closed issues are not monitored closely. Since the original issue did not follow the issue template at all there was even less reason to look into this.

@devel0 Thank you very much for adding a repro. This helped a lot. Please open a new issue the next time. A maintainer can then decide if they want to merge issue or keep the other one abandoned.

[oliviertassinari]

@eps1lon I'm curious. What solution do you envison? I'm wondering about the trdeoff.

[eps1lon]

Use context instead of cloneElement

[Atralbus]

Additionally, we get the same error when we use IconButton component inside ButtonGroup which sounds like it might be a valid element. demo

[aleoyakas]

The same console error occurs when you use <Tooltip />

The following code:

<Tooltip
  title={t("jobHeader.downloadTooltip") as string}
  disableHoverListener={!disabled}
>
  <div>
    <Button
      variant="contained"
      onClick={downloadFunction}
      disabled={disabled}
    >
      {text}
    </Button>
  </div>
</Tooltip>

Gives similar console errors for the following properties:

• disableFocusRipple
• disableRipple
• fullWidth
• disableElevation

[silently]

This thread is confusing: the title refers to the fact some props are badly camel-cased (badly only regarding React) and many comments here respect this topic: those from Manasamahesh, devel0 and even aleoyakas which is in my opinion not off-topic.

What I don't get is the link with context and cloneElement and the replies from eps1lon.

The fix seems to be renaming some props (fullWidth -> fullwidth) so that we don't have those logged errors printed by React in the console devtools.

devel0 provided a repo to reproduce the error so it seems well specified.

[eps1lon]

What I don't get is the link with context and cloneElement and the replies from eps1lon.

ButtonGroup injects some properties into its children (using cloneElement) assuming they implement the fullWidth prop. Button does this but not IconButton which forwards excess props to the underlying DOM component. However, <div fullWidth> is not implemented by react-dom and it assumes that fullwidth is a non-standard HTML attribute in which case it should be lower cased.

The solution is to use a context instead of cloneElement so that children have to opt-in to button group props instead of having to opt-out.

As a temporary workaround use the following pattern:

function ButtonGroupIconButton(props) {
  // intercept props only implemented by `Button`
  const { disableElevation, fullWidth, variant, ...iconButtonProps } = props;
  return <IconButton {...iconButtonProps} />;
}

export default function BasicButtonGroup() {
  return (
    <div>
      <ButtonGroup color="primary" aria-label="outlined primary button group">
        <ButtonGroupIconButton>
          <ArrowBackIcon />
        </ButtonGroupIconButton>
        <ButtonGroupIconButton>
          <ArrowForwardIcon />
        </ButtonGroupIconButton>
      </ButtonGroup>
    </div>
  );
}

-- https://codesandbox.io/s/iconbutton-in-buttongroup-2yhmu?file=/demo.js

[silently]

The solution is to use a context instead of cloneElement so that children have to opt-in to button group props instead of having to opt-out.

thanks for the response,
I wonder if renaming props (like fullWidth -> fullwidth) over the codebase can be considered a solution?

[eps1lon]

I wonder if renaming props (like fullWidth -> fullwidth) over the codebase can be considered a solution?

That would be a breaking change for Button and not in line with how we name or props.

[eps1lon]

For the people stuck on v4 or who need a quick fix:

function ButtonGroupIconButton(props) {
  // intercept props only implemented by `Button`
  const { disableElevation, fullWidth, variant, ...iconButtonProps } = props;
  return <IconButton {...iconButtonProps} />;
}

export default function BasicButtonGroup() {
  return (
    <div>
      <ButtonGroup color="primary" aria-label="outlined primary button group">
        <ButtonGroupIconButton>
          <ArrowBackIcon />
        </ButtonGroupIconButton>
        <ButtonGroupIconButton>
          <ArrowForwardIcon />
        </ButtonGroupIconButton>
      </ButtonGroup>
    </div>
  );
}

-- codesandbox.io/s/iconbutton-in-buttongroup-2yhmu?file=/demo.js