Skip to content
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

Jump to bottom

[css-grid] fit-content() vs 'stretch' alignment #1732

Closed
fantasai opened this issue Aug 17, 2017 · 9 comments
Closed

[css-grid] fit-content() vs 'stretch' alignment #1732

fantasai opened this issue Aug 17, 2017 · 9 comments
Labels
Closed Accepted by CSSWG Resolution css-grid-1 Tracked in DoC
Milestone
css-grid-1 CR 201...

Comments

@fantasai
Copy link
Collaborator

@rachelandrew and @mrego found some problems with the spec for fit-content() and its interaction with stretch content alignment of the tracks:

So I thought this was a clear issue and I've already a patch for it. But now reading the specs I'm not 100% sure, so let's clarify this before landing the patch.
The relevant sections of the spec are:

The max track sizing function for the fit-content() tracks is "max(auto, argument)".
In the case "auto" is the maximum should it be considered an "auto max track sizing function" or not?

Would be ok to avoid stretching in all cases? Should we stretch any of them?

IMHO, I believe that we should avoid stretching for fit-content(). That would be simpler to implement too. But I'd love to hear the feedback from Rachel and Tab on the topic. Thanks!

When we wrote the spec for fit-content(), the auto in the formula was intended as a replacement for the min-content argument of the shrink-to-fit formula, as is made clear in the parenthetical “(i.e. minmax(auto, max-content))”. Removing the word auto and substituting the parenthetical in its place makes the intended behavior much clearer.

However, there is a remaining question: should that max-content argument actually be auto, i.e. respond to stretching (which is the only difference between auto maximums and max-content maximums)? It would still be clamped by the argument as Rachel requests, since once the track hits that limit it should be treated as a fixed-size track:

For this purpose, ''fit-content()'' tracks are treated as ''max-content'' until they reach the limit specified as the ''fit-content()'' argument, after which they are treated as having a fixed sizing function of that argument.

I'm leaning towards yes, since having fit-content() behave exactly like auto aside from clamping at its argument seems like a useful and friendly behavior, but other thoughts welcome~
@mrego
Copy link
Member

mrego commented Aug 18, 2017

If I'm understanding your proposal, you mean that the fit-content() tracks will grow up to the fit-content() argument.
So for example a track with fit-content(200px) if it has a content of 50px, it'll be stretched to 200px (if the available space is enough) but not larger than that.

Probably that can make sense from the authors point of view, but it'll make more complex the implementation of the stretch phase of the algorithm.
For example if we have a grid with 1200px width and these columns: fit-content(200px) auto fit-content(450px). Imagine they've no contents yet so with justify-content: start all of them are 0px.
I guess we'd need to do something like this to stretch them:

  1. We've 1200px available so we divide them between the 3 columns: 400px each.
    1st column can only grow up to 200px. 2nd and 3rd columns grow to 400px.
    At this point we've: 200px 400px 400px.
  2. We still have 200px available, we divide now only between the 2 columns that still can grow: 100px each.
    2nd column grows to 500px, but 3rd column can only grow up to 450px.
    Now we've: 200px 500px 450px.
  3. At the end we still have 50px available, and only 2nd column can grow (the other two have already reached their limit).
    2nd column grows to 550px.
    The final result would be 200px 550px 450px.

Probably we'd need to specify how this will work in 11.8. Stretch auto Tracks.

BTW, this is an editorial nit, but shouldn't the 11.8 section mention that tracks only stretch if justify|align-content is stretch. That's somehow mentioned in the summary of the track sizing algorithm:

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.

@svillar
Copy link

svillar commented Aug 18, 2017

I'm leaning towards yes, since having fit-content() behave exactly like auto aside from clamping at its argument seems like a useful and friendly behavior, but other thoughts welcome~

That's was my first thought when @mrego told me about the issue. Whoever (now with my implementor hat on) there are edge cases. For example, what about a fit-content(100px) track hosting a 200px item? It'll be sized to 200px, so we have already surpased the limit, would it make sense not to stretch it? I don't know 😄 I guess we need the opinion of the authors here to know what do they expect.

From the implementation POV I think either solution would be fine anyway.

@javifernandez
Copy link
Contributor

With this proposal contradicts, if we don't add additional info, what the Alignment spec states about the 'stretch' value:

If the combined size of the alignment subjects is less than the size of the alignment container, any auto-sized alignment subjects have their size increased equally (not proportionally), while still respecting the constraints imposed by max-height/max-width (or equivalent functionality), so that the combined size exactly fills the alignment container.

There are different cases:
1- not possible to use all the available space
2- tracks grow not equally

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed fit-content() vs 'stretch' alignment.

The full IRC log of that discussion <dael> Topic: fit-content() vs 'stretch' alignment
<dael> github topic: https://github.com//issues/1732
<dael> fantasai: rachelandrew and rego found spec issues about stretch and fit-content and stretch will stretch past the specified limit and that doesn't make sense.
<rachelandrew> the interop issue is described here https://github.com/rachelandrew/gridbugs#14-fit-content-is-stretching
<dael> fantasai: There's 2 options. 1) only have stretch stretch auto tracks but not fit-content. 2) Have stretch work on fit-content up to the limit.
<dael> fantasai: 2 is more complex to impl but may be more expected.
<dael> Bert: I'm having trouble visualizing. Other people understand?
<dael> fantasai: I'm happy to defer if people want more time.
<dael> gregwhitworth: I prefer to wait. I didn't review in depth.
<dael> Bert: How much time?
<dael> gregwhitworth: Just a week. I want to run Rego's comment on impl past our devs.
<dael> fantasai: There's also two options for if we do stretch...do we stretch other tracks once these have reached their limit or just say nope.
<dael> jensimmons: From author side havign a track with fit-content grow and shrink would be expected. And I think that once the fit-content limit is hit having additional space appear get distributed through the rest of the grid would also make sense, not have the grid stop growing.
<dael> jensimmons: I don't know the implication for impl. Super quick issue reading from rachelandrew's github of grid bugs it looks like that's what other authors expect. Correct me if I'm wrong rachelandrew
<dael> rachelandrew: I thinkt he author that raised it thought stretching was correct, but I found authors assume Chrome is correct. They tend to assume the browser they user the most is correct so I take it with a pinch of salt.
<dael> Bert: So if we come back next week that's enough time to investigate?
<dael> jensimmons: Is what I tried to articulate what FF does?
<dael> rachelandrew: FF doesn't stretch. If you align to start you get the same with everything. Chrome is stretching.
<dael> jensimmons: It would be nice...Oh there's a demo here.
<dael> jensimmons: I'm fine punting to next week.
<dael> fantasai: If people have opinions or thoughts please add comments to the issue.
<dael> Bert: So there are impl difference so at least one will have to change.
<dael> Bert: Please add your thoughts to the issue and we'll come back next week.
<gregwhitworth> Edge doesn't stretch either in the case provided

@css-meeting-bot
Copy link
Member

The Working Group just discussed fit-content() vs 'stretch' alignment.

The full IRC log of that discussion <dael> Topic: fit-content() vs 'stretch' alignment
<dael> github: https://github.com//issues/1732
<dael> astearns: I'm not sure what's left. Can anyone summerize?
<tantek> looks like we discussed it last week
<dael> TabAtkins: No one removed the agenda+

@astearns astearns removed the Agenda+ label Aug 30, 2017
@mrego
Copy link
Member

mrego commented Sep 12, 2017

Could you please re-discuss this topic and make a resolution?

  • 23 August you talked about 1 week to check the examples and define the expected behavior.
  • 30 August you didn't discuss further.

This is not something big but it's currently an interop issue, so it'd be nice to know what's the expected behavior to update implementations.
Thanks!

@rachelandrew
Copy link
Contributor

I've shown fit-content now to a few people and asked opinions. People generally expect it not to grow past the clamping, so my opinion is that for author friendliness the value passed in should be a max, and other tracks should stretch.

@css-meeting-bot
Copy link
Member

The Working Group just discussed fit-content() vs 'stretch' alignment, and agreed to the following resolutions:

  • RESOLVED: fit-content does not grow past its arguement due to alignment stretch
  • RESOLVED: keep fit-content behavior as-is, to not grow past max-content in presense of stretch
The full IRC log of that discussion <dael> Topic: fit-content() vs 'stretch' alignment
<dael> github: https://github.com//issues/1732
<dael> Rossen_: fantasai can you take this?
<dael> fantasai: I haven't looked lately. Looking now.
<dael> Rossen_: I think you brought this up 3 weeks ago.
<dael> fantasai: I think in order to make progress on writing modes we should do florian's extra.
<dael> Rossen_: Grid issue you want to discuss on GH?
<fantasai> https://github.com//issues/1732
<dael> fantasai: no...it's okay.
<dael> fantasai: Issue was we have a stretch value for alignt and justify content. These align and justify grid tracks from a high level. One of the options is strech, kinda like flex lines. If you choose stretch we do so by distributing space to auto-size columsn euqually. No auto sized columns, nothing happens.
<dael> fantasai: Q is if it should apply to fit-content or not. Looks like rachelandrew asked a few authors and they expect it not to go past argumenet of fit content, but there was a question of should we apply the extra space at all.
<dael> fantasai: WE can say fit content doesn't go past its argumenet due to stretching.
<dael> Rossen_: I'm in favor as an impl.
<fantasai> s/WE/For sure I think we/
<dael> Rossen_: Obj to resolving that fit-content does not grow past its arguement due to alignment stretch.
<dael> RESOLVED: fit-content does not grow past its arguement due to alignment stretch
<rachelandrew> this was the issue as it came up https://github.com/rachelandrew/gridbugs/issues/9
<dael> fantasai: Second is should fit-content grow past it's max content...if we're below the argument, the clamping one, do we grow fit-content it the stretch is set.
<dael> fantasai: For that question, I don't know. We don't seem to have any feedback one way or another of what should happen there.
<dael> jensimmons: Alternately the content would not stretch?
<dael> fantasai: If there's auto tracks there, if not start.
<dael> jensimmons: And they could adjust by not using stretch?
<dael> fantasai: Yes, case where it would make a difference is if there's fit-content and an auto column. They have very similar behavior except fit-content has additional clamp.
<dael> Rossen_: This scenario was a canonical example we had originally when working on grid. The ability to have a menu system based on size of items inside and let the extra space go into the rest of the content area. Using fit-content with auto columns or fr columns was the way to achieve this.
<dael> Rossen_: If you take away the guar. of fit-content that's not just for grid and we start changing what fit-content means then this is a pretty, in my opinion, radiacal change.
<dael> Rossen_: I haven't seen any really strong cases to change that and I would prefer if we don't (as an impl)
<dael> jensimmons: You want to stretch beyond max-content?
<dael> Rossen_: I don't.
<dael> fantasai: Auto will go beyond max-content, but fit-content won't.
<dael> Rossen_: fit-content was designed to fit content
<dael> rachelandrew: I'd agree with Rossen_ I don't think people would expect it to stretch. I'd prefer fit-content stays the way it is.
<dael> Rossen_: We're at top of hour.
<dael> fantasai: We should go for resolving
<tantek> +1
<dael> Rossen_: Obj to keep fit-content behavior as-is, to not grow past max-content in presense of stretch
<dael> RESOLVED: keep fit-content behavior as-is, to not grow past max-content in presense of stretch

@mrego
Copy link
Member

mrego commented Sep 21, 2017

A test has been added as part of the Blink patch to modify the behavior: web-platform-tests/wpt#7436

The test is: https://github.com/w3c/web-platform-tests/blob/master/css/css-grid-1/alignment/grid-fit-content-tracks-dont-stretch-001.html

@fantasai fantasai added this to the css-grid-1 CR 2016-09-29+ milestone Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Closed Accepted by CSSWG Resolution css-grid-1 Tracked in DoC
Projects
None yet
Milestone
css-grid-1 CR 2016-09-29+
Development

No branches or pull requests
7 participants
@mrego @fantasai @astearns @svillar @javifernandez @rachelandrew @css-meeting-bot