-
Notifications
You must be signed in to change notification settings - Fork 672
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][cssom] Disallow repeat() syntax in grid-template-rows/columns resolved values #2427
Comments
Yes, I agree it'd be nice to just have one option here, and avoiding I created a very simple example to show the issue: https://wptest.center/#/nsrrq1 |
If Edge is actually the only browser using |
Agenda+ because the editors don't care which way this goes, just tell us which way to edit it. ^_^ |
I'm not sure I quite agree with the argument here. Dealing with repeating syntax isn't new - see repeating gradients as an example. Expanding to a variable list that happened to fit a given content (in the case of auto placement) will give authors more trouble for parsing that if they deal with the short form. My preference would be for keeping the current text and open bugs for other impls to catch up. |
The Working Group just discussed
The full IRC log of that discussion<dael> Topic: Disallow repeat() syntax in grid-template-rows/columns resolved values<dael> github: https://github.com//issues/2427 <dael> Rossen_: Summary: there was a question about hey we are seeing a couple of ways repeat() is being serialized, why can't we have one? <dael> Rossen_: Edge supports serialization of repeat() by using repeat syntax and computed values inside. <dael> Rossen_: FF, Webkit, and Chrome serialize as a list of computed values. <dael> Rossen_: Question from TabAtkins and fantasai was just tell us which you want, we don't care. <leaverou_> What if the number to repeat by is a var()? <Rossen_> https://wptest.center/#/nsrrq1 <dael> Rossen_: Seems Igalia is pushing for simpliest which is not use repeat() syntax and they asked MS to voice our thoughts. My position is having repeat() serialize as a list is more difficult for editors and script to handle. Test case inside the issue serialization is okay, but if the repeat >2 in chome if you use 5000 you get 100 columns and it drops. FF is between <fantasai> leaverou_, it's the used value that gets returned by gCS <leaverou_> Oh right <dael> Rossen_: Having to parse this many is harder. Finally, this isn't new. grandant-repeat() has the same challenge but we're not serializing a list. <dael> Rossen_: Our opinion is preserve the repeat syntax and have others catch up. <dael> Rossen_: Looking for other opinions. <dael> AmeliaBR: What if author has spec columns that could be collapse? <dael> AmeliaBR: If the author has literally specified multiple coluns without using repeat syntax but they could be collapsed using repeat would you serialize that using repleat? <dael> Rossen_: Not really. That's like saying if we have width 10px why not serialize as calc(10px). I don't believe shorter is the goal here. <dael> AmeliaBR: Just so long as there's a clear definition. You agrue keep as spec by author <dael> Rossen_: Right. Roundtrip of repeat serializes as repeat, not a bunch of values. <Rossen_> grid-template-columns: repeat(2000, 100px); <dael> Rossen_: In the test case from Igalia you have the repeat syntax that looks okay, but if you try this ^ the serialization is crazy. <dael> Rossen_: Any other opinions? <dael> Rossen_: If not, can we resolve? <dael> Rossen_: Objections to keeping repeat syntax as is currently in the spec and open bugs for other impl to do that? <dael> frremy: Spec is a may. So there is no implementation bug to file unless it's a must. <dael> Rossen_: A ha. good point frremy. I didn't realize this was a may. <dael> Rossen_: Objections to specifying the repeat() syntax serialization as a must? <AmeliaBR> +1 to interop! <dael> TabAtkins: Yes, we should spec one way or the other as a must. <dael> Rossen_: I'm not hearing objections. <dael> RESOLVED: serialization of the repeat() MUST use the repeat syntax |
OK, I didn't care which way this went, but the resolution above isn't valid. You can't serialize a specified value of We can have
|
Hi,
On 29/03/18 22:09, fantasai wrote:
We /can/ have |getComputedStyle| use |repeat()| syntax according to
some set of collapsing rules, but we /can't/ make it mirror the
structure of the specified style. Keeping in mind that we are
serializing the /used value/ here, the options here are:
* Don't use |repeat()| in serialization. Easy, not nice for scaling
up to giant grids.
* Use |repeat()| in serialization to collapse any sequence of
identically-sized columns. Simplest option.
* Use |repeat()| in serialization to collapse any sequence of
identically-sized columns only if they happen to be sourced from
the same specified |repeat()|. NOTE: A specified |repeat()| can be
split into a sequence of multiple serialized |repeat()|s and/or
externally-listed sizes if the repeated track list contains sizes
that are not |<length-percentage>|. E.g. |20px repeat(6, auto)|
can serialize out as |20px repeat(2, 20px) 35px repeat(3, 19px)|
if that is what the track sizes end up as.
* Some other option which is less aggressive than the previous, e.g.
only collapse columns if they're in a |repeat()| that only
contains |<length-percentage>| track listings.
I think the last option is feasible, form the implementation point of
view, and I think it address the main purpose of serializing computed
values using the repeat notation. Limiting it to same-equal tracks seems
useless to me, I'd rather avoid using repeat notation at all for
serialization. The third option seems too much complex to me.
|
I'd go for either one of these 2 options, I don't see the benefit of doing something more complex than that. |
I would just note that in addition to the size difference, you also have to take care of the line names being the same for the duration of the repetition. Still siding on the don't use repeat side, because 3 browsers are already doing that. |
The Working Group just discussed
The full IRC log of that discussion<dael_> Topic: Disallow repeat() syntax in grid-template-rows/columns resolved values<dael_> github: https://github.com//issues/2427 <fantasai> ScribeNick: fantasai <fantasai> TabAtkins: So we resolved to serialize out the repeat()s that were specified, <fantasai> TabAtkins: but actually you can't do that as fantasai points out in https://github.com//issues/2427#issuecomment-377357237 <fantasai> TabAtkins: So we need to choose some different option <fantasai> TabAtkins: There are a few possible options <fantasai> TabAtkins: First is to reverse previous resolution: don't use repeat() in serialization of gCS <fantasai> TabAtkins: This is very simple and straightforward <fantasai> TabAtkins: downside is it can potentially produce very long values for grid-template* <fantasai> TabAtkins: if you do something like grid-template-rows: repeat(10000, 1px) <fantasai> TabAtkins: Second option is to compress any adjacent tracks that have the same track size (and line names) <fantasai> astearns: Clarification on 2nd option, is that only when specified value was repeat() or anytime? <fantasai> TabAtkins: Anytime <fantasai> TabAtkins: more complicated variants are to track which tracks came from a repeat(), and then collapse them if possible <fantasai> (These options are summarized in https://github.com//issues/2427#issuecomment-377357237 ) <fantasai> TabAtkins: The Igalia folks don't like tracking which things were in repeat() originally <fantasai> TabAtkins: seems like too much complexity for little gain <fantasai> TabAtkins: I think the best thing to do is to not serialize repeat(). The only issue is a blow-out of string sizes if you have long ones <fantasai> TabAtkins: but there is a cap on the number of tracks so it won't be too crazy <fantasai> florian: I think I'd go with non-collapsing things <fantasai> florian: Seems to me that there are a lot of corner cases <dael> florian: I think I'd go with non-collapsing things as well. As i'm looking through there seems to be lots of corner cases. I'm not sure they'll all be fine. <dael> florian: Given there's limited value le's skip the pain. <fantasai> ScribeNick: dael_ <fantasai> ScribeNick: dae <fantasai> ScribeNick: dael <dael> florian: Given that you get things like calc that are somewhat the same. I'd rather just not go there. <dael> astearns: Only edge rep is melanierichards . Since Edge is the browser that retains repeat() and thought we should. I'd like to get an Edge opinion. <dael> fantasai: frremy says still siding on the don't use repeat() sidde <dael> TabAtkins: Rossen wants repeat() and frremt would rather not. <dael> astearns: Since frremy commented on the issue my Edge concern is satisfied. <dael> astearns: Other comments on if we should deal with repeats or throw them out? <dael> astearns: Prop: Specify the behavior in all the browsers except Edge. just don't use repeat() in grid serialization <dael> RESOLVED: Specify the current behavior in all the browsers except Edge. Just don't use repeat() in grid serialization |
@atanassov please confirm you're fine with this as I'd like to update the tests accordingly. |
https://drafts.csswg.org/css-grid/#resolved-track-list
I tend to think that the above sentence should be removed from the spec and that the resolved values should always use the expanded form, i.e one value per track. UAs are currently returning incompatible values: Edge use
repeat()
syntax and other UAs don't. It's a burden for script to have to deal with two formats. I think one-value-per-track is far easier to work with so I propose we settle on that as the mandatory format.The text was updated successfully, but these errors were encountered: