Skip to content

[Tabs] Add variant prop and deprecate fullWidth and scrollable props#13980

Merged
oliviertassinari merged 7 commits into
mui:masterfrom
rfbotto:add-variant-prop-to-tabs
Dec 25, 2018
Merged

[Tabs] Add variant prop and deprecate fullWidth and scrollable props#13980
oliviertassinari merged 7 commits into
mui:masterfrom
rfbotto:add-variant-prop-to-tabs

Conversation

@rfbotto

@rfbotto rfbotto commented Dec 21, 2018

Copy link
Copy Markdown
Contributor

Closes #13610

Deprecation

The Tabs fullWidth and scrollable properties can't be used at the same time. The API change prevents any awkward usage.

-<Tabs fullWidth>
+<Tabs variant="fullWidth">

@joshwooding

Copy link
Copy Markdown
Collaborator

@rfbotto I think the preferred route of deprecation, is to first display warnings when the deprecated prop is used and then in the next major version (v4) to remove them.

@rfbotto

rfbotto commented Dec 21, 2018

Copy link
Copy Markdown
Contributor Author

That's a good point @joshwooding, didn't think of it. Then it should be fine to keep this PR open and only merge it into the next major version and in the meantime create a PR with the deprecation warning for those props (in case the solution in this PR gets accepted).

@oliviertassinari oliviertassinari added the scope: tabs Changes related to the tabs. label Dec 24, 2018
@oliviertassinari oliviertassinari added the deprecation New deprecation message. label Dec 24, 2018
@oliviertassinari oliviertassinari added this to the v3.8.0 milestone Dec 24, 2018
@oliviertassinari
oliviertassinari merged commit 0102308 into mui:master Dec 25, 2018
@oliviertassinari

Copy link
Copy Markdown
Member

The deprecation path looks great, well done 👌 !

@eddy03

eddy03 commented Jan 2, 2019

Copy link
Copy Markdown

Nice. But I have some problem with this latest update.

My tab will use fullWidth in mobile and using default in Desktop.

Before this. It's very easy.

const {classes, fullScreen} = this.props

<Tabs fullWidth={fullScreen}
...

export default withStyles(styles)(withMobileDialog()(Module))

This latest update make my code become messy because variant props only accept 2 string.. Perhaps having a default value to set back to tabs behavior to default behavior is appreciated. 😺

@olee

olee commented Apr 12, 2019

Copy link
Copy Markdown
Contributor

Hi, I have to disagree with this change.
We are using scrollable and fullWidth together just fine, it only requires adding a minWidth to the Tab button components but otherwise it works just fine.
This is a breaking change for our layout - could this be reverted / are there other solutions to keep fullWidth and scrollable available at the same time?

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

Labels

deprecation New deprecation message. scope: tabs Changes related to the tabs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants