-
Notifications
You must be signed in to change notification settings - Fork 672
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-grid] What if base sizes of both auto/min-content and max-content minimums are being handled? #3621
Comments
Firefox, Chromium and Edge are not interoperable: https://jsfiddle.net/mafd8s3o/ .grid {
display: grid;
grid-template-columns: 200px 300px;
grid-template-rows: max-content fit-content(0);
}
.foo {
background: yellow;
}
.bar {
background: cyan;
}
.baz {
grid-area: 1 / 2 / 3 / 3;
background: magenta;
height: 300px;
} <div class="grid">
<div class="foo">foo</div>
<div class="bar">bar</div>
<div class="baz">baz</div>
</div>
|
AFAIK, Gecko has the correct layout for the given testcase. The reason is that "baz" has already grown both rows in the "For intrinsic minimums" step by its minimum contribution (300px, making each row 150px). So in the steps where the FYI, I asked a similar question in #209. (what Tab refers to as 12.5.1 there is now 11.5 step 3.6 "For max-content maximums", and 12.5.2 step 5 and 6 are now 11.5.1 step 2 and 3.)
The first row has As noted in #209, |
@MatsPalmgren If I understand correctly, the difference is in the "For intrinsic minimums" step. At that point, the growth limit of both rows is something like 18px (the height of the content). When distributing the 300px, the base sizes have already reached the growth limits, so we need to "Distribute space beyond growth limits". The spec says,
Here we are handling a track with an auto minimum (the 2nd row) and another one with a max-content minimum. Both conditions apply, so it's not clear among which tracks the 300px should be distributed. In fact this is not completely related to
|
Hmm, yeah, that's a good point. I'll have to run this in a debugger later to see exactly what we're doing...
"For intrinsic minimums" says "First increase the base size of tracks with an intrinsic min track sizing function". Since both rows have an intrinsic min sizing function, both are included in the set of tracks we should run "distributing extra space" over. The second bullet applies to row 1, and the first bullet applies to row 2 (since the "For intrinsic minimums" step handles base sizes). So it seems to me we should distribute to both rows in this section: 2.1.
Yeah, I'm not sure what that paragraph is trying to say really. Maybe that any
For the |
So I guess you are saying that when multiple bullets apply, we should consider the union? With the "if there are no such tracks, then all affected tracks" happening at the end after doing the union. Like
That would be clearer (and easy to implement in Chromium).
Yes, that's my understanding too. It would match Edge instead of Chrome. When distributing "For intrinsic minimums", Chromium only applies the first bullet, i.e. only tracks matching |
Yes, given that the affected tracks may contain tracks in both these categories, it doesn't make much sense to apply the "if there are no such tracks, then all affected tracks" to each bullet separately.
That seems like a bug in Chrome to me. "For intrinsic minimums" should include Regarding the meaning of the "For this purpose, I think that's the intention of the spec as it's written and it makes sense to me. However, for |
Fwiw, I did a quick hack to handle |
OK, I reviewed the old version of the Grid sizing algorithm... afaict the two branches are correct, they're just poorly labelled. I've clarified the conditions for each one, I think this is now correct. It doesn't handle fit-content() though, so I'll need to follow up on that point. |
…s to its max track sizing function. (We are not treating fit-content()'s min track sizing function as max-content ever! Related to #3621
@fantasai and I walked thru some examples including this specific one, and yes, we agree with Mats's interpretation in all aspects. In this case, the two rows would end up being 145px and 155px tall. To be clear: when fit-content() tracks are involved, the set of tracks that are growing when you're distributing space past limits can change over time due to fit-content() tracks freezing, and then potentially causing a "grow all tracks" switchover due to all the fit-content() tracks freezing. @Loirooriol Do you need anything clarified about the algorithm in that case? It read pretty clearly to us, even with fresh eyes that don't remember typing it out originally, but we're happy to make it clearer if necessary. |
…cks when distributing past growth limits. #3621
Yes I think it's clear enough now |
… 'base sizes'; remove nonsense application of 'intrinsic' to 'growth limits'. #3621
From https://drafts.csswg.org/css-grid/#algo-spanning-items,
But then https://drafts.csswg.org/css-grid/#extra-space says
What if the item spans a track with a
min-content
orauto
minimum, and another withmax-content
, and the base sizes of both tracks are being simultaneously handled because both are intrinsic? To which tracks do you distribute space? The union of both cases? The intersection (all affected tracks if empty)? Something else?The text was updated successfully, but these errors were encountered: