Skip to content
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

RadioButtonGroup usability issue (renders it's own children regardless of given children types) #8225

Closed
kubijo opened this issue Mar 30, 2021 · 4 comments · Fixed by #8283
Closed

Comments

@kubijo
Copy link
Contributor

kubijo commented Mar 30, 2021

Detailed description

What version of the Carbon Design System are you using?

carbon-components-react@npm:7.31.0

What did you expect to happen?

I've created a wrapper component for RadioButton to support our design addition of description text & then spent half an hour scratching my head as to why my component is not rendered.

It's because you are rendering your version of RadioButton regardless of what is given as children.

I'd consider this to be quite an ugly anti-pattern.
If you need to insist on using the only right component, why aren't you using data to render (an array of records as a prop for RadioButtonGroup) instead of remapping children.

Additional information

image

@vpicone
Copy link
Contributor

vpicone commented Apr 1, 2021

@kubijo Definitely agree, the fact that this works is evidence of that haha.

  <RadioButtonGroup>
    <h1
      id="radio-1"
      value="standard"
    />
    <html
      id="radio-2"
      value="default-selected"
    />
  </RadioButtonGroup>

Cloning the child rather than forcing our own RadioButton would likely be the best way forward here.

@andreancardona
Copy link
Contributor

@kubijo going to implement a solution to close this! :)

@tw15egan
Copy link
Member

tw15egan commented Apr 1, 2021

Looks like this was introduced way back in PR #124, it's almost about to start kindergarten now 😂 Definitely due for a refactor!

@andreancardona
Copy link
Contributor

PR can be seen here - will close once approved and merged: #8283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants