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

[Divider] Height is not correct for vertical divider in a flexbox #18022

Closed
2 tasks done
hdstusnick opened this issue Oct 24, 2019 · 4 comments · Fixed by #19614
Closed
2 tasks done

[Divider] Height is not correct for vertical divider in a flexbox #18022

hdstusnick opened this issue Oct 24, 2019 · 4 comments · Fixed by #19614
Labels
bug 🐛 Something doesn't work component: divider This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@hdstusnick
Copy link

When a divider component with vertical orientation is inside a container with flex and height set to 100%, the height is calculated as 0px in chrome.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The vertical dividing lines do no appear to show up on the page in chrome, but when inspecting the page, I can find the <hr> tag and see that it has height: 100% and a calculated height of 0px. When opening the page in firefox the vertical lines appear as expected.

This only happens when the parent component has flex properties. If I wrap the divider component in an empty <span>, the lines do appear.

The height of the lines should be calculated to be the full hight of its parents when set to height: 100%.

Expected Behavior 🤔

In all cases, the height of the divider component should be calculated as the height of its parent when set to height: 100%

Steps to Reproduce 🕹

Open this link in both chrome and firefox to see the the issue: https://codesandbox.io/s/reverent-kare-iuonx

Your Environment 🌎

Tech Version
Material-UI v4.5.1
React 16.10.2
Browser Chrome
@oliviertassinari
Copy link
Member

@hdstusnick In a flex container, use:

hr {
  align-self: stretch;
  height: auto;
}

I don't know how we could internalize this style. Maybe with a flexItem?: boolean prop?

@oliviertassinari oliviertassinari added component: divider This is the name of the generic UI component, not the React module! discussion labels Oct 24, 2019
@mirismaili
Copy link
Contributor

Is the below bug is related to this issue? As you can see, the vertical divider is not visible (its calculated height is 0!).

image

That's from the official example in documentations:

https://p5xpy.csb.app/


I tested it on Chrome 78.0.3904.108 (Official Build) (64-bit) and Firefox 69.0 (64-bit).

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. and removed discussion labels Dec 3, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 3, 2019

@mirismaili Yes, I believe it's exactly the problem we are discussing here. I would propose that we move forward with the flexItem prop so people can declare when the parent is a flex container so the divider can use a different strategy to cover the whole screen: #18022 (comment)

@oliviertassinari oliviertassinari changed the title Height is not calculated correctly for vertical divider in a flexbox in chrome [Divider] Height is not correct for vertical divider in a flexbox Dec 3, 2019
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Dec 3, 2019
@captain-yossarian
Copy link
Contributor

@mirismaili @oliviertassinari Can I pick it up ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: divider This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants