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

[css-grid] Unintentional change to grid container baseline? #3645

Closed
MatsPalmgren opened this issue Feb 11, 2019 · 7 comments

Comments

Projects
None yet
4 participants
@MatsPalmgren
Copy link

commented Feb 11, 2019

There was a change made in the last bullet point under §10.6. Grid Container Baselines that looks unintentional. It used to say that the first item should be used, but now it doesn't. It appears that part of this edit was not discussed in the CSSWG meeting, nor mentioned in the resolution, AFAICT, so it looks like an unintentional change to me. It appears that the discussion is mostly about which box (content/block/margin) to use to synthesize the baseline, not that items in other tracks should be ignored. That edit was for issue #373 BTW.

Testcase:

<style>
  .grid {
    display: inline-grid;
    grid: 100px 100px 100px / 10px;
    border: 1px solid;
  }
  .item {
    background: lime;
  }
</style>

A<div class="grid">
  <div class="item">B</div>
</div>

A<div class="grid">
  <div class="item" style="grid-area:2/1">B</div>
</div>

Firefox takes the item in the 2nd row into account while Chrome doesn't so we now disagree on this testcase.

I'd like clarification if the change to ignore all items except those in the first(last) track was intentional or not?

@MatsPalmgren

This comment has been minimized.

Copy link
Author

commented Feb 12, 2019

Fwiw, it seems a bit inconsistent that we baseline-align with the "B" here, but not in the previous example:

<style>
  .ib {
    display: inline-block;
    border: 1px solid;
  }
  .ib > div {
    height: 100px;
  }
</style>

A<div class="ib">
  <div></div>
  <div>B</div>
  <div></div>
</div>

@fantasai fantasai added the css-grid-1 label Feb 13, 2019

@fantasai

This comment has been minimized.

Copy link
Collaborator

commented Feb 13, 2019

I don't think the change was intentional, I think it was an accidental oversimplification. I guess the question is now, what's the preferred behavior. I don't think the result of that oversimplification is it: I'd say either we align to the contents of the first item, or we synthesize a baseline off the dimensions of the first grid track itself. Falling back to synthesizing off the grid container makes sense for an empty grid, but not really for one with content...

@jensimmons @rachelandrew @LeaVerou What would you expect in @MatsPalmgren example (baseline aligning a grid that contains only a single grid item in 2nd row)?

@javifernandez

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

As far I understand the 3 steps to determine the grid container's baseline, the grid in your first example has no first natural baseline, and it should synthesize one (step 3). The steps 1 and 2 state clearly that the items used to determine the grid s baseline should have an area intersecting the first row.

The change @MatsPalmgren mentioned, if I'm not wrong, stated originally:

and the first (last) such grid item (in grid order) has a baseline set parallel to the relevant axis, ...

I'd always understood that such first item was one among those their area intersects with the first row, since there may be many; that's why we have to use the first one in grid order.

@MatsPalmgren

This comment has been minimized.

Copy link
Author

commented Feb 14, 2019

I think we should definitely use items in other tracks if the first track is empty. I don't feel strongly about exactly how a baseline is derived from that item though. Using its natural baseline, if it has one, or synthesizing one if it doesn't, make the most sense to me. I think if we remove the "whose area intersects the first (last) row" condition from Step 2 then we'd get that behavior. A slightly more advanced scheme would be to remove that condition from Step 1 too.

Using the track itself makes no sense to me since baseline alignment is a content-based alignment scheme and the grid itself is not content (and to be clear: by "the grid" I mean the abstract concept outlined with a dashed border in this figure, not the grid container box). I don't see how that would be useful anyway when that track is empty. If that track is intrinsically sized it'd be zero, otherwise it's likely a definite size so alignment to that position can likely be achieved through other means (relative position or margin or whatever on the grid container).

It seems to me that if the author intentionally wants to align the grid lines to the baseline it should be a separate concept, perhaps as a new value for alignment-baseline.

@javifernandez

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

I understand @MatsPalmgren 's point, however, I find the first (last) row condition very intuitive to match the fist (last) baseline value. In this case, tracks behave line lines when we are talking about a general text box's baseline. I agree, though, that the grid is not content hence they are not its track, but I wouldn't remove the condition of using the first (last) row.

@javifernandez

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Using the track itself makes no sense to me since baseline alignment is a content-based alignment scheme

I forgot to comment about this statement, which I don't think it can be interpreted reading the spec and it wasn't also in my comments. We indeed don't want to use the track itself, since tracks are not content, but the items in that track.

If there are no items in the first track, then synthesizing makes sense, in my opinion.

@css-meeting-bot

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

The CSS Working Group just discussed Unintentional change to grid container baseline?, and agreed to the following:

  • RESOLVED: Change the algorithm take baseline from the first grid content instead of synthesize from first row
The full IRC log of that discussion <dael> Topic: Unintentional change to grid container baseline?
<dael> github: https://github.com//issues/3645
<dael> fantasai: Error I want to revert at least. What would we expect baseline alignement of grid with a single item in 2nd row. We can use the first item in the first row for where we derive baseline from
<dael> fantasai: I think we should do the baseline of the first available grid item even if not on first row, but wanted to check
<dael> astearns: Prefer FF behavior. Better to take baselines we have then punt
<dael> astearns: Looking through comments to see if anybody disagrees
<dael> astearns: Only thing javier brings up is if there's no content in first track you can synthesize
<dael> fantasai: If no content in grid we synthesize.
<dael> fantasai: Question is if we have an empty row do we look at next row?
<dael> AmeliaBR: When first item is it grid layout order?
<dael> fantasai: Yes
<dael> AmeliaBR: First item in DOM is in a different row we skip over that and focus on rows and columns?
<dael> fantasai: Yes
<dael> astearns: Looks like I was confused on rows and columns
<dael> astearns: There's an actual difference in opinion between impl
<dael> fantasai: jensimmons rachelandrew opinions?
<dael> jensimmons: Not really
<dael> astearns: If the first track is empty then is the design using that track for space?
<dael> astearns: In which case does aligning grid on 2nd track baseline void that decision?
<dael> fantasai: If you're using it for space, we've seen cases to use first track to make padding, seems reasonable to use the content of the 2nd row in that case
<dael> astearns: First track is for padding if grid is not baseline align?
<dael> fantasai: We have a grid with an empty first row and content in 2nd row. Do we treat this grid as not having baseline or do we take baseline from 2nd row. That's the question
<dael> jensimmons: How would that effect sizing of first row?
<dael> fantasai: Doesn't. Effects how that grid aligns with other content. If you have grid inside a table cell and another grid next to it
<dael> rachelandrew: In that example 2nd row makes sense. It does seem using first row as padding is the common case. Trying to think if other cases
<gregwhitworth> Agreed with fantasai and rachelandrew
<dael> astearns: If you have 2 grids in adjacent table cells and you are using the first row for space in the first grid and both have a BG color so you can see the space added if those 2 are not baseline aligned then the content in the first is pushed down by the amount of first row. If you do baseline align the 2 table cells the first grid's space is maintained and push table height up in FF case. In chrome case space is maintained and push table cell height down
<dael> astearns: That's the case for this in a concrete case
<dael> [mic problems with someone, possibly fantasai ]
<fantasai> [not me]
<dael> astearns: I think it does make a bit more sense to take baseline from first available content in the grid. I don't know that it is something that is going to be used often. Prob an edge case that will not come up often
<dael> jensimmons: I think it might come up. Not sure edge case. I can see real world use cases for this. not 2 grid items in a table cell, but 2 grid items in a grid
<dael> astearns: Or a flex container. Or if we get to baseline aligning in other cases
<dael> jensimmons: Options is align second row with other grid or...?
<dael> astearns: Options are at moment algo says you take baseline of content in first row and if there isn't you synthesize. So if first row is empty spec sasy you synthesize a baseline. Considering saying take first content in grid and use that for baseline
<dael> jensimmons: Result of synthezation? Just make up a line? I guess yes
<dael> rachelandrew: I can think of using baseline of first content item, that can be explained. You can say you haven't got content in the first track so we're using first item to work out baseline. Being worked out on something that doesn't exist is harder to explain. As someone who explains this stuff to people ut's easier to work off first item that appears
<dael> jensimmons: Lean that way too. I can see where people would make sep grids, put in a grid, and then align baselines. At this point you can't have an empty row with a visual thing. But if we get to where you can style grid rows there's a case to show on nothingness
<dael> astearns: Will show if there's border or BG
<dael> jensimmons: I lean toward do the first row with content in it. THat makes sense as an author. I think that's what rachelandrew is saying
<bkardell_> it is what I would expect too
<dael> astearns: Converging on take baseline from the first grid content instead of synthesize from first row
<dael> astearns: Objections to take baseline from the first grid content instead of synthesize from first row and see how Blink and WK impl take to it?
<jensimmons> I also think this makes baseline alignment more useful
<dael> RESOLVED: Change the algorithm take baseline from the first grid content instead of synthesize from first row

fantasai added a commit that referenced this issue Jun 7, 2019

[css-grid-1] Revert unintentional change to ignore grid items past 1s…
…t row for purpose of finding grid container baselines. Also clarify traversal order. #3645

@fantasai fantasai closed this Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.