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-contain-3] container :/ size #7142

Closed
andruud opened this issue Mar 15, 2022 · 8 comments
Closed

[css-contain-3] container :/ size #7142

andruud opened this issue Mar 15, 2022 · 8 comments

Comments

@andruud
Copy link
Member

andruud commented Mar 15, 2022

While it's awesome that the new container shorthand grammar allows for ASCII-emojis in the middle of the declaration, it has some drawbacks as well:

  • An empty value is a valid declaration: container:. (Equivalent to none / none).
  • Values equivalent to none / none (the initial values) would serialize as an empty value (e.g. via gCS) per cssom shorthand serialization algos.
  • Values equivalent to none / <something> would serialize as a string starting with /, which looks very odd. (E.g. container: / size).

It seems more normal to make the container-name part non-optional, i.e.:

  <container-name>? [ / <container-type> ]?
                  ^
                 Drop
@mirisuzanne
Copy link
Contributor

I like this change. It further encourages naming containers, and also makes it clear that leaving a name off would set the name to none. If authors want to add a container-type without setting the name, we already have a longhand for that.

@mirisuzanne
Copy link
Contributor

Agenda+ for a resolution

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed container ;/ style.

The full IRC log of that discussion <fantasai> subtopic: container ;/ style
<fantasai> github: https://github.com//issues/7142
<fantasai> github: https://github.com//issues/7142
<fantasai> miriam: Wit the container shorthand, it takes 2 values, and because one of them accepts custom idents and other accepts keywords and hard to distinguish, we have a slash separator
<fantasai> miriam: at this point both are optional
<fantasai> miriam: so can get into situation where you can mek this frowny face in the property declaration syntax
<fantasai> miriam: he suggests it looks weird, unsure if it's an actual problem
<fantasai> miriam: one possibility is to make container-name required in the shorthand
<fantasai> miriam: the only reason to use the shorthand would be to set both
<fantasai> miriam: with it required, would have to set 'none' as container name if you want to not have a name
<fantasai> miriam: I don't feel strongly one way or another
<emilio> q+
<fantasai> miriam: we do want to encourage names, so ...
<TabAtkins> Fine with this, or we can fix the optionality by using the `[...]!` syntax in the grammar
<Rossen_> ack emilio
<fantasai> emilio: I see a very similar issue and what we're testing
<fantasai> emilio: what WPT is testing is container-type being mandatory and container-name being optional
<fantasai> emilio: why would you specify just the name?
<TabAtkins> (To resolve the "empty value becomes valid" problem raised.)
<fantasai> miriam: Right now we have default container-type of style on all elements
<fantasai> miriam: so that makes it optional
<fantasai> miriam: sometimes would just want to add a name to the style container
<fantasai> emilio: but shorthand expands to set container type to none
<fantasai> fantasai: we changed the initial value to style
<fantasai> Rossen_: seems we're running out of time here
<fantasai> Rossen_: not hearing any pushback on makign container name required, is it something we can resolve?
<fantasai> miriam: That's just for the shorthand syntax
<fantasai> emilio: ...
<fantasai> emilio: It makes sense to check what WPT tests are checking, optional name and required type
<fantasai> Rossen_: sounds we're not ready to resolve today
<fantasai> Rossen_: we'll end the call here, and recommend for posting opinions in issue
<fantasai> Rossen_: and we can bring it first thing next week
<emilio> ftr I filed https://github.com//issues/7180 yesterday

@mirisuzanne
Copy link
Contributor

I (hopefully) addressed some of the confusion here in a related issue: #7180 (comment)

I'll also get the spec is up to date with our resolutions, which should help clarify even more…

@SebastianZ
Copy link
Contributor

Copying my point from #7180 (comment) over to here:

Making all values of a shorthand optional is incorrect. As @tabatkins said on the call, you might use [ ... ]! to force a value to be set. Though this comes with some downsides. As @andruud already pointed out, the current definition allows values starting with a slash, which is probably not something you'd expect. And if the slash is meant to go away when only one value is specified, container names can clash with the container type keywords.

So, either we make the container name obligatory as @andruud wrote and which I support, or we allow to only specify a container type (without leading slash), and clarify in the prose that the container type keywords cannot be used as container names.

Regarding the slash: This could either be solved via the syntax I suggested in the other issue, or by generally implicitly omitting slashes at the beginning or end of values. I actually tend to the latter option. Therefore I created #7200 now.

Sebastian

@mirisuzanne
Copy link
Contributor

To clarify: The current spec only allows you to drop the slash when providing names without types, not the other way around. So the current spec does not have any clash between names and type keywords. That's what leads to the :/ syntax currently, if you want to set only a type.

@SebastianZ
Copy link
Contributor

SebastianZ commented Apr 6, 2022

If you only want to set a type, the syntax currently allows you to skip the name completely but requires the slash. E.g. you'd have to write /size. :/size means the colon is interpreted as the container name and size as the container type. :/ alone is invalid as of the current syntax because if the value contains a slash, it requires to define a container type.

My point is that the current syntax definition is invalid because it allows empty values. The correct syntax definition for allowing both values to be optional would be <'container-name'> [ / <'container-type'> ]? | <'container-type'>. (And if we agree on #7200, the syntax would just be <'container-name'>? / <'container-type'>?).
And that syntax would require to define to exclude container types from names in order to allow specifying only a container type. Otherwise a value like size or style would then be ambiguous.

Anyway, I think we all want the same, drop the question mark from the <'container-name'>, i.e. require a container name in the shorthand. This resolves the issue about empty values being valid and pushes the usage of container names.

Sebastian

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-contain-3] container :/ size, and agreed to the following:

  • RESOLVED: Require the container name in the shorthand syntax
The full IRC log of that discussion <dael> Topic: [css-contain-3] container :/ size
<dael> github: https://github.com//issues/7142
<dael> miriam: Since last week the spec is now up to date with current state of all the syntax changes
<dael> miriam: Only thing missing from ED is CSSOM resolutions. All syntax is up to date which should help with this convo
<dael> miriam: Sebastian pointed out in thread currently it allows the shorthand syntax for container which takes name and type...currently both are optional meaning it could be empty as well as original issue where if only a type have to start with a \
<dael> miriam: Proposal here is we could require a container name in the shorthand
<dael> miriam: since we are right now container type of style is default on every element and heavily encourage names. If you just need a type you can use the longhand
<dael> miriam: That's the prop. Open for questions
<dael> astearns: If I recall correctly have plenty of places where shorthands cannot express every possible value longhands can do
<dael> miriam: And you could express it, but have to explicitly use 'none' in name of syntax
<dael> astearns: Prop: Require the container name in the shorthand syntax
<dael> miriam: Right
<dael> astearns: Any concerns?
<dael> astearns: Obj?
<dael> RESOLVED: Require the container name in the shorthand syntax

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

5 participants