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

Nesting Astro components prevents transition directives from animating #8392

Closed
1 task
sam-high opened this issue Sep 4, 2023 · 11 comments · Fixed by #8449 or #8646
Closed
1 task

Nesting Astro components prevents transition directives from animating #8392

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

Comments

@sam-high
Copy link

sam-high commented Sep 4, 2023

Astro Info

Astro                    v3.0.12
Node                     v16.20.0
System                   Linux (x64)
Package Manager          unknown
Output                   static
Adapter                  none
Integrations             none

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

No response

Describe the Bug

Nested View Transitions stop animating when HTML with Transition Directives is nested too deep within Astro components.

e.g. when nesting an image

<img
  transition:name="test"
  transition:animate="initial"
  src="https://placehold.co/600x400/png"
/>

This level of nesting works

Images (Astro Component)
    Image (Astro Component)
        HTML image (with Transition Directives)

This level of nesting or more does not work

Images Container (Astro Component) ** Extra level
    Images (Astro Component)
        Image (Astro Component)
            HTML image (with Transition Directives)            

If I can help make this issue clearer please let me know.

What's the expected result?

Nested View Transitions to animate no matter how deeply they are nested.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ojxahh?file=src%2Fpages%2Findex.astro

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
@matthewp matthewp self-assigned this Sep 5, 2023
@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) feat: view transitions Related to the View Transitions feature (scope) and removed needs triage Issue needs to be triaged labels Sep 5, 2023
@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.

@matthewp
Copy link
Contributor

matthewp commented Sep 8, 2023

Ah, thanks, will take a look.

@matthewp matthewp reopened this Sep 8, 2023
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Sep 8, 2023
@sam-high
Copy link
Author

sam-high commented Sep 8, 2023

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.

You are right. I have updated the Link to Minimal Reproducible Example to Astro 3.0.12 and unfortunately, it does not resolve the issue.

Thanks for looking into the issue @matthewp

@martrapp
Copy link
Member

martrapp commented Sep 18, 2023

Fwiw, an even more condensed version https://stackblitz.com/edit/github-ojxahh-hrpswf?file=package.json,src%2Fpages%2Findex.astro

Boils down to this

not-working-page.astro // lacks view-transition-name
  Layout.astro
    ImageContainer.astro // remove this and it works
      Images.astro
        Image.astro // sets transition name

@matthewp matthewp removed the needs triage Issue needs to be triaged label Sep 20, 2023
@matthewp
Copy link
Contributor

Are you happening to use transition:name and transition:persist on the same element?

@adrianosmond
Copy link

Fwiw, an even more condensed version https://stackblitz.com/edit/github-ojxahh-hrpswf?file=package.json,src%2Fpages%2Findex.astro

Boils down to this

not-working-page.astro // lacks view-transition-name
  Layout.astro
    ImageContainer.astro // remove this and it works
      Images.astro
        Image.astro // sets transition name

Not in this reproduction ☝️
And also not in the more convoluted case I was working on that led me to find this GH issue

@sam-high
Copy link
Author

sam-high commented Sep 21, 2023

No, transition:name and transition:persist are not used on the same element on the Minimal Reproducible Example or in my production code.

Only transition:name and transition:animate are being used together.

@matthewp
Copy link
Contributor

Thanks, this is the top of my list of issues to fix.

@matthewp
Copy link
Contributor

I'm pretty sure I know what's causing this.

@matthewp
Copy link
Contributor

So what's happening here is we use this plugin to track dependencies, so we know which components are going to be appending styles (and scripts) to the head: https://github.com/withastro/astro/blob/main/packages/astro/src/vite-plugin-head/index.ts

When we find a component that does this, such as one that has transition styles, it walks up the tree and marks parent components as having head metadata in them (the in-tree value).

In Image reproduction what happen is, the Image component loads on the index page, it walks up its parents, but the not-working.astro page has not been loaded yet, so it is not a parent. Later when not-working.astro loads, it never gets marked as having styles in-tree. So when we go to render it the page we don't know to wait on Image. So it doesn't.

The fix is that when a new module comes in, we need to check if it is marked as in-tree or self and then propagate this up. This should ensure that the metadata is in the entire tree at all times.

@matthewp
Copy link
Contributor

PR up: #8646

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: view transitions Related to the View Transitions feature (scope)
Projects
None yet
4 participants