-
Notifications
You must be signed in to change notification settings - Fork 672
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] Intrinsic size of grid containers #2303
Comments
The quoted spec text was added in this thread, here: https://lists.w3.org/Archives/Public/www-style/2016Jan/0163.html The principle it's trying (and maybe failing?) to uphold is this one:
The idea is that if all our sizes are auto and we degenerate down to one grid track/one item, a shirnkwrapped (e.g. floated or abspos'd) grid should behave similar to a shrinkwrapped block or flexbox. |
…izing. r=dholbert When sizing the container under a min- or max-content constraint, the item's min/max-content contribution needs to be clamped (when Automatic Minimum Size / clamping applies) if its size is 'auto'. That'll give the container the right intrinsic size. In Reflow, we'll size the track initially to the clamped min-content contribution again (in the Resolve Intrinsic Track Sizes step), but since the container now has a definite size we'll grow the track in the Maximize Tracks step up to its limit (i.e. the clamp size). For more details on the underlying issue, see: w3c/csswg-drafts#2303
FYI, this is now fixed in the latest Firefox Nightly.
clamped: means shrink the margin box size to fit a fixed track max-sizing function (if there is one), without making the content-box size negative (as usual per Grid §6.6) With this definition of "behave as auto". (ditto for So, the trick is to always clamp the size in "For auto minimums" (when AMS and clamping applies). This gives the grid container the desired intrinsic size. Then, in layout (no sizing constraint), we'll return a clamped min-content contribution there, but since the grid container now has a definite size (the intrinsic size), we'll grow the track in the "Maximize Tracks" step up to the desired size. As far as I can tell, this gives the same layout as in Chrome for all the tests I looked at. |
…n when sizing under a min-content/max-content constraint. Part I: non-spanning items. #2303
OK, I've made the spec changes for non-spanning items. Haven't figured out the correct edits for spanning items yet. |
[ For the archives, here's a parallel thread between me and Mats from around the time this issue was filed. I think my goal here is to make sure I catch all of the relevant edits brought up in both discussions on this issue. ] |
…n when sizing under a min-content/max-content constraint. Part I: non-spanning items, revised. #2303
Proposed edits for spanning items (accounting also for #3565): diff --git a/css-grid-1/Overview.bs b/css-grid-1/Overview.bs
index 5b8ed42..9913a83 100644
--- a/css-grid-1/Overview.bs
+++ b/css-grid-1/Overview.bs
@@ -3975,9 +3975,9 @@ Resolve Intrinsic Track Sizes</h3>
limited by the <a>max track sizing function</a>
(which could be the argument to a ''fit-content()'' track sizing function)
if that is <a lt="fixed sizing function">fixed</a>,
- else by it's own <a lt="min-content contribution">min-</a>/<a>max-content contribution</a>
+ else by it's own <a>min-content contribution</a>
if the <a>max track sizing function</a>
- is ''grid-template-columns/min-content''/''grid-template-columns/max-content'';
+ is ''grid-template-columns/min-content'';
and ultimately floored by its <a>minimum contribution</a> (defined below).
Otherwise,
@@ -4018,11 +4018,7 @@ Resolve Intrinsic Track Sizes</h3>
<li id="algo-spanning-items">
<strong>Increase sizes to accommodate spanning items crossing content-sized tracks:</strong>
Next, consider the items with a span of 2
- that do not span a track with a <a>flexible sizing function</a>,
- treating a <a>min track sizing function</a> of ''auto''
- as ''min-content''/''max-content''
- when the grid container is being sized under a
- <a lt="min-content constraint">min</a>/<a>max-content constraint</a> (respectively):
+ that do not span a track with a <a>flexible sizing function</a>.
<!-- auto-min contribution <= min-content contribution <= max-content contribution -->
<ol>
@@ -4032,21 +4028,53 @@ Resolve Intrinsic Track Sizes</h3>
an <a lt="intrinsic sizing function">intrinsic</a> <a>min track sizing function</a>
by <a href="#extra-space">distributing extra space</a> as needed
to accommodate these items’ <a>minimum contributions</a>.
+
+ If the grid container is being sized under a
+ <a lt="min-content constraint">min-</a> or <a>max-content constraint</a>,,
+ use the items’ <a>limited min-content contributions</a>
+ in place of their <a>minimum contributions</a> here.
+ For an item spanning multiple tracks,
+ the upper limit used to calculate
+ the item’s <a>limited min-/max-content contribution</a>
+ (see above)
+ is the greatest of
+ * infinity, if it spans any tracks
+ with an ''grid-template-columns/auto'' or ''grid-template-columns/max-content''
+ <a>max track sizing function</a>
+ * its own min-content contribution, if it spans any tracks
+ with an ''grid-template-columns/min-content''
+ <a>max track sizing function</a>
+ * the sum of the <a lt="fixed sizing function">fixed</a>
+ <a>max track sizing functions</a>
+ of any tracks it spans,
+ if it spans any such tracks.
+
<li>
<strong>For content-based minimums:</strong>
Next continue to increase the <a>base size</a> of tracks with
a <a>min track sizing function</a> of ''min-content'' or ''max-content''
by <a href="#extra-space">distributing extra space</a> as needed
to account for these items' <a>min-content contributions</a>.
+
<li>
<strong>For max-content minimums:</strong>
- Third continue to increase the <a>base size</a> of tracks with
+ Next, if the grid container is being sized
+ under a <a>max-content constraint</a>,
+ continue to increase the <a>base size</a> of tracks with
+ a <a>min track sizing function</a> of
+ ''grid-template-columns/auto'' or ''max-content''
+ by <a href="#extra-space">distributing extra space</a> as needed
+ to account for these items' <a>limited max-content contributions</a>.
+
+ In all cases,
+ continue to increase the <a>base size</a> of tracks with
a <a>min track sizing function</a> of ''max-content''
by <a href="#extra-space">distributing extra space</a> as needed
to account for these items' <a>max-content contributions</a>.
<li>
If at this point any track’s <a>growth limit</a> is now less than its <a>base size</a>,
increase its <a>growth limit</a> to match its <a>base size</a>. |
And the fixed gutters between all the spanned tracks? This concept is also used in https://drafts.csswg.org/css-grid/#min-size-auto, you could name it and reference it.
Maybe the limited min-/max-content contribution should be fully defined in its own subsection (together with the minimum contribution, I guess). Providing a simpler definition in the place where it's defined, and then adding amendments in references to it is less clear. Because not all references will be next to the amendments, and following the link won't provide the full definition. |
No need to mention
The
I'm not convinced this is the right approach. Imagine you have an item with a min-content contribution of 25px spanning a track with I think it should be more like:
|
OK, given the way space is distributed, a maximum as you said may be better. However, it still doesn't seem completely right. The idea is that, when sizing under a max-content constraint, items shouldn't contribute more space than what they will end up using. But imagine you have a single grid item with a minimum contribution of 20px, a min-content contribution of 25px, and a max-content contribution of 100px. It spans two columns sized with When sizing the container under a max-content constraint, the limited max-content contribution will be 50px (100px clamped between 20px and 50px). This will be distributed equally, so both columns will end up with a base size of 25px. Then the minimum contribution, 20px, is distributed for growth limits. The 2nd column already has a growth limit of 50px, so it will remain that way, but the first column will change its growth limit from infinity to the base size, 25px. Then the 2nd column will be maximized and the grid container will be 75px wide. The you do layout for real. This time the IMO the sizing algorithm seems too aggressive reducing the growth limit when it was infinity and becomes finite. If there is free space, the 1st column should probably be allowed to grow until it reaches 25px (the min-content contribution, as dictated by the max track sizing function). This can be an unrelated issue, though. |
OK, in the interest of getting this fixed, I simplified down the patch proposed in #2303 (comment) to just deal with fixed sizes and not min-content limits. We'll handle those in #3565.
Fixed gutters are defined to act like fixed tracks for the purpose of the sizing algorithm. Calling them out here would imply that substitution is not always valid. @Loirooriol Would you mind copying your comments about handling min-content clamping over to #3565? Or I can do that later. @MatsPalmgren I'd appreciate your review on the changes, see 3b26eeb and 92a3d3a Let me know if they're enough to close out this issue and the thread in https://lists.w3.org/Archives/Public/www-archive/2018Mar/0004.html or if there's still something not right with the spec. |
Well they are already called out in https://drafts.csswg.org/css-grid/#content-based-minimum-size, which is sometimes used to size tracks. |
…izing. r=dholbert When sizing the container under a min- or max-content constraint, the item's min/max-content contribution needs to be clamped (when Automatic Minimum Size / clamping applies) if its size is 'auto'. That'll give the container the right intrinsic size. In Reflow, we'll size the track initially to the clamped min-content contribution again (in the Resolve Intrinsic Track Sizes step), but since the container now has a definite size we'll grow the track in the Maximize Tracks step up to its limit (i.e. the clamp size). For more details on the underlying issue, see: w3c/csswg-drafts#2303 UltraBlame original commit: 82bd129fd8059961fca6eca013cf7ed84ccee897
…izing. r=dholbert When sizing the container under a min- or max-content constraint, the item's min/max-content contribution needs to be clamped (when Automatic Minimum Size / clamping applies) if its size is 'auto'. That'll give the container the right intrinsic size. In Reflow, we'll size the track initially to the clamped min-content contribution again (in the Resolve Intrinsic Track Sizes step), but since the container now has a definite size we'll grow the track in the Maximize Tracks step up to its limit (i.e. the clamp size). For more details on the underlying issue, see: w3c/csswg-drafts#2303 UltraBlame original commit: 82bd129fd8059961fca6eca013cf7ed84ccee897
…izing. r=dholbert When sizing the container under a min- or max-content constraint, the item's min/max-content contribution needs to be clamped (when Automatic Minimum Size / clamping applies) if its size is 'auto'. That'll give the container the right intrinsic size. In Reflow, we'll size the track initially to the clamped min-content contribution again (in the Resolve Intrinsic Track Sizes step), but since the container now has a definite size we'll grow the track in the Maximize Tracks step up to its limit (i.e. the clamp size). For more details on the underlying issue, see: w3c/csswg-drafts#2303 UltraBlame original commit: 82bd129fd8059961fca6eca013cf7ed84ccee897
@fantasai Can you explain what the goal of these spec changes are exactly? Otherwise, I can't tell whether the spec achieves that goal or not. |
@MatsPalmgren The goal was to resolve all the issues discussed in your thread, linked above. I believe some amount of your confusion was resolved by @Loirooriol's comments earlier today... so let me know if you still need an explanation, because to be honest it's really difficult for me to page this issue back into my head, but if you need me to I will try harder tomorrow. Wrt the the sample testcase you gave in your comments on 3b26eeb AFAICT it will render as you expect, because that changeset floors the item's contribution to the track size by its “minimum contribution”. In the case of auto, this is the min-content size clamped by the max track sizing function to zero. But in the other cases, it's equivalent to its min-content contribution, where the preferred size ('width') or explicit minimum size ('min-width') you've given controls that size (and the AWS is not invoked). |
OK, no worries. I'll take a look at the spec anyway... One thing that immediately stands out is that non-spanning/spanning items are treated differently for (Also, an unrelated nit: in the non-spanning case you say "Otherwise, set the track’s base size to the maximum of its items’ minimum contributions, floored at zero.", but in the spanning case you say " to accommodate these items’ minimum contributions". The "floored at zero" is missing in the latter case. Fwiw, it's a bit silly that we need to handle this here - maybe css-sizing could floor it so that all size contributions are non-negative?) |
For the spanning case, the spec says
Where the limited min-content contributions is
So the "floored by its minimum contribution" seems included. The problem that I see is that the spec says the "limited min-content contribution" should be used even when sizing under a max-content constraint. Shouldn't it use the "limited max-content contribution" in that case?
I think it's not important, because https://drafts.csswg.org/css-grid/#extra-space says
So currently we use
But contributions can actually be negative, right? Like if you add a box with a very negative margin, the containing block can shrink. |
Good points.
Note that Step 1 is for |
Ah you are right, I already suspected I was missing something since when that text was written it looked good to me. |
Closing per @Loirooriol's assessment that the spec is correct, though implementations still need fixing. |
The spec text is very clear about this and has been around for a long time:
But it seems not all the browsers are doing it, or at least not all
are doing it properly.
Firefox is following the spec, and it runs the track sizing algorithm twice
(one under min-content constraint and another under max-content constraint)
to compute the min|max-content widths of the grid container.
However Blink and WebKit follow a different approach.
In this case they just run the track sizing algorithm once
with an available space of zero and use the sum of base sizes
as min-content size and the sum of the growth limits as max-content size.
The reason why these browsers do it that way is because
this was suggested back in 2013 by Julien Chaffraix in
www-style
and never modified later.
Regarding Edge I don't know what's their implementation, but I'm finding
the same issues than in Blink and WebKit. So I guess Edge is either
not following the spec or not implementing it properly.
Let's go to the examples and why I'm bringing up this issue.
The key part of the track sizing algorithm that is affected by this
is when we resolve intrinsic track sizes
(for both non-spanning and spanning items we have a similar text):
Imagine the following example:
The intrinsic sizes should be:
50px
.100px
.As the grid container is
inline-grid
, its sized asfit-content
:min(max-content size, max(min-content size, stretch-fit size))
.In this example,
min(100px, max(50px, 20px)) = 50px
.Note: This also matches what happens in a similar case using
inline-block
instead of
inline-grid
.In Blink and WebKit we always go through the Otherwise paragraph,
so the min-content size is
0px
and the final width is20px
.The same result happens on Edge, though I don't know how is that implemented.
In any case it seems quite clear that this is a bug in Blink, WebKit and Edge
and their behavior should be modified.
Note that to fix this, we'll need to follow a similar approach than Firefox,
which means that the track sizing algorithm will be run 3 times:
(one under min-content constraint and another under max-content constraint).
This is not all, now let's go to a different example:
To calculate the min-content size, all the browsers seem to apply the clamping
described in Automatic Minimum Size of Grid Items section
of the spec. So the min-content size is
0px
.However to calculate the max-content size, Firefox uses
the max-content contribution as requested by the spec, that's
100px
.This makes that the intrinsic size of the grid container is
100px
.However during layout the track sizing algorithm is run again,
and as during layout it doesn't know if it's under a min-content
or max-content constraint, it goes through the Otherwise sentence
and apply the clamping, so the track has
0px
width.This causes a weird effect as the track is
0px
but the grid container is
100px
.In Blink and WebKit the result is better, the grid container is
0px
as we always go through the Otherwise sentence and apply the clamping.
Edge has the same output too.
In a Firefox bug
@MatsPalmgren suggests that we apply the automatic minimum size clamping
when we are under a min-content constraint, max-content constraint
or no constraint.
That would make Firefox has the same output than the other browsers
in this last example.
It'd be nice to get the feedback from @atanassov about this issue,
as we don't know how the intrinsic size is being calculated in Edge.
Check the following codepen to see the examples live and play with them:
https://codepen.io/mrego/pen/xYdvbZ
The text was updated successfully, but these errors were encountered: