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] Grid track sizing items spanning a flexible track #2177

Closed
mrego opened this Issue Jan 9, 2018 · 16 comments

Comments

Projects
None yet
7 participants
@mrego
Member

mrego commented Jan 9, 2018

I've been checking the following example of CSS Grid Layout:

<div style="width: 50px; height: 50px; border: solid thick;
            display: grid; grid-template-columns: auto 1fr;">
  <div style="background: magenta; width: 100px;
              grid-column: span 2;">item</div>
</div>

The grid container has a fixed with of 50px and has 2 columns "auto 1fr".
The grid has only one item with a fixed width of 100px which is spanning both columns.
The result in all the browsers is that the columns are "0px 50px" so the item overflows the columns.

The reason is the following text in the spec:

consider the items with a span of 2 that do not span a track with a flexible sizing function

Which causes that the item is ignored for resolving intrinsic track sizes step.

This is somehow unexpected to me as the auto column is expected to be big enough to avoid overflow due to its contents.
If you have only 1 column or 2 auto columns, then there's no overflow regarding the columns sizes (of course there's going to be always overflow regarding the grid container size as it's fixed to 50px while the item is 100px).

I created a codepen with several examples that has the same output in all the browsers: https://codepen.io/mrego/pen/vpdybY

Output of the codepen examples

Is this the expected behavior or not? What's the rationale behind hat sentence on the spec? Thanks!

@mrego mrego added the css-grid-1 label Jan 9, 2018

@fantasai

This comment has been minimized.

Contributor

fantasai commented Jan 15, 2018

That clause dates from the original Microsoft algorithm, see https://www.w3.org/TR/2014/WD-css-grid-1-20140513/#translated-algo

I think the reason it was included had something to do with wanting the flexible track to accommodate the spanning item, rather than making any adjacent auto tracks accommodate it: this makes the auto tracks more tightly wrapped around their contents while the flexible track grows. I'm not sure it's working as intended though. :/

@tabatkins

This comment has been minimized.

Member

tabatkins commented Feb 14, 2018

So the confusion is between the results of case 2 (the fr column grows bigger than the container) and cases 4/5 (the fr column won't exceed the container).

This seems like an accidental result of the change we made to have 1fr imply an auto minimum - this has the desired effect in case 2 (the fr grows larger to accommodate the item's minimum size), but then the algorithm itself skips over the track when the item spans, so we don't get the opportunity to have the same effect.

Hmm, we don't want to lose the case 2 behavior, so we'll have to look into this and see if there's a reasonable way to make case 4/5 match expectations, without losing the intent of the original MS algo (make auto tracks tightly wrap and let fr tracks take up the slack).

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 14, 2018

OK, Tab and I looked over this issue. As far as we can tell, there are two principles here that we want to follow:

  • Flexible tracks absorb spanning items' content, leaving auto tracks tightly-wrapped around their contents insofar as possible. (See #2177 (comment))
  • Content-sized tracks such as auto tracks and flexible tracks with a minimum (e.g. minmax(min-content, 1fr) honor their minimum and ensure that no item spanning them causes overflow.

The current algorithm is failing at #2 because of its heuristic for #1. Consider a spanning item of 100px, a 50px-sized grid, and:

  • Track sizing of minmax(auto, 1fr) minmax(max-content, 2fr)
  • Track sizing of auto minmax(10px, 1fr)

In the first case, we're completely ignoring the minimums. In the second case, the first track sizes at 0px because it skips the spanning item, and the second track maxes out at 50px, causing the item to overflow the tracks.

We're not sure if there's a good way to fix this while adhering to both principles, since it seems to require resolving flexible tracks and then cycling back to re-resolve intrinsic track sizes. We'd like to hear back from the Microsoft folks who developed the algorithm on what they think about this situation. @atanassov?

@tobireif

This comment has been minimized.

tobireif commented Mar 7, 2018

It seems I'm affected by this [1], so I hope that @atanassov et al can resolve this 😀(and that everything will be consistent in browsers soon, eg via test-cases & browser-tickets)

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=817329
(Mainly: I want the text to fill the whole available width.)

@atanassov

This comment has been minimized.

Collaborator

atanassov commented Mar 27, 2018

Some of the original reasoning for the behavior (at least looking through some old test cases) suggests this was intended for scenarios where a spanning header was supposed to influence the size of columns in an article. I don't believe we have a strong reason and/or content that depends on this behavior at this point so we could go either way.

Assuming we want to keep the exception, it'd be good to explore tweaking the heuristic with something like:

  • Spanners that cross tracks that have content-based mins AND flexible maxes only contribute content sizes to those tracks; otherwise they participate normally.

Since flexible tracks now have auto minimums by default, this will ensure that spanners crossing flexible tracks only contribute to non-flexible tracks if all the flexible tracks have explicit fixed mins. It seems like that embodies the spirit of the original heuristic (spanners typically rely on flexible tracks to fit) while also satisfying the requirement that items crossing content-sized tracks shouldn’t overflow their tracks.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Apr 10, 2018

The Working Group just discussed Grid track sizing items spanning a flexible track, and agreed to the following resolutions:

  • RESOLVED: edit in Rossen proposal in https://github.com/w3c/csswg-drafts/issues/2177
The full IRC log of that discussion <dael> Topic: Grid track sizing items spanning a flexible track
<dael> github: https://github.com//issues/2177
<dael> rego: Quite related, You have an item that spans 2 columns. One of them is outside it's flexible track and we weren't sure what would be the best result. We're not sure what's the core approach. Rossen was tyring to clarify in the last comment.
<dael> astearns: There's a specific tweak to the algo you propose?
<dael> rego: Original algo was aligned that you don't expand a track with a flexible track sizing. Maybe we can open the use cases.
<dael> fantasai: We should try Rossen suggestion in the issue and see if it works. I haven't fully through it through but that's what I'd want to try.
<dael> Rossen: When we looked we played with magizine layouts. One recurring thing was when you have a splash page when an item that spans x columns and want to influence rest of layout. Those columns need to be flexible when you're on a browser. That was the original motivation.
<dael> Rossen: Since then in the issue...we can go either way. I don't think there's enough concept to support one or the other. What I proposed should work.
<dael> fantasai: Makes sense to me.
<dael> astearns: Do we need a resolution to edit in Rossen proposal?
<dael> fantasai: Resolution to add Rossen proposal and we'll revisit
<fantasai> s/revisit/revisit if there's a problem/
<dael> RESOLVED: edit in Rossen proposal in https://github.com//issues/2177

fantasai added a commit that referenced this issue Apr 20, 2018

@fantasai

This comment has been minimized.

Contributor

fantasai commented Apr 20, 2018

OK, Tab and I edited in some changes to address this case. The changes are a little more complicated than what Rossen outlined, because we realized we needed to intelligently handle the case of the item spanning multiple tracks sized with fr units. :)

@MatsPalmgren @mrego @atanassov Please review the changes and let us know if they seem OK? 25e3f63 Thanks~

@svillar

This comment has been minimized.

svillar commented Apr 23, 2018

I might have missunderstood the proposed change but I think something like that does not work at least for three reasons. I'll use an example to illustrate what I mean:

<div style="display:grid; grid-template-columns: 1fr auto 2fr">
     <div style="width: 50px; grid-column: 2/4;"></div>
     <div style="width: 100px; grid-column: 1/3;"></div>
     <div style="width: 90px; grid-column: 1/4;"></div>
</div>

(I don't specify a width for the grid container. Let's just assume that we have exactly the amount of free space required by the examples not to enlarge tracks in "Expand flexible tracks" step)

If we apply a change like the one proposing we'd be breaking at least 2 principles & 1 heuristic of grid:

  • Order independence
  • fr tracks must retain their respective proportions
  • Tightly-wrapped tracks

In this example, procesing the items in this source code order will generate the following result

  1. the 50px item sizes the tracks to [0 0 50]
  2. the 100px item sizes them to [100 0 50]
  3. the 90px one fits and there is no need to enlarge tracks

Instead should we process the third item in the first place we'd have:

  1. the 90px item sizes the tracks to [30 0 60]
  2. the 50px item fits in [30 0 60]
  3. the 100px one sizes the tracks to [100 0 60]

In none of the two cases the grid is tightly packed. Agreed that's a desirable effect from the heuristics but not a requirement. However as you can see, relative proportions of fr tracks are not preserved and order independence is also broken.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Apr 30, 2018

@svillar I think you're missing one of two things: 1) The space distributing algorithm maintains the planned increase separately from the track sizes subtracted out to find the space to distribute exactly so that we don't have this order dependence. 2) The step we added invokes space distribution for all items spanning flexible tracks at once (“together, rather than grouped by span size”) so that the planned increase isn't cleared out between them--i.e. items spanning flexible tracks aren't ordered by span size, as other items are.

So the processing of items will generate the following result:

  1. The 50px item folds into the planned increase -> [0 0 50].
  2. The 100px item folds into the planned increase -> [100 0 50].
  3. The 90px item folds into the planned increase -> [100 0 60].
  4. The planned increase is added to the track sizes (currently [0 0 0]) yielding [100 0 60] at the end of intrinsic track sizing.
  5. If there is space available afterwards, the Expand Flexible Tracks step inflates the 60px track to match the ratios, yielding [100 0 200] (or as close to it as it can get given the limits of available space).

The algorithm preserves order independence. It doesn't maintain the fr ratios when there isn't space to do so, this is intentional. It isn't tightly packed always, as in this example--in order that simple cases with spanning item being the largest maintain their ratios insofar as possible in the available space.

Does this clear things up?

@fantasai fantasai added the Agenda+ label Apr 30, 2018

@fantasai

This comment has been minimized.

Contributor

fantasai commented Apr 30, 2018

(Agenda+ to request WG review of changes.)

@svillar

This comment has been minimized.

svillar commented May 2, 2018

Ah OK, then I had clearly missunderstood the changes.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented May 2, 2018

The Working Group just discussed Grid track sizing items spanning a flexible track, and agreed to the following resolutions:

  • RESOLVED: Grid items spanning fr tracks do contribute to fr resolution for intrinsic sizes with the addition that fr tracks not 1 to 1 ratio get contributed to based on that ratio.
The full IRC log of that discussion <dael> Topic: Grid track sizing items spanning a flexible track
<dael> github: https://github.com//issues/2177
<dael> fantasai: This was a case where when MS designed algo if you had an item that spanned an auto track and a flexible track the space rewuired to fir the item would go intot he flexible track. this is frequently used in design where the auto track is tight to the content only in it.
<dael> fantasai: We inherited this in algo. When you shrinkwrap the grid we don't have a point at which we consider the size of the spanning item. Flexible track goes to 0 and the other track doesn't expand so it overflows.
<dael> TabAtkins: Manuel's picture in the first message shows the problem. #1, 2, and 3 thegrid expands to the full width of the item. But in the 2fr columns they don't. In section 5 is the problem. 1 auto and 1 fr. a single auto or 2 autos exapnd correctly, but because the interaction of auto and fr you don't get the good behavior.
<dael> fantasai: Rossen_ explained high level how to incorporate this into algo. TabAtkins and I tried to do that, but we realized you might have multi fr tracks or a different ratio. We added a new section and changed how space is distributed to try and keep the ratio as far as we can. There are conflicting requirements like try and shrink as small as you can. You don't always have tightest or idea flex, but you gete close.
<dael> fantasai: We'd like WG review on if this is the right way or if there's other ideas on how to do this.
<dael> Rossen_: What you desc for multi fr track with a different ratio then 1 to 1 your proposal is reasonable and what I'd expect. From that PoV I'm fine with the proposal. I haven't done a full review, but I won't block because I trust you did the changes you outlined and it sounds good to me.
<dael> Rossen_: Is this agenda+ to elevate awareness or do you want to resolve?
<dael> fantasai: If everyone who wants to look has let's resolve but if anyone wants time to review that's fine.
<dael> Rossen_: Does anyone want more time to review? I personally don't.
<dael> Rossen_: Then can we resolve on the current proposal? Grid items spanning fr tracks do contribute to fr resolution for intrinisic sizes with the addition that fr tracks not 1 to 1 ratio get contributed to based on that ratio.
<dael> Rossen_: fantasai if you've got better wording.
<dael> fantasai: looks good to me.
<dael> Rossen_: Opinions or objections?
<dael> RESOLVED: Grid items spanning fr tracks do contribute to fr resolution for intrinsic sizes with the addition that fr tracks not 1 to 1 ratio get contributed to based on that ratio.
@fantasai

This comment has been minimized.

Contributor

fantasai commented May 4, 2018

OK @svillar :) Let me know if the spec needs clarification, though.

@fantasai fantasai closed this May 4, 2018

fergald added a commit to fergald/csswg-drafts that referenced this issue May 7, 2018

@mrego

This comment has been minimized.

Member

mrego commented Jun 8, 2018

I've a question about this change, on a previous comment you said:

  • Content-sized tracks such as auto tracks and flexible tracks with a minimum (e.g. minmax(min-content, 1fr) honor their minimum and ensure that no item spanning them causes overflow.

But the change affects also non-spanning items, see https://drafts.csswg.org/css-grid/#algo-single-span-items:

  1. Size tracks to fit non-spanning items: For each track with an intrinsic track sizing function and not a flexible sizing function, ...

Now it says "and not a flexible sizing function" so if we have a non-spanning item on a flexible track. The item could end up overflowing the track as their contents would be ignored.
Check the discussion in the following bug for an example:
https://bugzilla.mozilla.org/show_bug.cgi?id=1381512#c6

Do we really want this behavior for non-spanning items?

@mrego mrego reopened this Jun 8, 2018

@fantasai

This comment has been minimized.

Contributor

fantasai commented Jun 12, 2018

@mrego Yes, those are now handled together with other items that span flexible tracks in https://drafts.csswg.org/css-grid-1/Overview.html#algo-spanning-flex-items

I think this makes sense? Let me know if it doesn't make sense. :) Certainly it shouldn't be overflowing.

@mrego

This comment has been minimized.

Member

mrego commented Jun 13, 2018

Yes, I think that makes sense. I missed that part, that would avoid overflow in that case. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment