GeorgeHastings / embed-unicornstudio

https://embed-unicorn-studio.vercel.app
7 stars 2 forks source link

Bug: `data-us-lazyload="true"` Attribute Not Working with `UnicornStudio.init()` #4

Open elliottmangham opened 2 months ago

elliottmangham commented 2 months ago

Hey πŸ‘‹

Issue Description: When using the vanilla JavaScript approach, the lazyLoad feature works perfectly as shown in the example below:

UnicornStudio.addScene({
    element,
    lazyLoad: true,
    filePath: 'example.json'
});

However, when using the data attribute approach with UnicornStudio.init(), the data-us-lazyload="true" attribute seemingly has no effect. I have tried various US instances on a page with scroll requirements, some with lazyload enabled and others not, but they all start right away, and I can see a data-us-initialized="true" attribute being set.

Example:

<div
    id="webgl"
    data-us-project-src="example.json"
    data-us-lazyload="true"
></div>

Expected Behaviour: β€’ Scenes with data-us-lazyload="true" should initialise only when they scroll into view. β€’ Scenes without data-us-lazyload="true" should initialise immediately.

Actual Behavior: β€’ All scenes, regardless of the data-us-lazyload attribute, initialise immediately upon page load. β€’ The data-us-initialized="true" attribute is set on all scenes right away.

Steps to Reproduce:

  1. Add multiple scenes to a page using data attributes, some with data-us-lazyload="true" and some without.
  2. Initialise the scenes using UnicornStudio.init().
  3. Observe that all scenes initialise immediately, regardless of the data-us-lazyload attribute.

Environment: β€’ UnicornStudio version: 1.3.1 β€’ Browser: Chrome (Arc), Version 1.60.0 (53678), Chromium Engine Version 128.0.6613.138 β€’ OS: macOS 15.0 (24A335)

Additional Context: It would be beneficial to have the data-us-lazyload="true" attribute work similarly to the lazyLoad: true option in the JavaScript API for better performance and resource management in single-page applications.

Thank you for considering this issue!

GeorgeHastings commented 2 months ago

Hey there @elliottmangham !

First of all thank you for the thoroughness here, super helpful.

Is the only observable evidence of initialization the data-us-initialized being set? That param gets put on the element as a "tag" once its picked up by the init function, so it doesn't get added again in subsequent init calls β€” it doesn't actually mean that the scene is rendered.

Lazyload defers creating the webgl resources until the scene enters the viewport, but the scene object will be created on pageload.

Let me know if this matches up with what you're seeing.

elliottmangham commented 2 months ago

Hey @GeorgeHastings,

Thanks for getting back to me!

Not just the data-us-initialized attribute, I can also see the canvas markup being rendered too before it comes into view. When I replace this with the JS route with lazyload parameter set to true, none of the HTML is rendered until it comes into view as it should. So it's a comparable difference.

Screen recording: https://cloud.cdrs.dev/vSyC7KJc

The HTML in the above recording is:

<div
id="webgl"
data-us-project-src="/wp-content/themes/cdrs/assets/static/v05ec6QWcRv6bfjYiI9Z.json"
data-us-scale="1"
data-us-dpi="1.5"
data-us-lazyload="false"
data-us-disableMobile="true"
data-us-alttext="Welcome to Unicorn Studio"
data-us-arialabel="This is a canvas scene"
></div>
<div
id="webgl"
data-us-project-src="/wp-content/themes/cdrs/assets/static/7MmQPv66zXMr23MJv25n.json"
data-us-scale="1"
data-us-dpi="1.5"
data-us-lazyload="true"
data-us-disableMobile="true"
data-us-alttext="Welcome to Unicorn Studio"
data-us-arialabel="This is a canvas scene"
></div>

On the other hand, this is my test with JS addScene method and lazyload set to true, which lazy loads fine: https://cloud.cdrs.dev/mhnB2tcB

I think the the issue is the [data-us-lazyload] attribute. What do you think?

On a related note, since data-us-initialized doesn't necessarily mean it has rendered, it might be helpful having another attribute to indicate it has rendered, like data-us-rendered or a modifiable .has-rendered class.

Thanks again George!

GeorgeHastings commented 2 months ago

I see. Agree on the naming and parameters β€” can definitely make this more intuitive.

However this still looks correct. The scene is created and the canvas will be injected on load β€” but none of the heavy lifting will happen until the scene enters the viewport.