Material-ui: [RadioButtonGroup] breaks when nesting non-RadioButton components

[bshyong]

I'm trying to do some simple styling. The following example fails.

<RadioButtonGroup>
  <div className="col6">
    <RadioButton />
  </div>
  <div className="col6">
    <RadioButton />
  </div>
</RadioButtonGroup>

I have written a patch that resolves this; let me know if I should issue a pull request.

[oliviertassinari]

I'm wondering, why aren't you using the className property of the RadioButton?

[bshyong]

Olivier, I need to nest the buttons in a div with clearfix for the layout that I want. If you look at the internal implementation of RadioButtonGroup it Iterates through and adds props to RadioButtons - I don't see a reason for it to nuke your layout structure or for the component to break if you have a nonRadioButton element nested inside. Thoughts?

[oliviertassinari]

let me know if I should issue a pull request

Would be interesting to see what you came up with to solve this issue.

[bshyong]

@oliviertassinari

Leaves structure alone but recursively maps through all radio buttons and creates them correctly

  radioButtonMapFunc(option) {
    if (option.props.children === undefined) {
      let {
        name,
        value,
        label,
        onCheck,
        ...other,
      } = option.props;
      if (option.type.displayName === 'RadioButton') {
        return <RadioButton
          {...other}
          ref={option.props.value}
          name={this.props.name}
          key={option.props.value}
          value={option.props.value}
          label={option.props.label}
          labelPosition={this.props.labelPosition}
          onCheck={this._onChange}
          checked={option.props.value === this.state.selected}/>;
      }
    }

    return {
      ...option,
      props: {
        ...option.props,
        children: React.Children.map(option.props.children, this.radioButtonMapFunc, this)
      }
    }
  },

  render() {
    let options = React.Children.map(this.props.children, (option) => {
      return this.radioButtonMapFunc(option)
    }, this);

    return (
      <div
        style={this.prepareStyles(this.props.style)}
        className={this.props.className || ''}>
        {options}
      </div>
    );
  },

[suricactus]

Hey, any progress on this? I ended up with the same bug, using react-flexgrid components.

[bshyong]

PR here https://github.com/callemall/material-ui/pull/3627

[rkmax]

I'm having troubles also for this. because I need render a radios and text fields together. any progress?

[bshyong]

@rkmax my PR hasn't been merged into master but you can take a look here https://github.com/callemall/material-ui/pull/3627

You could copy the RadioButtonGroup.js component from my repo and try it out for your own use (keeping everything else the same)

[RohovDmytro]

Hey, any progress with this? Merging would be great.

[coreyar]

The solution @bshyong wrote works great.

You will need to update line 112 to read option.type.name === 'RadioButton'.

[bshyong]

If possible, someone can reopen and merge this fix to resolve this issue: https://github.com/callemall/material-ui/pull/3627

[mbrookes]

@bshyong https://github.com/callemall/material-ui/pull/3627#issuecomment-217744111. However, focus has moved to next. I would suggest waiting for @oliviertassinari's input on whether a revised PR would be accepted before doing any further work on it.

[bshyong]

Makes sense. I would stick with the recursive approach for now, as I would like to avoid the use of context unless it is absolutely necessary. https://facebook.github.io/react/docs/context.html#why-not-to-use-context

We can revisit if this is still a problem once next is complete

[mbrookes]

I would like to avoid the use of context

I'm very familiar with that page, but I think this use-case sits outside that warning.

From a brief look at the code, it does seem next will have a similar issue. Whether addressing it in the library is appropriate or should be handled by the dev is another question. It's a simple enough matter to make the wrapper component pass props to the RadioButton.

[car3oon]

Hi guys
Any progress on merging this issue?

[wcandillon]

I'm also interested to see this feature shipped.

[marutha]

I am also interested to see this

[thiagoxvo]

Same here...

[fabyeah]

I need this to display additional content below the selected radio button.

[bshyong]

@fabyeah my PR hasn't been merged into master but you can take a look here #3627

You could copy the RadioButtonGroup.js component from my repo and try it out for your own use (keeping everything else the same)

[oliviertassinari]

In order to keep the implementation simple and slim, I'm closing the issue with the following solution.
On the v1 branch, the canonical way of using the RadioButtonGroup component is:

<RadioGroup
  aria-label="gender"
  name="gender2"
  className={classes.group}
  value={this.state.value}
  onChange={this.handleChange}
>
  <Radio value="male" />
  <Radio value="female" />
  <Radio value="other" />
  <Radio value="disabled" disabled />
</RadioGroup>

Still, one might want to change the way the radios are rendered. We have the FormControlLabel component for this use-case:

<RadioGroup
  aria-label="gender"
  name="gender2"
  className={classes.group}
  value={this.state.value}
  onChange={this.handleChange}
>
  <FormControlLabel value="male" control={<Radio />} label="Male" />
  <FormControlLabel value="female" control={<Radio />} label="Female" />
  <FormControlLabel value="other" control={<Radio />} label="Other" />
  <FormControlLabel value="disabled" disabled control={<Radio />} label="Disabled" />
</RadioGroup>

So, either you can get along with the FormControlLabel component or you can start from the FormControlLabel implementation to build your own. Let me know if it's no enough.

[oliviertassinari]

I'm reopening as #10576 makes me believe we should be documenting this behavior and add an example of how to forward the properties.

[oliviertassinari]

Workaround n°1.

The RadioGroup component is using the React cloneElement API: #12921. This is a problem because it's hidden from our users, it's happening behind the scene. The immediate children of the RadioGroup should handle the injected properties. If they don't, the control is lost.
Now, you can expose the injecteed properties with a Wire component and apply them where needed:

const Wire = ({ children, ...props }) => children(props);

// …

<RadioGroup
  aria-label="Gender"
  name="gender1"
  className={classes.group}
  value={this.state.value}
  onChange={this.handleChange}
>
  <Wire value="female">
    {props => (
      <Tooltip title="text">
        <FormControlLabel
          control={<Radio />}
          label="Female"
          {...props}
        />
      </Tooltip>
    )}
  </Wire>

https://codesandbox.io/s/zr7x1xx32x

Workaround n°2.

Form libraries like react-final-from can handle the state centralization problem, you can choose not to delegate this task to the RadioGroup. By handling each Radio independently, you can workaround the problem.

[onpaws]

Thanks Oliver the <Wire> component appears to be working for my needs