[css-grid] Regarding the 'subgrid' feature #958

Open
MatsPalmgren opened this Issue Jan 19, 2017 · 11 comments

Projects

None yet

8 participants

@MatsPalmgren

https://drafts.csswg.org/css-grid/#subgrids

I've re-read the latest spec text and it still looks rather immature to me. I suspect that it needs several rounds of feedback from implementors (while we're actually implementing it!) before it will mature. As you know, there have been many significant improvements in other parts of the Grid/Alignment specs based on our feedback and I suspect that this will be even more true for subgrid given that it's a relatively complex sub-feature of Grid.

@jensimmons, @meyerweb and others have expressed that subgrid is an important feature. I fully agree. This makes me think that we should be extra careful in designing this feature and that the spec for it shouldn't be finalized before everyone can give solid feedback on it, based on experimental implementations (i.e. Nightly/Canary builds).

IIUC, the CSS Grid spec will soon have CR status. Firefox and Chrome will soon release their initial implementations of Grid layout, but both vendors have chosen to exclude subgrid in the first version (there are several good reasons for this, but I won't go in to that here).

In this situation, I believe that it would be bad to finalize the syntax and semantics of the subgrid feature at this point - it's definitely not ready to have CR status IMHO. If the CSSWG intends to move Grid to CR soon, then I think you should exclude the subgrid feature completely and instead move that to the next level of the Grid spec. Otherwise, I think Grid CR status should be gated on solid implementation feedback for subgrid, preferably from two independent implementations.

Thanks for your consideration.

@rachelandrew
Contributor

I've been very vocal about my belief that subgrid is a requirement, and have been talking about this with authors and looking at use cases for the last couple of years [1].

The revised specification came about after a conversation with developers from Igalia [2], as an attempt to move the feature forward. Given that this has not resulted in any implementor doing so, I would be inclined to agree with Mats that the feature should be more clearly worked through, rather than this revised attempt pushed into L1.

The revised spec solves a class of issues that authors expect grid to solve for them. For example in this post[3], the author wants to be able to line up the internals of a set of boxes laid out by grid. In this case we know the number of rows required in the subgrid so the fact that subgrids are in both dimensions would not be an issue. There are a number of cases however [4]where people will want more auto-placement of items in a subgrid, and I don't see how this will work with the revised spec.

It's been relatively difficult to get good author use cases for more than the most trivial of things, with grid behind a flag. I'm starting to see that pick up now as people realise this is shipping in browsers. I think in 6 months time we're going to have a lot more to work with in terms of seeing how people really want to use this feature. My fear in punting it to L2, without clear interest from implementors is that it disappears into some distant future.

So at this stage I agree with Mats, this feature still needs a significant amount of discussion, it needs more author use cases (which I will continue to collect as I talk to authors). Grid is already at CR and my understanding in terms of process is that Grid can't exit CR with features that do not have two interoperable implementations, so a decision will need to be made at some point about subgrid. Whatever that decision is I very much hope that it comes along with interest from both the WG and implementors to move this feature along. My feeling is we will start to get a lot of demand for it from authors very soon.

  1. Modern CSS Layout Power and Responsibility
  2. Subgrids thinking out loud
  3. CSS Grid AMA Issue 13
  4. A Revised Subgrid Specification
@dauwhe dauwhe added the css-grid-1 label Jan 26, 2017
@fantasai
Contributor
fantasai commented Feb 1, 2017 edited

As Rachel notes, Grid is already in CR. If feedback ends up destabilizing the spec for subgrid, we will pull it back into L2 WD. Since we haven't received much in the way of such feedback, I'm inclined to leave it in CR until we do. :)

(From the CSSWG perspective, CR is intended as "we can't do anything more to improve this feature without implementation, and don't expect any changes unless they come from implementation feedback, so please try to implement it". That said, there didn't used to be much distinction between "implement it" and "ship it" in the past, and now there is, so there is that.)

@MatsPalmgren
MatsPalmgren commented Feb 2, 2017 edited

Well, just to give you one example: the display:subgrid value, which implies a subgrid's tracks must be tied to its parent grid in both axes. This is something that wasn't in the spec originally, but was added for vague reasons of maybe being "simpler".

From my experience in implementing the rest of the Grid spec in Gecko, this seems misguided. Pretty much all algorithms so far are implemented in a way that they apply to each axis independently, i.e. you have some data/state per track, and a set of those for each axis, then you run the same algorithm once for the column axis and then for the row axis. I don't see why subgrid poses any problem in this respect. So I think we should, as a starting point for implementation, use the original subgrid keyword in grid-template-rows/columns, which allows for this:
https://www.w3.org/TR/2015/WD-css-grid-1-20150917/#track-sizing

Removing the ability to specify subgrid in just one axis takes away creative freedom from authors, something I, unlike the CSSWG apparently, take very seriously. We shouldn't do that without strong reasons based on actual implementation experience. There is no evidence to support that display:subgrid is in any way simpler than the previous version of the spec. (I could be wrong of course, but I don't think anyone can tell before trying to implement it.)

Regarding "Grid is already in CR" -- subgrid is marked as at-risk, thus it can be removed from the L1 CR version of the spec (or better, replaced with "TBD in Level 2"). I think it's clear that the subgrid part of CSS Grid is not mature enough to be standardized in its current form and that doing so is actually harmful.

To be clear, personally I'm not even going to start implementing the subgrid feature unless I have assurances that Grid spec changes are welcome, and that for example supporting subgrid in just one axis will be accepted if implementation experience shows that it is in fact not hard to support. And as long as subgrid is in the L1 CR, I'm not confident that would be the case.

@rachelandrew
Contributor

Mats - as already noted (and as an author who is also a CSS WG Invited Expert) I agree with you, and mentioned some of the history of the change to the spec in my comment.

Whether subgrid stays in the spec for now or is removed is mostly process if no-one is implementing it. However I would love to have some discussion around what subgrid should be. I have a reasonable idea of what authors might want - and I think once grid ships we'll see many more examples which I am already looking out for in order to collate real use cases. I'm not an implementor and so have to base what I know about what is and is not hard to implement upon what implementors say.

@tabatkins
Member

[...] takes away creative freedom from authors, something I, unlike the CSSWG apparently, take very seriously.

This is unnecessary hostility, and helps nobody. Please leave it out of any future communication.

Well, just to give you one example: the display:subgrid value, which implies a subgrid's tracks must be tied to its parent grid in both axes. This is something that wasn't in the spec originally, but was added for vague reasons of maybe being "simpler".

No, it's actually 100% required for the simplification of subgrid that ended up making it palatable to implementors.

The subgrid feature, as written now, effectively promotes the subgrid's children up to being child boxes of the parent grid - they position themselves in the parent grid exactly like the parent grid's own children do. They just receive a bit of "magic margin" if they're against the edge of the subgrid, equal to the subgrid's own m/b/p, to make it look like they're positioned inside the subgrid. The whole feature is just a slightly souped-up version of display: contents, because contents didn't address the "visual grouping" use-case that originally motivated subgrids.

Allowing the subgrid to only inherit the parent grid's tracks in one dimension changes the entire feature, making the subgrid actually a full grid container for its children, with a complicated relationship between its tracks and those of the parent grid. It's possible to define how this works - we kinda did it, tho you had to read very closely to correctly understand how it worked - but it's a lot of extra complication for relatively little benefit. The feature as currently designed is vastly simpler and allows almost as much stuff.

To be clear, personally I'm not even going to start implementing the subgrid feature unless I have assurances that Grid spec changes are welcome, and that for example supporting subgrid in just one axis will be accepted if implementation experience shows that it is in fact not hard to support.

You're free to make implementation decisions as you like, and refusal to implement something does have the possibility of killing the feature entirely. But it seems clear from your comments that you haven't looked at the feature in enough depth to understand the simplicity of the current approach, and the complexity of the previous approach. I'd spend a bit more time thinking on this before making your final decision.

@fantasai
Contributor
fantasai commented Feb 2, 2017

something I, unlike the CSSWG apparently, take very seriously

The CSSWG is composed of implementer representatives as well as various other experts, and it's our job to take feedback and process it: yours as well as everyone else's. The feedback on Subgrid from implementers (specifically, from Igalia) was what prompted the restriction you are objecting to.

To be clear, personally I'm not even going to start implementing the subgrid feature unless I have assurances that Grid spec changes are welcome, and that for example supporting subgrid in just one axis will be accepted if implementation experience shows that it is in fact not hard to support.

If you and Igalia can agree on that point, I am sure we would be happy to re-open the discussion and make any necessary changes. Rachel would also be happy as she's concerned about the restrictions as well. :)

Again, we did the best we could processing feedback on subgrid, and put the resulting draft--which was stable, no issues were reported against it--in CR along with the rest of the spec. If there are reasons to pull it back to WD for further re-work, we will.

@MatsPalmgren

The subgrid feature, as written now, effectively promotes the subgrid's children up to being child boxes of the parent grid - they position themselves in the parent grid exactly like the parent grid's own children do. They just receive a bit of "magic margin" if they're against the edge of the subgrid, equal to the subgrid's own m/b/p, to make it look like they're positioned inside the subgrid. The whole feature is just a slightly souped-up version of display: contents, because contents didn't address the "visual grouping" use-case that originally motivated subgrids.

This is the first time I've heard subgrid described as "a slightly souped-up version of display: contents". It seems like a bad analogy to me. For example, the children of a subgrid should not be placed as if it has display:contents. Here's an example https://people-mozilla.org/~mpalmgren/tests/grid/grid-subgrid-1.html that use display:contents on a grid item. Two things to note here: the "B" jumps out from the "subgrid" and is placed in the first slot before the "A" (due to dense), this would not occur if this was a subgrid (without display:contents). Secondly, the "D" is auto-placed inside the "subgrid" (adjacent to "C"), this would not occur either if this was a subgrid (instead, the "D" would be placed before the "A"). So, subgrid vs display:contents have completely different effects on placement.

The primary feature of display:contents is that it doesn't have a [principal] box. A subgrid on the other hand MUST have a box, because it needs to be able to have a border/padding/margin/background/opacity/transform/relative position/clipping/scrolling/and everything else that CSS associates with a box. It should also be the CB of its abs.pos. descendants when its properties says so (position:relative/transform etc). A display:contents element can have/be none of those things.

(FYI, it just so happens that I'm the person that implemented both grid layout and display:contents in Gecko, so I know intimately how these work internally. I can tell you with 100% certainty that implementing subgrid as "a slightly souped-up version of display: contents" would fail disastrously. I would advice against having that mental model of subgrid because it's very likely to be misleading rather than helping.)

As far as understand the Grid spec, the following are true:
A. a subgrid item MUST have a principal box
B. that box establishes a grid formatting context
C. the subgrid's items cannot be placed outside the subgrid
D. the parent's items cannot be auto-placed inside a subgrid
E. subgrid items contributes to the Track Sizing Algorithm of the parent grid and the track sizes are taken from the parent
F. subgrid items participate in baseline alignment together with the parent's items (and other subgrid's items) in the same track

Given that list, I think a better mental model for subgrid is that it is just like an ordinary grid container, with a few exceptions, like E and F above, plus the bullet points that the Grid spec currently have (I suspect that we'll modify/add to that list once we start implementing it, but it looks like a good starting point).

I do think that we should keep an open mind about adding "in one or both axes" as the original subgrid spec said, because that would be useful to authors and there is no evidence to suggest it's hard to implement at this point, so we should at least try before rejecting it.

@MatsPalmgren

The feedback on Subgrid from implementers (specifically, from Igalia) was what prompted the restriction you are objecting to.

Right, I'm well aware of that. My impression though, is that their conclusions were not based on actually trying to implement it.

I think you need to separate "an implementer gives feedback based on his/her opinion" vs "an implementer wrote code that tried to implement what the spec says and ran into unforeseen problems". The latter category is of course what should count as "evidence based on implementation experience". The former is of course valuable, but the latter must be taken much more seriously.

Again, we did the best we could processing feedback on subgrid...

Yes, and I appreciate that. But that doesn't mean subgrid is ready to be enshrined with CR status. Unlike the rest of the Grid spec, which is now implemented (for the most part) in two major browsers, the spec for subgrid has not been battle-tested like that and it's better for everyone if it remains in draft status, as I argued in the OP.

Subgrid is both an important feature (we should try hard to get it right) and also a rather complex feature (it's unlikely that we'll get it right before starting to actually implement it). I see no benefits at all to freezing the feature in its current form before we have actual implementation feedback.

@frivoal
Contributor
frivoal commented Feb 3, 2017
@MatsPalmgren

CR ... means “we can't do anything more to improve this feature without implementation, and don't expect any changes unless they come from implementation feedback, so please try to implement it”
[...]
CR does seem the right level, and I don't see what harm it does being at that level.

The harm is that the Grid CR is recommending us to implement a feature that is sub-optimal for authors. It would be better if implementers tried to implement the older version of the subgrid spec (where subgrid could be specified independently for each axis) and then report back if there are any actual implementation issues with that. It's quite clear from the quote above though, that CR means the discussion is over and that alternative solutions aren't going to be considered.

[if] subgrid remains unimplemented (or incompletely so), it will probably be pushed to the next level

That would be unfortunate, when I'm telling you I'm willing to investigate if subgrid could work independently in each axis right now. But I'm not going to invest that time unless the CSSWG would welcome such a spec change, which seems very unlikely when the CR precludes that solution.

@atanassov atanassov added the Agenda+ label Mar 14, 2017
@dbaron
Contributor
dbaron commented Mar 15, 2017

This was discussed in today's teleconference, concluding with:

RESOLVED: Move subgrid to level 2 of Grid

@atanassov atanassov added Needs Edits and removed Agenda+ labels Mar 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment