Discussion: replace bracket syntax used in property names

jonathantneal commented 2 years ago

This issue is to discuss whether it would be possible and best for corset to avoid the bracket syntax used in CSS property names. The brackets would be avoided to follow the CSS Syntax rules for consuming a list of declarations, which limits property names to identifiers.

Current Syntax

::part(increment) {
  event[click]: var(--inc);
}

Possible Solutions

An event-set property to add a given event type with a given event listener.

::part(increment) {
  event-set: click var(--inc);
}

Questions:

Would it replace the event listener of an existing event type?
Or would it add a new event listener to the given event type?
How is this handled currently?

matthewp commented 2 years ago

Can you explain how this new event-set property is meant to relate to event? Does it mean to replace it or is this a separate new property?

To answer one question:

How is this handled currently?

Currently if you use event without a key (the bracket syntax) it replaces all event listeners on the element (all that were set via Corset, that is).

If you use event[type] with the key it replaces the event for that type. You cannot currently have multiple listeners for the same type. That would be a desirable feature to have.

To your questions I would add:

Would it replace all event listeners on the element.

The reason why the bracket syntax was added was because I believed that declarations were always destructive, meaning they replaced all previous declarations of that property. If that's not the case then it changes the reason for the syntax.

I want to understand this better so I'm going to play around with counter-reset to see if it is additive or destructive. If there are other similar properties in CSS please let me know.

matthewp commented 2 years ago

Ok, at least for counter-reset, it appears to be purely additive. I couldn't cause it to interfere with other counters: https://codepen.io/matthewp/pen/qBpONKw. So that's a good reason to pursue changing the way that event, attr, class-toggle, prop, and data work.

On Twitter you suggested this syntax for explicitly removing existing listeners, which I like:

button {
  event: * unset;
}

In Corset, what should unset mean generically? Should it mean to remove the thing or should it mean to reset it to its original value? For example, if a class exists in the original HTML before Corset runs, should unset add the class back when unset it used? How should this relate to a potential initial?

As an example:

<main class="app"></main>

main {
  class-toggle: app false;
}

main.condition {
  class-toggle: app unset;
}

Should the app class be added back in this situation?

matthewp commented 2 years ago

In order to support multiple events of the same type, I think there needs to be a way to name an event. I want to preserve this ability:

button {
  event: click var(--close);
}

.modal button {
  event-capture: click true;
}

The above is modifying the same event listener in another selector. Another use-case is overriding a listener itself:

button {
  event: click var(--close);
}

button.open {
  event: click var(--open);
}

This is not meaning to add a second event listener, but rather to replace the previous one.

I think we can get the best of both worlds by allowing a listener to be named similar to how a grid line can be named.

button {
  event: [close] click var(--close), [second] click var(--second);
}

.modal button {
  event-capture: [close] click true;
}

In this way we can modify the [close] named event without modifying another of the same type.

The [name] syntax should be optional; if you omit a name then that becomes the default listener for that type. Most of the time name shouldn't be needed, but it would be very useful for mixins.

brunostasse commented 2 years ago

I think the grid line naming convention fits great here.

Although the counter() held value is additive, it is important to note that the value is not held directly by a property, it is accessed through a function. To my knowledge, all CSS property declarations are indeed destructive, and it would be good to respect that. Note also that the counter model is pretty much an exception, not widely used, and doesn't feel very intuitive in CSS land.

I think such comma-separated list of shorthand property values is only used in one place currently in CSS: for the transition property. It is interesting to note that with this syntax, if you do this:

.element {
  transition-property: opacity, left, top, height;
  transition-duration: 3s, 5s;
}

It is treated like this:

.element {
  transition-property: opacity, left, top, height;
  transition-duration: 3s, 5s, 3s, 5s;
}

I.e. the values are repeated as necessary. You can read about it here and here. So in your last example, because you have only set the event-capture value for one listener, that value should be repeated for the others.

However, you actually forked the model here. The way you set the value of event-capture defers from the way it would be set for the transition CSS property: you used the identifier of the listener (actually you used both identifiers: the custom name (close) and the type (click)). Therefore, it wouldn't make sense for the value to be repeated for the other listeners, because it targets precisely this one. I think such syntax is relevant, and that in that case the values of the other listeners should not be changed.

The model could be: to modify a value with longhand property, you have to use the identifier of that value. The order doesn't matter, and what is not declared remains untouched.

Now, this only allows to update the values of an existing listener, not to add new ones or remove some without touching the other. Here are some ideas to further circumvent the destructivity of declarations and allow that:

Allow a `current` value for all multi-binding properties:

I know of two ways in CSS to capture and use the current value of a thing. The attr() function, which allows to get the value of an attribute of the element, and the currentColor value, used to get the current value of the color CSS property for the element.

Both concept could be used to preserve the current value of a list of values. I propose using a current value to get the list of current values of the property. Combined with other values, it can be used to add/update/remove only specific values in a list. Examples:

Add a listener to existing listeners:

.element {
  event: click var(--onclick), touchstart var(--ontouchstart);
}
.element.active {
  event: current, touchend var(--ontouchend);
}

Update a listener value:

.element {
  event: click var(--onclick), touchstart var(--ontouchstart), touchmove var(--ontouchmove);
}
.element.active {
  event: current, click var(--increment) true false true;
}

Allow an `initial` value:

With a listener identifier, using the initial value would reset the matching listener to their initial spec value, i.e. nothing, causing it to be removed. This value could be used with any property.

Remove the increment listener:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event: current, [increment] initial;
}

Remove all click listeners:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event: current, click initial;
}

Allow an `all` value:

Just like for the transition CSS property, the all value could be used to set values for all possible listeners (in CSS it's all the possible properties). This seems more relevant than *, which is only used in selectors.

Set common values for all listener:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event-signal: all var(--signal);
}

Remove a certain value for all listeners:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event-signal: all initial;
}

Remove all listeners:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event: all initial;
}

Allow a `none` value:

The none value set the value of a property to nothing, so this can be used to remove all listeners, just like all initial does.

Remove all listeners:

.element {
  event: [increment] click var(--increment), [display] click var(--display);
}
.element.active {
  event: none;
}

I have mainly talked about listeners here, but this would apply to class, attr, prop and data as well.

I think these additions cover pretty much everything one might want to do with multi-binding properties?

matthewp commented 2 years ago

@brunostasse Very open to all of these ideas! initial and all have been discussed in the past, so definitely on board with those. The nice thing about unset over none is that unset will likely have value in other properties. I'd rather have 1 globalish value than several, and I think the * unset syntax makes a lot of sense.

brunostasse commented 2 years ago

@matthewp When you say unset will likely have value in other properties are you referring to attributes for instance, which would revert back to the value set in the original HTML? Because that wouldn't be quite the effect of unset in CSS.

The global values in CSS (which can be used with any property) are:

initial refers to the value for the property as defined in the specification.
inherit refers to the value for the property as defined in the parent element.
unset refers to the value for the property as defined in the parent element if the property is naturally inherited as defined in the specification (that concerns just a few properties, mostly related to text styling), otherwise it refers to the value for the property as defined in the specification (i.e. it's equivalent to inherit if it inherits naturally, otherwise initial).
revert is the same as unset, but if the value is not naturally inherited, it refers to the value for the property as defined by the user-agent (i.e. the browser).
revert-layer, is used in the context of CSS layers (which is new) and from what I understand it refers either to the value set for the property in the previous layer, or if there is no previous layer, to the value in the current layer, and if there is none, to the value set in the previous style-origin (so probably either the user styles or the browser styles).

none is not a global value, it can only be used with property accepting it (like display or pointer-events) and has a special meaning with each property, but basically it always makes its effect null.

So, since no event, attribute, prop, etc, is inherited from the parent element (right?) initial and unset would behave similarly in Corset. You could possibly use revert to revert to the value set in the original HTML, but that wouldn't be quite exact either. The most relevant might be to create a revert-sheet value, which reverts back the current sheet and therefore goes back to the values set before that, that is in the original HTML.

Do you affect a different meaning to * and all? In CSS * refers to all elements in a selector, and all refers to all properties in a property value.

matthewp commented 2 years ago

Hey @brunostasse sorry for taking so long to get back and thanks for the detailed thoughts. I had created another issue for initial: https://github.com/matthewp/corset/issues/65. The thinking there was more around like reseting a text property, but I suppose it really is the same as what we are talking about here. Do you think initial is a better use of what we're describing here? If not I think I might stick with the unset idea.

As far as * vs. all vs something else, still very much in the air. I'm only aware of all being used as a property, which is not what we're discussing here. Is there a use of all as a value in CSS? If not, would it still make sense to make one here?

brunostasse commented 2 years ago

Hi @matthewp, so yeah:

initial fits better here. initial is straightforward, it always refers to the spec value, and can be used safely to reset the value of a property. unset is more complicated, because it can refer to either the parent element value, or the spec value. It doesn't make much sense in Corset's context, where values are not inherited from parent elements.
all also fits better. all can be used as a value in the transition-property CSS property (see here), and refers to all properties that can be transitioned. So it makes total sense to use it here, much more than * which is only used in selectors.

So you want:

button {
  event: all initial;
}

brunostasse commented 2 years ago

I have seen in your blog post about v2.0 that you're considering making properties additive. I have to say it's a big paradigm shift, and it might confuse people coming from CSS. Having to first reset a property value and then set its new value to replace the original one also feels a bit cumbersome.

button {
  event: click var(--increment);
}

button.disabled {
  event: all initial;
  event: click var(--alert);
}

If I may, I have an alternate proposal to replace the bracket syntax that might fit better the CSS paradigm. The idea is to use the nested longhand property syntax from Sass and PostCSS, like this:

button {
  event: {
    click: var(--increment);
    mouseenter: var(--display-stuff);
  }
}

button.disabled {
  event: {
    click: var(--alert);
  }
}

This syntax is not valid CSS, but feels close, and is supported by all tooling that works with both Sass and PostCSS. Properties remain destructive, so you stay in the CSS paradigm, but you can easily add a new one or replace an existing one without touching the others. You can also remove all grouped properties with event: all initial. Each property also have its own space, contrary to the current multi-binding syntax, which gives more freedom for the syntax of the value.

This can be used for attr, event, prop, data...!

matthewp commented 2 years ago

Hey @brunostasse, by additive I don't mean that it adds an extra event like your example implies.

button {
  event: click var(--increment);
}

button.disabled {
  event: click var(--alert);
}

This will replace the click event, not add a second one. This is because click becomes the key. In Corset 1.0 you cannot have 2 event listeners at all, in 2.0 you will be able to using named events which act as the key.

button {
  event: [one] click var(--increment);
  event: [two] click var(--alert);
}

This would add two click events, because you have given them distinct names. You could think later reset a single one:

button {
  event: [one] click var(--increment);
  event: [two] click var(--alert);
}

button.disabled {
  event: [two] unset;
}

This would remove the [two] listener but leave [one] untouched.

To explain what I mean by additive it's best to look at other properties like class-toggle and attr. Event is a bit different because it's the only one where it's possible to have multiple of the same key (you can't have multiple attributes of the same name, for ex).

button {
  class-toggle: one true;
  class-toggle: two true;
}

In Corset 1.0 this only adds the two class. That's because every time Corset sees a class-toggle declaration it treats the list as the only classes that should exist on the element. In 2.0 it will have one and two be set. You can then remove them via unset (or initial, naming tbh).

Thanks also for the perspective on unset vs. initial. I'll think more about this and respond later. I might create a spreadsheet showing how unset / initial work in CSS to compare how they might work in Corset, to better show the similarities and differences.

brunostasse commented 2 years ago

Oh alright, so the additivity is only for the values accepting the custom-key-value syntax.

So, in this case:

button {
  event: [one] click var(--increment);
  event: [two] click var(--alert);
}

button.enabled {
  event: focus var(--handle-focus);
}

button.disabled {
  event: [three] mouseenter var(--handle-enter), [four] mouseleave var(--handle-leave);
}

When button.enabled is matched, the two click handlers are removed from the element, and the focus handler is added?
When button.disabled is matched, the two click handlers are preserved, and the mouseenter and mouseleave handlers are added?

matthewp commented 2 years ago

No,[one] and [two] are only ever removed if their selector no longer matches or if you added an unset/initial declaration.

You can think about the [] syntax as a key in Map. If you don't provide a [] key then there is an implicit key that is the event type. So you can pretend that it's written like this: [focus] focus var(--handle-focus). There's always an implicit key.

So the same rules apply whether you provide an explicit key (tentatively called a named binding) or not. You can only ever use focus once, unless you provide a key.

Events are the only property that will use this feature. The [] syntax will likely be turned off for class-toggle, attr, prop, and data. That's because those properties can never have multiple of the same keys. You can't add the disabled class more than once; it's either on or off. Likewise for attributes, etc.

So it might be easier to understand the behavior if you think about these properties first and think about events only after you understand, for ex. class-toggle.

To illustrate, take this example:

button {
  class-toggle: one true;
}

button[disabled] {
  class-toggle: two true;
}

In Corset 1.0 only the two class will be on. That's because the class-toggle property is destructive. It only applies the classes in that exact declaration.

In Corset 2.0 both the one and two classes will be on. I think this is the behavior you want most of the time.

matthewp commented 2 years ago

This feature is down to 1 skipped test. Can see the branch here: https://github.com/matthewp/corset/compare/labeled-events?expand=1

Naming is still very much up for discussion. I think the behavior as described here is the way to go. Will follow up on naming.

matthewp commented 2 years ago

I should say, everything is open to discussion, but the branch implements the additive behavior discussed in this thread.

@brunostasse brings up an interesting idea for continuing the destructive behavior but adding a current as a way to keep previous bindings: https://github.com/matthewp/corset/issues/130#issuecomment-1113855189

Curious @jonathantneal your thoughts on this idea.

My instinct is that Corset, unlike CSS, is mostly made up of properties that contain multiple values. class-toggle, attr, and event are properties you use all of the time. Unlike in CSS where you mostly use single-value properties and the multi-value ones are the exception.

So I say this to say, it feels like it is ok for Corset to deviate in this respect. But obviously I did feel differently before when I made them be initially destructive. So I appreciate all thoughts on this.

matthewp commented 2 years ago

On initial vs unset. In Corset, the only time we get a value from a parent is for vars. So unset will always mean the same as initial.

One could see some value in having unset-like behavior in the future though:

body {
  --role: "User";
}

.admin {
  --role: "Admin";
}

.admin.revoked {
  --role: unset;
}

Given that, does it make more sense to:

Add this feature as initial and leave unset for a future feature.
Add both keywords
Just go with unset

I don't think I will implement the unset for vars right away. I think it would be nice to support but might take some work. But I could be wrong and it's not so bar. So I'm leaning towards either (1) or (3) here.

matthewp commented 2 years ago

Actually I think I lean towards initial now. Since I don't plan on implementing inherit it doesn't make sense to use unset which people would expect inherit behavior.

matthewp commented 2 years ago

Draft PR: https://github.com/matthewp/corset/pull/134

matthewp commented 2 years ago

Discussed more with @brunostasse offline and this really isn't initial behavior because it doesn't remove non-sheet defined values. For now I think we're going to go with revert-sheet, similar to revert-layer in CSS.

button {
  class-toggle: one true;
}

button[disabled] {
  class-toggle: all revert-sheet;
}

brunostasse commented 2 years ago

`current` value:

After giving it more thoughts, it appears to me that the current value would be inconvenient and error-prone.

Because the selector matching mechanism already provides a way to remove event listeners/attributes/props, it seems likely that users will more often want to add ones than replace those already in place. So users would have to preprend most of their declarations with current , which would be cumbersome, and worse, might cause bugs when they forget it.

So I think it's better to discard that idea.

The additivity of the new syntax:

Comparison between the additive new syntax and the proposed nested property syntax:

With your additional explanations I realised that the nested property syntax I proposed would actually behave the same way as the new additive one @matthewp.

input {
  attr: {
    type: "number";
    name: "savings";
  }
  event: {
    click: var(--alert);
  }
}

input.important {
  attr: {
    required: true;
  }
  event: {
    click: [whatever] var(--increment);
  }
}

Would behave the same as:

input {
  attr:
    type "number",
    name "savings";
  event: click var(--alert);
}

input.important {
  attr: required true;
  event: [whatever] click var(--increment);
}

So we can actually see the second syntax as a terser version of the first one.

The new additive syntax expanded for demonstration:

It also really behave as the current bracket syntax, and another way to see and understand it is by expanding it to individual CSS properties, like so (this is just a "demonstration" syntax):

input {
  attr_type: "number";
  attr_name: "savings";
  event_click: var(--alert);
}

input.important {
  attr_required: true;
  event_click_[whatever]: var(--increment);
}

With such "expanded" syntax, it is quite obvious (from a CSS standpoint) to understand why updating the value of one attribute or event won't affect the others.

We can also see how the all value works. This:

button {
  event: all initial;
}

button.important {
  event: all click initial;
}

Could be written as:

button {
  event_all: initial;
}

button.important {
  event_click_all: initial;
}

So the "additivity" in the new syntax is actually quite understandable from a CSS point of view when seen like that.

The new additive syntax as an improvement over CSS, that fits right in:

One way to present this could be as a "correction", or "improvement", of the CSS multi-value syntax (used for the transition and animation properties), which is often tricky to use because of its destructivity.

However, as you pointed out elsewhere, it doesn't even have to be considered as a "correction" or "deviation" from CSS, because additivity is also made possible in CSS with the animation-composition value (https://drafts.csswg.org/css-animations-2/#propdef-animation-composition)!

The default animation-composition value is replace (destructive), but it can be changed to add (additive). I think it could be totally acceptable to say that, in Corset, all multi-value properties have add as *-composition default value.

All that to say that I think the new syntax with "additive" behaviour can fit alright in the CSS paradigm, and seems like the right choice considering Corset's needs.

bramus commented 2 years ago

Also see https://github.com/w3c/csswg-drafts/issues/1594 which talks about Additive Cascade. No progress or consensus in it right now, but maybe worth a read.

matthewp commented 2 years ago

Maybe it would make sense to support the keywords add and replace, but default to add for these multi-value properties. Although I can't think of a reason why you'd use replace so maybe that only makes sense if it's the default.

matthewp commented 2 years ago

Additive behavior was merged in https://github.com/matthewp/corset/pull/134. It will likely be a while before 2.0 is released so plenty of time to change things if needed.

matthewp commented 2 years ago

@brunostasse and I went back and forth about the naming of the unset property offline and we feel like revert-sheet is the best match to what this is actually going to do. It's not going to unset attributes / classes, etc. that were not added by the sheet, so unset felt wrong in this case.

matthewp commented 2 years ago

2.0 is released: https://github.com/matthewp/corset/releases/tag/v2.0.0

matthewp / corset