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

(fxFlex): zero value for Shrink xor Grow results in overflowing content when basis is px #153

Closed
somombo opened this issue Feb 1, 2017 · 4 comments · Fixed by #160
Closed
Assignees
Labels
flexbox has pr A PR has been created to address this issue in-progress
Milestone

Comments

@somombo
Copy link
Contributor

somombo commented Feb 1, 2017

I initially posted this on Stack Overflow and then in the Gitter chatroom but have not gotten any responses. So I figured I'd just go ahead and file it here.

Setting px basis value in fxFlex does not behave as expected when either of (but not both) shrink xor grow are set to zero.

See my plunkr examplifying this.

Here is an excerpt of the code with some Comments:

    <!-- Box A -->
    <div fxLayout="row" >
       <div fxFlex="0 1 2000px" >  
           <p>... Really Really long Lorem Ipsum ...</p>
           <!-- expecting flex div to shrink to viewport size, 
                with no horizontal scrolling necessary-->
    <!-- Box B -->
    <div fxLayout="column" >
       <div fxFlex="1 0 200px" >  
           <p>... Really Really long Lorem Ipsum ...</p>
           <!-- expecting flex div to grow to accommodate, 
                content size with vertical scrolling if necessary-->

In addition, here are screenshots of what I'm getting ("Actual") using only fxFlex API, versus what I should be getting ("Expected") if I used equivalent pure-css flex.

You can see these interactively at the plunkr above. Click on the view to switch between Actual and Expected.

Note that if your screen's viewport width happens to be greater than 2000px, then you'll need to make it smaller manually in order to see the effect shown in the screenshot below for "Box A".

actual-flex

expected-css

@somombo somombo changed the title Zero value for Shrink xor Grow results in overflowing content when basis is px. (fxFlex): zero value for Shrink xor Grow results in overflowing content when basis is px Feb 1, 2017
@somombo
Copy link
Contributor Author

somombo commented Feb 1, 2017

According to 2016 css spec,

"[flex-grow: 0 with flex-shrink=1 ] Makes the flex item inflexible when there is positive free space, but allows it to shrink to its minimum size when there is insufficient space."

This means if the flex container is smaller than the flex item as is the case in example with "Actual Box A" above, then the 0 1 ... growth-shrink factor should cause the flex-item to shrink as is shown in "Expected Box A". We see that this is not the case, and after playing with it a little I found that simply removing the min-width allows the Actual to appear as Expected in both the Box A and Box B cases

So Without loss of generality,
it looks like the problem is stemming from min-width being set to basis value whenever grow==0 (...and max-height being set whenever shrink==0 respectively). I think this should be prevented.

Looks like this issue is a bug and the code is not addressing these degenerate scenarios correctly ?

I am happy to provide PR :-)

@ThomasBurleson
Copy link
Contributor

@somombo - flexbox CSS has many known issues (see issues tab) and known flexbox bugs. The min-width settings were attempts to address those bugs.

Perhaps one solution is to browser detect for IE and only use the min-width/max-width settings for those browsers.

This needs significant exploration and regression testing.

@ThomasBurleson ThomasBurleson added this to the v2.0.0-rc.0 milestone Feb 2, 2017
@ThomasBurleson ThomasBurleson self-assigned this Feb 2, 2017
@somombo
Copy link
Contributor Author

somombo commented Feb 3, 2017

I'll call Known Issues "KI's" and Flexbugs "FB's" .

By "max/min-size", what I mean is max/min-height and/or max/min-width (will be more clear in context)

The KI#7 (regarding percentage heights of children) is the only KI that does appear to be listed among the FB's. However if you try setting max/min-height/width for that, it does not resolve it. So without loss of generality, I will focus only on the FB list.

I am having trouble determining which FBs specifically, are resolved by setting max/min-size equal to the basis size.

@ThomasBurleson, you suggested detecting for IE, so I figured that maybe the issue at hand is one of the IE related FBs. But if you look through all the IE related FB workarounds (FB# 2-8 & 12-13) none of them say anything about setting max/min-size equal to the basis size. Here is a summary of the FB workarounds listed on Flexbugs Repo:

  1. [COS] Set flex-shrink: 0
  2. Set max-width: 100%; box-sizing:border-box;
  3. substitute height for min-height OR add flex container wrapper
  4. Include unit for flex-basis, Use 0% instead of 0px
  5. add container-wrapper
  6. use explicit grow, shrink and basis (longhand)
  7. add wrapper OR set width to 100% and flex-basis to auto
  8. Use longhand OR set flex-basis: auto and e.g. width: calc(100%/3)
  9. [CEFOS] add container-wrapper
  10. [F] Set nested container to display: inline-flex
  11. [S] if you're using both a min and a max size declaration, set the flex basis to a value that is somewhere in that range.
  12. add display: <non-inline-value> to flex item/pseudo element
  13. declare flex-basis seperately (use longhand)
  14. [CFSO] flexcontainer {writing-mode: vertical-lr;} and flexitem {writing-mode: horizontal-tb;}

It appears the only FB that has anything to do with max-size and basis is FB#11, which is in fact related to Safari and not IE. In addition, KI#1 also notes that this issue has been resolved in Safari version >10.

So if this is the case, my question is:

Is it worth supporting a bug-fix for an bug that affects only one browser version;

The answer may very well be 'yes, this fix needs to be supported'. In which case I think we need to explore very carefully what exactly the workaround for FB#11 is recommending, and what the side effects may be, e.g. in what scenarios applying our version of the workaround brakes compliance with the flexbox-2016 css spec

somombo added a commit to somombo/flex-layout that referenced this issue Feb 7, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
somombo added a commit to somombo/flex-layout that referenced this issue Feb 7, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
somombo added a commit to somombo/flex-layout that referenced this issue Feb 7, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
@ThomasBurleson ThomasBurleson added the has pr A PR has been created to address this issue label Feb 7, 2017
somombo added a commit to somombo/flex-layout that referenced this issue Feb 7, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
somombo added a commit to somombo/flex-layout that referenced this issue Feb 7, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
somombo added a commit to somombo/flex-layout that referenced this issue Feb 8, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
somombo added a commit to somombo/flex-layout that referenced this issue Feb 9, 2017
handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
tinayuangao pushed a commit that referenced this issue Feb 9, 2017
)

handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes #153

Signed-off-by: Somo <somo@mombo.solutions>
karlhaas pushed a commit to karlhaas/flex-layout that referenced this issue May 3, 2017
…ngular#160)

handle various scenarios of grow/shrink being zero:
*  set `min-width`/`min-height` only if `grow != 0`
*  set `max-width`/`max-height` only if `shink != 0`
*  set both min and max if both `grow == 0` AND `shink == 0`
*  add tests for these scenarios
*  remove max-width from  test "should set a min-width when the shrink == 0"

fixes angular#153

Signed-off-by: Somo <somo@mombo.solutions>
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flexbox has pr A PR has been created to address this issue in-progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants