[css-grid] Applying 'justify-content' content distribution is in the wrong place in the overall grid sizing algo

The Grid spec currently says that "the tracks are aligned within the grid container according to the align-content and justify-content properties." in step 4, after the sizes of both columns and rows have been determined. This leads to the wrong layout because intrinsic row sizes are then determined using the wrong Containing Block inline-size for spanning items. Here's a testcase demonstrating the wrong layout in Chrome (the row size is too large for the text). The layout in Firefox is IMHO the correct one - it sizes the row based on the height of the text when the CB inline-size is 500px, which is what the grid-column: span 2 grid area is sized to after justify-content is applied.

The correct place to apply justify-content in the overall algo is between step 1 and 2, i.e. first you determine the column sizes, then you apply justify-content. This allows you to calculate the correct CB inline-size (and padding/margin percentage basis) for the items. Now you can calculate the correct row sizes, and after that apply align-content.

@tabatkins

I knew this would end up in a issue here. I discussed the case with @tabatkins today, and he was pretty sure Chromium was right.
The spec even has a note about this, so it seems intentional:

Note: This can introduce extra space between tracks, potentially enlarging the grid area
of any grid items spanning the gaps beyond the space allotted to during track sizing.

Anyway let's see what other people think about this (CC @fantasai & @atanassov).
Or if someone recalls the reason why this is done this way.

BTW, Firefox and Edge behave the same for this case, and different from Chromium and WebKit.
Note that for the same example with an orthgonal item neither of Edge, Chromium or WebKit recompute the size of the column. Firefox just don't take orthogonal into accounts so the result is different.

IIRC we considered the effect of alignment on same-axis sizing, e.g. it would add some slack to a spanning element which was spanning only max-content sized tracks. However, we didn't consider the effect of alignment on sizing in the other axis, and it does seem to me that doing so would be an improvement in most cases. (If orthogonal cases don't get quite the same benefits that's probably OK, as long as we don't break them.)

If we do this as part of the track sizing algorithm, this will conflict with what was discussed at #2409.

For example you have 2 rows of 100px each, and then you've a orthogonal item spanning both rows.
In #2409 we said that the item should consider it has 200px height, for the purpose of computing the proper intrinsic size with baseline.
But, if there's align-content: space-between the final height of the item could be 300px or 500px (it'd depend on the grid container's height). We couldn't compute that in advance to know the estimated height to do the intrinsic size computation.

@mrego

@mrego I'm not following. We use 200px height to calculate the intrinsic width of the orthogonal item, sure. And then we start the track sizing algorithm, sizing the columns based on that information. Whether we perform alignment of the tracks after that set of tracks is sized or at the end of the algorithm, the final size of the orthogonal item will still be 300px or 500px. If we apply alignment within the grid algorithm, it seems to me we get a better result regardless, no?

The proposal, if I understand correctly (just so we're all clear) is to change from:

Size the columns, making some estimates about row sizes (if needed for orthogonal flows)
Size the rows, using the column sizes we just calculated (if needed for orthogonal flows)
Size the column, using the row sizes we just calculated (skip if not needed)
Size the rows, using the column sizes we just calculated (skip if not needed)
Align the tracks
Lay out items into the resulting grid areas

to:

Size the columns, making some estimates about row sizes (if needed for orthogonal flows). Align the columns (so colspans in the next step are more accurate).
Size the rows, using the column sizes we just calculated (if needed for orthogonal flows). Align the rows (so rowspans in the next step are more accurate).
Size the column, using the row sizes we just calculated (skip if not needed). Align the rows (so colspans in the next step are more accurate).
Size the rows, using the column sizes we just calculated (skip if not needed). Align the rows.
Lay out items into the resulting grid areas

Whether we perform alignment of the tracks after that set of tracks is sized or at the end of the algorithm, the final size of the orthogonal item will still be 300px or 500px. If we apply alignment within the grid algorithm, it seems to me we get a better result regardless, no?

The issue is that baseline alignment in the row-axis (justify-self) may depend on the final size of the row, as explained in #2409.

@javifernandez

@javifernandez I'm not understanding why this is a problem. :(

This issue is about the place to run the Content Alignment logic; with the new baseline alignment integrated in the track sizing algorithm, the place where we finally run the Content Alignment logic will have an impact in the result of the Baseline Alignment, generating different results.

Hmm, indeed. Case in point:

<style>
.grid { 
 display: grid; 
 grid: 100px 100px auto / auto; 
 align-content: space-between; 
 height: 300px;
}
.ortho {
 grid-row: 1 / span 2;
 font: 50px Ahem;
 writing-mode: vertical-rl;
 background: purple;
}
.content {
 grid-row: 3;
}
</style>
<div class=grid>
  <div class=ortho>É É ÉÉ</div>
  <div class=content>foo</div>
</div>

Today, the spec says that the orthogonal item can tell how large its containing block is (200px high, as it spans only rows with fixed max track sizing functions). The actual height of the block might not be 200px, as it depends on how much space is left over in the grid after the content-sized track is sized.

If we move the distribution up in the algorithm, then we have to weaken the condition we're using - the amount of gap, and thus the size of the grid area, is content-sized if any track in the entire grid is content-sized. Per the spec, this means we have to assume the grid area is infinite, and we'll get the original bad layout from #2409.

(On that note, we should probably make it clearer that gaps are considered in the calculation of the grid-area size, even if we leave distribution at the end.)

No, we don't change the rules for estimating the height of the rows/gaps in step 1. It still starts out assuming the fixed heights and no content distribution gaps -- but at the end of step 2, we take the actually-calculated row sizes (and alignment) into account when performing the resizing check in step 3.

@fantasai

@fantasai points out that I'm wrong - I'm talking about Step 1 in my previous comment, but row-distribution would happen in a new Step 2.5. So for the purpose of Step 1, the gap is still 0, and the item would still lay out into a fixed size of 400px.

The distribution would then likely change the min-content contribution of the item, when we hit step 3, but that will just make us cycle once, and hopefully with a closer-to-correct min/max-content contribution.

@javifernandez

@javifernandez Given the above, do you agree that it's okay to move content distribution up into new steps 1.5 and 2.5 (and eliminate step 4)? In the common case you'll get tighter, more acccurate grid sizes; in some cases (ortho flows, column-wrap flexboxes, etc) you might trigger a step-3 cycle more often than the spec currently does, but it should give a better result, too.

@javifernandez

@javifernandez Given the above, do you agree that it's okay to move content distribution up into new steps 1.5 and 2.5 (and eliminate step 4)?

@tabatkins are you talking about the original 6 steps fantasai described ?

Size the columns, making some estimates about row sizes (if needed for orthogonal flows)
Size the rows, using the column sizes we just calculated (if needed for orthogonal flows)
Size the column, using the row sizes we just calculated (skip if not needed)
Size the rows, using the column sizes we just calculated (skip if not needed)
Align the tracks
Lay out items into the resulting grid areas

Wouldn't be the step 5 the one that should be removed ? I don't follow why we should remove step 4, neither in the 6 original steps or in the new ones (reduced to 5 steps)

In any case, and with some caution until I had the time to implement a prototype for this change, I think those new steps 1.5 and 2.5 will produce better results and I'm for it. I'd like to know the @MatsPalmgren 's opinion about this, since he was the one creating the issue; besides, I'm curious about how and why he implemented it in the first place, because this is the Firefox's current behavior, right ?

@MatsPalmgren

@MatsPalmgren @javifernandez @mrego Would you mind looking this over to make sure it adequately captures what we've discussed?

The changes look good to me.

Bug 1463665 [wpt PR 11115] - [css-grid] Content Alignment as part of …

One question about the final text in the spec, we still need the last step (now number 5):

Finally, the grid container is sized using the resulting size of the grid as its content size, and the tracks are aligned within the grid container according to the align-content and justify-content properties.

If content alignment was already done as part of the previous steps, why we still need it?

@mrego

@mrego The previous steps calculate the effective column/row gaps taking content alignment into account, but they don't actually apply alignment e.g. centering to the tracks.

(Of course if in your implementation you've worked this into your algorithm earlier, you don't need to do it at this point, but it's generally true that CSS implementations don't need to follow the steps in the spec so long as they return the expected output.)

CSS grid elements with justify-content: space-around have extra white…

Bug 1463665 [wpt PR 11115] - [css-grid] Content Alignment as part of …

Navigation Menu

Repository navigation

[css-grid] Applying 'justify-content' content distribution is in the wrong place in the overall grid sizing algo #2557

24 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[css-grid] Applying 'justify-content' content distribution is in the wrong place in the overall grid sizing algo #2557

Description

Activity

mrego commented on Apr 12, 2018

fantasai commented on Apr 20, 2018

mrego commented on Apr 23, 2018

fantasai commented on May 1, 2018

javifernandez commented on May 1, 2018

fantasai commented on May 4, 2018

javifernandez commented on May 4, 2018

tabatkins commented on May 17, 2018

fantasai commented on May 17, 2018

tabatkins commented on May 17, 2018

tabatkins commented on May 17, 2018

javifernandez commented on May 21, 2018

24 remaining items

javifernandez commented on Jun 14, 2018

mrego commented on Aug 3, 2018

fantasai commented on Aug 24, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions