Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-content doesn't work correctly with flex. #4022

Closed
langley-agm opened this issue Aug 5, 2015 · 24 comments
Closed

md-content doesn't work correctly with flex. #4022

langley-agm opened this issue Aug 5, 2015 · 24 comments
Assignees
Labels
needs: feedback The issue creator or community need to respond to questions in this issue needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community needs: team discussion This issue requires a decision from the team before moving forward. P2: required Issues that must be fixed. ui: flexbox ui: layout
Milestone

Comments

@langley-agm
Copy link

Maybe it works as expected but if it does, I'm not sure I fully understand the purpose of md-content and the documentation resumes up to one single line. I made a shorter example of my issue.

I want two divs with overflowing content each covering half the page,

This example works as I would expect (the two divs are immediate children of the body):

http://codepen.io/langley/pen/Nqorzv

However if they are not direct children of the body, but a div instead, or even another md-card, md-whiteframe or any other, it stops working:

http://codepen.io/langley/pen/waNWYe

In order to make it work I have to apply height: inherit or height: 100% to the parent:

http://codepen.io/langley/pen/KpJMbR

It's not a big deal in this example but if the object I want to be scrollable is a nested child of 10 elements, I have to apply this to every single parent, not just the closest.

I see that in the demo a fixed px value is used, but I thought the whole idea of being responsive is to use flex-xx style and % instead of fixed px values ?

@nkoterba
Copy link

nkoterba commented Aug 6, 2015

@langley-agm I'm not sure how to do that cool little thumbs up emoticon, so I'll just have to do:

+1 +1 +1

Been struggling all day to get a md-content nested inside several layers of divs with layout="column" and "flex" to correctly scroll.

Using latest chrome and MD 0.10.0 (no RCs yet for me).

I agree that the demo of md-content uses a fixed height, which always works but like you said totally defeats the purpose of flexbox.

I'm not sure if this is a bug, intentional design, or what.

My current hack/workaround is to give the md-content a style of height: 1px. This seems to trick (at least Chrome, nervous about FF and IE) into correctly "boxing" md-content so my inner "overflowing" content scrolls (and not the whole parent div).

http://codepen.io/anon/pen/MwLQqJ

Here's another code-pen that is the current demo MD uses in their documentation for md-content. All I've done is add more content to ensure scrolling will occur and remove the "fixed" height of the md-content element.

You see the same behavior you describe above.

What I would expect to happen is that if I add flex to the md-content, it should fill all the space after the md-toolbar but stop at the bottom of the parent div and the md-toolbar should not scroll, just the content in the md-content container.

@juandiago
Copy link

@nkoterba thnks for the height:1px workaround.

Works like a charm!

@topherfangio
Copy link
Contributor

Hi all, I am looking into this issue, but I'm having trouble understanding the height: 1px fix/workaround. When I apply that to the md-content, it gives me a 1px tall block of text (which doesn't seem like a proper fix).

Can one of you produce a quick demo that shows the issue and has the CSS fix commented out so that I can comment it back in and see the difference?

@topherfangio topherfangio added the needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue label Aug 14, 2015
@topherfangio topherfangio self-assigned this Aug 14, 2015
@juandiago
Copy link

Here you go:

http://codepen.io/anon/pen/gpyEjV

Just remove the height:1px from the md-content

Hope it helps!..

@nkoterba
Copy link

@topherfangio @juandiago demo is a good example. I updated the codepen to use the "master" CSS and JS files as it was still on 0.10 (instead of 0.10.1 or master):

http://codepen.io/anon/pen/vOMMbX

Issue still exists. As @juandiago said, if you remove the height: 1px then the entire "page" scrolls versus just the stuff in the top md-content.

@topherfangio
Copy link
Contributor

I can confirm that the following code should fix the issue:

md-content[flex] {
  height: 0px;
}

We have a team meeting today, let me confirm with the others that this shouldn't break anything else.

@topherfangio
Copy link
Contributor

Seems the same as #3901. We're still trying to determine if there is a better fix or not.

@topherfangio topherfangio added this to the 0.12.0 milestone Aug 27, 2015
topherfangio added a commit that referenced this issue Aug 29, 2015
In some cases, browsers have issues with rendering flex layouts
when the height of an element is not set. This simply forces each
layout element (which is the most common case when using flex) to
the height it would normally be.

Fixes #4022. References #3901.
topherfangio added a commit that referenced this issue Aug 31, 2015
In some cases, browsers have issues with rendering flex layouts
when the height of an element is not set. This simply forces each
layout element (which is the most common case when using flex) to
the height it would normally be.

Fixes #4022. References #3901.
@ThomasBurleson
Copy link
Contributor

Closed with SHA 303ab0d

topherfangio added a commit that referenced this issue Sep 1, 2015
Originally, it seemed that adding the `height: initial` made no
significant change to the layout system except to fix #4022,
however, multiple users have experienced major issues with
their layouts: #4397.

We are reverting so that we can take some more time to work
with the community to find a better fix.

Reverts commit 303ab0d
@topherfangio
Copy link
Contributor

Due to some unexpected breaking in other developers applications (#4397), we have decided to remove this change from 0.11 and get some more community feedback before pushing a fix in 0.12.

Reverted in commit 44a7e52

@topherfangio topherfangio reopened this Sep 1, 2015
@topherfangio topherfangio added needs: team discussion This issue requires a decision from the team before moving forward. needs: feedback The issue creator or community need to respond to questions in this issue needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community and removed needs: demo A CodePen demo or GitHub repository is needed to demonstrate the reproduction of the issue labels Sep 1, 2015
kennethcachia pushed a commit to kennethcachia/material that referenced this issue Sep 23, 2015
Originally, it seemed that adding the `height: initial` made no
significant change to the layout system except to fix angular#4022,
however, multiple users have experienced major issues with
their layouts: angular#4397.

We are reverting so that we can take some more time to work
with the community to find a better fix.

Reverts commit 303ab0d
@nkoterba
Copy link

Just wanted to say this is definitely still broken as of Sept 28, 2015 on the HEAD version.

Check out the codepen: https://codepen.io/anon/pen/avBMKd

Thankfully, I followed #4453, which suggests the following fix that I currently have commented out in the codepen.

Just re-enable the suggested "fix" CSS, and md-content scrolls again!

Update:

  • Originally, without a defined height, the scrollbar would appear but scroll all content.
  • Now, without a height, there is no scrollbar, but nothing scrolls.

Perhaps 3rd time will be the charm?

@ThomasBurleson
Copy link
Contributor

We just updated master with stability fixes in layout.css.
With those fixes and tweaks to your demo fixes the Toolbar with Scroll Demo works

Note the overrides to the docs.css with:

body {
  height : 100%;
  overflow : auto; 
}

Also this class is invalid: class="md-flex"
Please use [flex], [flex-md], etc.

@ThomasBurleson ThomasBurleson modified the milestones: Backlog, 1.1.0 Jan 15, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.0, 1.3.0, Backlog, Layout Engine Apr 21, 2016
@ThomasBurleson ThomasBurleson modified the milestones: - Layout, Deprecated May 26, 2016
@ThomasBurleson
Copy link
Contributor

This issue is closed as part of our ‘Surge Focus on Material 2' efforts.
For details, see our forum posting @ http://bit.ly/1UhZyWs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: feedback The issue creator or community need to respond to questions in this issue needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community needs: team discussion This issue requires a decision from the team before moving forward. P2: required Issues that must be fixed. ui: flexbox ui: layout
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants