issue with new lazyload feature?

[futtta]

@softmanro a LYTE-user is using your lazyload feature (see original post on the wordpress.org forum) and seems to be running into an issue with the image not being visible;

There is a problem with mobile thumbnails. See here https://imgur.com/a/ytPulsV They are not being loaded.

If I remove the code add_filter( ‘lyte_use_internal_lazyloader’, ‘__return_true’); then it worked again. Once, reloading the page again did not show it for the second time. But it is connected to the lazy load feature. I checked the code and see here https://imgur.com/a/ArL7JZ2 The image is there but you force background image: none so it’s hidden behind the background…

Edit: Yeah, it’s because of the lazy load. I just deleted the cache. Right now the thumbnails are loaded but it’s immediately after the page load and not lazy loaded. Could you please fix it so we can use the lazy load feature but having the thumbnails working fine?

any idea what is happening here?

[softmanro]

Well, I'm not sure what could be causing this behaviour and I don't seem to be able to reproduce this.
Looking at the highlighted area in the second picture, there is actually not an issue with the wyl-lazy CSS class. That one is only applied to the elements not above the fold. While the user scrolls and that area reaches above the fold, the class gets removed (as it seems to be in that picture as well), leaving the actual thumbnail visible instead.

Let me know if the user is able to reproduce the issue and explain the steps required for that.

[futtta]

I've asked keengamer to join this conversation :)

[Stirius]

Hi this is KeenGamer, different account.

Well, I can just provide as much info as I can. I'm not sure how this add_filter( 'lyte_use_internal_lazyloader', '__return_true'); works for your plugin (feature) but if it's in our functions then it makes the issues as described above. Right now we do not have the staging site, it needs to be redone at the moment.

Also we just switched to the default settings with Lyte plugin to use the local cache and the image size is below 20 Kb so it doesn't ring any issues even for google fortunately. And I don't want to test it more because that would mean deleting the cache and for our 10k posts it's just timeconsuming to resave it again.

But, we use also WP Rocket with this settings https://prnt.sc/268ohtl Could be a conflict with this? Plus we have also Cloudflare's APO https://blog.cloudflare.com/automatic-platform-optimizations-starting-with-wordpress/ which saves on the edge the whole HTML etc. But this should not affect the lazyload in my opinion.

Also, if you go first to the desktop version of the page (not visited before so no cache is used, not even from Lyte plugin), the image is loaded fine. And if you go afterwards to the mobile version, the image is also loaded fine. However, if you visit the page FIRST using mobile then the image is not there. If you switch to desktop, reload, the image is there and afterwards it's also there for the mobile version. So it seems like there must be some conflict or a small bug just for the mobile view and it's caching, lazyloading.

Hope this helps :-).

[believableUN]

Hi,
Can confirm, I am having the same exact issue and not using WP rocket or Cloudflare.
background: none forces a blank image.

[futtta]

and you're also using the new lazyload functionality (by using the filter) @believableUN ?

do you have a stagingsite where you could leave this active for @softmanro and me to see?

[believableUN]

Yes, I am using the lyte_use_internal_lazyloader filter.
Thanks

[softmanro]

@believableUN I've managed to reproduce this using the link that you provided. It looks like the javascript which should remove the wyl-lazy class fails to run (or do its work). I need to understand why that happens. @futtta any idea what could be triggering this?

Here is how the code looks like when showing blank instead of image:

[futtta]

@softmanro could the problem be that DomContentLoaded (the event you hook into) has already fired when the JS (due to it being async) is executed? maybe we need some test/ fallback there?

[softmanro]

Yes, that's it. Due to the scripts being loaded asynchronous, the DOMContentLoaded fires sometimes before we can hook on to it.
A possible work-around would be to hook to window load event, what do you think @futtta?

[softmanro]

We could also set the lazyload script to be defer instead of async and keep the DOMContentLoaded hook, which should also work. I'm not sure what the best option would be, so looking forward to other points of view.

[futtta]

that might work, as deferred scripts execute (just) before DCL?

[softmanro]

Yes, that is accurate. The only issue would be if some other plugin messes with defer attribute from the script.
I've tested both the window load event and the defer approach and the results are positive, but I'm not sure which one to pick in order to cover the most comprehensive user base.

[Stirius]

I'm not sure about the defer version. There is a lot of caching plugins which work and have defer (delay) javascript features. We have also DOM and the HTML doc loaded really fast from Cloudflare APO cache. But if you manage to run it AFTER the DOM and the HTML is loaded I would prefer this. I can test it because we use a lot of optimization features and plugins to speed up our site...

[futtta]

I choose to keep async and switch the hooked event from domContentLoaded to load as a test, just committed the change here, could you test @Stirius & @believableUN ? Download https://github.com/futtta/wp-youtube-lyte/archive/refs/heads/main.zip and install instead of "normal" version.

[Stirius]

Hm, so far so good. But would be good to change the version number to be able to see which one is activated. Also you cannot have "main" as the folder name. The cache files and php is not using that path...

[futtta]

the official release (once we're ready) will have an updated version number. re folder name; it's "wp-youtube-lyte-main" for the github version, "main" is just the name of the default branch in github :)

…

On 07/01/2022 12:01, Stirius wrote: Hm, so far so good. But would be good to change the version number to be able to see which one is activated. Also you cannot have "main" as the folder name. The cache files and php is not using that path... — Reply to this email directly, view it on GitHub <#29 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABMIMO6QQN6RVC5QVEJXELUU3B2FANCNFSM5LKPEKAA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[Stirius]

Alright. It works for me so if @believableUN will confirm it as well then you can push it live in my opinion...

[believableUN]

Working good now, thanks a lot!

[softmanro]

Thanks for bringing this issue up @believableUN, @Stirius and for validating the fix.

[futtta]

Thanks indeed everyone, I'll probably push out an official update this weekend! :)

[futtta]

couldn't wait, just pushed out the release :)