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

[Tab] children are not accepted, even though docs state "Any other properties supplied will be spread to the root element" #11860

Closed
2 tasks done
jedwards1211 opened this issue Jun 14, 2018 · 6 comments
Assignees
Labels
component: tabs This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented Jun 14, 2018

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The docs for Tab say:

Any other properties supplied will be spread to the root element (ButtonBase).

Therefore if I render

<Tab>
  <Sometimes />
  <You />
  <Need />
  <More />
  <Than />
  <One />
  <Icon />
</Tab>

all of those children get spread to the ButtonBase.

Current Behavior

The children are not spread to the ButtonBase or rendered in any way. The docs lie...

Steps to Reproduce (for bugs)

https://codesandbox.io/s/wo977lzr98

Context

I need to put both a progress spinner and a delete button on a tab, but Tab only provides label and icon properties, which is a bit annoying, because this looks gross:

<Tab
  label={
    <React.Fragment>
      The label
      {working && <Spinner />}
      <DeleteButton onClick={...} />
    </React.Fragment>
  }
/>

Your Environment

Tech Version
Material-UI v1.2.1
React v16.4.0
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 14, 2018

The children are not spread to the ButtonBase or rendered in any way. The docs lie...

I guess we should document that there is no children property, and warn if someone tries to us it?

for cases when the desired content doesn't fit neatly into the label/icon dichotomy

I think that it can be better handled with the component property.

@oliviertassinari oliviertassinari added the component: tabs This is the name of the generic UI component, not the React module! label Jun 14, 2018
@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Jun 14, 2018

I think that it can be better handled with the component property.

I don't see a component property for Tab in the docs, nor is there evidence for one in the source code

It's certainly possible to pass the children through, as I do in my PR.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 14, 2018

@jedwards1211 The component property is inherited by the ButtonBase.

It's certainly possible to pass the children through, as I do in my PR.

I have two issues with that.

  1. The children are added to an opinionated position. Better let user control the position:
<Tab component={props => <button {...props}><Prefix />{props.children}<Sufix /></button>} />
  1. The problem isn't scoped to the tab component. You can find it on the Chip and possibly more components.

@jedwards1211
Copy link
Contributor Author

Ah right, I forgot that ButtonBase takes care of that. Okay, that works for me, thanks!

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 14, 2018

Wait, shouldn't we warn or document the missing children support?

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Jun 14, 2018
@jedwards1211
Copy link
Contributor Author

Ah, yes that's true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tabs This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants