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-align] percentage gap definition should be clearer for the block direction #4664

Open
cbiesinger opened this issue Jan 10, 2020 · 6 comments

Comments

@cbiesinger
Copy link

https://drafts.csswg.org/css-align-3/#propdef-row-gap

Consider this testcase:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7659
Should the flexbox height (the black border) expand to include the full flex line?

There is an indefinite height. The pre-gap flexbox height would be 100px. What Firefox does is compute the gap at 50% * 100px = 50px, so the content is now 150px high, but it keeps the flexbox height at 100px.

The spec should be clearer on how this works:

  • Should percentages resolve against an indefinite height?
  • If yes, should it affect the intrinsic block size of the flex container?

cc @dholbert

@cbiesinger cbiesinger added the css-align-3 Current Work label Jan 10, 2020
@MatsPalmgren
Copy link

Compare this corresponding Grid testcase:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7661
Firefox and Chrome agrees on the layout of this test and it renders exactly as your Flexbox testcase does in Firefox.

The relevant spec for this is the section on cyclic percentages in css-sizing. (Which could definitely be improved by explicitly including the gap properties in the text there (the table in d.), but I think it's clear it should use the same rules as padding/margin (although it has a different percentage basis).)

So, I think a cyclic percentage gap should follow the "a cyclic percentage is resolved against zero for determining intrinsic size contributions" under d. And then, the last bullet says: "Otherwise, the percentage is resolved against the containing block’s size. (The containing block’s size is not re-resolved based on the resulting size of the box; the contents might thus overflow or underflow the containing block)."
I think that's the spec text that applies in the Grid case.

Is there any reason a block-axis gap in Flexbox should behave differently to Grid?

Fwiw, @mrego and @dholbert agreed this is the right layout back when we implemented gaps for Flexbox in Gecko.

@mrego
Copy link
Member

mrego commented Jan 13, 2020

Yeah we had long discussions about this thing on grid layout: #509 and #1921.

So when Firefox was implementing gaps for flexbox I suggested we should keep consistency with what we are doing for grid layout, and people seem to agreed.

I believe it's good we keep a similar behavior in both flex and grid, despite it can be strange in some cases it'd be better than having different ones.

@cbiesinger
Copy link
Author

OK, so it seems clear that the Firefox behavior is what's desired.

However, I disagree that css-sizing is the right spec for this. gaps are not sizing... At the very least there should be a non-normative note in css-align about this, but I really do think that it should be defined there.

I guess what I would like is a definition of this in terms of the flexbox algorithm:
https://drafts.csswg.org/css-flexbox/#layout-algorithm

I suppose from a spec perspective the argument is that flexbox block sizes should first be determined based on https://drafts.csswg.org/css-flexbox/#intrinsic-sizes and percentages then resolve based on that. Unfortunately no browser implements that yet, making this harder.

@cbiesinger
Copy link
Author

(cc @davidsgrogan )

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Apr 24, 2020
This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}
@fantasai
Copy link
Collaborator

@cbiesinger We added some cross-links. Let me know if it's acceptable, or if you want something else.

Wrt handling of gaps in the flexbox algorithm generally, we need to draft up a Flexbox Level 2 to do that properly. It's on the to-do list... but basically they can be treated similar to margins on the flex items.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 1, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194
xeonchen pushed a commit to xeonchen/gecko that referenced this issue May 1, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 1, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194
xeonchen pushed a commit to xeonchen/gecko that referenced this issue May 1, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: 5f6680be8c1fb6229fc67131a0a44b9273d246ea
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: eb89edcb321e41cd109e4be74a30e043df6ba8e4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: 5f6680be8c1fb6229fc67131a0a44b9273d246ea
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: eb89edcb321e41cd109e4be74a30e043df6ba8e4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: 5f6680be8c1fb6229fc67131a0a44b9273d246ea
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue May 3, 2020
…estonly

Automatic update from web-platform-tests
[css-flexbox] Implement gap/gutters

This only works when you enable "Enable experimental web platform
features" in about:flags.

This patch doesn't implement a weird case that firefox supports:
resolving a percent column-gap when the flexbox's height is indefinite.
This patch resolves it to 0. We may wait until that is fixed before
exposing this to the web without the about:flag. The relevant test is
http://wpt.live/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-column-row-gap-004.html
The spec issue is w3c/csswg-drafts#4664.

This patch does not include the few engine-specific bits in legacy, so
gap only works in FlexNG.

Change-Id: I691889cf4b9346e94c83ef655dc9fd6eebca640d
Bug: 762679, 845235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2162149
Commit-Queue: David Grogan <dgroganchromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: Christian Biesinger <cbiesingerchromium.org>
Cr-Commit-Position: refs/heads/master{#762264}

--

wpt-commits: 344e2aa99d6555e12d0003fb6cbf7abf0c5fd64a
wpt-pr: 23194

UltraBlame original commit: eb89edcb321e41cd109e4be74a30e043df6ba8e4
@cbiesinger
Copy link
Author

@fantasai Thanks. If I understand correctly, gap percentages against an indefinite size resolve to zero. SGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants