[Tabs] Remove duplicate styles

[cmfcmf]

It looks like the first [theme.breakpoints.up('sm')] was being overwritten by the second theme.breakpoints.up('sm'), therefore the intended increased padding on sm and bigger screens did not take effect.

• I have followed (at least) the PR section of the contributing guide.

(Note that I am not sure whether we actually want the padding or whether the Material Design spec has changed over the years and the padding should be different. However, the current state is clearly a bug)

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against 1891455

[oliviertassinari]

Related to #15824 and #15324.

[oliviertassinari]

What do you think about?

• removing the min-width style
• keeping the mobile & desktop padding at 24px

[eps1lon]

The demos now look off:

1. small tab size but full width tablist = space used that is not needed
   e.g. https://deploy-preview-23561--material-ui.netlify.app/components/tabs/#basic-tabs
2. Individual tabs should use some min-width
   Otherwise they wrap on smaller tablist sizes. You can reproduce this in the visual regression test (https://www.argos-ci.com/mui-org/material-ui/builds/5415 docs-components-tabs/NavTabs.png) or by resizing the window in the docs

[oliviertassinari]

1. small tab size but full width tablist = space used that is not needed

@eps1lon Are you suggesting that we rework the demos? I like the idea! I think that they are too much mobile-driven. Google Cloud seems to better leverage them:

The way Stripe, Ant Design, and the other handles it is cleaner:

We could have a simple border-bottom style, using <Box sx={{ borderBottom: 1, borderColor: 'divider' }} />. It's used in some of the demos of https://material.io/components/tabs.

---

2. Individual tabs should use some min-width

The demo impacted is https://deploy-preview-23561--material-ui.netlify.app/components/tabs/#nav-tabs. A min-width seems to would only impact the visual regression test, not the outcome in developers's app: https://codesandbox.io/s/navtabs-material-demo-forked-e3s0l?file=/demo.js. I think we are good on this end.

This screenshot would suggest we need a min-width, but the official implementation doesn't have one.

[eps1lon]

A min-width seems to would only impact the visual regression test, not the outcome in developers's app:

We support resizing the window in our demos do we not?

This screenshot would suggest we need a min-width, but the official implementation doesn't have one.

Ok? Sounds like you're suggesting our components should not accept CSS properties that aren't used in the official implementation. Tabs vertically resizing when I resize the page horizontally does not sound ok to me.

Not having min-width by default is understandable since it's almost always either too much or too little. In the end the Tabs should not change dimensions in the y-axis on resize.

small tab size but full width tablist = space used that is not needed

Retracting it for now. Personally, I would always add a visual separator for tabs so that it's obvious where they end and what "dead space" is. But that's out-of-scope for this PR-

[oliviertassinari]

We support resizing the window in our demos do we not?

@eps1lon Did you try with a min-width? I see no impacts. Proof: open the demo, from the master branch, with the min-width in a code sandbox and resize it: https://codesandbox.io/s/ubt9g

Tabs vertically resizing when I resize the page horizontally does not sound ok to me.

At least, it's not the default, it only happens because the tabs are set full-width, which is expected. By default, it scrolls :).

[eps1lon]

At least, it's not the default, it only happens because the tabs are set full-width, which is expected. By default, it scrolls :).

So do you agree that this is bad behavior or not? We shouldn't document problematic code.

Did you try with a min-width? I see no impacts. Proof: open the demo, from the master branch, with the min-width in a code sandbox and resize it: codesandbox.io/s/ubt9g

I'm strictly speaking in the context of our docs (resized horizontally to minimum possible):

https://next--material-ui.netlify.app/components/tabs/#nav-tabs:

https://deploy-preview-23561--material-ui.netlify.app/components/tabs/#nav-tabs:

[oliviertassinari]

So do you agree that this is bad behavior or not? We shouldn't document problematic code.

@eps1lon Yes and no. In full-width mode, the Material Design guidelines ask to wrap the text if there isn't enough space. https://material.io/components/tabs#anatomy, https://material.io/archive/guidelines/components/tabs.html#tabs-content. I think that behavior is fundamentally correct. However, I also think that we should avoid text wrapping as much as possible for the demos. For instance, we could use shorter labels, as done in the guidelines, or reduce the padding.

I'm strictly speaking in the context of our docs (resized horizontally to minimum possible):

Does it come from the change of horionztal padding? In next, we have 12px and in PR 24px.

I have reduced the ambition of the pull request, it removes the dead code now, and improve the tests to avoid relying on implicit sizes

[oliviertassinari]

For future pull requests, it seems that we could significantly improve the https://next.material-ui.com/components/tabs/ page:

1. Use the Experimental API in all the pages, much better
2. Update the demos to focus on desktop, the current app bar is overused. Have a few demos for mobile when it makes sense
3. Update the styles to match the Material Design guidelines

cc @mbrookes

[oliviertassinari]

@cmfcmf Thanks for raising the issue with the dead code :)