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.

Sign up for GitHub

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

Jump to bottom

In rare cases, CSS for transition scopes is not generated #8399

Closed
1 task done
martrapp opened this issue Sep 4, 2023 · 10 comments · Fixed by #8449
Closed
1 task done

In rare cases, CSS for transition scopes is not generated #8399

martrapp opened this issue Sep 4, 2023 · 10 comments · Fixed by #8449
Assignees
@matthewp
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: view transitions Related to the View Transitions feature (scope)

Comments

@martrapp
Copy link
Member

martrapp commented Sep 4, 2023

Astro Info

Astro                    v3.0.8
Node                     v20.5.1
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

A certain structure of nested components leads to CSS not being generated for transition scopes.
First found here: https://discord.com/channels/830184174198718474/1147944562682253394 by Nash

What's the expected result?

CSS generation works independent of the component structure

Link to Minimal Reproducible Example

https://github.com/martrapp/ronijaakkola-website.git

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Sep 4, 2023
@martrapp
Copy link
Member Author

martrapp commented Sep 4, 2023 •
edited
Loading

Preliminary analysis so far:
The required style sheet is pushed to SSRResult._metadata.extraHead in renderTransition().

In this example, however, the transition:name is not found until after the <head> rendering is complete.
Two maybe-head instructions follow after the transition:name is found, but they do nothing because the head is already closed.

@lilnasy
Copy link
Contributor

lilnasy commented Sep 5, 2023

Related #7761

@matthewp matthewp added feat: view transitions Related to the View Transitions feature (scope) and removed needs triage Issue needs to be triaged labels Sep 5, 2023
@matthewp matthewp self-assigned this Sep 5, 2023
@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Sep 5, 2023
@matthewp
Copy link
Contributor

matthewp commented Sep 5, 2023

This is a head propagation bug, i'll take a look.

@hkfi
Copy link

hkfi commented Sep 5, 2023

I ran into this issue as well. However, I had my <ViewTransitions /> as a child of <body> instead of <head>. My issue was resolved after moving <ViewTransitions /> to be a child of <head>.

@matthewp
Copy link
Contributor

matthewp commented Sep 6, 2023

I'm still looking at this. It wasn't what I was expecting but pretty confident a fix is close.

@matthewp matthewp mentioned this issue Sep 7, 2023
Open
1 task
@matthewp
Copy link
Contributor

matthewp commented Sep 7, 2023

Think I have a solution here, working on a test.

@matthewp matthewp mentioned this issue Sep 7, 2023
Merged
@matthewp
Copy link
Contributor

matthewp commented Sep 7, 2023

Fix is in #8449.

@adrianosmond
Copy link

I don't think that the fix that landed in 3.0.12 has fixed the stackblitz linked in the original post. If you go to the stackblitz link, update Astro to 3.0.12 and click the "Not working" link you'll see that you only get a full page cross-fade transition. On inspecting the (on the not working page, not the landing page) you can see it has a data-astro-transition-scope but no CSS with a view-transition-name for it has been generated.

@martrapp
Copy link
Member Author

martrapp commented Sep 8, 2023 •
edited
Loading

3.0.12 fixes this issue (#8399) and the repository linked above now generates the required CSS:

<head>
  ...
  <style>
    [data-astro-transition-scope="astro-z5xyy7w3-1"] { view-transition-name: title-project-1; }@layer astro { ::view-transition-old(title-project-1) { 
  ...

@adrianosmond what stackblitz are you looking at?

@adrianosmond
Copy link

I'm sorry - I had 2 tabs open about issues with View Transitions and I meant to post this in #8392 🤦

Will post there instead. Ignore me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewp matthewp

Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: view transitions Related to the View Transitions feature (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix multi-layout head injection withastro/astro
5 participants
@matthewp @adrianosmond @hkfi @lilnasy @martrapp