-
Notifications
You must be signed in to change notification settings - Fork 695
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-flexbox] [css-sizing-4] Interaction of flexbox minimum height and aspect-ratio minimum height #6069
Comments
The Flexbox automatic minimum size rules were drafted with the assumption that only replaced elements had an aspect ratio. The Sizing rules, which make the content-based size a very strong constraint on the auto min size, are correct for non-replaced elements; but the Flexbox rules, which make the "content-based" (aka replaced element natural size) a weaker constraint, are correct for replaced elements. Probably what we need to fix here is to distinguish content-based sizes of non-replaced elements which should be a stronger constraint than a transferred size vs content-based sizes of replaced elements (those derived from the natural size of the replaced content), which should be a weaker constraint than a transferred size. So this needs some tweaks to merge the two into a consistent set of rules. |
Agenda+ to confirm that we want to make non-replaced elements ignore any transferred preferred sizes for the purpose of automatic minimum sizing. Remember, this is the minimum size below which a flex item, by default, cannot shrink. We don't want to be too constraining, so that when it's sensible to shrink it can shrink. Current spec:
Proposed spec:
In all cases, we apply an opposite-axis minimum transferred through the aspect ratio if any: min(specified size, max size, opposite-min * aspect-ratio) Alternately we can ignore the transferred min of a non-replaced element. [All of these are clamped by the appropriate maximum sizes.] The various inputs into this are
|
(cc @aethanyc since this affects tests they wrote) I think that is an outcome that makes sense; it is certainly consistent with block layout in the sense that aspect ratio is "ignored" if it would cause content to overflow. |
The proposed change -- non-replaced elements ignore any transferred preferred sizes for the purpose of automatic minimum sizing -- looks reasonable to me. (cc @dholbert) |
(Also a reminder that grid probably needs a similar change) |
Echoing @aethanyc :
Yup, seems reasonable to me, too. |
…size in content-based minimum size. #6069
…in content-based minimum size. #6069
OK, we've updated the Flexbox and Grid specs to exclude non-replaced elements from using the transferred size suggestion:
Note: We also redrafted Flexbox’s definition to be a bit to be easier to read and to match Grid more closely in f86bc38 Keeping Agenda+ to confirm with the CSSWG. |
The CSS Working Group just discussed The full IRC log of that discussion<dael> Topic: [css-flexbox] [css-sizing-4] Interaction of flexbox minimum height and aspect-ratio minimum height<dael> github: https://github.com//issues/6069 <fantasai> https://github.com//issues/6069#issuecomment-790046695 <dael> Rossen_: Summary ^ <fantasai> https://github.com//issues/6069#issuecomment-797028657 <dael> Rossen_: Can you repeat it? <dael> fantasai: Prop is a non-replaced element ignores transfered sizes for auto min sizing <dael> fantasai: auto min siziing in flex and gri where initial value is auto. You take size of min-content so you don't shrink flex item too much. On images with aspect ratio if you set for row flexbox a height on the image we transfer that through a-r and modifies minimum <dael> fantasai: Makes sense for images and that's what it was designed for. For non-replaced boxes this makes less sense. When we thought through we considered don't do this. Make this behavior restricted to replaced elements. Don't apply to non-replaced elements that have a-r added by a-r property <Rossen_> q? <dael> florian: You're left with min prior to a-r being used? <dael> iank_: min-content or intrinsic <dael> fantasai: For relaced element natural and min-content size is same. <dael> fantasai: For repalced if you spec a size on the axis we clamp auto-min by that size. <dael> fantasai: Other than that we transfer that and other conent and min that. <dael> fantasai: Non-replaced we don't transfer <dael> florian: If the transfer a-r would have made min size smaller clearly good to not do. If tranfer a-r makes min-size bigger is it not good to honor it? <dael> fantasai: Trying to remember the logic <dael> fantasai: This was because on items with an a-r...consider a block with a-r. min-height: auto. Decided that means element with an a-r will ignore a-r if content is larger than height it would have imposed by a-r. 1:1 a-r block. 100px width which gives 100h. 500px content so I have to block 500px <dael> fantasai: When you turn it into a column flexbox we want same. But if we transfer size we can't get same behavior b/c made auto-min dictated by a-r. To have same behavior of the content blowing out the a-r in both blocks we can't use same transfer behavior we have for replaced <dael> florian: Makes sense, thank you <dael> fantasai: More like thanks cbiesinger <dael> iank_: This means a min-content size will ignore tranferred min and max heights. But min-content contribution will respect them? <dael> fantasai: min-content contribution will have to take into account...[thinks]...something complicated about this but I don't remember <dael> fantasai: I think min-content contribution needs to take it into account. I remember complixity on how defined in a different issue and I'm not sure if it was relevent there <dael> cbiesinger: min-content contribution doesn't matter? <cbiesinger> s/?/in flexbox?/ <dael> cbiesinger: flexbox intrinsic sizes are defined in flexbox spec. <dael> Rossen_: Got it <dael> iank_: I asked about it to make sure it is applied there. grid spec does use the min-content contributions directly <dael> iank_: In certain circumstances <dael> fantasai: yeah <dael> cbiesinger: I don't think we can use that in content contribution b/c don't know content-height. We would create circular <dael> Rossen_: Yes <dael> Rossen_: We're getting to top of hour. Doesn't sound like we're ready to resolve on this. not quite. <dael> Rossen_: Perhaps I would suggest we take it back to the issue and answer the questions there and then resolve during the F2F. Let's make sure we're not missing any of the points raised here <dael> Rossen_: Let's end here |
The CSS Working Group just discussed
The full IRC log of that discussion<astearns> topic: [css-flexbox] [css-sizing-4] Interaction of flexbox minimum height and aspect-ratio minimum height<astearns> github: https://github.com//issues/6069 <fremy> cbiesinger: the aspect ratio spec says that min-height auto and you compute it using the aspect ratio, you expand to the content height <fremy> cbiesinger: but in a column flex box, the height is the minimum of the transferred height and the content height <fremy> cbiesinger: it's not clear to me which one should win <fremy> cbiesinger: fantasai clarified the aspect ratio version should win <fremy> cbiesinger: but does the working group agree to this? <fremy> fantasai: this is different for replaced and non-replaced elements <fremy> fantasai: because the intrinsic size of replaced elements is not as strict <fremy> fantasai: so we need to keep that behavior for them <fremy> fantasai: but for non-replaced elements, the intrinsic size is more meaningful <fremy> florian: I'm not sure about what the alternative is? <fremy> cbiesinger: width:100px + aspect ratio 1:1 but intrinsic-height:200px <fremy> florian: ah ok, got it <fremy> cbiesinger: because it used to be the minimum <fremy> jensimmo_: the default of flexbox is that it's shrink then grow <fremy> jensimmo_: but the proposal is that for replaced elements, we make aspect ratio "work" <fremy> fantasai: yes, that is the heart of the issue <fremy> (some discussion about magic css jokes) <fremy> astearns: last week there was more contention <fantasai> [jen's explanation was that if there's a bunch of elements you want to be squares, unless too much content want to grow, that's the behavior we specced for block boxes with aspect-ratio; and the issue is making sure it works the same for flexbox] <fremy> iank_: that was me last week <fremy> iank_: I am now fine with this <fremy> astearns: is the spec edit correct for what we discussed <fremy> fantasai: I think it is correct <fremy> fantasai: but we need to update the spec <fremy> fantasai: proposal is non-replaced elements with an aspect ratio honor the intrinsic size in flexbox <cbiesinger> for min-height:auto <fremy> jensimmo_: I am seeing a lot of tutorials about aspect ratio on replaced elements <fremy> jensimmo_: in particular content-size fit etc <fremy> fantasai: that should continue to work <cbiesinger> s/content-size fit/object-fit/ I think? <fremy> fantasai: that is why we don't do this for replaced elements <fremy> fantasai: non-replaced elements have an intrinsic size that depends on content, that might not be easy to resize <fremy> jensimmo_: the previous text for images would still apply then? <fremy> fantasai: yes <fremy> astearns: any objection to keep fantasai's edit in the ED? <fremy> RESOLVED: non-replaced elements with an aspect ratio honor the intrinsic size in flexbox for min-height:auto <fremy> <br length=15min /> <leaverou_> Partial regrets for Chris, he's having a migraine. May join later <fremy> TabAtkins: might be a dwarf propaganda map, hiding their continent from the view of others then! that would be smart of them... <TabAtkins> House Sivis is gnomish, not dwarvish <fantasai> ScribeNick: fantasai |
I'm a little skeptical by the grid behaviour now - and it doesn't seem correct to me. E.g. consider this testcase: is it really correct that we ignore the transferred size suggestion for non-replaced elements? |
… element. If the flex item is a non-replaced element and its min-width/min-height is 'auto', the spec has changed so that it has no transferred size suggestion now. https://drafts.csswg.org/css-flexbox-1/#min-size-auto This patch also updates WPT tests to fix #27878 Differential Revision: https://phabricator.services.mozilla.com/D112830 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1703304 gecko-commit: fb14e6d29a44a79efc296cc727f92aa58ea4dacc gecko-reviewers: dholbert
@bfgeek I think it is. The goal of the automatic minimum size is to prevent things from becoming so small that their contents are unreadable (overflowing or shrunk to zero). The preferred size still accounts for the aspect ratio, so if the column is allowed to grow wider the item will be square as intended, but when there isn't enough space in the column (and the item's own contents are small enough to fit), it won't overflow the column. That's a good behavior for the automatic minimum size, imho. If the author wants the aspect ratio to force overflow, they can set the width or min-width to max-content instead of auto. |
That was a WG resolution from quite a few months ago: w3c/csswg-drafts#6069 Also update bug numbers of some tests that were assigned to this bug but still do not pass. Fixed: 1204827 Change-Id: I7ff892e751a21a4348d2bc7c18ca5eb1b085f940 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3173718 Commit-Queue: David Grogan <dgrogan@chromium.org> Auto-Submit: David Grogan <dgrogan@chromium.org> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org> Cr-Commit-Position: refs/heads/main@{#923654}
https://drafts.csswg.org/css-flexbox/#min-size-auto
https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum
If an item has an aspect ratio and is a flex item, both specs impose a minimum size on a flex item. Consider:
In this case, the flex item has two constraints:
What should the used minimum size be? max(100px, 500px)? Or should the flexbox one "override" the aspect-ratio one?
https://github.com/web-platform-tests/wpt/blob/master/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html relies on the latter interpretation.
The text was updated successfully, but these errors were encountered: