Skip to content
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 percentage heights of grid items during min-content sizing #2674

Closed
FremyCompany opened this issue May 11, 2018 · 13 comments

Comments

@FremyCompany
Copy link
Contributor

FremyCompany commented May 11, 2018

When a grid item

  • is found inside a container whose track is mimax(min-content, 1fr) or 1fr
  • has height:100%

How do you compute the min-content height of the track?

  • Usually in CSS we ignored height:100% as height:auto
  • However sometimes we resolve to height:0 during that phase
    (for instance see table cells children having overflow:scroll)

https://wptest.center/#/gawww8 (replaced element)
https://wptest.center/#/v298y2 (in wrapper)
https://wptest.center/#/18osz0 (in wrapper with overflow)

Currently:

  • Edge does auto auto auto
  • Chrome/Firefox do 0 auto auto
  • My feeling is we should all do auto auto 0

Is some behavior defined in the spec? Is it the behavior we want?

@FremyCompany FremyCompany changed the title [css-grid] Resolving percentages [css-grid] Resolving percentage heights of grid items during min-content sizing May 16, 2018
@fantasai
Copy link
Collaborator

fantasai commented May 17, 2018

OK, so, I think you're missing a few things.

  • The min-content contribution of an image with a percentage size is zero. See https://drafts.csswg.org/css-sizing-3/#percentage-sizing and [css-sizing] percentage [max-]width|height and intrinsic sizes #765 This is why your first test yields a min-content track height of zero. (The max-content contribution will still be the size of the image, IIRC; but we are not using it in this case.)
  • The min-content contribution and max-content contributions of a block are both the height after layout, so the previous rule has no effect on the calculations in the second case. Also percentage heights in auto-sized blocks are always ignored (treated as auto) per CSS2.1. That takes care of the second case.
  • You're expecting the third case to be different from the second. And it is, but only because we're supposed to use auto, not min-content as the min track sizing function. You're right that the min track sizing function should resolve to zero in that case: overflow is not visible (or clip), and thus the automatic minimum size doesn't apply.

So the base sizes of the tracks should be 0 auto 0. :) And the behavior of 1fr rows and 1fr columns should be symmetric.

@tabatkins
Copy link
Member

tabatkins commented May 17, 2018

Agreed, 0 auto 0 are the correct answers per spec. All browsers are a little wrong. ^_^

@tabatkins
Copy link
Member

tabatkins commented May 17, 2018

@MatsPalmgren @mrego @javifernandez Do y'all agree with the above? If so, we need to file bugs on all the browsers.

@MatsPalmgren
Copy link

MatsPalmgren commented May 17, 2018

So the base sizes of the tracks should be 0 auto 0

@fantasai that statement makes no sense. Base sizes are the result from the Track Sizing Algorithm - they can't be "auto". They are always a definite size. I'm assuming you mean "the block-size of the grid item after it is flowed in a containing block (CB) that has an inline size of 200px". So, I assume you mean "0 ~200px 0".

Also, I don't understand why you think the base size should be zero in the third test. You are correct that Automatic Minimum Size doesn't apply, but that doesn't make the min-content contribution zero - it's still "the block-size after flowing it", which in this case is 204px (the CB inline size is definite (200px), which makes the <figure> width 200px which provides a CB inline size for the image which becomes 200px in height, which makes the <figure> grid item have block-size 204px in Nightly because the <figure> (a block) baseline adds 4px (adding vertical-align:top makes it 200px). I can't find any spec that says that overflow != visible (and a block-size that "behaves as auto") on non-replaced blocks implies they have zero min-content size. Please point it out if I missed it, thanks.

@FremyCompany
Copy link
Contributor Author

FremyCompany commented May 18, 2018

Also, I'm still a bit confused. If the rule that makes the image be 0px tall is in css-sizing, why doesn't the same happen inside a flexbox?

https://wptest.center/#/ji5gi2
https://wptest.center/#/4ndzg9

In both case the flexbox is size vertically as the image.

Is that because it uses max-content and not min-content?

https://wptest.center/#/tn72m1

Even so, I'm still a bit perplexed by

https://wptest.center/#/3o8fj2

Why doesn't the image stop at 21px, that seems to be a stricter constraint than its max-content contribution, it does act like if 32px was its min-content contribution here, am I missing something?

@fantasai
Copy link
Collaborator

fantasai commented May 19, 2018

@fantasai that statement makes no sense. Base sizes are the result from the Track Sizing Algorithm - they can't be "auto". They are always a definite size. I'm assuming you mean "the block-size of the grid item after it is flowed in a containing block (CB) that has an inline size of 200px". So, I assume you mean "0 ~200px 0".

@MatsPalmgren Yes, that's what I meant.

Also, I don't understand why you think the base size should be zero in the third test. You are correct that Automatic Minimum Size doesn't apply, but that doesn't make the min-content contribution zero

Right, but we're not looking at the min-content contribution when we're calculating the base size. 1fr expands to minmax(auto, 1fr), not minmax(min-content, 1fr). The grid container is not being sized under a min/max-content constraint--it's explicitly sized at 300px. So the auto minimum is supposed to mean “use the automatic minimum size”, which due to the overflow setting is zero in this case. https://www.w3.org/TR/2017/CR-css-grid-1-20171214/#algo-single-span-items It seems that this is working fine in browsers when calculating columns, it's just not happening for rows. (Note the ED has some half-baked edits on this section, so linking to the CR.)

The point of auto is to add some automagic to the minimum (primarily to account for overflow), so that it is not always the min-content size--if we simply used the min-content size here always, we'd be defeating the point.

@FremyCompany Yes, in those first two cases, your flex container has an auto height, so it first calculates the container size from the max-content contribution of the items, then it resolves the percentage against that. In this case, the max-content contribution of the image is larger than the text, and is controlling the size of the container. Resolving 100% against that means the image simply is the size it is.

In the third case, the float's size is resolved as clamp(min-content, CB, max-content). The min-content size of the image is zero, the max-content is 32px, and CB is 9px, so we get 9px for the float, and the image's 100% width resolves against that.

In the last case, the percentage is attempting to resolve against an indefinite size (because while the max-height is definite, the height is not). And such percentages do not resolve; the percentage height is thrown out and treated as the initial value, auto. Meanwhile the width resolves against the float's width, which is clamp(min-content, CB, max-content) = max-content of the image = 32px.

@MatsPalmgren
Copy link

MatsPalmgren commented May 19, 2018

Right, I'm aware we're in the "height that behaves as auto" part of min-size contribution. Note that it says "computed min-width or min-height value" though. The computed min-height value is auto so it says we should return the outer size for height:auto which is the flowed block-size. I'm guessing you meant "used min-width or min-height value", because then there is now a clause in AMS that says "the automatic minimum size is otherwise zero, as usual" in the current draft (it was previously undefined). Then you would get the desired "outer size with height:0" result in the third test.

I agree with the results you suggest for the testcases and I've filed a bug to fix the layout for the third testcase in Gecko.

@FremyCompany
Copy link
Contributor Author

FremyCompany commented May 19, 2018

Alright, thanks for helping clear that up!

@MatsPalmgren
Copy link

MatsPalmgren commented May 19, 2018

min-size contribution says "... as its specified size (width or height, whichever matches the relevant axis) ...". Did you mean to use the term preferred size rather than "specified size" here?

Also, I'm still not sure how min-size contribution results in a row base size of zero in the first grid test above.
Here's a somewhat simplified example: https://wptest.center/#/9tgtf3
The image height behaves as auto when calculating its min-size contribution in the block axis so the resulting min-size contribution is the outer size after replacing height with used value of min-height. min-height is auto, thus AMS applies. There is no "specified size suggestion" (since height behaves as auto). The transferred size is 10px (width:10px transferred through a 1:1 ratio). Thus the min-size contribution is 10px, which makes the row size 10px. The final image size is then 10px x 20px since height:200% resolves to 2 * the row size during layout.

I don't see anything that would suggest that the second bullet in Percentage Sizing would apply in this case, since that rule is specifically restricted to "when calculating the min-content contribution". I don't think we're calculating a min-content contribution under the transferred size suggestion. If you want image grid items with a percentage preferred size to contribute zero to the track base size then I think you need to include min-size contribution too. Something like "when calculating a min-content or min-size contribution".

@MatsPalmgren
Copy link

MatsPalmgren commented May 27, 2018

@fantasai please confirm that you mean "when calculating a min-content or min-size contribution" rather than just "when calculating the min-content contribution" for replaced boxes (under the second bullet). Also, it's currently undefined how percentages should be calculated when calculating a max-content contribution for replaced boxes. Presumably the spec should say that they behave as auto there.

Also, for clarity, I'm reading the "for the purpose of calculating the box’s intrinsic size contribution" in the first bullet to mean (exactly) "when calculating a min-content, min-size, or max-content contribution". Please correct me if that's not an exact equivalence.

@fantasai
Copy link
Collaborator

fantasai commented May 29, 2018

@MatsPalmgren Right, I think the ED’s insertion of “computed” is wrong, and it should be “used”.

Yes, “specified size” should be “preferred size”. We didn't have the term “preferred size” until recently. :/

Also, I'm still not sure how min-size contribution results in a row base size of zero in the first grid test above.

... I think you're right, the transferred size is currently defined to override the percentage clause. I'm not entirely sure what we should be doing here, need to think about that. :/ I suspect that, since the effects of the percentage clause on replaced elements is a specified size effect, it should override the transferred size effect. Assuming that's the right way to go, we'll need to make that clear.

Also, it's currently undefined how percentages should be calculated when calculating a max-content contribution for replaced boxes.

It's defined here, roughly speaking: https://drafts.csswg.org/css-sizing-3/#intrinsic-contribution The exact rules for this calculation are rather complicated (see CSS2 chapter 10) so in the interest of not introducing errors and keeping css-sizing scoped to where we could possibly get it to CR several years ago (heh) we're effectively importing them by reference. Probably worth closing this if-branch as you suggest, though, for editorial reasons.

Also, for clarity, I'm reading the "for the purpose of calculating the box’s intrinsic size contribution" in the first bullet to mean (exactly) "when calculating a min-content, min-size, or max-content contribution".

Correct.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 1, 2018
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.
mcmanus pushed a commit to mcmanus/gecko that referenced this issue Jun 2, 2018
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.
jryans pushed a commit to jryans/gecko-dev that referenced this issue Jun 5, 2018
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.
@fantasai fantasai added this to the css-grid-1 CR 2017-12-14+ milestone Jan 23, 2019
@fantasai fantasai added the Target Revision: Current label Feb 20, 2019
fantasai added a commit that referenced this issue Feb 20, 2019
@fantasai
Copy link
Collaborator

fantasai commented Feb 20, 2019

please confirm that you mean "when calculating a min-content or min-size contribution" rather than just "when calculating the min-content contribution" for replaced boxes (under the second bullet).

Fixed s/computed size/used size/ and s/specified/preferred/ in 8d71a73

Also, it's currently undefined how percentages should be calculated when calculating a max-content contribution for replaced boxes. Presumably the spec should say that they behave as auto there.

Added missing case in c0a4c578

If you want image grid items with a percentage preferred size to contribute zero to the track base size then I think you need to include min-size contribution too.

Attempted to fix via 534f792

@MatsPalmgren Let me know if anything here still seems unresolved.

Agenda+ for review of that last fix, to make the cyclic percentage rules that zero out percentages apply to automatic minimum size calculations, making the “specified size suggestion” definite (and equal to zero, in the case of a pure percentage).

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Mar 13, 2019

The CSS Working Group just discussed Resolving percentage heights of grid items during min-content sizing (review last fix), and agreed to the following:

  • RESOLVED: Make the same result for min content sizing apply to automatic minimums
The full IRC log of that discussion <dael> Topic: Resolving percentage heights of grid items during min-content sizing (review last fix)
<dael> github: https://github.com//issues/2674#issuecomment-465490328
<fantasai> https://github.com/w3c/csswg-drafts/commit/534f79283a4687a5f0bc722baed5792e2c54433a
<dael> fantasai: There's some sizing rules where we decided percentages resolve to 0 something depending on when you're tacking intrinisic sizes. Clarifies the 0 basis also applies for automatic min size so that it ends up being definite
<dael> fantasai: Automatic min size is a derivation of min content sizing so this is saying same rules for min-content sizing apply to automatic minimum
<dael> astearns: Reasonable to me
<dael> astearns: Other comments or concerns?
<dael> astearns: Objections to make the same result for min content sizing apply to automatic minimums
<dael> RESOLVED: Make the same result for min content sizing apply to automatic minimums

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.

UltraBlame original commit: 547112cf70032742672310176b0b879f2e177c55
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.

UltraBlame original commit: 547112cf70032742672310176b0b879f2e177c55
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
… in min-size contributions according to spec. r=dholbert

Background:
w3c/csswg-drafts#2674

calc() percentages for replaced boxes are still not according to spec.
Bug 1463700 will fix that.

UltraBlame original commit: 547112cf70032742672310176b0b879f2e177c55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment