withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.26k stars 2.44k forks source link

View Transition resets html attributes #7765

Closed florian-lefebvre closed 1 year ago

florian-lefebvre commented 1 year ago

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

matthewp commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

florian-lefebvre commented 1 year ago

Swup as well: https://swup.js.org/events/#list-of-all-events

florian-lefebvre commented 1 year ago

Awesome thank you!

Tc-001 commented 1 year ago

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 commented 1 year ago

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

matthewp commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

surjithctly commented 1 year ago

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 commented 1 year ago

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

Tc-001 commented 1 year ago

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

chenbinli-dev commented 1 year ago

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 commented 1 year ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

I can see Oliver answered you 👍

danielo515 commented 7 months ago

Can you share a link to the discord thread?

TheElegantCoding commented 7 months ago

try with this link, discord

danielx-art commented 6 months ago

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 commented 6 months ago

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 commented 4 months ago

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


        <!-- 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>