w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.48k stars 659 forks source link

[css-flexbox-1][css-sizing-3] Definiteness of `min-height: min-content` #6457

Open fantasai opened 3 years ago

fantasai commented 3 years ago

via https://github.com/w3c/csswg-drafts/issues/4310 ...

https://drafts.csswg.org/css-flexbox/#min-size-auto

Nonetheless, although this may require an additional layout pass to re-resolve percentages in some cases, this value (like the min-content, max-content, and fit-content values defined in [CSS-SIZING-3]) does not prevent the resolution of percentage sizes within the item.

I think this parenthetical is trying to say that min-size: min-content|max-content|fit-content do not prevent the resolution of percentage sizes same as min-size: auto. But I can't find where this is specced?

(Also none of the browsers support these keywords on min-block-size. But they parse it.)

fantasai commented 3 years ago

I guess the question is do we make this parenthetical reality? Or clarify that these cases are in fact indefinite?

dholbert commented 3 years ago

(Also none of the browsers support these keywords on min-block-size. But they parse it.)

Could you elaborate on the lack-of-support?

I believe we (Firefox at least) treat these keywords as auto in the block axis, which is what the spec calls for: https://drafts.csswg.org/css-sizing-3/#valdef-width-min-content

Quoting that spec-text on what these keywords mean in the block axis (using min-content as an example here):

* min-content
     [...] for a box’s block size, unless otherwise specified, this is equivalent to its automatic size.

In that spec quote, automatic size is just linked to the definition of auto here: https://drafts.csswg.org/css-sizing-3/#valdef-width-auto

So: these keywords are specced to behave as auto in the block axis, basically.

So if you're seeing these keywords not having any special behavior in the block axis, I think that's correct :) though maybe I'm misunderstanding the issue you're seeing.

And also, I would expect these keywords to have the same definiteness guarantees as auto (at least in the block axis where they're literally equivalent... And probably also in the inline axis since sizes are typically more eagerly definite in the inline axis, I think? Though it'd probably be worth testing some inline-axis scenarios.)

davidsgrogan commented 3 years ago

So: these keywords are specced to behave as auto in the block axis, basically.

I think that is not the intent of the spec, based on https://github.com/w3c/csswg-drafts/issues/3973#issuecomment-705663905 and the following few comments.

dholbert commented 3 years ago

Hmm, interesting. I do see in the csswg discussion quoted there:

at one poin the spec defined min-content/max-content/fit-content to behave the same as 'auto' in the block axis

So that does indeed imply that the spec changed to mean something else. I guess in the spec-text that I quoted above, "block size" is probably literally & only referencing the block-size property (e.g. height, not max-height or min-height).

I guess we have https://bugzilla.mozilla.org/show_bug.cgi?id=1672559 filed to track changing the Firefox behavior here.

davidsgrogan commented 3 years ago

I think we are supposed to honor height: min-content also, but I can't come up with a case where that matters since in the block direction min-content size == max-content size, and height: auto resolves to that same size.

Like, the outer div's used height is 50px no matter if you honor height:min-content or not:

<div style="height:auto; max-height: 50px">
  <div style="height: 100px"></div>
</div>
<div style="height:min-content; max-height: 50px">
  <div style="height: 100px"></div>
</div>

Maybe there's an orthogonal writing mode case where honoring height:max-content matters? I don't know.

<div style="height:max-content; min-height: 50px">
  <div style="writing-mode: vertical-lr;">
    <!-- some children that change max-content size of outer div? -->
  </div>
</div>
Loirooriol commented 3 years ago

@davidsgrogan If I understand correctly, height: min-content would matter in grid (and probably flexbox too):

<div style="display: grid; grid-template-rows: minmax(0, 100px); height: min-content; border: solid"></div>

In https://drafts.csswg.org/css-grid/#algo-grow-tracks the base size of the row is 0px, the growth limit is 100px, but we don't distribute space under a min-content constraint. So the height should become 0px. Browsers just treat it as height: auto, though.

css-meeting-bot commented 3 years ago

The CSS Working Group just discussed [css-flexbox-1][css-sizing-3] Definiteness of min-height: min-content.

The full IRC log of that discussion <dael> Topic: [css-flexbox-1][css-sizing-3] Definiteness of min-height: min-content
<dael> github: https://github.com/w3c/csswg-drafts/issues/6457
<dael> astearns: fantasai are you on?
<dael> [silence]
<dael> astearns: Anyone on the call that can take this?
<dael> TabAtkins: fantasai and I have wanted to look but haven't had a workday. We have one Friday
<dael> astearns: Added to the agenda weeks ago
<dael> TabAtkins: A bunch of comments since then.
<dael> TabAtkins: If she knew what she wanted to talk about and it's still there sure. But had a lot of comments since
<dael> dholbert: I suggest we wait
tabatkins commented 3 years ago

As Oriol mentioned, min-content and max-content differ in the block axis when sizing flex and grid containers, because min-content sizing doesn't distribute any free space.

The original question still remains: the spec is currently implying that the content-based sizing keywords in the min-* properties don't prevent children from resolving %s against those sizes (even though the width/height properties themselves do). This is necessary behavior for the automatic minimum size (or else percentages would usually be unresolvable), which can be content-based, so it seems like it should be fine to specify that explicitly for all the content-based sizing keywords. But given that these keywords aren't otherwise representing definite sizes, do we actually want to?

fantasai commented 3 years ago

Agenda+ to bring the issue up with the WG.

css-meeting-bot commented 3 years ago

The CSS Working Group just discussed Definiteness of min-height: min-content.

The full IRC log of that discussion <fantasai> topic: Definiteness of min-height: min-content
<fantasai> github: https://github.com/w3c/csswg-drafts/issues/6457
<fantasai> “The original question still remains: the spec is currently implying that the content-based sizing keywords in the min-* properties don't prevent children from resolving %s against those sizes (even though the width/height properties themselves do). This is necessary behavior for the automatic minimum size (or else percentages would usually be unresolvable), which can be content-based, so it seems like it
<fantasai> should be fine to specify that explicitly for all the content-based sizing keywords. But given that these keywords aren't otherwise representing definite sizes, do we actually want to?”
<fantasai> TabAtkins: Original question of issue is still open
<fantasai> TabAtkins: for reasons of usability, we had to rule that 'min-size: auto' does not prevent percentages on your children from resolving
<fantasai> TabAtkins: even though technically the size of parent might be dependent on size of contents
<fantasai> TabAtkins: because if not, percentages would hardly ever resolve in flex or grid
<fantasai> TabAtkins: But we have also the min-content and max-content keywords
<fantasai> TabAtkins: Do we make these also not block percentage resolution on children?
<Rossen_> q?
<fantasai> TabAtkins: or do we want to hold the line, and have these content-based keywords block percentage resolution
<fantasai> TabAtkins: just like they do in the not-min sizing properties (width/height/etc.)
<fantasai> TabAtkins: Not necessarily need to resolve now, but wanted to bring up the quesiton
<fantasai> iank_: I'd want to discuss with David
<fantasai> dholbert: I think given how treated as 'auto' right now, there might be web-compat demand for them to have same definiteness properties.
<TabAtkins> fantasai: Given they're "treated as auto" and not commonly used, there's not much incentive for authors to use them right now, especially in a min-size property; feels unlikely that there's compat risk
<fantasai> Rossen_: Any concrete use cases?
<fantasai> TabAtkins: no, this is a question of consistency
<fantasai> TabAtkins: what kind of divergence do we want here
<fantasai> TabAtkins: we need an answer for the spec, but having use cases
<fantasai> dholbert: for non-definite case
<fantasai> dholbert: consider a deeply-nested flexbox case
<fantasai> dholbert: want content-based minimum
<fantasai> dholbert: but don't want perf complications
<fantasai> dholbert: so switch from auto to min-content
<fantasai> dholbert: That said, browser could also maybe detect the lack of percentages inside the element
<fantasai> dholbert: and not run the second layout pass
<fantasai> dholbert: Can't think of another justification for not being definite
<fantasai> jfkthame: agree
<fantasai> Rossen_: ...
<fantasai> Rossen_: I think the perf problem stated here is more of a speculation
<fantasai> Rossen_: less a concrete concern
<fantasai> Rossen_: my preference would be to keep it consistent
<fantasai> fantasai: consistency works both ways
<Rossen_> s/.../The deeply nested elements can be made context aware and resolve % only in the cases of having both % and min-content/
<fantasai> fantasai: these keywords in 'height' cause it to be indefinite
<fantasai> fantasai: so do we want to be consistent with that? or with 'min-height: auto'?
<fantasai> Rossen_: where do we go from here?
<fantasai> fantasai: Tab and I are happy for people to think about it. We just wanted to bring it up and explain on the cal.
dholbert commented 3 years ago

Is min-height:min-content supposed to work (i.e. be treated as the content-height) for elements inside of a block? (I think it is, based on https://github.com/w3c/csswg-drafts/issues/3973#issuecomment-705663905 and surrounding comments.)

If so, and if we resolve here that it doesn't prevent the element from being considered definite-height, then I worry that this could turn into "one weird trick" for getting block layout to have content-sizing combined with percent-resolution, and it would mean that browsers would have to start doing two-pass layouts inside of blocks in order to measure content and then resolve percentages.

I'm thinking of examples like this (see the question in the CSS code-comment here): https://jsfiddle.net/dholbert/gjwrtv1h/

<div class="test">
  <div class="pct">p</div>
  <div class="tall">t</div>
</div>
.test {
  height: 100px;
  width: 100px;

  /* If this is honored, we'll end up with a used height
     of 150px, the height of our orange child.
     And then the question is, how tall should our
     percent-height cyan child be? Should it be content-height,
     or 100px, or 150px? */
  min-height: min-content;
  border: 1px solid black;
}
.pct {
  height: 100%;
  width: 40px;
  background: cyan;
  display: inline-block;
}
.tall {
  height: 150px;
  width: 40px;
  background: orange;
  display: inline-block;
}
dholbert commented 3 years ago

If I'm not misunderstanding anything in my previous comment here, then I think I'd prefer that we have these min-content/max-content keywords consistently prevent sizes from being definite.

I think we can consider min-height:auto to have its own special exception from the "definiteness" rules, given that: (a) it's the default, and we don't want percentages to just be broken by default. (b) it doesn't require content measurement in every layout mode (e.g. min-height:auto just resolves to 0 for an element inside of a block) (c) even in layout modes where it does perform content measurement, it has special-cases where it still doesn't need to (e.g. overflow:scroll flex items in a vertical flexbox will resolve their min-height:auto to 0 without having to measure their content.)

In contrast: (d) min-height:min-content is not the default, so if we were to say it makes its element indefinite-height, that's not catastrophic; it's not going to break percentages across the board. (Just for that one element.) (e) min-height:min-content is (I think) intended to always trigger content measurement, in all layout modes. It's not as subtle "sometimes-yes-sometimes-no" like auto. (f) I don't like the idea of min-height:min-content being definite in some layout modes (e.g. flexbox) but not in others; and I don't like the idea of it being definite across-the-board, per my previous comment and code example. It feels most-coherent for them to be considered indefinite across-the-board.

Given the above, I'd lean towards speccing the content keywords as triggering indefinite sizes (or at least, I'd lean against speccing the opposite).

(note: I'm using min-height for brevity/intuitiveness here, but really I mean min-block-size)

tabatkins commented 3 years ago

+1 to all this reasoning; it matches with my and @fantasai's own thoughts.

So, Agenda+ to resolve that min-height:auto just has special behavior that allows child %s to still resolve, and it's not a precedent to apply to all the other content-based keywords. (Thus, we'll remove, or possibly keep-but-reverse, the note that's currently in the spec.)

davidsgrogan commented 3 years ago

I agree with (a), (b), (d), (e) without reservation.

I don't disagree with (c), I just don't understand its relevance.

I don't necessarily disagree with (f) either, I just want to bring up that the "one weird trick" situation mentioned above has benefits that have so far been ignored. Namely, authors would appreciate the ability to opt in to the poorer-performing but more powerful two-pass layout in block layout (i.e. I wholeheartedly agree with @fantasai's comment at https://github.com/w3c/csswg-drafts/issues/4311#issuecomment-881058999)

Three comments/questions:

  1. Am I misunderstanding something, or would this indeed be a compat-conserving way to hand authors a long-desired behavior they could opt in to?
  2. @dholbert , above you say you "worry ... that browsers would have to start doing two-pass layouts inside of blocks in order to measure content and then resolve percentages". Agree that browsers would have to do two-pass layouts. Is the 'worry' about layout speed? Or engine-implementation complexity?
  3. Seems to me the clear answer to the CSS-comment question above is '150px'. But maybe I'm missing something.

So, to be clear, I'm NOT advocating for making min-height: min-content allow child percentages to resolve, rather I'm trying to make sure both the pros and cons are being considered. Seems like only the cons have been discussed so far.

dholbert commented 3 years ago

I don't disagree with (c), I just don't understand its relevance.

Point (c) was intended as support for why it makes sense to keep min-height:auto in a separate category from "min-content" / "max-content", as not interfering with definiteness. The reason being: min-height:auto doesn't even require content measurement for some cases -- e.g. scrollable grid/flex items (point (c)), and children of non-grid/flex containers (point (b)).

  1. Am I misunderstanding something, or would this indeed be a compat-conserving way to hand authors a long-desired behavior they could opt in to?

It could be, but it feels like a potentially arcane way of doing that. If we want to address that use-case, perhaps there's a more direct way to do so?

  1. @dholbert , above you say you "worry ... that browsers would have to start doing two-pass layouts inside of blocks in order to measure content and then resolve percentages". Agree that browsers would have to do two-pass layouts. Is the 'worry' about layout speed? Or engine-implementation complexity?

I was mostly thinking about speed, yeah. But yes, also complexity to some extent -- because we would likely want & need to add optimizations and caching to avoid two-pass layouts where possible, and those would surely have tricky edge cases, and so there would be a lot of complexity and special considerations around those optimizations.

Basically, I just wanted to express that this would not be a trivial change.

  1. Seems to me the clear answer to the CSS-comment question above is '150px'. But maybe I'm missing something.

Based on where this thread is trending, I think the answer to my css-comment-question should in fact be "content-height", not 150px. Of course, the answer depends on whether we consider .test to have a definite height or not (once we have min-height:min-content implemented to actually do what it says).

That's why I brought up those three outcomes to compare among.

So, to be clear, I'm NOT advocating for making min-height: min-content allow child percentages to resolve, rather I'm trying to make sure both the pros and cons are being considered. Seems like only the cons have been discussed so far.

I think the pros that have been mentioned here were: (1) pro: consistency between the content-dependent sizing keywords (including auto) in the min-width/min-height properties. But as I noted in my most recent comment above, I think it in fact makes more sense to treat auto as an exception. (2) The "pro" that you mentioned, that it would provide the "one weird trick" to trigger percentage re-resolution in content-sized containers. If we really think that this is the right ergonomic way to provide that feature, then that's worth considering, I suppose. But I think that needs some further discussion/examples to demonstrate that this is in fact a good way to provide this behavior (rather than just being a hack).

davidsgrogan commented 3 years ago

It could be, but it feels like a potentially arcane way of doing that. If we want to address that use-case, perhaps there's a more direct way to do so?

Yes, you are right, "one weird trick" is probably not the best way to address that use-case.

  1. Seems to me the clear answer to the CSS-comment question above is '150px'. But maybe I'm missing something.

Based on where this thread is trending, I think the answer to my css-comment-question should in fact be "content-height", not 150px.

Ah, yes, you are right. I meant 150px if we went in the other direction on this issue.

Thanks for the good responses. I now get it and am in total agreement that min-height: min-content and friends should result in an indefinite height.

css-meeting-bot commented 3 years ago

The CSS Working Group just discussed [css-flexbox-1][css-sizing-3] Definiteness of min-height: min-content, and agreed to the following:

The full IRC log of that discussion <dael> Topic: [css-flexbox-1][css-sizing-3] Definiteness of min-height: min-content
<dael> github: https://github.com/w3c/csswg-drafts/issues/6457
<dael> TabAtkins: We resolved a few weeks ago...the discussion on the call was roughly that min-height:auto needs it special behavior but probably didn't want to extend to other content-sizing keywords
<dael> TabAtkins: Further discussion on issue went back and forth on minor details. Overall everyone agrees that's fine
<fantasai> proposal is https://github.com/w3c/csswg-drafts/issues/6457#issuecomment-917223624
<fantasai> "min-height:auto just has special behavior that allows child %s to still resolve, and it's not a precedent to apply to all the other content-based keywords. "
<dael> TabAtkins: Prop: Accept results of last discussion - all intrinsic sizing keywords besides auto make sizes indefinite for boxes
<dael> Rossen_: Everything but auto makes it indefinite?
<TabAtkins> So, Agenda+ to resolve that min-height:auto just has special behavior that allows child %s to still resolve, and it's not a precedent to apply to all the other content-based keywords. (Thus, we'll remove, or possibly keep-but-reverse, the note that's currently in the spec.)
<dael> TabAtkins: Yeah
<dael> TabAtkins: prop from issue ^
<fantasai> rationale is in dholbert's comment https://github.com/w3c/csswg-drafts/issues/6457#issuecomment-915427806
<dael> TabAtkins: content keywords make it indefinite
<Rossen_> q?
<dael> Rossen_: Reasonable. Any comments?
<fantasai> +1 from me (and dholbert obviously ;)
<dael> Rossen_: Obj?
<dholbert> +1 :D
<dael> RESOLVED: min-height:auto just has special behavior that allows child %s to still resolve, and it's not a precedent to apply to all the other content-based keywords. (Thus, we'll remove, or possibly keep-but-reverse, the note that's currently in the spec.)
tabatkins commented 2 years ago

Okay, so I fixed up the language incidentally in response to a different issue, but just now made it slightly clearer, and ensured that Grid uses the same language.

Just need to check if there are any tests for this behavior (both auto min not causing indefiniteness, and min-content/etc causing it).

benface commented 7 months ago

Sorry to bump this old issue with a question, but I'm wondering if browsers currently behave like the conclusion that was reached here. Maybe this is unrelated to the issue, but I am trying to use min-height: min-content (or even max-content or fit-content) and it's not working as expected. Here's a simple reproduction: https://codepen.io/benface/pen/ZEZOXre