Repository navigation
[css-grid] Overflow with auto-repeat and minmax() #4043
Description
Imagine the following example:
<div style="display: grid; width: 200px; border: solid thick;
grid-template-columns: repeat(auto-fill, minmax(200px, 100px));
grid-auto-rows: 100px;">
<div style="background: magenta;"></div>
<div style="background: cyan;"></div>
<div style="background: yellow;"></div>
</div>All implementations (Chromium, Firefox, WebKit and Edge) render it the same way. The use the 2nd argument of minmax(200px, 100px) to calculate the number of repetitions,
so they consider there are 2 columns. Then we have overflow because columns are 200px width.
I guess the reason for that is the following text in the spec:
if the grid container has a definite size or max size in the relevant axis, then the number of repetitions is the largest possible positive integer that does not cause the grid to overflow the content box of its grid container (treating each track as its max track sizing function if that is definite or as its minimum track sizing function otherwise, and taking gap into account)
The max track sizing function is:
If the track was sized with a minmax() function, this is the second argument to that function
It's true that minmax() definition already talks about the case were min is bigger than max:
If the max is less than the min, then the max will be floored by the min (essentially yielding minmax(min, min))
But it seems that none of the implementations catch that detail.
It seems we need to use the min in this case, so the grid of the example has only 1 column, unless there are some other weird implications from using the min in this case. So I guess that the spec text should be updated to be clearer about that and avoid similar misunderstandings in the future.
Activity
MatsPalmgren commentedon Jun 19, 2019
I think key word in "If the max is less than the min, then the max will be floored by the min" is "will", meaning that some other spec text will say how/when that's applied. Indeed, this is what falls out from the "if the growth limit is less than the base size, increase the growth limit to match the base size" which is applied at various stages through the Track Sizing Algorithm. The auto-repeat spec text in your first quote discuss min/max track sizing functions separately without mentioning any flooring though, so I think current UAs are correct in not applying it in this case. However, I agree that it's a reasonable change to add an explicit flooring step there. I'd prefer if the spec continues to make that explicit in each case though, so that it's clear when in the algorithms it should be applied (we've agreed on that in a previous spec issue, IIRC). Otherwise, I fear that it's open to interpretation when to apply it and that might lead to different rendering results.
Loirooriol commentedon Jun 20, 2019
That's right, see #3567. The min and max cannot always be compared a priori, so each time we handle max<min needs to be explicitly defined.
I agree the proposal makes sense, i.e.
And I think there is the remaining case of both sizing functions being indefinite, like a grid container with
width: max-content; max-width: 100px; grid-template-columns: repeat(auto-fill, 50%). This falls into this case because the max size is definite, but the preferred size is intrinsic, so the percentage track size is initially treated asauto, right?mrego commentedon Jun 20, 2019
The first sentence of the spec says:
So if the grid container size is indefinite, then we don't calculate the number of repetitions, even if they have a fixed size like
repeat(auto-fill, 100px).Loirooriol commentedon Jun 21, 2019
But
max-width: 100pxis definite so the condition holds, even ifwidthis indefinite, doesn't it?[css-grid-1] Floor max by min when calculating repetitions. #4043
fantasai commentedon Nov 1, 2019
OK, updated the spec to floor the max by the min. Agenda+ to confirm.
Split out the issue of resolving percentages in the indefinite-width, fixed-max-width case as #4480
heycam commentedon Nov 13, 2019
@MatsPalmgren does that text look good to you?
MatsPalmgren commentedon Nov 13, 2019
Yup, the new text looks clear to me.
cdoublev commentedon Nov 15, 2019
I have tried to read all discussions here (BugZilla), here (Chromium), and in this issue, but please forgive me if I missed the answer to the following question: does this update will also resolve the following case, or should I fill a new ticket? This is a minimal reproduction of the CodePen used by Rachel Andrew to explain minmax() on its personal website. I think it's also used in the ticket from BugZilla.
What is the expected behavior?
Each grid item should have a minimum width of 200px.
What went wrong?
When resizing the window, the min value in minmax() might be ignored for "B" and/or "C", ie. grid items placed before an "overflowing" area (grid cell-s-) generated by a spanning grid item.
Loirooriol commentedon Nov 15, 2019
@cdoublev When the window is small enough,
repeat(auto-fill, minmax(200px, 1fr))will only produce a single explicit column, otherwise there would be overflow. Then, item D has a span of 3, so 2 implicit columns are added at the end. These implicit columns are not sized withminmax(200px, 1fr), they obey thegrid-auto-columnproperty instead, which defaults toauto. To achieve your desired behavior, usegrid-auto-columns: minmax(200px, 1fr).cdoublev commentedon Nov 16, 2019
Thank you for this clarification. I would have thought that it makes more sense to create a new row and preserve the min value in minmax(), but I guess it would make an exception to the placement behavior.
Anyway, I'm not trying to achieve a particular behavior. I was just revisiting auto-fit/auto-fill behaviors, by reading Rachel's blog post, and I was "surprised" by the min value not being respected.
css-meeting-bot commentedon Nov 20, 2019
The CSS Working Group just discussed
Overflow with auto-repeat and minmax(), and agreed to the following:RESOLVED: Accept change in https://github.com/w3c/csswg-drafts/issues/4043The full IRC log of that discussion
<dael> Topic: Overflow with auto-repeat and minmax()<dael> github: https://github.com//issues/4043
<dael> astearns: fantasai do you want this?
<dael> TabAtkins: I'm happy to do it
<dael> TabAtkins: We missed where if you do a minmax function and a repeat autofil or autofill we previously based it on max track sizing function. You had to give a real length. minmax(100px, 50px) we would tile columns based on filling 50px and then at layout they're all too big.
<dael> TabAtkins: We now floor the max with the min. Need review from impl if they want. Otherwise should be relatively rubber stamp.
<dael> astearns: This has had review. Anyone want time?
<dael> astearns: Objection to Accept change in https://github.com//issues/4043 ?
<dael> RESOLVED: Accept change in https://github.com//issues/4043
[css-grid] Test autorepeat with max,min. w3c/csswg-drafts#4043
[css-grid] Test autorepeat with max,min. w3c/csswg-drafts#4043
Bug 1650905 [wpt PR 24470] - [css-grid] Maxmin autorepeat tests, a=te…
Bug 1650905 [wpt PR 24470] - [css-grid] Maxmin autorepeat tests, a=te…
[css-grid] When the max is less than the min in minmax(), the max wil…
[css-grid] When the max is less than the min in minmax(), the max wil…
Bug 1650905 [wpt PR 24470] - [css-grid] Maxmin autorepeat tests, a=te…