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

[css-grid] Resolving 'fr' flexible lengths during intrinsic sizing of grid items #1120

Closed
atanassov opened this Issue Mar 23, 2017 · 6 comments

Comments

Projects
None yet
5 participants
@atanassov
Collaborator

atanassov commented Mar 23, 2017

In section 7.2.3 of Grid we specify that resolving 'fr' units with summed value of less than 1, during intrinsic sizing, should match Flexbox. Looking at the current implementations in Chrome and Firefox the behavior doesn't seem to match that expectation.

Consider this test case. The expectation is that when the sum of all fractions is less than one we have the special case. The implementations behavior is inconsistent and it appears to be triggered when at least one 'fr' value is less than 1.

If these are implementation bugs I can follow up with filing them but I'd like to make sure the spec is clear and if not we should be more explicit.

Here's a reference of the correct behavior in flexbox here.

@atanassov atanassov changed the title from [css-grid] Resolving 'fr' flexible lengths during intrinsic sizing of drid tracks to [css-grid] Resolving 'fr' flexible lengths during intrinsic sizing of drid items Mar 23, 2017

@javifernandez

This comment has been minimized.

Contributor

javifernandez commented Mar 23, 2017

If I understood the issue correctly, the problem is not that the sum is less than 1, but that for indefinite sizes, we may have to use 1 for fractional flexible lengths.

https://drafts.csswg.org/css-grid/#algo-flex-tracks

If the free space is an indefinite length:
The used flex fraction is the maximum of:

    If the flexible track’s flex factor is greater than one, the result of dividing the track’s base size by its flex factor; otherwise, the track’s base size. 

This issue was addressed long time ago, see the following thread for details:
https://lists.w3.org/Archives/Public/www-style/2015Sep/0032.html

@tabatkins tabatkins changed the title from [css-grid] Resolving 'fr' flexible lengths during intrinsic sizing of drid items to [css-grid] Resolving 'fr' flexible lengths during intrinsic sizing of grid items Mar 30, 2017

@tabatkins

This comment has been minimized.

Member

tabatkins commented Mar 30, 2017

Interesting! Javi's right that "sum less than 1" isn't quite the issue. The different between Grid and Flexbox here is in how we calculate the size of the container when some of the items have <1 flexibility, and the available space is indefinite (such as in a float).

Specifically, Grid's behavior is defined in https://drafts.csswg.org/css-grid/#algo-flex-tracks, the "If the free space is an indefinite length" substep. In your examples, it ends up computing a "used flex fraction" of 100px. In the .5/1 example, then, the tracks get sized to 50px/100px, respectively, and the grid then shrinkwraps those two tracks, being 150px wide total.

In Flexbox, the behavior is defined in https://drafts.csswg.org/css-flexbox/#intrinsic-sizes, where the "max content flex fraction"s end up computing to 0, and each item has a flex base size of 100px (due to its contents), so the flex container's size is the sum of (FBS + MCFF) for each item, or 200px wide total. If there's a .5/1 flex on the children, the sum of flexes is >1, so they then distribute that 200px with a 1:2 ratio, making the items approximately 67px/133px wide. (If the sum is <1, they get sized to an appropriate fraction of the 200px, leaving some empty space in the flexbox. The container does maintain its 200px width regardless, tho.)

I'm pretty sure Flexbox is the one with the bad behavior here. The container should pay attention to <1 flex values when finding its size, like Grid does. I'll file a bug for Flexbox instead.

@atanassov

This comment has been minimized.

Collaborator

atanassov commented Apr 21, 2017

The Grid spec is still somewhat misleading. Consider the part of the spec:
https://drafts.csswg.org/css-grid/#fr-unit

In this part, the spec we refer to "free space" and show <flex> * <free space> / <sum of all flex factors> as shares of free space for each track.

free space is defined as: (https://drafts.csswg.org/css-grid/#free-space)
Equal to the available grid space minus the sum of the base sizes of all the grid tracks (including gutters), floored at zero. If available grid space is indefinite, the free space is indefinite as well.

This implies that for example like : (https://jsfiddle.net/73oq0wqc/)

<div style="display:grid;background: orange; -ms-grid-columns:minmax(50px, 1fr) minmax(50px, 2fr); grid-template-columns: minmax(50px, 1fr) minmax(50px, 2fr);width:150px;border:5px solid red;">
  <div style="-ms-grid-column: 1;grid-column-start:1;background:yellow;height:10px;"></div>
  <div style="-ms-grid-column: 2;grid-column-start:2;background:yellow;background:lime;height:10px;"></div>
</div>

Here, both tracks have base size of 50px, the the free space is 150 - 50 - 50 = 50. And two tracks divide this 50px 1 to 2. But that is not what is happening. Instead, the final track size is 50px and 100px in all browsers.

Finding the size for fr (https://drafts.csswg.org/css-grid/#algo-find-fr-size), "Let leftover space be the space to fill minus the base sizes of the non-flexible grid tracks. " note that this time only minus base size of non-flexible tracks. In our example, both are flexible tracks, so “leftover space” is the entire 150px.

A similar issue arises when the sum of fr-unit is less than one.

In https://drafts.csswg.org/css-grid/#fr-unit , there is a note: "Note: If the sum of the flex factors is less than 1, they’ll take up only a corresponding fraction of the free space, rather than expanding to fill the entire thing. This is similar to how Flexbox [CSS-FLEXBOX-1] acts when the sum of the flex values is less than 1. "

This implies that the following example: (https://jsfiddle.net/kb0exLay/)

<div style="display:grid;background: orange; -ms-grid-columns:minmax(50px, 0.3fr) minmax(50px, 0.6fr); grid-template-columns: minmax(50px, 0.3fr) minmax(50px, 0.6fr);width:200px;border:5px solid red;">
  <div style="-ms-grid-column: 1;grid-column-start:1;background:yellow;height:10px;"></div>
  <div style="-ms-grid-column: 2;grid-column-start:2;background:yellow;background:lime;height:10px;"></div>
</div>

Free space is 200 - 50 -50 = 100. And the base on the note above, 100*0.9 is consumed by two fr tracks, remaining only 10px.
In current implementations of Chrome and Firefox the remaining size is 20px and the two tracks consume 180px. Again, agreeing with https://drafts.csswg.org/css-grid/#algo-find-fr-size.


Note that https://drafts.csswg.org/css-grid/#algo-find-fr-size is quite different from what Edge does but we could update our implementation for interop.


One more part that is not entirely clear is determining the size of container.

For width: https://jsfiddle.net/Lbvz57zj/

Firefox appears to be determining the size of the box based on https://drafts.csswg.org/css-grid/#intrinsic-sizes. And then in second pass, use the determined width as definite
while distributing flexible sizes.

But in Chrome, they appear to be doing https://drafts.csswg.org/css-grid/#algo-find-fr-size directly with indefinite width. After one pass, they are doing it again with width from first pass.

They might be doing this due to https://drafts.csswg.org/css-grid/#algo-terms saying:
"
available grid space
Independently in each dimension, the available grid space is:
If the grid container’s size is definite, then use the size of the resulting content box.
If the grid container is being sized under a min-content constraint or max-content constraint , then the available grid space is that constraint (and is indefinite).
"
In both Firefox and Chrome, there are left over space when sum of fr-unit is less than one.

For height:
https://jsfiddle.net/4xhmato4/

Firefox and Chrome both appears to be doing one pass of https://drafts.csswg.org/css-grid/#algo-find-fr-size with indefinite space.
As a result, there is no left over space even when sum of fr-unit is less than one.

@w3c w3c deleted a comment from SebastianZ Aug 9, 2017

fantasai added a commit that referenced this issue Aug 17, 2017

[css-grid] Use leftover space instead of free space in description of…
… <<flex>> since that is more accurate wrt terminology in the algorithm. #1120
@tabatkins

This comment has been minimized.

Member

tabatkins commented Oct 13, 2017

Here, both tracks have base size of 50px, the the free space is 150 - 50 - 50 = 50. And two tracks divide this 50px 1 to 2. But that is not what is happening. Instead, the final track size is 50px and 100px in all browsers.

Indeed; this appears to be a terminology mixup. Locally in that section, "free space" is correctly defined as subtracting out only the non-flexible tracks, but the "free space" term in the grid algorithm, as you point out, has a different meaning of subtracting out all the base sizes. We've changed the term from "free space" to "leftover space" to be more consistent with the terminology in the Track Sizing Algorithm section.

Note that https://drafts.csswg.org/css-grid/#algo-find-fr-size is quite different from what Edge does but we could update our implementation for interop.

It would be interesting to know what, exactly, those differences are.

One more part that is not entirely clear is determining the size of container.

As far as I can tell, Chrome's Grid behavior is correct per spec right now, but implementations haven't yet caught up with the corresponding section in Flexbox. (We recently made some edits to this, and ended up reverting them; #1735 has the details.)


Let us know if there's anything left to do on this issue. (And again, we'd be interested to know about Edge's behavior for frs in Grid.)

@atanassov

This comment has been minimized.

Collaborator

atanassov commented Dec 6, 2017

In order to avoid blowing up the intrinsic content size when a single flexible track approaches 0 (even if the sum is > 1), the grid spec treats flex factors < 1 as 1 during https://drafts.csswg.org/css-grid/#algo-find-fr-size (otherwise if you had 0.01fr and 1fr tracks with similar max-content values, the second track would blow up in size). This is a slightly different approach than the one flexbox took when the space is indefinite, but when the space is definite I think they are equivalent. Edge and Firefox current implementations are closer to the flexbox algorithm, but we (Edge) can change if this is the behavior we want to stick with.

At first I thought Chrome's algorithm wasn't per-spec, but after reading through the algorithm carefully, I think Chrome's behavior does indeed match the spec. See https://codepen.io/anon/pen/JOqOey for an explanation.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Dec 7, 2017

The Working Group just discussed Resolving 'fr' flexible lengths during intrinsic sizing of grid items.

The full IRC log of that discussion <dael> Topic: Resolving 'fr' flexible lengths during intrinsic sizing of grid items
<astearns> github: https://github.com//issues/1120
<dael> astearns: I'm not up on this issue. What was your comment Rossen_ ?
<dael> Rossen_: We are most likely going to have to change. We're okay with it, it will be simple. I'd like TabAtkins and fantasai to give this a read one more time. If they're still okay with current resolution we don't have to re-open. If they find additional points we can bring it back. for now I don't see a reason to re-open
<dael> fantasai: Our analysis from the comment is there was a clarifaction place and a language point. We checked in those. We couldn't find any other wanted change then those errors. We checked that in and asked Rossen_ if there's anything else. I'm guessing he said no.
<fantasai> s/language point/error in terminology/
<dael> Rossen_: Mostly. We can discuss if after you read the comment you feel like we need to discuss.
<fantasai> s/those/those fixes/
<dael> astearns: Sounds like action is for fantasai to take a look and if everything is fine she can mark as commentor satisfied.
<dael> Rossen_: Yes.
<dael> astearns: Good fantasai ?
<dael> fantasai: Yeah.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment