-
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] Track sizing algorithm and gutters #2201
Comments
I think the fact that two independent implementations had the exact same bug is enough proof that the spec is misleading. I see several issues with the spec:
It might be easier to leave §11.7.1. "Find the Size of an 'fr'" as is though and instead fix it in §11.7. "Expand Flexible Tracks" by changing "space to fill of the available grid space" to "space to fill of the available grid space minus the size of all the gutters" (the definite case), and "space to fill of the item’s max-content contribution" to "space to fill of the item’s max-content contribution minus the size of the gutters it spans" (the indefinite case). The spec should also point out that if "space to fill" is negative then the "hypothetical fr size" is zero. |
I totally agree with Mats here and also share his point of view of what the spec should state. In fact, our final implementation to fix this bug is precisely subtracting the gutters to the 'space to fill' value. We also assumed that if "space to fill" is negative, then the "hypothetical fr size" is zero. Hence, I think these changes make sense. |
My concern about integrating gutters explicitly into the track sizing algorithm is that some places will remember to integrate them and others won't, and then we'll have actual problems. The spec very explicitly defines that gutters behave exactly like fixed-size tracks for the purpose of the grid sizing algorithm, which avoids any such synchronization problem. I'm happy to add a note reminding the reader of this fact at the top of the grid sizing algorithm, but I do not want to introduce spurious errors by pretending that gutters are potentially not exactly equivalent to fixed-size grid tracks for the purpose of sizing.
Gutters are defined exactly to behave as fixed-size tracks of the absolute gutter width. So of course they have base sizes and limits. As with any fixed-size track, these are defined to be the same. |
I understand the concerns. Actually, that is precisely what happened in two implementations, so both forgot to consider gutters as fixed-sized tracks for the purpose of computing the flex fraction. I guess both approaches have similar issues; perhaps that note at the beginning of the track sizing algorithm is a good compromise. |
I understand @MatsPalmgren position from the implementor POV but I also share @fantasai concerns. I'm reluctant to include anything related to gutters in the track sizing algorithm. Actually that's what we have been doing in Blink/WebKit, substract the size of the gutters from the available space and then run the algorithm normally. However, can we totally forget about gutters? Unfortunatelly not, as implementors we need to consider them in several places. Actually we're currently doing it without an explicit mention in the specs like [here] for example. IMO it's implementors work to detect where we need to consider gutters gutters from computations. Trying to reach some consensus what do you think about changing the status of the note mentioned by @mrego above, and transform it into a proper paragraph for implementors explaining that the gutters should be treated as fixed size tracks and that they should be likely considered in any computation which involves computing the size of spanning items or the available space in general. |
I'm aligned with @fantasai and @svillar opinions. For example in the text proposed by @MatsPalmgren:
Then when you do:
If you consider that the gutters are fixed tracks, you'll be subtracting their size again, which would be wrong. |
… like fixed-size tracks, and need to be incorporated into track size calculations accordingly. #2201
…acks, are spanned by any items which span across their corresponding grid line. #2201
OK, I've added a note to the Track Sizing section reminding everyone that they need to consider gutters as fixed-size tracks as defined in the Gutters section; and clarified the gutters section to note the relationship of the gutter to items which span across the line (which is, to span the gutter). @MatsPalmgren @mrego @svillar @javifernandez Let me know if these changes are sufficient! Thanks! |
LGTM. |
There's some text about this in gutters definition:
And the track sizing algorithm has a note about gutters:
But they're not mentioned in the rest of the algorithm.
I believe the text in the spec is correct, but it has lead to confusion in 2 implementations (Blink/WebKit and Firefox), so probably it can be clarified to avoid this kind of mistakes.
The problem is detailed in the following Chromium bug, but basically if you have a content based grid container (e.g. a floated grid container for columns or one with
height: auto
for rows) both implementations were wrongly calculating the size of a flexible track.The text on the spec says:
Implementations were subtracting non-flexible tracks, but forgetting about gutters (which should be considered non-flexible tracks too for this step.
@MatsPalmgren suggested I open this issue to try to clarify things if possible.
FWIW, Edge is right on this, and their behavior is the expected one.
The text was updated successfully, but these errors were encountered: