Skip to content

[css-grid] minmax(auto,min-content) under a max-content constraint #3565

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

Closed
Loirooriol opened this issue Jan 29, 2019 · 32 comments
Closed

[css-grid] minmax(auto,min-content) under a max-content constraint #3565

Loirooriol opened this issue Jan 29, 2019 · 32 comments

Comments

@Loirooriol
Copy link
Contributor

Consider this grid: https://jsfiddle.net/xnvrk7h8/

.grid {
  display: grid;
  grid: auto / minmax(auto, min-content);
  float: left;
  border: 3px solid;
}
.item {
  background: blue;
}
.float {
  float: left;
  width: 50px;
  height: 50px;
}
<div class="grid">
  <div class="item">
    <div class="float"></div>
    <div class="float"></div>
  </div>
</div>

float: left sizes the grid under a max-content constraint (assuming the window is wide enough).

The grid item has a max-content width of of 100px when the floats are in the same line, and a min-content of 50px when they are in different lines.

According to https://drafts.csswg.org/css-grid/#algo-single-span-items,

If the track has an auto min track sizing function and the grid container is being sized under a min/max-content constraint, set the track’s base size to the maximum of its items’ min/max-content contributions, respectively, each limited by the max track sizing function if that is fixed and ultimately floored by its minimum contribution.

The track does have an auto min track sizing function and the grid container is being sized under a max-content constraint. So the track’s base size should be set to 100px. This is not limited by min-content because it's not fixed, and the minimum contribution is less than 100px:

The minimum contribution of an item is the outer size that would result from assuming the item’s used minimum size (min-width or min-height, whichever matches the relevant axis) as its preferred size (width or height, whichever matches the relevant axis) if its computed preferred size behaves as auto, or else the item’s min-content contribution.

Here the computed width is auto, and the used value of min-width: auto is the content size suggestion, 50px.

If the track has a min-content max track sizing function, set its growth limit to the maximum of the items’ min-content contributions.

So the growth limit is initially set to 50px, but

if a track’s growth limit is now less than its base size, increase the growth limit to match the base size.

So both the base size and the growth limit should be 100px, the track is frozen, and the rest of the algorithm doesn't seem relevant.

However, this is not what implementations do:

results

Note that in Firefox the column is 50px wide but the grid container is 100px wide, so strange, @MatsPalmgren. This is solved by using min-width: 0 in the grid item, which alters the minimum contribution but shouldn't affect the result.

Is my understanding of the spec correct? Should it be changed to match implementations?

@fantasai
Copy link
Collaborator

I think you're right, something's not right here. Probably the automatic minimum should be limited by min-content here.

@fantasai
Copy link
Collaborator

OK, I think the correct edit here is to remove “if that is fixed” from the spec text you quote:

If the track has an auto min track sizing function and the grid container is being sized under a min/max-content constraint, set the track’s base size to the maximum of its items’ min/max-content contributions, respectively, each limited by the max track sizing function if that is fixed and ultimately floored by its minimum contribution.

What do you think?

@fantasai
Copy link
Collaborator

CC @MatsPalmgren

@Loirooriol
Copy link
Contributor Author

I think the correct edit here is to remove “if that is fixed”

I don't think the sentence makes sense without “if that is fixed”, because it's not clear how a length should be limited by something which is not a length or a resolveable percentage.

For example, if the max track sizing function is the keyword max-content, it needs to be defined that in this case the limit is the item’s max-content contribution.

It can also be a <flex> value, and these are not obvious because at that point fr is not known yet.

@fantasai
Copy link
Collaborator

So “if that is fixed, else it's own min-content/max-content size if the max track sizing function is min-content/max-content” ?

@Loirooriol
Copy link
Contributor Author

Loirooriol commented Jan 29, 2019

Basically yes, but IMO the sentence would bee too long and a bit confusing, there is already a min/max-content dichotomy for the size constraint, one could think that the this min/max-content is also with respect to that.

I would prefer a refactoring like

If the track has an auto min track sizing function, set the track’s base size to the maximum of its items’ foobars.
The foobar of an item is calculated as follows:

  1. Let max be the max track sizing function if that is fixed, else the min/max-content contribution if the max track sizing function is min/max-content, respectively, else infinity.
  2. If the grid container is being sized under a min/max-content constraint, return the item’s min/max-content contribution, respectively, limited by max and floored by the item’s minimum contribution.
  3. Return the item’s minimum contribution, floored at zero.

fantasai added a commit that referenced this issue Jan 29, 2019
… the items' contributions when sizing auto tracks under a min/max-content constraint. #3565 [Tentative pending WG review.]
@fantasai
Copy link
Collaborator

OK, I split the sentence. Didn't refactor the whole thing, but did some editorial tweaking to try to make it clearer. Let me know if the edits look OK and we'll ask for WG review. :)

@Loirooriol
Copy link
Contributor Author

Your refactoring with "limited min-/max-content contribution" looks good too.

I'm just not sure about the use of "min-content/max-content size" instead of the preceding "min-/max-content contribution". Not much sure about the difference, I believe one is an inner size and the other an outer size, here you probably want the outer?

And when it says "else by it’s own min-content/max-content size if that is min-content/max-content", "that" seems to refer to the own size instead of to the max track sizing function. Consider repeating "max track sizing function" to make it clear.

@fantasai
Copy link
Collaborator

You're right, that's clearly the wrong one. Done, and done. Thanks!!!

@fantasai
Copy link
Collaborator

Agenda+ to review the proposal:

When sizing auto-minimum tracks under a min-/max-content constraint, we use the item's min-/max-content contribution. But we limit that contribution by the track's max size if it's fixed (length/resolved %). This change makes it so that we also limit the contribution in the case of a min-content/max-content max track size, which fixes Oriol's example above to yield more intuitive behavior.

The value we're using to represent such an intrinsic max track size is the item's own contribution to such a size, which is I think a sufficient proxy for the actual max track size.

CC @MatsPalmgren @mrego @javifernandez

@fantasai
Copy link
Collaborator

@Loirooriol I think actually we can simplify to only do this for min-content max track sizes, limiting by max-content seems like it'd be a no-op?

@Loirooriol
Copy link
Contributor Author

Sure, the non-fixed limit is only useful when the max track sizing function is min-content and the grid container is being sized under a max-content constraint.

@Loirooriol
Copy link
Contributor Author

Another case: fit-content(0) under a min-/max-content constraint.

Its max track sizing function is max-content, so the base size is not limited and is 50px/100px, respectively. Only the growth limit is handled especially to be 0, but this is useless because later it's floored by the base size.

However, Chromium, Firefox and Edge produce a 50px column with min-width: auto and a 0px column with min-width: 0. So I say that the "limited min-/max-content contribution" should be limited by the fit-content argument.

@fantasai
Copy link
Collaborator

fantasai commented Feb 1, 2019

@Loirooriol If the limited min-/max-content contribution were limited by the fit-content argument, fit-content(0) would never yield anything greater than a zero-width column. But that's not what happens. I think there is something to fix here, but it's not the same issue, exactly...

@Loirooriol
Copy link
Contributor Author

fit-content(0) would never yield anything greater than a zero-width column

Not true, thanks to the minimum contribution.

@fantasai
Copy link
Collaborator

fantasai commented Feb 1, 2019

Right. Forgot that. Fixed, I think?

@fantasai
Copy link
Collaborator

fantasai commented Feb 1, 2019

Wondering now whether to simplify out that no-op or leave it as-is because it more accurately reflects the principle of the thing...

@Loirooriol
Copy link
Contributor Author

Yes, that's good. Well, the "could" sounds a bit strange to me, like implying uncertainty.

Maybe slightly in favor of simplifying the no-op in order to shorten a bit the long sentence, but I don't care.

@MatsPalmgren
Copy link

The track does have an auto min track sizing function and the grid container is being sized under a max-content constraint. So the track’s base size should be set to 100px.

I think this is the basic misunderstanding. The min-/max-content constraint is only used while determining the grid containers intrinsic size, not while flowing the box after that size has been determined. (i.e. the "Otherwise" part of "For auto minimums")

I stepped through this in Gecko in a debugger and jotted down some notes in case it's helpful:

grid container min-content size:
  1. initialize => base=0 limit=unconstrained
  2. intrinsic track sizing
       auto min-sizing,
         min-width:auto => item min size = 50px
         base=50px limit=50px
  => 50px

grid container max-content size:
  1. initialize => base=0 limit=unconstrained
  2. intrinsic track sizing
       auto min-sizing,
         item max-content contribution = 100px
         base=100px limit=unconstrained
       min-content max-sizing,
         item min-content contribution = 50px
         base=100px limit=50px
         base > limit =>
           base=100px limit=100px
  => 100px

Assuming the grid has a CB that is at least 100px, then its
intrinsic content-box width is 100px.  Now, flow the container
with that CB width:

grid container reflow:
  1. initialize => base=0 limit=unconstrained
  2. intrinsic track sizing
       auto min-sizing,
         min-width:auto => item min size = 50px
         base=50px limit=unconstrained
       min-content max-sizing,
         min-width:auto => item min size = 50px
         base=50px limit=50px
     => base=50px limit=50px
  3. distribute free space (100px)
       no growable tracks
  4. stretch flex tracks
       no flex tracks
  => base=50px limit=50px
=> grid area width is 50px

(I'll be happy to elaborate in more detail if needed.)

AFAICT, Gecko implements exactly what the spec says and thus its rendering is correct.

We could cap the item's max-content contribution also for min-content to get the rendering in Chrome, if that's desirable. Or we could make them "growable" to get the rendering in the "Expected" image in the OP. I don't feel strongly that anything needs to be changed in the spec though. If the result in Firefox isn't what the author wants they should just use min-content track sizing (to get the result in Chrome) or auto track sizing (to get the result in "Expected"). Whatever you decide, please make sure it makes sense also when span > 1 and when there's a mix of fixed and min-content max-sizing tracks.

@SelenIT
Copy link
Collaborator

SelenIT commented Feb 5, 2019

I guess the problem is more float-related than Grid-related. It seems to be technically not defined yet how should the preferred width be calculated in presence of floats. My understanding is also that preferred width should account for all the floats and the content that flows around them when they are placed in the single line, but Gecko and WebKit/Blink seem to disagree.

@Loirooriol
Copy link
Contributor Author

@MatsPalmgren You are right, I missed that layout is done again "for real" once the container size is known. Firefox follows the spec before fantasai's edits, but the result just seems weird to me.

@SelenIT I floated the grid container in order to size it under a max-content constraint (assuming enough available space) instead of width: max-content which is not supported in Edge. And I floated the contents of the grid item instead of using inline-blocks so that the whitespace between them would have no effect. But the example would be the same without floats. The grid spec defines what to do when sizing under min/max-content constraints, which is what floats do (by default, and among other things). The bug you linked seems unrelated, that's about the max-content contribution of a non-BFC-root block container whose line boxes are shortened by a float participating in the same BFC.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed minmax(auto,min-content) under a max-content constraint, and agreed to the following:

  • RESOLVED: we will handle min-content track sizes as a clamp on automatic sizes
The full IRC log of that discussion <dael> Topic: minmax(auto,min-content) under a max-content constraint
<dael> github: https://github.com//issues/3565#issuecomment-461242081
<dael> astearns: Discussed last week. Sounds like the one thing waiting on is the verbiage for what to do when spans >1
<dael> astearns: Is fantasai on yet?
<dael> astearns: Is there anyone else who wanted to go over last week's discussion or have anything new?
<dael> astearns: Rossen it sounds like on this item we were waiting to resolve on fantasai addressing a comment from Mats. She did in the issue. Should we call for resolution?
<jensimmons> oh sorry — I was totally confused. I thought you were asking for new agenda items for the call still… SORRY. That’s what I get for doing more than thing at the same time
<dael> oriol: fantasai proposed edits and they're not yet in the spec. I had some complaints against them and they need to be tweaked a bit. It would be better to discuss with fantasai
<dael> fantasai: I'm here
<dael> astearns: oriol did you want to discuss on the call or do it in the issue?
<dael> oriol: I wrote them in the issue. mostly it was that fantasai proposed to clamp as an upper limit that's the maximum of value. I thought it would make more sense to sum values rather than max of them. Some other minor corrections that maybe can be in github before making edits
<dael> fantasai: It's a max instead of sum because we want it to just fit within the number of tracks it spans. If it spans 50px and then one that's min-content we want it to fit in 50px plus whatever is left. If we add to it the 50px we're giving it more space then it needs
<Rossen> just a sed
<dael> astearns: It sounds to me that there are 2 issues. THe one on the agenda and another that has some ramifications on this one?
<Rossen> sec... having problem connecting on the phone
<dael> florian: I think ramifications are a follow up to the thing on the agenda
<dael> oriol: The item in the agenda was for the non-spanning case and Mats wanted to handle spanning case as a generalization of this and it make it more complicated
<dael> astearns: Okay, thank you
<dael> astearns: I could see two ways going forward. One is figure out the ins and outs of spanning case on the call. Sounds like discussion is between fantasai and oriol so might not be most efficient. Other is resolve on accepting these edits and if needed open a second issue for spanning case
<dael> fantasai: Can resolve on principle of handling min content track sizes as a clamp on automatic sizes in a similar way fixed sizes are a clamp
<dael> astearns: Make sense oriol ?
<dael> oriol: Yes
<dael> astearns: We're asking for 2 resolutions? One to accept the edits proposed in the issue and second is on principle?
<dael> fantasai: Just principle. Deal with the edits in the issue that is open on that
<dael> astearns: Issue on agenda needs resolution. Is resolving on the principle sufficient?
<dael> fantasai: I think so
<dael> astearns: Prop: Close the issue by resolving we will handlemin content track sizes as a clamp on automatic sizes
<dael> astearns: Objections?
<dael> Rossen: This is on #3565?
<dael> falken: Yes
<dael> s/ falken /fantasai
<dael> Rossen: sgtm
<dael> RESOLVED: we will handle min-content track sizes as a clamp on automatic sizes

@Loirooriol
Copy link
Contributor Author

Some of my points from #2303:

  • 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.

  • The limited min-/max-content contribution should take fit-content() into consideration in the spanning case.

  • Clamping by the min-content contribution may not ensure that spanning items won't contribute more space than what they will end up using.

    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 minmax(auto,min-content) minmax(auto,50px).

    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 auto minimums only distribute the minimum contribution, 20px. So the base size of each column becomes 10px. 20px are distributed again for the growth limits, the first one becomes 10px and the 2nd one remains 50px. So this time the columns will sum 60px. This is smaller than the grid container.

    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.

@fantasai fantasai removed this from the css-grid-1 CR 2017-12-14+ milestone Jul 21, 2020
tabatkins added a commit that referenced this issue Jul 17, 2023
@tabatkins
Copy link
Member

A narrow fix here: The use of "limited min-/max-content contribution" in the non-spanning auto minimums step was wrong, it should have been only min all along. Unsure why we wrote it that way, but it came from 7cacfbf back in 2016, so probably we were still just trying to figure out how this works. In the OP's case this'll give us the Chrome behavior (a width of 25px).

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-grid] minmax(auto,min-content) under a max-content constraint, and agreed to the following:

  • RESOLVED: accept latest change
The full IRC log of that discussion <fantasai> See https://github.com//issues/3565#issuecomment-1638901643
<bramus> TabAtkins: if you look at very first comment, oriol has a test case here with some example and the wg decided earlier that chrome and edge are correct here.
<bramus> … earlier we tried to implement that resolution in spec and we did it wrong, reverted, and did it again
<bramus> … issue ended up being simpler thatn we thought
<fantasai> Final fix was much simpler than we thought: https://github.com/w3c/csswg-drafts/commit/35ed93fc9296e6541e83c8cca4b705b7ba82c31a
<bramus> … we believe only problem was in handling non spanning items in a max content constraint was
<fantasai> s/max content/min content/
<bramus> … we were using the limitex max content size
<dholbert> s/limitex/limited/
<bramus> … we should have been using limited min content size when resolving base sizes of tracks
<bramus> … once we fixed that, algortihm made more sense in general
<fantasai> s/of tracks/of auto tracks/
<bramus> … this text also dated back to 2016 which was early in grid spec text, so we believe this was a mistake we made early on
<bramus> … this mistake was not copied over into spanning case but firefox behavior in previous issue was them doing the same thing in spanning case
<bramus> … if you do it in either case, it should be ok. we think spec text is ok now.
<bramus> astearns: you were nodding along?
<bramus> oriol: yeah, we discussed yeasterday and I think the current solution is the correct one
<bramus> astearns: other comments?
<bramus> astearns: proposed resolution to accept latest change as the solution to the earlier resolution
<bramus> astearns: objections?
<bramus> RESOLVED: accept latest change

@yisibl
Copy link
Contributor

yisibl commented Jan 23, 2024

Does this have a wpt test yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants