w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 660 forks source link

[css-grid-1] Track Sizing Algorithm question #2873

Open tabatkins opened 6 years ago

tabatkins commented 6 years ago

Issue copied from original email from Jason.W He (hekylinwork@gmail.com):


In case of following grid.

--- 65% -- | -- 35% -- | min-content --- 65% -- | -- 35% -- | auto --- 65% -- | -- 35% -- |

Two column: 65%, 35% Two row: min-content, auto

template-areas: "head side" "body side"

For row #1's track size, which it use min-content to define height of that row. When calculate height, why is row #1 been calculated using items with span of 2?

From spec, it consider items with span of 1 first, then consider at span of 2 that do not span a track with a flexible sizing function. Which the second row auto get caught.

When change second row's auto to 1fr (flexible sizing function), it now has correct behaviour.

I have sample to reproduce this under https://codepen.io/Nness/pen/MrYMzQ?editors=1100#

When increment items inside area side. The area head's height now resize according to side.

Is this logical and correct behaviour to increase size of single span item by multiple span item?

It make sense to increase total size of multi-span grid track by item span across multiple track. How is that logical to change size of single-span grid track by item span across multiple track?

All the browser I have tested have exactly same result, does that means they all according to spec and have correct behaviour?

If use flexible sizing function is the only way to get this working and it is according to spec, then in which situation we want to use auto to have multiple span item change single span track's height behaviour?

svillar commented 6 years ago

From spec, it consider items with span of 1 first, then consider at span of 2 that do not span a track with a flexible sizing function. Which the second row auto get caught.

This is not correct. The second row is auto sized. Flexible sized tracks are those using fr. See https://www.w3.org/TR/css-grid-1/#layout-algorithm

When change second row's auto to 1fr (flexible sizing function), it now has correct behaviour.

Not sure what you mean with "correct" but I guess I've explained it in the previous comment.

Nness commented 6 years ago

This is correct behaviour. Which second row is 1fr 2018-07-05_08h04_52

This is incorrect behaviour. Second row is auto 2018-07-05_08h05_07

The question is, is second one acceptable result? If it is according to spec, then what kind of scenario it trying to cover? Why do we want content inside item that span multiple tracks to increase height of single track item?

This is not related to total size, I mean individual size. You can check on sample: https://codepen.io/Nness/pen/MrYMzQ?editors=1100#

Then add more <div>push</div> into <div class="item-side">

fantasai commented 6 years ago

I think it makes sense to fix this--to make min-content fit tighter than auto/max-content when possible. We should draft up a change proposal for it for review... @svillar What do you think?

tabatkins commented 1 year ago

Flagging for f2f. We think the author expectation of this case is fairly clear and the spec would be reasonably straightforward to fix, but at this point the compat impact is very worrying and requires analysis.

Loirooriol commented 1 year ago

The problem seems to be in this line: https://github.com/w3c/csswg-drafts/blob/0dcb036011e9ff870bc3f5de03d53210749ca0b6/css-grid-1/Overview.bs#L4425 It treats all intrinsic max track sizing functions equally instead of prioritizing min-content vs auto/max-content.

css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [css-grid-1] Track Sizing Algorithm question, and agreed to the following:

The full IRC log of that discussion <fantasai> https://github.com/w3c/csswg-drafts/issues/2873#issuecomment-402552212
<emilio> fantasai: so we got an issue with a test-case with two columns, and two rows, one min-content, one auto
<emilio> ... the results that they want is the first result, which makes sense
<emilio> ... the one they get is the second
<emilio> ... the q for the working group is do we think we can fix it and do we want to?
<emilio> ... there's a compatibility concern
<emilio> q+
<emilio> fantasai: I can explain the result
<emilio> ... the first step is satisfying the minimum
<emilio> ... so min-content expands to minmax(min-content, min-content), auto expands to minmax(min-content, max-content)
<emilio> ... there's extra magic but that's roughly what happens
<emilio> ... so in the first pass we look at the items with span of 1 and the first row becomes 1 em tall, the second becomes 3em tall
<emilio> ... then we look at the spanning item
<emilio> ... and that's 10em, and since both have a min of min-content I'll distribute the extra space equally into two
<emilio> ... so the change we'd have to make is if you have two columns that are min-content but one has a max-content maximum we prefer distributing into it
<emilio> ... that's the technical direction, the question would be do we want to
<astearns> ack emilio
<TabAtkins> fantasai: Currently we grow all tracks as needed fo rmin-content sizes, then another for ....
<TabAtkins> fantasai: we'd probably put a phase between min-content and max-content
<TabAtkins> fantasai: so do we want to pursue this or does it seem unreasoanble
<emilio> dholbert: is max-content special here or would a 300px be treated similarly?
<emilio> ... same question for min-content
<florian> q+
<emilio> fantasai: when we discussed this it seemed reasonable to try to fix, author expectation seems to make more sense
<emilio> ... main q is is it web compatible
<astearns> ack florian
<emilio> florian: I think we should try, accounting with dholbert's nuanced
<emilio> ... also cross-checking with the spanning issue discussed earlier
<emilio> ... but in terms of exploring yeah
<emilio> iank_: my gut feeling is that it's hard to check whether this is web compatible, a bit of a webcompat black box
<emilio> fantasai: my guess is that authors that hit this change min-height or use fr
<emilio> iank_: yeah for this case it's clear but for other cases not so more
<emilio> astearns: iank_, are you saying this is kind of a blackbox because we don't have a solution?
<emilio> iank_: no, I just not have a good sense of how web compatible this would be
<dholbert> emilio: this seems like the sort of thing you need to try and see if it impacts the rendering/webcompat
<fantasai> emilio: and just see if you get bugs
<dholbert> emilio: I don't want to argue against it, but as dholbert noted, I'm not sure min-content / max-content are special
<dholbert> emilio: maybe we want to generically distribute space to tracks with larger maximums
<emilio> fantasai: I'm not sure doing that would be compatible, we distribute evenly in a bunch of cases
<emilio> ... min-content is special in the sense that is specifies a desire of making it as compact as possible
<emilio> dholbert: not so sure, it's more about not wanting content clipped
<emilio> astearns: there are ways to fixing things to get what you want and by not doing anything we don't risk anything
<emilio> fantasai: the workaround has some side effects that might not be totally desirable
<emilio> astearns: so... strawpoll?
<emilio> (1) do nothing, (2) try to figure out a solution for this
<florian> 2
<astearns> 2
<rachelandrew> 2
<schenney> 2
<argyle> 2
<Sammy_Gill> 2
<bramus> 2
<miriam> 2
<TabAtkins> abstain
<oriol> 2, but not sure if possible due to compat
<SebastianZ> 2
<emilio> 0
<dholbert> 2 (weak preference)
<iank_> 0
<fantasai> personal preference for 2
<stewart> 2
<emilio> RESOLVED: Keep working on a solution for this and take it back to the group
<emilio> <br>
tabatkins commented 1 year ago

Okay, explaining the current behavior took some doing. Here's a testcase showing it more simply:

<!doctype html>
<div class="container">
  <div class="item-head">
    <div class=block style="height: 20px;"></div>
  </div>
  <div class="item-body">
    <div class=block style="height: 80px;"></div>
  </div>
  <div class="item-side">
    <div class=block style="height: 150px"></div>
  </div>
</div>
<style>
.container {
  background-color: #fefefe;
  display: grid;
  grid-template-columns: 150px auto;
  grid-template-rows: min-content auto;
  grid-template-areas: 
    "head side"
    "body side";
}
.item-head {
  background-color: #d4edda;
  grid-area: head;
}
.item-body {
  background-color: #fff3cd;
  grid-area: body;
}
.item-side {
  background-color: #cce5ff;
  grid-area: side;
  overflow:hidden;
}
.block {
  width: 100px;
  background: #0002;
}
</style>

As you can see, the blue spanning item gives all of its content's height to the "min-content" track, and none to the "auto" track. This is the opposite of what the OP asks for, and is confusing to boot!

This happens because the blue item is scrollable (via overflow:hidden). This means it doesn't get an automatic minimum size. In section 11.5, step 3.1, we distribute zero space, as that's the element's "minimum size". Then in step 3.2 we distribute min-content height, which is 150px, to the min-content track only - 150px to distribute, minus 100px of space already present in the track base sizes, means the "min-content" track grows by 50px. Then we're done, all items are considered and all space is distributed.

If the blue item is not scrollable, it has an auto min size of its min-content height, and so step 3.1 distributes that space to both tracks equally. We might still want to prioritize the auto track over the min-content track in this case. But it's not the problem OP is running into.

To address OP's problem, we need to address the fact that the auto track, which has absorbed the spanning item's minimum contribution (0px), isn't absorbing any of the spanning item's min-content contribution (150px). But it's not clear how to do that in the algorithm without breaking other expectations.

fantasai commented 1 year ago

Note: The desired behavior is likely more closely represented (in the current algorithm) by this:

  grid-template-rows: minmax(auto, min-content) auto;

instead of

  grid-template-rows: min-content auto;

Still leaves open the idea of prioritizing auto/max-content tracks over min-content, though, to get it to size tightly around the min-content track rather than distributing the spanner equally.

tabatkins commented 1 year ago
Notes for future Tab and fantasai

In the spirit of fr tracks, let spanned auto maximum tracks absorb space over intrinsic minimums?

Possible Idea:

  • address all items not spanning fr or auto
  • address all items spanning auto
  • address all items spanning fr

Other Possible idea

  • in distribute across spanning tracks, split "distribute up to limits" into two phases, preferentially distributing into auto tracks
  • maybe only do that for step 3.2 specifically, under some restrictive conditions?
Loirooriol commented 1 year ago

I also analyzed the testcase with more detail after the meeting, but forgot to post my thoughts.

Basically, some confusing behavior of grid is that auto has completely opposite meanings when used as a min track sizing function vs a max track sizing function.

The example here uses min-content, auto, i.e. mimax(min-content, min-content), minmax(auto, auto). This is a somewhat contradictory input: the 1st track is requested to grow more as a minimum, but grow less a maximum. In CSS, minimums win, so that's why basically all of the contribution of the spanning item goes into the min-content track.

As Tab says, the effects vary a bit depending on the automatic minimum size. Removing the overflow distributes the size as a minimum contribution, so both rows get half of it since both are intrinsic. With overflow, the minimum contribution is 0, so the size is distributed as a min-content contribution, and it goes to the 1st track.

I don't think it's possible at this point to change the above. As fantasai says, to get the desired behavior, authors should use a different grid-template-rows, weakening the min track sizing function of the 1st track, and boosting the min track sizing function of the 2nd track.

grid-template-rows: minmax(auto, min-content) minmax(min-content, auto);

That does the trick when the minimum contribution is 0, but otherwise the size will be distributed equally, just like with

grid-template-rows: min-content minmax(min-content, auto);
grid-template-rows: minmax(auto, min-content) auto;

And it's at this point where I think we can improve the situation by doing what I said in https://github.com/w3c/csswg-drafts/issues/2873#issuecomment-1641000730

The growth limits have been set tight to the size of the non-spanning items, so if we prioritize respecting the limit of a track with a min-content max track sizing function over respecting the limit of a track with a max-content or auto max track sizing function, the extra contribution of the spanning item will go into the 2nd track.

css-meeting-bot commented 8 months ago

The CSS Working Group just discussed [css-grid-1] Track Sizing Algorithm question, and agreed to the following:

The full IRC log of that discussion <emilio> oriol: this was about a test-case with 2 tracks, sized min-content and auto
<emilio> ... resulting behavior was not what the author expected
<emilio> ... discussed in july and resolved to try improving it
<emilio> ... after a detailed analysis my understanding is that there were two problems
<emilio> ... one was the author's fault
<emilio> ... auto track sizing had different meanings if used as a max or min
<emilio> ... so we had a smaller minimum, so I think the author should fix the minimums using minmax
<emilio> ... this will provide the expected behavior on some cases
<emilio> ... but not others. To address those others is that when we're distributing, right now we try to respect the growth limit, but if all tracks have reached the limit then we distribute between tracks that have intrinsic sizes
<emilio> ... but we don't differentiate between min and max-content
<emilio> ... so to fix this the proposal is to first try to distribute first to the max-content max-sizing functions, then min-content
<emilio> ... fit-content() is a bit trickier
<emilio> ... will initially behave as max-content, and at some point it'd be fixed so it'd use the min-content rules
<emilio> ... not sure if there's websites that would break
<emilio> ... would improve the test-cases here and it seems reasonable
<emilio> dholbert: in the block axis min/max-content are the same, does it make sense to differentiate between them?
<emilio> fantasai: min-content / max-content contributions are the same but the sizing keyword is different so we could give them different behavior
<emilio> oriol: those are keywords for track-sizing which aren't the same as css-sizing
<emilio> emilio: so this would still apply in the block axis
<emilio> oriol: yeah this doesn't depend on whether the contributions are the same, just about prioritizing
<emilio> ... height: min/max-content behaves the same as auto, which is true in implementations but not in the spec for grid/flex at least
<emilio> dholbert: [quotes spec]
<emilio> ... but yeah sounds fine
<emilio> astearns: concerns? oriol can you summarize the proposal?
<emilio> oriol: when distributing past growth limits, we distribute prioritizing tracks that have max-content max-track-sizing function
<emilio> RESOLVED: when distributing past growth limits, we distribute prioritizing tracks that have max-content max-track-sizing function