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] place-self should not accept <overflow-position> keyword #2276

Closed
javifernandez opened this issue Feb 5, 2018 · 5 comments
Closed
Assignees
Labels
css-align-3 Current Work

Comments

@javifernandez
Copy link
Contributor

We have agreed in issue #1446 that the alignment shorthands don't accept complex values, including keywords.

However, the current place-self syntax allows such complex values:

https://drafts.csswg.org/css-align/#place-self-property

<‘align-self’> <‘justify-self’>?

@javifernandez
Copy link
Contributor Author

javifernandez commented Feb 5, 2018

Umm, perhaps I misunderstood issue #1446, so it seems that issue claims that both place-items and place-self should allow <overflow-keyword>. Then, the problem is that place-items doesn't allow it:

https://drafts.csswg.org/css-align/#place-items-property

[ normal | stretch | <baseline-position> | <self-position> ] [ normal | stretch | <baseline-position> | <self-position> ]?

@javifernandez javifernandez changed the title [css-align] palce-self should not accept <overflow-position> keyword [css-align] place-self should not accept <overflow-position> keyword Feb 5, 2018
@javifernandez
Copy link
Contributor Author

I think there we have a temporary agreement (issue #595) of using simple keywords in the Alignment shorthands (this seems what the place-items syntax defines), while there is an ongoing discussion (#1002) about using slashes to avoid ambiguities.

In any case, the spec needs to be updated so the three shorthands are consistent.

@fantasai
Copy link
Collaborator

Fixed in 3e27468

@fantasai
Copy link
Collaborator

Actually, I think that's the opposite of fixed. I think we're tired, we'll fix everything on Thursday. :)

@fantasai fantasai reopened this Mar 27, 2018
@tabatkins
Copy link
Member

No, it's fixed. The discussion history was a bit confusing, tho:

  1. Initially place-items was just <'align-items'> <'justify-items'>.
  2. In [css-align] Add shorthands for alignment properties #595 (comment) Mats points out the grammar is ambiguous and suggests using a / between the terms.
  3. In [css-align] Allowing fallback alignments without breaking shorthands #1002 fantasai offers up a slightly different usage of the / to avoid needing it for the common case (so it's more like the slash usage in border-radius).
  4. In [css-align] Fix order of unsafe/safe keyword wrt alignment keyword? #1001 fantasai offers a different suggestion entirely, that of just specifying the relative order of the overflow and position keywords (at this time they could be given in either order, leading to the grammar ambiguity Mats found). This ends up being accepted by the WG.
  5. There was some confusion around this time due to some incomplete/incorrect edits.
  6. Last week we were doing a round of edits on this spec, realized that now that [css-align] Fix order of unsafe/safe keyword wrt alignment keyword? #1001 is resolved on, the grammar ambiguity is indeed gone, and we can return to the original, simple grammar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-align-3 Current Work
Projects
None yet
Development

No branches or pull requests

3 participants