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-1] Where are the "named elements" map cleaned up? #9669

Closed
nt1m opened this issue Dec 2, 2023 · 11 comments · Fixed by #9769
Closed

[css-view-transitions-1] Where are the "named elements" map cleaned up? #9669

nt1m opened this issue Dec 2, 2023 · 11 comments · Fixed by #9769
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-view-transitions-1 View Transitions; Bugs only

Comments

@nt1m
Copy link
Member

nt1m commented Dec 2, 2023

I noticed while reading the spec that the "named elements" map is never cleaned up, so it might become stale with elements that no longer have a view-transition-name. But it's also possible that I missed something here.

@khushalsagar @noamr @vmpstr @fantasai @tabatkins

@nt1m nt1m added the css-view-transitions-1 View Transitions; Bugs only label Dec 2, 2023
@nt1m
Copy link
Member Author

nt1m commented Dec 2, 2023

Similarly I don't see where the pseudo element tree is being torn down in the algorithms.

@nt1m nt1m changed the title [css-view-transitions-1] Where is the "named elements" map cleaned up? [css-view-transitions-1] Where is the "named elements" map and pseudo elements cleaned up? Dec 2, 2023
@nt1m nt1m changed the title [css-view-transitions-1] Where is the "named elements" map and pseudo elements cleaned up? [css-view-transitions-1] Where are the "named elements" map and pseudo elements cleaned up? Dec 2, 2023
@nt1m
Copy link
Member Author

nt1m commented Dec 2, 2023

Nevermind for the pseudo element tree. "show view transition tree" covers it.

@nt1m nt1m changed the title [css-view-transitions-1] Where are the "named elements" map and pseudo elements cleaned up? [css-view-transitions-1] Where are the "named elements" map cleaned up? Dec 2, 2023
@noamr
Copy link
Collaborator

noamr commented Dec 2, 2023

What would be the observable effect of specifying this? the map gets populated twice and is no longer in use after setting the pseudo elements style, but I don't know what it would mean that it holds stale elements after this.

If you change view transition names after capturing the new state it's too late for this view transition as everything is already captured.

@nt1m
Copy link
Member Author

nt1m commented Dec 2, 2023

@noamr The named elements map is used as input to build the pseudo element tree, so it's rather important that cleanup is specified. Otherwise, implementing as it is means that the pseudo element tree will be built with non-existent groups if you do the following steps:

  • start a view transition with A & B having view-transition-names
  • wait for it to end
  • set view-transition-name: none on B
  • start another view-transition

-> B gets constructed along with A (even though B has view-transition-name: none)

@noamr
Copy link
Collaborator

noamr commented Dec 2, 2023

@noamr The named elements map is used as input to build the pseudo element tree, so it's rather important that cleanup is specified. Otherwise, implementing as it is means that the pseudo element tree will be built with non-existent groups if you do the following steps:

  • start a view transition with A & B having view-transition-names

  • wait for it to end

  • set view-transition-name: none on B

  • start another view-transition

-> B gets constructed along with A (even though B has view-transition-name: none)

Oh I see. The named element map is associated with a particular ViewTransition object, so starting a new one would ignore any existing one from a previous transition.

@nt1m
Copy link
Member Author

nt1m commented Dec 2, 2023

Ah right, that makes sense thanks!

@nt1m nt1m closed this as completed Dec 2, 2023
@nt1m nt1m added the Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. label Dec 2, 2023
@nt1m
Copy link
Member Author

nt1m commented Dec 2, 2023

I wonder if it makes sense to add a note if this is implicit.

@noamr
Copy link
Collaborator

noamr commented Dec 3, 2023

I wonder if it makes sense to add a note if this is implicit.

It's explicit though:

https://www.w3.org/TR/css-view-transitions-1/#viewtransition-named-elements

"A {{ViewTransition}} has the following:"

@nt1m
Copy link
Member Author

nt1m commented Dec 3, 2023

That part is explicit, but I think it would be useful to mention a note about clean up: "Note: Since this is associated to the ViewTransition, this means the map is cleaned up along with the view transition when that ends" or something. Because naturally when you're looking to implement this, you're looking for where things are getting cleaned up.

@khushalsagar
Copy link
Member

Sounds reasonable @nt1m. Feel free to send a PR or let me know if you'd like me to do that. :)

@nt1m
Copy link
Member Author

nt1m commented Jan 5, 2024

I sent a PR: #9769

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-view-transitions-1 View Transitions; Bugs only
Projects
None yet
3 participants