w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.38k stars 643 forks source link

[scroll-animations-1] `scroll-timeline` and `view-timeline` shorthand syntax #7627

Closed bramus closed 1 year ago

bramus commented 1 year ago

The syntax for the scroll-timeline shorthand is defined as such:

scroll-timeline: <'scroll-timeline-axis'> || <'scroll-timeline-name'>

The syntax for the view-timeline shorthand is defined as such:

view-timeline: [<'view-timeline-name'> || <'view-timeline-axis'>]#

It’s odd that the scroll-timeline has the name mentioned first, while in view-timeline it is the axis. Even though the order of both does not matter – due to || – I would like to see these listed in the same order for consistency reasons.


Going further, maybe we should disallow reordering the values? There is precedent to have the *-name first: the container shorthand has the container-name first and then the container-type.

container: <'container-name'> [ / <'container-type'> ]?

Maybe we can update the scroll-timeline and view-timeline shorthands to follow the form of the container shorthand? Like so:

scroll-timeline: <'scroll-timeline-name'> [ / <'scroll-timeline-axis'> ]?
view-timeline: [<'view-timeline-name'> [ / <'view-timeline-axis'> ]?]#
andruud commented 1 year ago

Yeah, it's weird that the order is reversed. Is that intentional @fantasai?

Especially since we don't disallow block/inline/etc from <custom-ident>, so:

See also https://github.com/w3c/csswg-drafts/pull/7694 (which doesn't address the ordering, but it's related).

fantasai commented 1 year ago

Pretty sure that's not intentional, I agree we should fix it to be consistent.

fantasai commented 1 year ago

@bramus Flipped ordering of scroll-timeline as you suggested. I agree we should discuss fixing the ordering, since reorderability is creating an unfortunate parsing ambiguity here. :(

cdoublev commented 1 year ago

Going further, maybe we should disallow reordering the values? There is precedent to have the *-name first: the container shorthand has the container-name first and then the container-type.

animation has animation-name last (conflict with animation-fill-mode for none).


See also #7694 (which doesn't address the ordering, but it's related).

I presume that the topic related to this PR is about whether none can repeat. Some context: none can repeat in animation for animation-name (but the <custom-ident> for @keyframes prelude excludes the none keyword) but it cannot in transition for transition-property (there is a minor issue related to how this is enforced).

The current view-timeline-name syntax does not allow repeating none but the current view-timeline syntax does. There is also:

If view-timeline-name has more names than view-timeline-axis has specified axes, the excess timelines use the last view-timeline-axis value. If view-timeline-name has fewer names than view-timeline-axis has specified axes, the used view-timeline-axis list is truncated.

If this difference is unintentional and none should not repeat in view-timeline AND view-timeline-name, it seems simpler and clearer to enforce it in prose rather than in the syntax (cf. my suggestion in the linked issue related to transition).

cdoublev commented 1 year ago

After reading this thread, I wonder if none could not be further restricted to be used alone in transition and view-timeline.

First, animation-name: none is basically a special value for disabling an animation or set of animations; there's really no reason to use it with a list, only on its own, since when animation-name is none all the other properties are irrelevant.

Would there be a reason to use transition: none 1s; or view-timeline: none inline?

animation: none 1s would still have to be valid because none must be assigned to animation-fill-mode.

fantasai commented 1 year ago

Agenda+ to discuss if we want to address the parsing ambiguity by fixing the order of arguments, or something else.

The proposal is to have:

scroll/animation-timeline: <name> <axis>?

rather than

scroll/animation-timeline: <name> || <axis>
css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [scroll-animations-1] `scroll-timeline` and `view-timeline` shorthand syntax, and agreed to the following:

The full IRC log of that discussion <dael> Rossen_: Who can summerize?
<fantasai> -> https://github.com/w3c/csswg-drafts/issues/7627
<dael> bramus: I can. In the spec there's a difference between scroll and view timeline argument order.
<dael> bramus: Proposal was to have them use same syntax
<fantasai> scroll/animation-timeline: <name> || <axis>
<fantasai> scroll/animation-timeline: <name> <axis>?
<dael> fantasai: Other point you raised is timeline name and axis are in same namespace. I put this on agenda to ask WG if we shoudl require an order and require name always? We could change the current syntax
<bramus> q+
<dael> fantasai: Other ways to disambig but asking if this is what we want to do or something different
<Rossen_> ack bramus
<dael> bramus: One question on prop new is there is no slash separator but container queries requires one after the name. Seems like a difference
<dael> miriam: Likely b/c you can have multiple names
<bramus> q-
<dael> bramus: Don't think we want to allow multiple names on scroll timeline, do we?
<dael> fantasai: Do it with a comma sep syntax and axises are listed with name. Container property doesn't have that; doesn't tie two properties with lists. Only one type of contianment
<dael> fantasai: Here we have a list of timelines that have their own properties
<dael> flackr: I'm in favor of fixing the order
<bramus> +1
<dael> Rossen_: Any opposing opinions?
<dael> Rossen_: Prop: Have parsing with fixed order of name and axis
<dael> Rossen_: Objections?
<dael> RESOLVED: Have parsing with fixed order of name and axis
<dael> Rossen_: fantasai anything else on this?
<dael> fantasai: nope

Official Minutes

fantasai commented 1 year ago

Fixed in https://github.com/w3c/csswg-drafts/commit/c57ee4f2be513edcb89974c0f861578d7b13c551