-
Notifications
You must be signed in to change notification settings - Fork 708
[css-grid] Doubts regarding implied minimum size of images #1117
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
After thinking on this for a while. I think now I understand the difference between 1) and 2). The difference is that in the track sizing algorithm in both cases the 1st row has:
As in the case 1) there's no constraint on the height of the container, the row grows up to 200px. |
I believe than 3) and 4) should behave the same than 1) and 2) respectively. The height of the 1st row should be 200px for 3) and 50px for 4). And then for 5), 6), 7) and 8) I believe the height of the row should be 200px, matching Chrome behavior. So summarizing IMHO there are bugs in both engines:
Sorry for all the noise on this... I was getting very confused before. 😌 |
Ok, so more thoughts on this... after some discussions with @svillar. Thanks! Coming back to case 3). I believe now it's not a Chrome bug either. Let me explain why. <div style="display: grid; width: 200px;">
<img style="width: 100%;" src="http://placehold.it/50x50">
<div style="background: yellow;">Item (2nd row)</div>
</div> The problem is that the item in the 2nd row was adding noise to the example. And I was missing it. A) <div style="display: grid; width: 200px;">
<img style="width: 100%;" src="http://placehold.it/50x50">
</div> Here to resolve the size of the If we use Now the example with the item. Imagine that it has exactly 100px width: <div style="display: grid; width: 200px;">
<img style="width: 100%;" src="http://placehold.it/50x50">
<div style="background: yellow;">Item (2nd row)</div>
</div> Here, to calculate the size of the Again if we use So we believe Chrome behavior is the expected one. Does it make sense? Next you can see an image with the results of the cases explained here. |
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the autmoatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
Short version: percentages against indefinite sizes are nasty. :( Wrt #1117 (comment) comment A Wrt comment B, I'm confused how we got to a row height of 100px. |
I guess you mean C because in B the cell is 50x50. As rego explains the item in the second row (the one with text) forces the column to be 100px wide (so the image on the first row). Then when computing rows we end up with a first row of 100px (because the image respects the aspect ratio). After running the track sizing algorithm we proceed to compute the alignment. The default value for justify-content is stretch, so the column becames 200px wide (the size of the grid container), but the rows are unaffected (we don't run the track sizing algorithm again). After doing, once the tracks sizes are set, we proceed to layout/reflow the grid items. The image becomes 200px wide (as it has width:100%) and so the height (to respect the aspect ratio). That indeed causes an overflow, but that's the expected behavior in this case. |
I'm a bit confused here: looking at the JSBin from the first comment, Chrome's behavior is identical for 1, 2, and 5-8 (the row is 200px tall). For 3 and 4, the row is 50px tall, and the image is squished to 200x50. |
Never mind, my Chrome was out of date. Now that I've let it update, it matches what Manuel described. |
After some thought, @tabatkins and I think we found what's wrong in the spec, and how to fix it. With respect to examples 1/2/5/6: Grid is using the same "automatic minimum size" algo that Flexbox does, which sets the min to the smaller of the content size and the specified/transferred size. This is correct for Flexbox - it gives us a reasonable minimum size that the image won't shrink below. But it serves a different purpose in Grid, effectively setting the layout size of the track while the item itself sizes into that containing block, potentially overflowing it. (Or, in other words, Flexbox actually sizes the item with the information; Grid sizes the track, and then the item lays itself out, and we don't want the item to overflow its track by default.) Therefore, in Grid it doesn't make as much sense to pay attention to the content size when a specified/transferred size exists. So, we think the right fix is to just change Grid's automatic minimum size to be the specified size if it exists, else the transferred size if it exists, else the content size. This will make examples 1, 2, 5, and 6 all render basically the same way, with a 200px tall first row, and the image filling that row. (Examples 3, 4, 7, and 8 rely on a different issue, so we won't worry about them here right now.) This gives more sensible results in size-constrained grids when the item's transferred size is larger than its content size, by keeping the contents of the track in sync and avoiding overlap of adjacent grid items. Note: A similar consideration about overlapping overflow was used in resolving issue 283. Refiling this particular issue as #1149 The second problem is in examples 3/4/7/8: @mrego writes:
The problem here is that the row height of 50px is entirely unexpected. The author would expect this to behave similar to fr sizing: the content and the row height are calculated based on the stretched track size, not the initial track size. The responsible spec prose is in https://drafts.csswg.org/css-grid/#algo-overview where Refiling this issue as #1150 Please let us know if this seems to address all of the doubts or if we missed something. :) |
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the autmoatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
Yeah it sounds good to split this into 2 different issues. Thanks for taking a look! |
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the automatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the automatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
This is a test created from issue w3c/csswg-drafts#1117. The test checks how the automatic minimum size of images affect to the grid container sizing and the track sizes. The test also verifies the size of the image in the different cases. The test combines the following cases: * A grid container with fixed width (smaller and bigger than the image). * A constrained grid container. * A grid container without default "justify-content: stretch". * A grid container with an extra test item. * An image with a fixed width (smaller and bigger than the image original size). * An image with a percentage width.
Section: 6.6. Implied Minimum Size of Grid Items.
This comes from a bug reported to Firefox by @jensimmons, after taking a look I'm quite lost now and I'm not sure if I understand even the simpler cases. 😞
So let's start with some examples and try to explain what's going on.
You can check the examples live in a jsbin.
1) 50x50 image with
width: 200px
:My doubt here comes about how the size of the 1st row is calculated. In both Chrome and Firefox it's 200px.
The rows are
auto
and the image hasmin-height: auto;
.The image has a specified width of 200px, and as it has aspect ratio its transferred size is 200px too. But its content size is 50px.
According to the spec:
So I'm not sure if the height of the row should actually be 50px instead of 200px.
I should be missing something else, but I don't know why right now.
2) 50x50 image with
width: 200px
and grid container withheight: 10px
:In this case both Chrome and Firefox calculate the size of the 1st row as 50px.
Is that right, why is this different from 1)?
3) 50x50 image with
width: 100%
and grid container withwidth: 200px
:I'd expect this to be equivalent to 1). As the 100% width on the image should be resolved to 200px.
However in Firefox it's like 2), the row has 50px (and in Chrome it seems there's a bug as the size of the first row is 100px, which doesn't make a lot of sense.
According to the spec, the content size is 50px and the transferred size is 200px, so the minimum is 50px. So it seems Firefox is right on this one.
Problem here is that the 2nd row will be overlapping the image which doesn't seem what the user will want.
@jensimmons commented on the bug report that this won't be what authors want.
4) 50x50 image with
width: 100%
and grid container withwidth: 200px
andheight: 10px
:I'd expect this to be equivalent to 2). Again the 100% width would be resolved to 200px.
But here the results vary. In Chrome the first row has 50px, but in Firefox it has only 10px.
5) 500x500 image with
width: 200px
:Here both browsers behave the same and have a height of 200px for the 1st row.
This makes sense according to the spec as the transferred size is 200px and the content size is 500px. So the minimum is 200px.
6) 500x500 image with
width: 200px
and grid container withheight: 10px
:Again both browsers have the same behavior and the result is the same than 5).
Again the transferred size 200px is smaller than the content size 500px.
7) 500x500 image with
width: 100%
and grid container withwidth: 200px
:Here in Chrome the size of the 1st row is 200px.
In Firefox it's 500px, as it consider that the 100% width is indefinite, so it doesn't have transferred size.
Anyway it seems @MatsPalmgren agrees that this should be considered definite in the bug report. Which will lead to the same output than Chrome.
8) 500x500 image with
width: 100%
and grid container withwidth: 200px
andheight: 10px
:Here Chrome has a size of 200px for the row. However Firefox has 10px.
I believe that 200px for the 1st row should be the expected behavior.
Could someone help to clarify this topic? I'm almost certain I'm missing something else from the specs, at least to explain case 1) but I cannot find it. 😕
Thank you very much.
The text was updated successfully, but these errors were encountered: