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

[Tabs] Remove min-width media query #15824

Closed
2 tasks done
smeijer opened this issue May 24, 2019 · 6 comments · Fixed by #26458
Closed
2 tasks done

[Tabs] Remove min-width media query #15824

smeijer opened this issue May 24, 2019 · 6 comments · Fixed by #26458
Labels
breaking change component: tabs This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process new feature New feature or request
Milestone

Comments

@smeijer
Copy link

smeijer commented May 24, 2019

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Tabs should render in a certain style/layout, depending on the width of the Tabs (container), instead of depending on the width of the screen.

Current Behavior 😯

Tabs base their width on the screen, and not on the tabs container.

<div style={{ width: 350, border: '1px solid red' }}>
  <Tabs variant="fullWidth" scrollButtons="auto">
    <Tab label="Tab 1" />
    <Tab label="Tab 2" />
    <Tab label="Tab 3" />
    <Tab label="Tab 4" />
  </Tabs>
</div>

material-tabs

Steps to Reproduce 🕹

Link present: https://x87uz.codesandbox.io/
Link editor: https://codesandbox.io/embed/materialuitabs-x87uz

  1. Open the preview screen
  2. Notice that only TAB 1 and TAB 2 are visible
  3. Open devtools, responsive mode
  4. Scale screen to 350 px
  5. Notice that TAB 1, TAB 2, TAB 3 and TAB 4 are visible.

The tab container is in both cases, 350 px wide.

Context 🔦

Tabs should render the same based on the width of the tab container. Material is often used and optimized for mobile layouts. This case confirms it.

What I'm doing, is rendering multi-column layout on desktop, and on mobile this translates to multiple single-column screens. So on desktop you would see column 1 + 2, while on mobile you see 1 or 2.

Thereby, my tabs are rendered in a single column, and on desktop optimized for a fixed width. While they scale to fullWidth on mobile.

Your Environment 🌎

Tech Version
Material-UI v3.9.3
React v16.8.2
Browser Chrome 74
@oliviertassinari
Copy link
Member

@smeijer The title of the issue sounds reverse. We are using media queries right now, and you want to rely on JavaScript layout width computation to switch between the different breakpoints, right?

@smeijer smeijer changed the title [Tabs] Using media queries instead of javascript width detection [Tabs] Are using media queries instead of javascript width detection May 24, 2019
@smeijer
Copy link
Author

smeijer commented May 24, 2019

@oliviertassinari, correct. I've updated the title. I think it should be more clear now.

The problem of the media query is that it's based on the window size instead of the container size. I think this can give problems on other components as well, but it's the Tab's that caused me problems.

Basically what I'm saying is, that I'm not sure if a component (Tabs in this case), should be basing it's display properties on the window size. It should make more sense if it's basing its properties on the element size.

There are workarounds for this if we would want to stick to css media queries, for example with css-element-queries, but I'm not sure if that's the way to go and if it plays nicely with css-in-js.

@smeijer smeijer changed the title [Tabs] Are using media queries instead of javascript width detection [Tabs] Base their display logic on screen width, instead of element width May 24, 2019
@smeijer smeijer changed the title [Tabs] Base their display logic on screen width, instead of element width [Tabs] Should use their root-element width, instead of screen width May 24, 2019
@oliviertassinari oliviertassinari added the component: tabs This is the name of the generic UI component, not the React module! label May 24, 2019
@oliviertassinari oliviertassinari added this to the v5 milestone May 24, 2019
@oliviertassinari
Copy link
Member

@smeijer Your issue comes from the media query we have. I believe it's something we will adress in #15324, in 6 months from now. We should be able to remove the media query, following the specification.

For now, you can override the min-width CSS value.

@oliviertassinari oliviertassinari added the new feature New feature or request label May 24, 2019
@oliviertassinari oliviertassinari changed the title [Tabs] Should use their root-element width, instead of screen width [Tabs] Remove min-width media query May 24, 2019
@chrisVillanueva
Copy link

@oliviertassinari

A quick follow up. Should we continue to follow your most recent recommendation to resolve this issue?

you can override the min-width CSS value.

Or has the issue been resolved in the latest next branch build?

Thanks!

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 24, 2021

@chrisVillanueva material.io mentions 90px as the min-width and 360px as the max-width. Its what Vuetify implements. It's not what Google and most of the design community tend to do (no min-width) but it seems to be already a great improvement (a smaller value, and no more media query). I think that we could solve this issue like this.

@chrisVillanueva
Copy link

@oliviertassinari
That makes sense. Appreciate the response.

@oliviertassinari oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: tabs This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants