-
Notifications
You must be signed in to change notification settings - Fork 747
[css-view-transitions-1] Resolve open issues. #8948
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
Conversation
This change fixes the following issues inlined into the spec: * The link for "paint order" doesn’t seem right. Is there a more canonical definition? The DOM order of `::view-transition-group` pseudo-elements needs to match the paint order of the corresponding DOM elements. This is why elements are traversed in paint order when adding them to named elements. The definition of paint order used here is correct. * Isolation and the dynamic setting of blending is only necessary... This was resolved in w3c#7814. * Refactor this so the algorithm takes a set of elements that will be captured... This was a problem because there was no way to refer to an element participating in a transition. Presence of view-transition-name was not enough. Now there's a bool on element to refer to this state. * Specify what happens with mix-blend-mode. Adds spec text to clarify this. The issues remaining are related to web animations, will take those up in a follow up PR.
css-view-transitions-1/Overview.bs
Outdated
Issue: Specify what happens with 'mix-blend-mode'. | ||
|
||
* 'mix-blend-mode' on the element is ignored during the capture. | ||
|
||
Note: 'mix-blend-mode' defines how the element is composited into its parent stacking context. | ||
Since the element skips painting into its stacking context, as defined above, 'mix-blend-mode' is a no-op. | ||
The blending used to composite the image is defined by the 'mix-blend-mode' on the replaced pseudo-element drawing this image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should get its own commit. I also don't think I understand what it's saying...
Let's say I have a view transition capturing the root and element foo, and foo is semi-transparent with a non-normal mix-blend-mode
. And let's say I define a very basic animation, e.g. sliding the old root out to the right and element foo out to the left to reveal the new document underneath.
Wouldn't ignoring mix-blend-mode
on the element mean that as soon as the transition starts, the blending of foo against its background would change, resulting in a visible change in the color of foo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Let me take this in a follow up. Lmk if you think we should resolve on an issue based on the clarification below.
Wouldn't ignoring mix-blend-mode on the element mean that as soon as the transition starts, the blending of foo against its background would change, resulting in a visible change in the color of foo?
You're right about the change in blending here. We could make it work by copying over the mix-blend-mode
from the element to the replaced pseudo where the image will be drawn. But it only works if the element's closest ancestor that is also being captured into an image is the root. For example, if you have a tree like this:
root
|_B mix-blend-mode: multiply;
|_C mix-blend-mode: multiply;
The blending order in the DOM is : Blend(root, Blend(B, C). If B and C are being captured for a transition, then the pseudo tree is:
::view-transition-group(root)
|_::view-transition-image-pair(root)
|_::view-transition-old(root)
|_::view-transition-new(root)
::view-transition-group(B)
|_::view-transition-image-pair(B)
|_::view-transition-old(B)
|_::view-transition-new(B)
::view-transition-group(C)
|_::view-transition-image-pair(C)
|_::view-transition-old(C)
|_::view-transition-new(C)
root, B and C become siblings so the blending order is Blend(Blend(root, B), C) which is not the same as what's in the DOM.
We have the same problem with a bunch of other properties that depend on the DOM's hierarchy. For example, as soon as the transition starts you'll stop being clipped by an ancestor which is participating in a transition. Nested transition groups is the principled fix. Authors would opt into this mode in which case group pseudo for C will be nested inside group pseudo for B which itself will be nested inside group pseudo for root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you're saying makes sense, but I'm not clear on what the right way forward is here. :) So let's open a new issue with your explanation and go from there. My first impression is that honoring mix-blend-mode on the pseudo-element is both the right thing to do here and also the right thing to do in the nested case, i.e. they should be consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Issue is here: #8962.
This change fixes the following issues inlined into the spec:
The DOM order of
::view-transition-group
pseudo-elements needs to match the paint order of the corresponding DOM elements. This is why elements are traversed in paint order when adding them to named elements. The definition of paint order used here is correct.This was resolved in #7814.
This was a problem because there was no way to refer to an element participating in a transition. Presence of view-transition-name was not enough. Now there's a bool on element to refer to this state.
Adds spec text to clarify this.
The issues remaining are related to web animations, will take those up in a follow up PR.