-
Notifications
You must be signed in to change notification settings - Fork 647
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-1] Should view transition names be tree scoped? #10145
Comments
+1 to view transition names being tree-scoped. The implication of the current spec/implementation is that custom elements that are supposed to be encapsulated can animate each other's internals if they know the internal vt-names. |
More work in the implementation but makes sense to me. |
The CSS Working Group just discussed
The full IRC log of that discussion<fantasai> khush: Spec currently traverses all DOM elements, including those in shadow DOM<fantasai> khush: Suggestion is to limit to tree-scoped <TabAtkins> +1 to this <fantasai> khush: so this would exclude things in shadow DOM <fantasai> astearns: I've written some tests for tree-scoping, btw, and interop is terrible <fantasai> khush: Are you talking about other features that are similar? <fantasai> astearns: Yeah, I did @Property rules and @xxx rules, which are supposed to be tree-scoped, and they're not handled correctly <astearns> zakim, open queue <Zakim> ok, astearns, the speaker queue is open <fantasai> astearns: so probably some other things are also broken <emilio> fantasai: two things, khush does this mean that for v-t that you'll never be doing transitions on anything in the shadow dom? <emilio> khush: yeah this means that you couldn't use something inside the shadow dom as an independent thing <kizu> q+ <emilio> ... scoped transitions would allow you to do that <emilio> ... but only as a unit inside the DOM <emilio> fantasai: to alan, there's two different kinds of scoping, at-rules and name defining <emilio> ... those are quite different mechanism <astearns> ack kizu <fantasai> s/defining/defining on elements/ <emilio> ... I'd expect less issues with things like timeline scopes and so on <fantasai> kizu: Wrt other scoping things, we also have timeline-scope and anchor-name <fantasai> kizu: Might want to have a way to expose these names, since might have cases where you want to expose <khush> q+ <fantasai> kizu: maybe this is a more general issue that we need to handle in CSS, and have it work consistently <astearns> ack khush <TabAtkins> +1, we need a general mechanic for weakening shadow encapsulation <TabAtkins> But we should be consistent for now <fantasai> khush: kizu's point reminds me of ::part(), could use CSS to expose names from within shadow DOM outside it <fantasai> khush: maybe if you expose the element could allow it to match <fantasai> khush: could make it more complicated to implement <fantasai> khush: we already have things in the platform that allow such information, in those cases do you want naming in CSS to also be exposed? <fantasai> astearns: That would be a separate feature, though. This is for default behavior for view transitions <fantasai> khush: Does it make sense to make this resolution cover other cases also? Have a general principle <fantasai> astearns: I think a separate issue on general mechanism, because we do want to consider them all <fantasai> astearns: don't want to have a specific escape route just for VT <TabAtkins> Agree, separate issues <fantasai> RESOLVED: view-transition-name lookup is tree-scoped |
https://bugs.webkit.org/show_bug.cgi?id=273883 rdar://127995859 Reviewed by Antti Koivisto. Corresponding spec issue: w3c/csswg-drafts#10145 Corresponding spec PR: w3c/csswg-drafts#10306 This ignores view-transition names from shadow DOM, given the pseudo-elements are linked to the document element, exposing shadow DOM names to the view transition pseudo-elements would be a violation of shadow DOM principles. We don't check for the tree scope directly in `forEachRendererInPaintOrder` because a shadow DOM element might have flat tree descendants that have an outer tree scope. We also use the `Styleable`'s element instead of `RenderElement`'s element because for pseudo-elements we want to consider the associated element's tree scope. * LayoutTests/TestExpectations: * LayoutTests/platform/glib/TestExpectations: * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::captureOldState): (WebCore::ViewTransition::captureNewState): Canonical link: https://commits.webkit.org/278704@main
View-transition names have been working seamlessly inside shadow trees until now. After updating Chrome to version 126, view-transition-name no longer pierces the shadow tree. |
Perhaps we should follow Scroll Timeline example here which treats everything as a flat tree:
In ViewTransitions, we limit |
I do not think we should violate shadow DOM principles by exposing shadow DOM names to the root element (in the light DOM). Having a way to run transitions outside of the root element seems like a better solution to the problem @shonya3 is facing. |
I'm not sure I understand the distinction. Transitions can run outside of the root element, but they can't reference things within the shadow DOM. Running a transition within the shadow DOM would only solve some cases, but not all of them. For example, icon to detailed view morph wouldn't be possible if the icon is a part of a custom web component. |
If |
I think this discussion transcends View Transitions and touches all CSS properties that define a name: In general I think we should respect the aspect that these names don’t leak out of shadow roots. However, in some cases authors would explicitly want this, e.g. websites where they have a big tree of Web Components. (For context: the re-opening of this issue was sparked by this report from an author in which they let us know that the View Transitions from the overview to the detail page on their site are no longer working in Chrome 126. Culprit is the In #8915 (comment) there was talk of breaking style containment into two parts: containing names and all the rest. I would love to see a solution where names are contained by default – to not break one of the premises of using shadow DOM – but that there is a way to make the names visible only after the author opts-in (and only for open shadowroots). This “contain by default” could be part of the UA stylesheet – e.g. |
My understanding from @bramus is also that To support cases that were broken by this change, we'd like to have some way of loosening this restriction, whether it's with |
If you end up with such a component, the components are probably poorly separated in the first place. To me, it sounds like scoped element transitions would be the better solution to this problem. |
That doesn’t work for websites with a tree of custom elements. In https://divicards-site.pages.dev/ for example the author has a Even if they did set that
|
Before jumping to the solutions, I want to see how far |
^ that is the question on this issue: #10303. And reading through the discussion here, I also feel like |
This is worth discussing in the context of all named references for CSS in general, added #10304 to the agenda with more context for it. |
One of our public-facing apps at Microsoft was also broken by this change. Our app is built entirely with web components; a tree of web components with shadow roots. In our case, when the user navigates our SPA, we want to view transition everything except that header and footer. So we assigned those a separate We'd love a way to say, "this web component has a different view-transition-name" so that it can remain in place during view transitions. I don't feel strongly about any of the proposals so far. So long as we can do what we did before: give elements different view-transition-names so they don't take part in view transition. |
So if I understand correctly, the issue here was not about mixing light-DOM elements with shadow-DOM elements, but rather that all the elements participating were inside the shadow DOM, and now this can't work? |
Correct, it's not about mixing light DOM with shadow DOM. Our whole app is built with web components, so it's all shadow DOM. And the issue is that, we don't want our header and footer to take part in view transition. Since header and footer don't change, we want them to stay in place. We accomplished this by setting You can see it for yourself in this minimal repo. Or you can see it on our live public-facing site, apps.microsoft.com - just click a link and observe that the header and footer participate in the view transition. We want a way to say, "This element shouldn't participate in the view transition", even if that element is inside a shadow root. |
But you’re calling |
I think we should find a way where the pseudos are also defined in the shadow tree, but we have to deal with edge cases like |
Thanks @JudahGabriel! We're taking this breakage very seriously and we'll find a solution. @shonya3 does your use case involve sharing view-transition-names across shadow boundaries, or it's like this comment, where the captured element is encapsulated within a shadow DOM? |
Gotcha. I see that you're using shadow parts already in the first example? Do you think enabling |
If I may mention my own use case here: my team is working on an app built entirely with web components, so we have nested components with shadow roots. Chrome 126 broke our View Transitions. Enabling That sounds very impractical. A solution like a CSS attribute, as mentioned here, looks like it would help us a lot more in that case. |
Thanks for sharing! Do your transitions cross shadow boundaries (an element in a shadow transitions into an element in another shadow/light DOM etc) or it's more like wanting transitions within the same shadow root to work? |
The transitions cross shadow boundaries. However, there is still a "transition container" element that contains every transitioned element within its shadow root (or its children's shadow roots, or their children's, etc). I feel like The main pain point remains that we would also have to modify the whole chain of ancestor elements, from the "transition container" all the way up to the root of the document, to use And it would make it harder to move the |
@Remiscan gotcha. So sounds like in the long run, something like a combination of Thanks for sharing in detail, we're discussing a few options internally and want to understand the use cases better. |
Summarizing internal conversation about this: We're hitting the same snag that we've hit in anchor positioning and view-timeline, where to support In other specs the current direction is to have these names operate on the flat tree, and scope them using a separate property (e.g. In any case, we should be consistent here and in anchor-positining/scroll-driven positioning. Proposing that if anchor positioning and scroll-driven animations don't change course to use shadow encapsulation, including figuring out the "where did the style come from" issue, then we revert the view-transition change so that view-transition-names are in the flat tree to be consistent, and introduce |
Enabling view-transition-name for ::part looks logical and natural to me. But the main concern for me is whether it will work without much hassle. |
No, anchor positioning is tree-scoped, and there's no plan to change it. I think the issue and proposal in #10325 was just misunderstood. The anchor-scope property does not replace tree-scoping, it's just an additional mechanism on top of it. Scroll-driven animations however is more like what you describe. For some reason I never understood, timeline names are not tree-scoped. #8135. But I'm not sure we should draw inspiration from this, we might want to fix SDA instead. |
Thanks @andruud, this makes sense. So to clarify, the resolution here is fine and matches anchor positioning, and should also support things like putting a |
… being name-based. The previous fix for using the tree context made it inconsistent with how shadow DOM styling works for things like anchor positioning, and made it so that e.g. `::part` cannot set a `view-transition-name`. Refactored to make the `view-transition-name` a tree-scoped name, rather than check the tree context of the element. Closes w3c#10145
@shonya3 @Remiscan @JudahGabriel as of Chrome M127 you should be able to give a |
Just implemented my first try with new approach.
p-card::part(card),
p-home::part(card) {
view-transition-name: card;
}
this.paginated.map(card => {
return html`<li>
${card === this.activeCard
? html`<e-card-with-sources
.name=${card}
.divcordTable=${this.divcordTable}
.cardSize=${this.cardSize}
.sourceSize=${this.sourceSize}
@navigate=${() => this.#setActiveCard(card)}
part="card"
></e-card-with-sources>`
: html`<e-card-with-sources
.name=${card}
.divcordTable=${this.divcordTable}
.cardSize=${this.cardSize}
.sourceSize=${this.sourceSize}
@navigate=${() => this.#setActiveCard(card)}
></e-card-with-sources>`}
</li>`;
}); And there is activeCard?:string global state to set it on p-home render and update on navigate(emits by divination card element on any inner link element click). |
The spec currently traverses all DOM elements to look for view transition names, see text here. This should probably be tree scoped similar to other CSS naming conventions like scroll timeline and I'm guessing anchor positioning (@tabatkins ?).
@nt1m fyi.
The text was updated successfully, but these errors were encountered: