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] Automatic Minimum Size for grid items should not min against content size #1149

Closed
fantasai opened this issue Mar 30, 2017 · 5 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-grid-1 Tracked in DoC

Comments

@fantasai
Copy link
Collaborator

Continuing from #1117 examples 1/2/5/6...

Grid is using the same "automatic minimum size" algo that Flexbox does, which sets the min to the smaller of the content size and the specified/transferred size. This is correct for Flexbox - it gives us a reasonable minimum size that the image won't shrink below. But it serves a different purpose in Grid, effectively setting the layout size of the track while the item itself sizes into that containing block, potentially overflowing it. (Or, in other words, Flexbox actually sizes the item with the information; Grid sizes the track, and then the item lays itself out, and we don't want the item to overflow its track by default.)

Therefore, in Grid it doesn't make as much sense to pay attention to the content size when a specified/transferred size exists. So, we think the right fix is to just change Grid's automatic minimum size to be the specified size if it exists, else the transferred size if it exists, else the content size.

This will make examples 1, 2, 5, and 6 all render basically the same way, with a 200px tall first row, and the image filling that row. (Examples 3, 4, 7, and 8 rely on a different issue, so we won't worry about them here right now.) This gives more sensible results in size-constrained grids when the item's transferred size is larger than its content size, by keeping the contents of the track in sync and avoiding overlap of adjacent grid items.

Note: A similar consideration about overlapping overflow was used in resolving issue 283.

The proposed fix is to define the automatic minimum size for grid items inline in the spec as: the specified size if it exists, else the transferred size if it exists, else the content size.

@mrego
Copy link
Member

mrego commented Apr 3, 2017

It's really nice we're finally having a specific text in Grid Layout spec about automatic minimum size. IMHO it was pretty complex to understand the text in Flexbox and how it applies to Grid.

About the proposal to use:

the specified size if it exists, else the transferred size if it exists, else the content size

On a first sight it looks good, I don't know about specific use cases though. Probably web authors should be the want checking what makes more sense for them.

@atanassov
Copy link
Contributor

atanassov commented Aug 1, 2017

Here's a relevant existing test case we have. Based on the resolution of this issue I would expect the result of the green image to be 1x2 rectangle of 100px by 200px.

https://test.csswg.org/suites/css-grid-1_dev/nightly-unstable/html/grid-minimum-size-grid-items-007.htm

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Automatic Minimum Size for grid items should not min against content size, and agreed to the following resolutions:

  • RESOLVED: is either it's defined size, content size, or transfer size
The full IRC log of that discussion <dauwhe> topic: Automatic Minimum Size for grid items should not min against content size
<astearns> github: https://github.com//issues/1149
<dauwhe> TabAtkins: in flex we do to some effort to find minimum size
<dauwhe> ... if it's image we try to figure out what information is important
<dauwhe> ... this is useful because flex uses this to size the image
<dauwhe> ... grid uses the info to size the track, and then lets the image size to the track
<dauwhe> ... the big difference is say you have a specified size of 100px and intrinsic size is 50px
<dauwhe> ... we use 50px in flex
<dauwhe> ... grid doesn't have to worry about shrinking
<dauwhe> ... "So, we think the right fix is to just change Grid's automatic minimum size to be the specified size if it exists, else the transferred size if it exists, else the content size."
<dauwhe> ... this better matches author intent for automatic minimum sizing of images
<dauwhe> (tab draws on whiteboard)
<dauwhe> ... image is 50px
<dauwhe> ... put in grid container, set to 100px
<dauwhe> ... in flex, minimum value is 50, it's allowed to shrink to that
<dauwhe> ... in grid, we prefer to say let's stick with 100, 'cause the author said so
<dauwhe> ... and avoid risk of overflow
<dauwhe> Rossen: additional piece: if you open the test case
<dauwhe> https://test.csswg.org/suites/css-grid-1_dev/nightly-unstable/html/grid-minimum-size-grid-items-007.ht
<dauwhe> TabAtkins: sets itself to 100px, but grid width is 10 x 10
<dauwhe> ... if we went by previous algo, track would be 50px
<dauwhe> ... maintaining aspect ratio is important
<dauwhe> astearns: any other comments?
<dauwhe> Rossen: this one has height set
<dauwhe> ... current spec says minimum of instrinsic and transfer?
<dauwhe> fantasai: use specified if you have it, content if you don't
<dauwhe> TabAtkins: edge does it
<dauwhe> ... any objections?
<dauwhe> astearns: hearing no objection,
<dauwhe> RESOLVED: is either it's defined size, content size, or transfer size
<dauwhe> (do what 1149 says)
<dauwhe> fantasai: there was one about stretching tracks

@fantasai
Copy link
Collaborator Author

fantasai commented Aug 9, 2017

Edits checked in:

The automatic minimum size for a grid item in a given dimension is its specified size if it exists, otherwise its transferred size if that exists, else its content size, each as defined in [CSS-FLEXBOX-1]. However, if the grid item spans only grid tracks that have a fixed max track sizing function, its specified size and content size in that dimension (and the input to the transferred size in the other dimension) are further clamped to less than or equal to the stretch fit the grid area’s size (so as to prevent the automatic minimum size from forcing overflow of its fixed-size grid area).

@mrego Would you mind reviewing to make sure this is all sane, since I can't review my own edits? ;) (I mean, I can, but a different set of eyes would be better.)

@mrego
Copy link
Member

mrego commented Sep 8, 2017

@fantasai I think the new text looks good.

BTW, I'm fixing this on Blink and WebKit and updating the WPT tests accordingly.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
foolip pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
foolip pushed a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
MXEBot pushed a commit to mirror/chromium that referenced this issue Sep 10, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
hubot pushed a commit to WebKit/WebKit-http that referenced this issue Sep 12, 2017
…um size

https://bugs.webkit.org/show_bug.cgi?id=176688

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the tests related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-006.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-007.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-008.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-009.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:
* web-platform-tests/css/css-grid-1/grid-items/support/100x50-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/25x50-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/50x100-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/50x25-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/w3c-import.log:

Source/WebCore:

CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch use the WPT tests updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
Modified so it always returns the transferred size (if any).


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@221910 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@fantasai fantasai added the Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. label Oct 17, 2017
bertogg pushed a commit to Igalia/webkit that referenced this issue Oct 31, 2017
… automatic minimum size

https://bugs.webkit.org/show_bug.cgi?id=176688

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the tests related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-006.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-007.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-008.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-009.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:
* web-platform-tests/css/css-grid-1/grid-items/support/100x50-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/25x50-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/50x100-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/50x25-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/w3c-import.log:

Source/WebCore:

CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch use the WPT tests updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
Modified so it always returns the transferred size (if any).

git-svn-id: http://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.18@223361 268f45cc-cd09-0410-ab3c-d52691b4dbfc
rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this issue Nov 8, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this issue Nov 16, 2017
CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch modifies GridTrackSizingAlgorithmStrategy::MinSizeForChild()
so it always returns the transferred size (if any).
This also updates the tests to check the new behavior.

BUG=763265

Change-Id: If6134e7d031a3f9693b8d9ca62f8f41d3146add2
Reviewed-on: https://chromium-review.googlesource.com/657098
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#500799}
@fantasai fantasai added this to the css-grid-1 CR 2016-09-29+ milestone Jan 22, 2019
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
…um size

https://bugs.webkit.org/show_bug.cgi?id=176688

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the tests related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-006.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-007.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-008.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-009.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:
* web-platform-tests/css/css-grid-1/grid-items/support/100x50-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/25x50-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/50x100-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/50x25-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/w3c-import.log:

Source/WebCore:

CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch use the WPT tests updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
Modified so it always returns the transferred size (if any).


Canonical link: https://commits.webkit.org/193251@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@221910 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit to WebKit/WebKit that referenced this issue Dec 1, 2022
… automatic minimum size

https://bugs.webkit.org/show_bug.cgi?id=176688

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the tests related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-006.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-007.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-008.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-009.html:
* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:
* web-platform-tests/css/css-grid-1/grid-items/support/100x50-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/25x50-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/50x100-green.png: Removed.
* web-platform-tests/css/css-grid-1/grid-items/support/50x25-green.png: Added.
* web-platform-tests/css/css-grid-1/grid-items/support/w3c-import.log:

Source/WebCore:

CSS WG has agreed to modify the spec so now the transferred size is used
(if it exists) independently if it's bigger or smaller
than the content size.
See: w3c/csswg-drafts#1149

The spec text (https://drafts.csswg.org/css-grid/#min-size-auto):
  "The automatic minimum size for a grid item in a given dimension is
   its specified size if it exists, otherwise its transferred size
   if that exists, else its content size"

This patch use the WPT tests updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minSizeForChild const):
Modified so it always returns the transferred size (if any).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-grid-1 Tracked in DoC
Projects
None yet
Development

No branches or pull requests

5 participants