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] Allowing fallback alignments without breaking shorthands #1002

Open
fantasai opened this Issue Feb 2, 2017 · 11 comments

Comments

Projects
None yet
7 participants
@fantasai
Contributor

fantasai commented Feb 2, 2017

When adding the place-* shorthands, we realized that specifying a fallback alignment for the content distribution keywords (space-around etc.) resulted in parsing ambiguities. We debating using a slash in the shorthand, but decided not to because it would make 2-axis alignment syntax inconsistent with similar syntax elsewhere in CSS, e.g. for scroll-snap-align or background-position.

An alternate idea is to use the slash to separate the fallback from the initial. Imho this also helps make it clearer that we're declaring a fallback alignment, since slashes are used to indicate alternates in general typographic usage.

Examples:

justify-content: space-around / center; /* space-around, fall back to center - both axes */
place-content: space-around / center; /* space-around both axes, fall back to center */
place-content: space-around start / center; /* space-around b-axis, start x-axis, fallback center */

(I think it's probably best to consider this for Level 4, leaving fallback keywords out for Level 3 so we can take Level 3 to CR asap.)

@tabatkins

This comment has been minimized.

Member

tabatkins commented Mar 14, 2017

This sounds like the values are basically parallel; why not just use commas to separate the fallback from the normal?

@fantasai

This comment has been minimized.

Contributor

fantasai commented Apr 19, 2017

@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented Apr 19, 2017

An alternate idea is to use the slash to separate the fallback from the initial.

I think that would be very confusing because slash is already used in the CSS Grid shorthands (grid, grid-template, grid-area etc) to separate row-axis from column-axis values. A slash in the place- shorthands should have the same meaning, i.e. place-content: <align-value> / <justify-value>.
Examples:

place-content: space-between start / center; /* space-between b-axis with start fallback, center i-axis */
place-content: space-between / center; /* synonym to "space-between center" */
@javifernandez

This comment has been minimized.

Contributor

javifernandez commented Apr 19, 2017

i agree that a slash would be better used as a separator for row-axis and column-axis values. It even makes the shorthand's syntax clearer, IMO

@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented May 20, 2017

I don't think this syntax issue should be deferred to L4, because adopting the proposal to use / as the separator will be impossible at that point since it wouldn't be backwards compatible. place-content:space-between start currently means align-content: space-between; justify-content: start whereas if we adopt / as the separator and allow for fallback values in the shorthands, then it's a single value and means align-content:space-between start; justify-content:space-between start.

It seems to me that / was actually the preferred separator by most people when the shorthands were discussed in issue #595. Using space as the separator was only accepted under the assumption that fallback values and <overflow-position> would never be allowed in the shorthands. It seems that assumption is now false and I would like to withdraw my "commenter satisfied" for issue #595, fwiw :-).

If you don't want to resolve on an extended syntax here for L3, then I request that these shorthands be deferred wholesale to L4 to avoid creating a situation were the preferred syntax can't be introduced because it would break the web.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Jun 27, 2017

No, I was suggesting that the slash separates the initial and the fallback, not the axes. So we'd have align-content: space-between / center for the longhand. This would help make it clearer that center is a secondary value, vs. the current syntax align-content: space-between center.

Then for the shorthand, place-content: space-between / center would set both align-content and justify-content to space-between / center. In cases where you want different values in each axis, we'd follow the border-radius shorthand pattern, e.g. place-content: space-evenly / start center would assign align-content: space-evenly / start; justify-content: space-evenly / center.

(Syntax of border-radius is border-radius: [ <length> | <percentage> ]{1,4} [ / [ <length> | <percentage> ]{1,4} ]?, described in https://www.w3.org/TR/css3-background/#corners, for reference.)

@astearns astearns removed the Agenda+ F2F label Aug 1, 2017

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Aug 2, 2017

The CSS Working Group just discussed Syntax for fallback alignment in the place-* shorthands, and agreed to the following resolutions:

  • RESOLVED: slash must be use to separate main value and fallback value in shorthand and longhands alignment properties
The full IRC log of that discussion <fremy> Topic: Syntax for fallback alignment in the place-* shorthands
<TabAtkins> GitHub: https://github.com//issues/1002
<fremy> TabAtkins: we would like to allow all combinations in the shorthand that are possible in the longhands
<fremy> TabAtkins: but if we just allow spaces, this would be ambigous
<fremy> TabAtkins: we need to have a separator to know which values are fallback for what
<fremy> TabAtkins: usually this kind of separator is the slash "/"
<fremy> TabAtkins: but if we decide to use a separator, should we do axis-1 / axis-2 or value/fallback value/fallback
<fremy> TabAtkins: former is annoying because you always need the slash
<fremy> TabAtkins: I believe the latter is better for that reason
<Rossen> q?
<fremy> fantasai: sounds it would make the longhand easier to read
<fremy> fantasai: (the slash)
<fantasai> s/sounds/
<fantasai> s/sounds//
<fremy> TabAtkins: there would be a second proposal to make the longhand also have the slash
<fantasai> s/second//
<fremy> fantasai: The proposal is to do both, not just for shorthand
<fantasai> https://github.com//issues/1002#issuecomment-311501471
<fantasai> align-content: space-between / center
<fremy> TabAtkins: even align-content would use space-between / center
<fantasai> lace-content: space-between / center
<fremy> TabAtkins: with center as the fallback
<fantasai> �place-content: space-evenly / start center
<fremy> Rossen: I like it better than what we have right now
<fantasai> align-content: space-evenly / start; justify-content: space-evenly / center
<fantasai> (same as above)
<TabAtkins> place-content: space-evenly space-between / center; <= different distribution, same fallback
<fremy> rachelandrew: I think it makes sense
<fremy> Rossen: anyone else?
<fremy> Rossen: let's resolve then
<fremy> Rossen: any objection to use the slash for fallbacks in all alignments?
<fremy> RESOLVED: slash must be use to separate main value and fallback value in shorthand and longhands alignment properties
@javifernandez

This comment has been minimized.

Contributor

javifernandez commented Jan 16, 2018

This issue still needs the changes in the spec. I know that Firefox started to implement the new behavior:

https://bugzilla.mozilla.org/show_bug.cgi?id=1430622

I'll start ASAP to implement it in WebKit and Blink as well, but I'd like to have the spec updated first.
Thanks.

@emilio

This comment has been minimized.

Collaborator

emilio commented Jan 16, 2018

We haven't implemented the new thing yet (that's a compat fix, we weren't implementing place-content in the new style engine correctly).

But we can do so as soon as the spec is changed as well, I'll file and implement as soon as @MatsPalmgren gives me the ok :).

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 18, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One fo the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 18, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One fo the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 18, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 18, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 19, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68
Reviewed-on: https://chromium-review.googlesource.com/868531
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: meade_UTC10 <meade@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#530416}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Jan 19, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68
Reviewed-on: https://chromium-review.googlesource.com/868531
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: meade_UTC10 <meade@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#530416}

ardalanamini pushed a commit to siliconjs/chromium that referenced this issue Jan 19, 2018

[css-align] overflow-position keyword must have a fixed position
There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

Bug: 802098, 802247
Change-Id: I57315a475940c00d0c0dafbb4f1b32a2c6c1ff68
Reviewed-on: https://chromium-review.googlesource.com/868531
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: meade_UTC10 <meade@chromium.org>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#530416}

hubot pushed a commit to WebKit/webkit that referenced this issue Jan 22, 2018

jfernandez@igalia.com
[css-align] 'overflow' keyword must precede the self-position and con…
…tent-position value

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

Reviewed by Antti Koivisto.

Source/WebCore:

There were several discussions to avoid ambiguities with the complex
values, specially when it comes to define the place-xxx shorthands.

One of the sources of problems is the 'overflow-position' keyword. The
CSS WG has decided to change the syntax of all the CSS Box Alignment
properties so that the 'overflow-position' keyword always precede the
'self-position' or the 'content-position' keywords.

w3c/csswg-drafts#1446 (comment)

In order to apply this change to the Content Distribution properties'
(align-content and justify-content) syntax I had to completely
re-implement their parsing function. Thanks to this I addressed also
the issue with the content-distribution fallback, which cannot be
specified explicitly now.

w3c/csswg-drafts#1002 (reference)

No new tests, just rebaselined the expected results of the test cases affected.

Despite the so many layout tests affected by this change, it's
unlikely that it might break any content in current web
sites. This patch changes the new CSS syntax, obviously backward
compatible, defined by the new CSS Box Alignment. The
'overflow-position' keyword is only used by the layout models
implementing the new spec, so far only CSS Grid Layout.
Considering that CSS Grid has been shipped last year, it's unlikely
that many sites are using the new CSS values.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::valueForItemPositionWithOverflowAlignment):
(WebCore::valueForContentPositionAndDistributionWithOverflowAlignment):
* css/CSSContentDistributionValue.cpp:
(WebCore::CSSContentDistributionValue::customCSSText const):
* css/StyleBuilderConverter.h:
(WebCore::StyleBuilderConverter::convertSelfOrDefaultAlignmentData):
* css/parser/CSSPropertyParser.cpp:
(WebCore::consumeOverflowPositionKeyword):
(WebCore::consumeContentPositionKeyword):
(WebCore::consumeContentDistributionOverflowPosition):
(WebCore::consumeSelfPositionOverflowPosition):

LayoutTests:

Rebaseline expected results of the test cases affected by this change.

* css3/parse-align-content.html:
* css3/parse-align-items.html:
* css3/parse-align-self.html:
* css3/parse-justify-content.html:
* css3/overwrite-self-alignment.html:
* css3/flexbox/new-alignment-values-invalid-if-grid-not-enabled.html:
* css3/overwrite-content-alignment.html:
* fast/css-grid-layout/grid-content-alignment-overflow.html:
* fast/css-grid-layout/grid-align-justify-overflow.html:
* fast/css/parse-justify-items.html:
* fast/css/parse-justify-self.html:
* fast/repaint/align-items-overflow-change.html:
* fast/repaint/align-self-overflow-change.html:
* fast/repaint/justify-items-overflow-change.html:
* fast/repaint/justify-self-overflow-change.html:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@227297 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented Feb 6, 2018

FYI, we're waiting for the shorthand value grammar updates before implementing this.

@javifernandez

This comment has been minimized.

Contributor

javifernandez commented Feb 6, 2018

We are waiting too before implementing it in WebKit and Blink.

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