Skip to content

[css-grid][css-flex] Indefiniteness when sizing grid tracks in a flexible flex item #4852

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 Mar 9, 2020 · 8 comments
Labels
Closed Rejected as Wontfix by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work css-grid-1

Comments

@Loirooriol
Copy link
Contributor

Consider this testcase:

.wrapper {
  display: inline-flex;
  flex-flow: column;
}
.height {
  height: 50px;
}
.min-height {
  min-height: 50px;
}
.grid {
  flex-grow: 1;
  display: grid;
  background: red;
}
.content {
  background: green; 
}
<div class="wrapper height">
  <div class="grid">
    <div class="content">content</div>
  </div>
</div>
<div class="wrapper min-height">
  <div class="grid">
    <div class="content">content</div>
  </div>
</div>
Firefox Chromium
firefox chromium

In the min-height case, the flex container is forced to be at least 50px tall, and then the grid container is also 50px tall due to flex-grow: 1.

But then, how tall should the grid row be? https://drafts.csswg.org/css-grid/#algo-stretch says

This step expands tracks that have an auto max track sizing function by dividing any remaining positive, definite free space equally amongst them. If the free space is indefinite, but the grid container has a definite min-width/height, use that size to calculate the free space for this step instead.

It seems to me that the free space is indefinite, since the flex container has min-height: 200px instead of height: 200px, so https://drafts.csswg.org/css-flexbox-1/#definite-sizes doesn't apply

If the flex container has a definite main size, a flex item’s post-flexing main size is treated as definite

And the grid container doesn't have a definite min-height. So the track shouldn't be stretched, and Chromium is right.

Something similar happens for block and flex layout. Testcase

Firefox Chromium
firefox chromium

However, from https://crbug.com/1055258, some people expect the track to be stretched like in Firefox (and old Edge). So maybe the spec should be tweaked? Or should Firefox just fix their implementation?

Note this is not just for grid-template-rows: auto, the behavior is the same for grid-template-rows: 1fr. But with grid-template-rows: 100%, Chromium stretches the row just like Firefox.

@tabatkins tabatkins changed the title [css-grid][css-flex] Indefineteness when sizing grid tracks in a flexible flex item [css-grid][css-flex] Indefiniteness when sizing grid tracks in a flexible flex item Jul 7, 2020
@fantasai fantasai added the Target Revision: Next Remove when done. label Jul 22, 2020
@fantasai
Copy link
Collaborator

The core concept of min-width/min-height, per CSS2.1, is that you calculate sizes without it, and then if that constraint is violated, redo the calculation with that value in place of width/height:
https://www.w3.org/TR/CSS2/visudet.html#min-max-widths

It seems to be what authors would expect. So I think we should try to keep to that idea, if we can.

@tabatkins
Copy link
Member

tabatkins commented Jun 17, 2021

That would mean a behavior change, in practice, for all the layout modes though, as demonstrated by Oriol's second testcase. I think we've strayed away from that original "just rerun layout using this value instead" conception pretty thoroughly.

@fantasai
Copy link
Collaborator

@dholbert @cbiesinger Thoughts on this issue?

@bfgeek
Copy link

bfgeek commented Jun 29, 2021

That would mean a behavior change, in practice, for all the layout modes though, as demonstrated by Oriol's second testcase. I think we've strayed away from that original "just rerun layout using this value instead" conception pretty thoroughly.

This isn't quite whats going on here - it wouldn't involve a change for all layout modes, just a clarification for what should happen with grid.

Firefox's behaviour is a little more self consistent just viewing grid layout by itself.

Effectively Firefox is applying the "grid rule" that once the "used block-size" is found, we re-compute the grid using that new "used" size in order to re-resolve percents, frs, %-grid-gaps, etc.

Normally we don't consider 'auto' tracks when re-computing the grid, as it (typically) doesn't change the result.
However flex-items are special in that they can "force" the initial block-size to be indefinite, even though

You can trigger "inconsistency" in Blink's behaviour by added a %-gap or similar to the above testcase. E.g. with a %-gap we behave like Gecko due to recomputing the grid using the "used" block-size.

I think I'd be happy to align on Firefox's behaviour here.

@Loirooriol
Copy link
Contributor Author

Thinking about this again, I agree with Ian.

@fantasai
Copy link
Collaborator

OK, seems like we have consensus in the issue. @Loirooriol Do you have any edits you'd propose to clarify this case?

@Loirooriol
Copy link
Contributor Author

I guess I was being confused by the interaction of flex and grid. Let's follow the specs step by step.

<div class="wrapper" style="display: inline-flex; flex-flow: column; min-height: 50px;">
  <div style="flex-grow: 1; display: grid; background: red">
    <div style="font: 20px/1 Ahem; background: green">content</div>
  </div>
</div>
  • In https://drafts.csswg.org/css-flexbox-1/#algo-main-item, we are in case C, so we size the flex item / grid container under a max-content constraint:
    • The auto min track sizing function sets the base size to the limited max-content contribution, which is the max-content contribution, which is 20px.
    • The auto (treated as max-content) max track sizing function sets the growth limit to the max-content contribution, 20px.
    • The base size and growth limit are both 20px, so the track is not maximized.
    • The auto max track sizing function doesn't stretch the row since free space is indefinite and the grid container has no definite min-height.
    • The final height of the row is the base size, 20px. And that's also the height of the grid.
  • So the flex base size is 20px. And the hypothetical main size is also 20px.
  • Then in https://drafts.csswg.org/css-flexbox-1/#algo-main-container we find the height of the flex container. That's the maximum of the 50px min-height and the max-content main size, defined in https://drafts.csswg.org/css-flexbox-1/#intrinsic-main-sizes
    • We subtract the flex base size (20px) from the max-content contribution (basically the max-content size, 20px). That's the max-content flex fraction, 0px.
    • Then the flex container’s max-content main size is 20px + 1*0px = 20px
    • So the height of the flex container is 50px.
  • We collect the flex item into a single flex line, and then resolve flexible lengths
    • The used flex factor is the flex grow factor (1), since the hypothetical main size of the flex item (20px) is smaller than the main size of the flex container are equal (50px).
    • The target main size is initialized to the flex base size, 20px.
    • The initial free space is the flex container’s main size minus the flex base size, 50px - 20px = 30px.
    • The remaining free space is the initial free space, 30px.
    • We distribute the 30px into the flex item by setting the target main size to the flex base size plus the whole remaining free space: 20px + 30px = 50px
    • There is no min/max violation, the sum of the adjustments is 0px, so we freeze the flex item, and exit the loop.
    • The used height of the flex item is the target main size, 50px.
  • Then we find the cross size, not much relevant
  • Now that we know the final size of the flex item / grid container, we lay out the grid again, "for real":
    • The auto min track sizing function sets the base size to the minimum contribution, which is the min-content contribution, 20px.
    • The auto (treated as max-content) max track sizing function sets the growth limit to the max-content contribution, 20px.
    • The base size and growth limit are both 20px, so the track is not maximized.
    • The auto max track sizing function stretches the row by distributing the 50px - 20px = 30px of free space. The base size becomes 50px.
    • The final height of the row is the base size, 50px.
    • The grid item is stretched into that size, so it becomes 50px tall.

So I guess Firefox is already behaving according to the specs, and it's a Chrome bug, and there is no need to edit the specs.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Indefiniteness when sizing grid track inside flex item, and agreed to the following:

  • RESOLVED: close no change, firefox is correct
The full IRC log of that discussion <fantasai> Topic: Indefiniteness when sizing grid track inside flex item
<fantasai> github: https://github.com//issues/4852
<fantasai> oriol: Case where we don't have interop between FF and Chrome
<fantasai> oriol: When have column flex container which has for example height set to some value bigger than the content
<fantasai> oriol: This flex item has a flex that causes it to expand
<fantasai> oriol: and this flex item is also a grid container
<fantasai> oriol: which contains an auto row
<fantasai> oriol: usually auto rows, if you have free space at end of track sizing, they are expanded to cover this extra space
<fantasai> oriol: specifically for the case where we set the height property, we have interop
<fantasai> oriol: but instead of setting height on flex container you set min-height, in Chrome the auto height doesn't grow
<fantasai> oriol: At first I was confused and thought Chrome was right, but actually after some feedback from iank_ I think it's actually Firefox which follows the spec
<fantasai> oriol: iank_ also said that he's willing to change Chrome to adapt to what FF is doing
<fantasai> oriol: So I guess we can close this no change, agreeing that Firefox is the right behavior
<fantasai> Rossen_: comments/objections?
<fantasai> iank_: ...
<fantasai> fantasai: I think authors would be happy with this resolution
<fantasai> RESOLVED: close no change, firefox is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Rejected as Wontfix by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-flexbox-1 Current Work css-grid-1
Projects
None yet
Development

No branches or pull requests

5 participants