-
Notifications
You must be signed in to change notification settings - Fork 711
[css-flexbox] Why does flex item aspect ratio sizing not take max-width into account? #3736
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
Comments
I don't think browsers implement that part of the spec yet, though I have a
patch to do so
…On Fri, Mar 15, 2019, 20:22 jonjohnjohnson ***@***.***> wrote:
https://jsfiddle.net/nfy4qe1v/ Here's your example and I see a 5x5, not
5x10 image.
Did you mean to use flex-direction: row? Regardless of the flex
containers own width? Which then means the image "stretches" to it's
intrinsic (10px) height unless you set align-items/align-self to a non
stretch (normal) value?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3736 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABahZX0Ig3hFNYJnbDfosxociud8ORANks5vXEdxgaJpZM4b3fED>
.
|
@cbiesinger Uh, that looks like a definite oversight. Should be factored in. |
OK, so looking at this more closely, it is handled in E (which is where we fall down to):
The max-content size takes into account any sizing constraints in the cross axis: We might want to make it clearer that this case is handled, though. |
But that's not what I'm concerned about (and anyway, why would we fall down to that?) If we have a replaced item, for the min-size calculation, we do: For the flex base size, we do: Shouldn't that be consistent? |
This causes a real compat issue: https://bugs.chromium.org/p/chromium/issues/detail?id=972576 |
@cbiesinger I'm not sure I understand your point about consistency. Those two calculations are consistent, are they not? The only difference is that the min-size uses the minimum size and the base size doesn't, which is appropriate. Anyway, to follow your example...
First, we calculate the minimum main size of this image, which is an automatic minimum size. The specified size is undefined, the transferred size is 5px, and the content size is 10px. The transferred size wins, so its minimum main size is 5px. Then we calculate the base size. The flex basis is |
Why doesn't it have a definite cross size? Remember point 1 in https://drafts.csswg.org/css-flexbox/#definite However, the same issue occurs if you give it a width: 100px (which is basically the 972576 case), and then the cross size is certainly definite. Also, my point about consistency was about how the cross size max size is treated, it seems inconsistent for no real reason? |
@cbiesinger Well, the flex container has a definite cross size, sure. Wrt the flex item itself... If it's not stretched, then it's not definite and would fall through E as I described above. If it is stretched... then you're right, we'd fall into B. The flex base size would be calculated from the cross size, which is max([container cross size], 5px), which is usually 5px. So let's assume 5px as the inner cross size. Transferred through the aspect ratio gives us a flex base size of 5px, and the image maintains its 1:1 aspect ratio. Basically, same result. I suppose the issue here is that “cross size” is not clarified as to whether it's the “used cross size” or the value of the cross size property? It should definitely be the used cross size that's used in this calculation, so that the max is taken into account. Lmk if this is the issue, or if there's something else. |
Why would it "usually" be 5px? That's only if the container is smaller than 5px. In my example the item is stretched (default value) and max(5px, cross size) is 10px. So with a 1:1 ratio, you'll get a 10px height (main size) and 5px cross size (because of max-width). Or, if you specify something like width: 100px; max-width: 5px;, we'll use 100px to compute main size but will clamp cross size to 5px. Yes, specifying that the cross size needs to be clamped by max (and min?) is the outcome I am hoping for. I would prefer not using the terminology "used cross size" because to me it's not clear that this implies applying min/max. |
Yeah, that's the only case I can think of. Your specific example would indeed be 5px by 5px afaict.
So as I mentioned, I think “cross size” under B should be “used cross size”, which means it's going to be 5px in that calculation. And thus the main size will resolve to 5px.
The “used cross size” maps to the “used height” in this case, right? I think that should be clear. And the used height is definitely applying min/max, otherwise getComputedStyle() wouldn't be honoring them either. |
… aspect ratio take the *used* cross size as input. #3736
Clarified to “used inner cross size” in the section defining the calculation of flex base sizes as a function of the cross size and an aspect ratio. @dholbert Would you mind reviewing this issue, please? :) |
OK, that looks good, thanks! |
I think the change makes sense, yeah. Thanks! |
Looks like this is tested by https://github.com/web-platform-tests/wpt/blob/master/css/css-flexbox/flex-aspect-ratio-img-row-010.html |
Here:
https://drafts.csswg.org/css-flexbox/#algo-main-item
The cross axis max-size is not taken into account. That differs from the min-size calculation here:
https://drafts.csswg.org/css-flexbox/#min-size-auto
This leads to unfortunate outcomes, e.g.:
Now, the algorithm will compute a height of 10px, but the width is limited to 5px, and the image is distorted.
The text was updated successfully, but these errors were encountered: