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-view-transitions-2] Specify 'view-transition-class' and corresponding algorithms #9773

Merged
merged 9 commits into from
Jan 31, 2024

Conversation

noamr
Copy link
Collaborator

@noamr noamr commented Jan 8, 2024

A view-transition-class property allows specifying view-transition pseudo-elements that apply to multiple participating elements without having to replicate them.

In this PR:

  • Specifying view-transition-class
  • Extending the pseudo-element syntax to support ::view-transition-foo(name.class)
  • Extending the capture algorithms and data structure to capture the classes
  • Added a simple example for using classes

Based on this resolution.
Closes #8319

Copy link
Member

@khushalsagar khushalsagar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd appreciate @tabatkins's review on this too. I feel like a good chunk of the VT spec is like html, so adding to the existing algorithms is hard.

If there's an existing style CSS specs use for this kind of addition to algorithms between levels of a module, would be good to stay consistent with that.

@@ -206,6 +209,83 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface;
```
</div>

### Example of using 'view-transition-class' ### {#vt-class-example}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding a similar sub-heading above for "cross-document view transitions".

css-view-transitions-2/Overview.bs Outdated Show resolved Hide resolved
css-view-transitions-2/Overview.bs Outdated Show resolved Hide resolved
Animation type: discrete
</pre>

The 'view-transition-class' works alongside 'view-transition-name', and is read at the same time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt that this block had a few redundant statements. A suggestion to reword, lmk what you think:

The 'view-transition-class' can be used to apply the same style rule to multiple [=named view transition pseudo-elements=] which may have a different [=view-transition-name=].

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

css-view-transitions-2/Overview.bs Outdated Show resolved Hide resolved
</pre>

A [=named view transition pseudo-element=] [=selector=] which has one or more <<custom-ident>> values
in its <<pt-class-selector>> would only match an element if the element's [=captured element/class list=]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"would only match an element" is confusing. Can we phrase this as, if the [=captured element/class list=] value in https://www.w3.org/TR/css-view-transitions-1/#viewtransition-named-elements for the pseudo-element's view-transition-name contains all of those values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

css-view-transitions-2/Overview.bs Show resolved Hide resolved
:: a [=/list=] of strings, initially empty.
</dl>

## Additions to capture algorithms: ## {#additions-to-capture-algorithms}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "view transition class algorithms"? Since this section is the set of algorithm changes required for the view-transition-class property.


## Additions to capture algorithms: ## {#additions-to-capture-algorithms}
<div algorithm="capture old classes">
When capturing the old state of the view transition, perform the following step when iterating on |captureElements| given |element| and |capture|:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wdyt about pulling steps 2-10 of this loop into a separate algorithm : https://www.w3.org/TR/css-view-transitions-1/#capture-old-state-algorithm:~:text=in%20the%20algorithm.-,For%20each%20element%20in%20captureElements,-%3A. That's where we're setting everything on the "capture" struct given an "element".

Then this spec, "add the following steps to this algorithm".

</div>

<div algorithm="capture new classes">
When capturing the new state of the view transition, perform the following step when iterating on |captureElements| given |element| and |namedElements|:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the same thing here for this loop: https://www.w3.org/TR/css-view-transitions-1/#capture-new-state-algorithm:~:text=set%20of%20strings.-,For%20each%20element%20of%20every%20element%20that%20is%20connected%2C%20and%20has%20a%20node%20document%20equal%20to%20to%20document%2C%20in%20paint%20order%3A,-If%20any%20flat.

But I also don't want to explode the number of sub-algorithms in the L1 spec. So if you have any other idea to better link to the exact spot that's good to. Another suggestion is, "Do this after step 4.7 of https://www.w3.org/TR/css-view-transitions-1/#capture-the-new-state". If you like that, I'm happy with the same for above as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's monkey-patchy, I don't like that... let's see what Tab has to say.

@noamr
Copy link
Collaborator Author

noamr commented Jan 12, 2024

I'd appreciate @tabatkins's review on this too. I feel like a good chunk of the VT spec is like html, so adding to the existing algorithms is hard.

If there's an existing style CSS specs use for this kind of addition to algorithms between levels of a module, would be good to stay consistent with that.

Yea doing this in a monkey-patchy way didn't feel exactly right. Will wait for Tab with applying the rest of the comments as if we go a different route I'll need to redo a lot of this.

'view-transition-name' is read. But unlike 'view-transition-name', 'view-transition-class' doesn't
have to be unique - an element can have several view-transition classes, and the same 'view-transition-class'
can apply to multiple elements. While 'view-transition-name' is used to match between the element
in the old state with its corresponding element in the new state, 'view-transition-class' is used
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a sentence to say explicitly that view-transition-class on its own does not affect the structure of the view transition pseudo element tree constructed.

I know this is what this says, but perhaps repeating in a different way is worthwhile even if non-normatively

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

:: No class would apply to the [=/element=], and pseudo-elements would have to match its 'view-transition-name'.

: <dfn><<custom-ident>>*</dfn>
:: All of the specified <<custom-ident>> values (apart from <css>none</css>) apply as classes for the [=/element=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should... I don't see how class being auto would be a useful reserved keyword since the class doesn't affect anything we do, just allow more selection

css-view-transitions-2/Overview.bs Show resolved Hide resolved
:: No class would apply to the [=/element=], and pseudo-elements would have to match its 'view-transition-name'.

: <dfn><<custom-ident>>*</dfn>
:: All of the specified <<custom-ident>> values (apart from <css>none</css>) apply as classes for the [=/element=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be defined a bit better I think, using [=captured element/class list=] perhaps, or reference the algorithm below.

noamr and others added 6 commits January 25, 2024 17:42
A `view-transition-class` property allows specifying view-transition pseudo-elements
that apply to multiple participating elements without having to replicate them.

In this PR:
- Specifying `view-transition-class`
- Extending the pseudo-element syntax to support `::view-transition-foo(name.class)`
- Extending the capture algorithms and data structure to capture the classes
- Added a simple example for using classes

Based on [this resolution](w3c#8319 (comment)).
Closes w3c#8319
Co-authored-by: Khushal Sagar <63884798+khushalsagar@users.noreply.github.com>
Co-authored-by: Khushal Sagar <63884798+khushalsagar@users.noreply.github.com>
Co-authored-by: Khushal Sagar <63884798+khushalsagar@users.noreply.github.com>
Copy link
Member

@vmpstr vmpstr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but let's make the changes to make this a diff spec as we agreed in the call today

css-view-transitions-2/Overview.bs Outdated Show resolved Hide resolved
css-view-transitions-2/Overview.bs Outdated Show resolved Hide resolved
css-view-transitions-2/Overview.bs Show resolved Hide resolved
noamr and others added 2 commits January 31, 2024 19:58
Co-authored-by: vmpstr <vmpstr@chromium.org>
Co-authored-by: vmpstr <vmpstr@chromium.org>
@noamr
Copy link
Collaborator Author

noamr commented Jan 31, 2024

This looks good to me, but let's make the changes to make this a diff spec as we agreed in the call today

Already done.

@noamr noamr merged commit 258a87e into w3c:main Jan 31, 2024
1 check passed
@noamr noamr deleted the vt2-classes branch January 31, 2024 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[css-view-transitions-2] Creating 'classes' of transition groups
3 participants