w3c / csswg-drafts

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

[css-grid-3][masonry] Reordering threshold #9328

Closed fantasai closed 1 month ago

fantasai commented 1 year ago

In https://github.com/w3c/csswg-drafts/issues/9041 @bfgeek suggested that there be some control over how sensitive masonry layout is to inherent order vs height differences in already-placed content.

E.g. adding a property like masonry-threshold: <length> where we only skip over a column if the next column is <length> less than or equal to the specified value.

SebastianZ commented 1 year ago

I am all for allowing to providing a way to influence the threshold. As with most properties, the default should be auto, though.

And it's probably too early to bikeshed but it might not be clear for authors what that threshold is meant for. So maybe it should be made more obvious by adding another word in between, e.g. masonry-placement-threshold or something similar. Benefit would also be that it could be combined with other placement related properties.

Sebastian

fantasai commented 1 year ago

Probably terrible bikeshedding ideas: masonry-strictness, masonry-adjust, masonry-skip, masonry-sensitivity...

bfgeek commented 12 months ago

Staircase problem:

Tracks heights:

[100px, 99px, 98px, 97px...., 80px]

Selecting 80px column is more correct here, looks incorrect if you pick the 100px column.

fantasai commented 12 months ago

@bfgeek Naively, if slack is 10px, you'd end up picking the 90px column, no?

css-meeting-bot commented 12 months ago

The CSS Working Group just discussed [css-grid-3][masonry] Reordering threshold, and agreed to the following:

The full IRC log of that discussion <TabAtkins> fantasai: In an earlier issue Ian suggested there should be some control over high sensitive masonry is to exact height differences
<TabAtkins> fantasai: Like if all your items are identical in size you'll lay out your content in perfect rows, straight across
<TabAtkins> fantasai: But if it's different sizes, column 2 was a little bit taller, you might skip from column 1 to column 3
<TabAtkins> fantasai: Ian suggested it might be useful to tweak the sensitivity so it's not just "is the difference 0", but to allow some amount of fudge factor
<TabAtkins> fantasai: So if the column 2 is only 10 px taller than column 1 and 3, you don't skip it, you place 1-2-3
<TabAtkins> fantasai: So do we want to add a control? Presuambly takes a length which is the fudge factor. And what do we call it?
<khush> q+
<TabAtkins> +1 to adding it
<TabAtkins> unsure about name, don't like any of the suggestions :/
<TabAtkins> khush: Have you seen use-cases where this is needed?
<TabAtkins> fantasai: The case Ian pointed out is - when you jump, it's harder to follow what's next. If you jump less often it's better to navigate for the user
<fantasai> One issue with masonry style layouts is that things can easily be visually out of order, e.g. if the current tracks are [100px, 99px] the next masonry item would be placed in the 2nd track, when the first would be more natural. A potentially solution to this is some user defined "threshold" to "place within the first track within Xpx of the smallest track"
<Rossen_> ack khush
<fantasai> masonry-threshold: <length>
<fantasai> -- iank
<TabAtkins> q+
<Rossen_> ack TabAtkins
<fantasai> TabAtkins: Use case is as described, when the mortar columns are fairly ragged, it's fine to track across columns
<fantasai> TabAtkins: when there is some sort of order
<fantasai> TabAtkins: but when differences are very small, looks very strange to skip over something
<iank_> One important note as well is the simple algorithm for this doesn't work - there is a staircase case where the simple algorithm doesn't work.
<fantasai> TabAtkins: if your column height differences are minimal, it looks weird to have a gap
<fantasai> TabAtkins: better to have gap at the end of the row instead
<fantasai> TabAtkins: this is an ability that masonry libraries have; not sure how common, but definitely some
<TabAtkins> iank_: [restates irc comment]
<TabAtkins> iank_: where even if all the tracks are off by 1px it looks very strang eto select the first one
<TabAtkins> iank_: So there needs to be a precise algorithm written that tests against the cases
<SebastianZ> q+
<Rossen_> ack SebastianZ
<TabAtkins> SebastianZ: so we're just talking about regarding the height of the already palced content?
<TabAtkins> fantasai: right
<fantasai> masonry-slack?
<astearns> we should look to see what the masonry libraries use
<TabAtkins> TabAtkins: I think we can resolve to add this ability. Should discuss names, all the ones in the issue are more complex words than I like to use in properties
<TabAtkins> TabAtkins: But I really like the masonry-slack fantasai just suggested
<TabAtkins> proposed resolution: Accept some form of masonry slack property; exact algo TBD; exact name TBD
<TabAtkins> RESOLVED: Accept some form of masonry slack property; exact algo TBD; exact name TBD
<TabAtkins> Agree with astearns we should do a quick survey to see if libs have consistency in naming
tabatkins commented 12 months ago

Naively, if slack is 10px, you'd end up picking the 90px column, no?

Yeah, my initial though on the algorithm would just be "among all the columns within slack distance of the shortest column, place in the startmost" (or whatever direction you're filling in), so in that example the columns from 90px to 80px would all be valid, and we'd place in the 90px column. This would just be a trivial adjustment from the current rule, which is to select the startmost among all the identically-shortest columns.

Loirooriol commented 6 months ago

among all the columns within slack distance of the shortest column, place in the startmost

As argued in https://github.com/w3c/csswg-drafts/issues/10232#issuecomment-2072636648, this doesn't seem right. Let's say we have 4 columns, we use a threshold of 75px, and we have a bunch of items which are all 30px tall. Then:

  1. Column sizes are 0px 0px 0px 0px, the shortest is 0px. We place the 1st item into the 1st column, since it's the startmost column within the threshold <= 0px + 75px.
  2. Column sizes are 30px 0px 0px 0px, the shortest is 0px. We place the 2nd item into the 1st column, since it's the startmost column within the threshold <= 0px + 75px.
  3. Column sizes are 60px 0px 0px 0px, the shortest is 0px. We place the 3rd item into the 1st column, since it's the startmost column within the threshold <= 0px + 75px.
  4. Column sizes are 90px 0px 0px 0px, the shortest is 0px. We place the 4th item into the 2nd column, since it's the startmost column within the threshold <= 0px + 75px.
  5. Column sizes are 90px 30px 0px 0px, the shortest is 0px. We place the 5th item into the 2nd column, since it's the startmost column within the threshold <= 0px + 75px.
  6. ...

So it ends up being super weird:

Col 1 Col 2 Col 3 Col 4
1 4 7 10
2 5 8 14
3 6 9 18
11 12 13
15 16 17

I think it should actually be:

  1. Let x be the last column that received an item, if any.
  2. If there is no x, pick the first column and abort these steps.
  3. Find the candidate columns within the threshold from the shortest one.
  4. Pick the startmost candidate column that comes after x and abort these steps, if any.
  5. Otherwise, pick the startmost candidate column.
Col 1 Col 2 Col 3 Col 4
1 2 3 4
5 6 7 8
8 9 10 11
12 13 14 15
16 17 18

Then masonry-auto-flow: next should be equivalent to masonry-threshold: calc(1px / 0).

It would also match Firefox here (with the initial masonry-threshold: 0px):

<style>
.grid {
  display: inline-grid;
  grid: masonry / repeat(3, 2ch);
  border: 1px solid;
}
item {
  border: 1px solid;
}
</style>
<div class="grid">
  <item style="grid-column: 2">1</item>
  <item>2</item>
  <item>3</item>
</div>
Firefox
My proposal
WebKit
Tab's proposal

But it would differ from both when adding a 4th item:

My proposal Firefox WebKit
Tab's proposal
tabatkins commented 6 months ago

Yeah, you're right, my proposed algo was bad. Masonry naturally displays a desirable "fill start-to-end" behavior when items are all the same size, and a small threshold just puts a little flexibility into that behavior; a large threshold should give it large flexibility, rather than drastically changing the behavior to something completely different.

I don't really like the behavior of your algo when explicitly-placed items get into the mix, tho. I've been playing around with it a bit, and I think we get the best results if we just don't update the "last placed" cursor for explicit items.

I got sniped and wrote up a live example of the layouts to play around with.

Loirooriol commented 6 months ago

@tabatkins From https://drafts.csswg.org/css-grid-3/#masonry-layout-algorithm

Items are placed in order-modified document order, but items with a definite placement are placed before items with an indefinite position (as in regular grid layout).

So I think the outcomes would be different? Though with masonry-auto-flow: next Firefox behaves as your depiction of "Oriol, ∞ threshold" indeed:

<style>
.grid {
  display: inline-grid;
  grid: masonry / repeat(5, 2ch);
  masonry-auto-flow: next;
  border: 1px solid;
}
item { border: 1px solid; }
item:nth-child(2) { grid-column: 4; }
</style>
<div class="grid">
  <item>1</item><item>2</item><item>3</item><item>4</item><item>5</item>
  <item>6</item><item>7</item><item>8</item><item>9</item><item>10</item>
</div>
Firefox WebKit

Anyways I don't have a strong opinion about explicitly placed items.

tabatkins commented 6 months ago

Ah, sorry, I was assuming masonry-auto-flow: ordered in these examples. It's not significantly different with definite-first, tho - I still prefer the look when things start from the start edge rather than after the final definite item.

fantasai commented 2 months ago

@Loirooriol Tab drafted and I edited in the proposal for a masonry-slack property based on these discussions. Let us know if there's anything that needs fixing!

Loirooriol commented 2 months ago

https://github.com/w3c/csswg-drafts/blob/ffda3a6f773d947693c285610981ed8a274f812a/css-grid-3/Overview.bs#L732

This should be a computed <length-percentage>, given that we need layout in order to resolve the percentage.

https://github.com/w3c/csswg-drafts/blob/ffda3a6f773d947693c285610981ed8a274f812a/css-grid-3/Overview.bs#L787

I think this is step 1 now.

Otherwise it looks reasonable. I have also filed some follow-up issues:

Loirooriol commented 2 months ago

https://github.com/w3c/csswg-drafts/blob/ffda3a6f773d947693c285610981ed8a274f812a/css-grid-3/Overview.bs#L733

Is this deliberate? It's surprising to see a <length-percentage> that animates discretely, but I guess not many people will want to animate this property...

fantasai commented 1 month ago

Thanks for the review, @Loirooriol! We fixed these. Closing out the issue.

Loirooriol commented 1 month ago

The animation type still seems wrong: https://github.com/w3c/csswg-drafts/issues/10931