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

View Transition resets html attributes #7765

Closed
1 task
florian-lefebvre opened this issue Jul 22, 2023 · 26 comments · Fixed by #7786 or #7823
Closed
1 task

View Transition resets html attributes #7765

florian-lefebvre opened this issue Jul 22, 2023 · 26 comments · Fixed by #7786 or #7823
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@florian-lefebvre
Copy link
Member

What version of astro are you using?

^2.9.1

Are you using an SSR adapter? If so, which one?

Vercel

What package manager are you using?

npm

What operating system are you using?

Windows

What browser are you using?

Brave

Describe the Bug

I have a script that toggles the .dark class on the html element. However, using view transitions, the html class is reset to its original state (ie. class specified in the layout) but the script is not re-run.

To reproduce:

  1. The page background will be red if your os is in dark mode
  2. navigate to another page, the background will be white again

What's the expected result?

I'd expect one of the following:

  1. html attributes are not reset to default because scripts are not re-run
  2. scripts re-run

I think 1. makes more sense to avoid flashes

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-8a4stc

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 Jul 22, 2023
@matthewp
Copy link
Contributor

Replacing the HTML attribute is what you would expect with View Transitions MPA mode so that I think is what we are going for. I would expect a script to run, though. Let me look into that.

@matthewp
Copy link
Contributor

Oh right, these are module scripts that get bundled. So those will not necessarily rerun (maybe if they are inline...). So we'll need some solution for that.

One person had suggested firing the load event. I'll have to see if other similar routers do that. Another option is a namespaced astro:load or something like that. Then you could listen to this event and do what you need. Want to first research what other similar routers do, if you happen to know and can provide context that would be appreciated.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Jul 22, 2023
@matthewp matthewp self-assigned this Jul 22, 2023
@matthewp
Copy link
Contributor

Turbo uses a turbo:load event: https://turbo.hotwired.dev/reference/events

@florian-lefebvre
Copy link
Member Author

@florian-lefebvre
Copy link
Member Author

Awesome thank you!

@Tc-001
Copy link
Contributor

Tc-001 commented Jul 25, 2023

Sadly this doesn't seem solve the theme toggle usecase unless I am missing something.

The current chain seems to be Old page -> Fetch new page -> Start crossfade -> End crossfade -> Dispatch astro:load

This means that the non-dark version of the page is still visible until the transition is completed

<script is:inline>
	document.addEventListener("astro:load", () => document.body.setAttribute("data-dark", "dark"))
</script>
Screencast.from.2023-07-25.23-07-40.webm

@matthewp
Copy link
Contributor

Ah good point, I think we need to run astro:load inside updateDOM before the transition is finished.

@matthewp matthewp reopened this Jul 26, 2023
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 26, 2023
@matthewp matthewp removed the needs triage Issue needs to be triaged label Jul 26, 2023
@matthewp
Copy link
Contributor

This one is tough b/c with the fallback the swap happens immediately, so there isn't time to wait for scripts to run. I could see solving this in a couple of ways:

  1. Preserve HTML attributes as a special-case for this use-case. I'm worried that there are other hidden use-cases this doesn't address though.
  2. Wait on head scripts in updateDOM which is what the view transition is waiting on. For fallback we need some other mechanism though. I've been thinking we might be able to mimick the "screenshot" feature of VT in the fallback by copying the DOM over to an iframe that is displayed over the page. If we do that we can think wait on the scripts to run in the real DOM before finishing the swap. I'm going to try this approach.

@Tc-001
Copy link
Contributor

Tc-001 commented Jul 26, 2023

Maybe you can have astro:beforeload or similar that has the new DOM as the event target, so you could apply any modifications to it instead of document

Something like

document.addEventListener("astro:beforeload", (e) => e.target.body.setAttribute("data-dark", "dark"))

But this won't work with some libraries that modify the document directly

On the other hand those usually don't modify the pages in a big way, so it's probably good enough to keep those in :load

@matthewp
Copy link
Contributor

That would work if the script is also on the old page. But if it's only on the new page then it won't because the script hasn't run yet.

But I might be overthinking it. For the use-case we are concerned about it's very likely that it is on both pages. In which case your idea would fix it.

@matthewp
Copy link
Contributor

astro:beforeload seems to work really well. Given that, is there a use-case for astro:load or should I remove it? I'm leaning towards removing it.

@florian-lefebvre
Copy link
Member Author

I think removing it is fine. If anyone needs it they will open a new issue I suppose

@surjithctly
Copy link

Please don't remove astro:load, I have a plugin which uses javascript to open dropdowns where its used DOMContentLoaded. beforeload won't work in this case.

I have concerns about backward compatibility. since astro:load won't work on previous versions, I cannot do this:

// won't work in old versions
document.addEventListener('astro:load', () => { 
  InitAstroNav();
}, { once: true });
// rans twice on the initial page load. 
 InitAstroNav();
document.addEventListener('astro:load', () => { 
  InitAstroNav();
}, { once: true });

Any solution to this?

@chenbinli-dev
Copy link

this is my first time trying to use astro and i ran into the same problem , so what should i do now ?

@Tc-001
Copy link
Contributor

Tc-001 commented Sep 11, 2023

Hello!
You should use https://docs.astro.build/en/guides/view-transitions/#astroafter-swap

@chenbinli-dev
Copy link

Hello! You should use https://docs.astro.build/en/guides/view-transitions/#astroafter-swap

Oh, I almost found this. Thank you very much!!!

@danielx-art
Copy link

Hello!

I could make things work with the astro:after-swap event, but with one exception, animating a canvas conmopnent:

This don't work for me when swaping between pages using View Transitions:

let t = 0;

function update(){
  //calculations
}

function show(){
  //actually draw on canvas
}

function animate() {
    //resets after 1024
    if (t > 1024) t = 0;
    requestAnimationFrame(animate);

    //responsiveness
    if (
      canvas.width != window.innerWidth ||
      canvas.height != window.innerHeight
    ) {
      setupCanvas();
    }

    ctx.clearRect(0, 0, herocanvas.width, herocanvas.height);
    update();
    show();

    t++;
  }

 document.addEventListener('astro:after-swap', animate);

@TheElegantCoding
Copy link

TheElegantCoding commented Mar 6, 2024

same here adding a class with animation or transition don't work in astro:after-swap

  const privateMessageImage = document.getElementById('privateMessageImage') as HTMLImageElement;

  document.addEventListener('astro:after-swap', () =>
  {
    containerPlay?.classList.add('image--active')
  });
.image
{
  visibility: hidden;
  opacity: 0;
  transition: opacity 0.5s, visibility 0.5s;
}

.image--active
{
  visibility: visible;
  opacity: 1;
}

@florian-lefebvre
Copy link
Member Author

The API has quite evolved since then! Would you mind seeking support on our Discord server or opening a new issue if you're sure there's one? Thanks!

@TheElegantCoding
Copy link

i open a discord issue with the name Re-run script tag in a component on navigation view transition you can see more details there i was waiting for a reply to see if i open an issue or not

@florian-lefebvre
Copy link
Member Author

I can see Oliver answered you 👍

@danielo515
Copy link

Can you share a link to the discord thread?

@TheElegantCoding
Copy link

try with this link, discord

@danielx-art
Copy link

Hi! Can't find it on Discord, and the provided link is broken. Still with the same problem, have a canvas element, use script tag to draw stuff on it, but on transition script is not re-run and canvas is not drawn, sad. You can see it on the background here when switching languages on the right upper corner.

@florian-lefebvre
Copy link
Member Author

This issue has been closed a while ago, can you open a new one with a minimal reproduction? That would help us thank you!

@Jean-Baptiste-Lasselle
Copy link

Jean-Baptiste-Lasselle commented Jun 7, 2024

Hey all, just to share my case, I solved thanks to you all:

  • I had a script tag which just has the https URL to a third party script (for github star rating component
  • And I solved my problem like below
		<!-- Place this tag in your head or just before your close body tag. -->
    <!-- script async defer src="https://buttons.github.io/buttons.js"></script -->
    <script async defer>

      document.addEventListener('astro:page-load', ()=>{
        
      /*----------------------------------------------------------
      --------------------0-GET DOM ELEMENTS----------------------
      ----------------------------------------------------------*/
        
        
        if (document.getElementById("github-star-rating-script")) {
          // document.getElementById("github-star-rating-script")?.remove();
          document.getElementById("github-star-rating-script")?.remove();
          console.info(`just removed github star rating script in <head>`)
        }
        var addScript = document.createElement("script");
        addScript.type = "text/javascript";
        addScript.id = "github-star-rating-script";
        addScript.src = `https://buttons.github.io/buttons.js`;
        (document.getElementsByTagName("head")[0] || document.documentElement ).appendChild(addScript);

        // console.info(`just added github star rating script in <head>`)
      })
    
      //document.addEventListener('astro:after-swap', animate);
    
    </script>

Also give another example because I think pretty commonly used (if you have better patterns i'm a taker), this one adds a copy/to/clipboard button to all of my code blocks as they are generated by markdown renderer:

     <script>
      document.addEventListener('astro:page-load', ()=>{
        const copyButtonLabel = "Copy Code";
        // use a class selector if available
        let blocks = document.querySelectorAll("pre");

        blocks.forEach((block) => {
          // only add button if browser supports Clipboard API
          if (navigator.clipboard) {
            let button = document.createElement("button");
            // let button = document.createElement("a");

            button.innerText = copyButtonLabel;
            // here i just add those classes because i work with tailwind / DaisyUI
            button.classList.add("btn");
            button.classList.add("mt-3");
            // button.classList.add("btn-active");
            // button.classList.add("btn-ghost");


            block.appendChild(button);

            button.addEventListener("click", async () => {
              await copyCode(block, button);
            });
          }
        });

        async function copyCode(block: any, button: any) {
          let code = block.querySelector("code");
          let text = code.innerText;

          await navigator.clipboard.writeText(text);

          // visual feedback that task is completed
          button.innerText = "Code Copied";

          setTimeout(() => {
            button.innerText = copyButtonLabel;
          }, 700);
        }
      });


     </script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
9 participants