swup / parallel-plugin

A swup plugin for animating the previous and next page in parallel  🎏
https://swup.js.org/plugins/parallel-plugin
MIT License
6 stars 1 forks source link

[Bug]: sometimes styles are not applied to .is-next-container #13

Closed ScoobyDid closed 9 months ago

ScoobyDid commented 9 months ago

What did you expect? 🧐

Styles to apply to .is-next-container before class is removed

What actually happened? πŸ˜΅β€πŸ’«

Styles are not applyed to .is-next-container before class is removed.

This issue only occurs when plugin is included from a CDN as UMD module (as it is suggested in documentation). It took me a while to figure out that forceReflow function is not included in output file - class is applied and then immediately removed:

Screenshot 2024-01-24 211447

Although it is included in module file, that's why it works correctly in your demo (it uses CDN link with module), but have inconsistent behavior in this demo (same code, but uses CDN UMD link)

Swup and plugin versions πŸ“

What browsers are you seeing the problem on? 🧭

Firefox, Chrome, Safari, Microsoft Edge

Relevant log output πŸ€“

No response

URL to minimal reproduction πŸ”—

https://stackblitz.com/edit/web-platform-bmtpwk?file=index.html,page-2.html,page-3.html,page-4.html,base.css,pages.css,transitions.css,swup.js

Checked all these? πŸ“š

daun commented 9 months ago

Thanks for investigating! This looks to be a minification issue β€” the forceReflow helper just reads the offsetHeight of the body if I remember correctly and Terser seems to discard any methods that just read and don't modify anything. We'll need to see if this can be configured or if we find a different workaround.

daun commented 9 months ago

There seems to be a magic comment to disable inlining of functions in microbundle. I'll see if that solves the issue.

export default function main(a, b) {
  const inlined = /*@__INLINE__*/ shouldBeInlined(a, b);
  const preserved = /*@__NOINLINE__*/ shouldBePreserved(a, b);
  return { inlined, preserved };
}
ScoobyDid commented 9 months ago

If this won't work properly, i think removing class within requestAnimationFrame should work just fine;

hirasso commented 9 months ago

You mean enable, right?

daun commented 9 months ago

@ScoobyDid Returning true should do the trick. We had started with requestAnimationFrame for the plugin, but after lots of trial and error it turned out that a forced reflow was required to get perfectly synchronous enter and leave animations.

@hirasso Now I'm confused 🀠 I assume we'd want to disable inlining to increase the chance of the function being kept in the output. But returning true should be the better solution here anyway.

hirasso commented 9 months ago

@daun oops no, I got it backwards πŸ˜… please ignore my comment 🫠

daun commented 9 months ago

Created a PR with a working fix on the core repo: #868

daun commented 9 months ago

This is fixed in the latest releases of swup and this plugin. Feel free to reopen if you're still seeing the issue, but I've confirmed the reflow helper is kept during minification. Thanks for reporting and investigating @ScoobyDid!

ScoobyDid commented 9 months ago

Awesome! Thanks for fixing it; tested it in this demo and now it works as expected!