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-sizing-4] Should contain-intrinsic-size be used for min-height: auto? #5537

Closed
cbiesinger opened this issue Sep 22, 2020 · 7 comments
Closed
Labels
Closed Rejected as Wontfix by Editor Discretion Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-sizing-4

Comments

@cbiesinger
Copy link

https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum

For a testcase like the the ones at https://dvoytenko.github.io/aspect-ratio-css/#x-intrinsic-and-float (make the window width smaller; should there be pink visible?), if the aspect-ratio calculated height of the box shrinks below the size from contain-intrinsic-size, should min-height: auto keep the height of the box at 300px (from contain-intrinsic-size)?

On the one hand, that's probably the straightforward interpretation of the intrinsic size; on the other hand, the content is not "real", should min-height: auto take it into account?

@cbiesinger
Copy link
Author

@bfgeek fyi

@dvoytenko
Copy link

Heads up, I've updated the https://dvoytenko.github.io/aspect-ratio-css/#x-intrinsic-and-float to use min-height: 0 to get the layout fixed, so please remove it to repro the issue that @cbiesinger describes. However, @cbiesinger's still stands.

@fantasai
Copy link
Collaborator

On the one hand, that's probably the straightforward interpretation of the intrinsic size; on the other hand, the content is not "real", should min-height: auto take it into account?

I think contain-intrinsic-size should be plugged into layout just like a real intrinsic size. Making exceptions here seems odd.

@tabatkins
Copy link
Member

tabatkins commented Sep 30, 2020

On first glance, I agree with @fantasai; c-i-s is meant to provide an intrinsic size, and min-height:auto pays attention to the intrinsic size, so it seems like they should work together.

Is there a reason we shouldn't do this? All I see here are some abstract test cases, with no reasoning given for why it should act one way or the other. If there's a compelling case to be made for letting ratio-dependent elements shrink below their c-i-s by default (and which somehow doesn't apply to a replaced element's "actual" intrinsic size), we can consider it; in the absence of that, tho, we have a generic switch you can toggle to get this behavior already (set min-height:0) which applies in a bunch more similar cases anyway.

@tabatkins
Copy link
Member

So our proposal is to close this issue no change. Does that work for people?

@dvoytenko
Copy link

I was highlighting in these tests the difference between content-intrinsic-size+aspect-ratio and img[width][height]+width:100%;height:auto. In other words, the difference between an explicit intrinsic size/aspect ratio and implicit intrinsic size/aspect ratio of a native replaced element. In layman's terms I don't see why these two cases should be so different. And I also dislike that conceptually the min-height: auto takes precedence over the explicitly specified aspect-ratio.

That being said, I understand that there might be some in-depth differences between replaced and non-replaced elements that justify the two issues are mentioned above. And as you mention, the fix is quite simple. Thus I don't feel very strongly that this must be resolved the way I describe. But for posterity, it might be good to clarify replaced-vs-non-replaced differences here.

@tabatkins
Copy link
Member

The two cases are different because c-i-s doesn't have anything to do with replaced elements. ^_^ c-i-s lets us skip actually doing the rendering work for an element's contents, and instead pretend that work had already been done and resulted in the content having the specified c-i-s size. So, it should be acting exactly like there was normal content in there of an appropriate size, and what you're seeing is exactly the behavior you'd get out of that: min-height:auto defaults to the min-content height (which is the full laid-out height), trying to avoid an accidental overflow situation.

I suspect the confusion here is that “intrinsic size” is commonly used to describe replaced elements, and not commonly used to describe normal elements, causing you to draw stronger parallels than actually exist. Hopefully this discussion has cleared things up now. I'm going to assume that your previous response means you're okay with us closing no change here, since specifying min-height: 0 does gives you the desired behavior.

@tabatkins tabatkins added Closed Rejected as Wontfix by Editor Discretion Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. labels Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Rejected as Wontfix by Editor Discretion Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-sizing-4
Projects
None yet
Development

No branches or pull requests

4 participants