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

Ability to disable image lazy loading listeners #7570

Open
4 tasks done
Xenossolitarius opened this issue Jun 14, 2024 · 9 comments
Open
4 tasks done

Ability to disable image lazy loading listeners #7570

Xenossolitarius opened this issue Jun 14, 2024 · 9 comments
Labels
feature request t0ggles Linked to the t0ggles task

Comments

@Xenossolitarius
Copy link

Xenossolitarius commented Jun 14, 2024

Clear and concise description of the problem

I am running a multi row (swiper) image gallery which is actionable and I am lazy loading the images with my custom logic. Because I need the performance I didn't want to relay on the swiper but unfortunately it attaches listeners anyway. Which makes it 100% unusable for my usecase. Currently I am patching the swiper to stop that behavior.

Suggested solution

diff --git a/shared/swiper-core.mjs b/shared/swiper-core.mjs
index 619c42a4ee03ac543db7869eb80aa7e782bccab2..06880c6f982f6d2d62dad27700a3ea13a5652a20 100644
--- a/shared/swiper-core.mjs
+++ b/shared/swiper-core.mjs
@@ -2860,9 +2860,9 @@ const events = (swiper, method) => {
   }
 
   // Images loader
-  el[domMethod]('load', swiper.onLoad, {
-    capture: true
-  });
+  // el[domMethod]('load', swiper.onLoad, {
+  //   capture: true
+  // });
 };
 function attachEvents() {
   const swiper = this;
@@ -3614,11 +3614,11 @@ class Swiper {
     if (params.breakpoints) {
       swiper.setBreakpoint();
     }
-    [...swiper.el.querySelectorAll('[loading="lazy"]')].forEach(imageEl => {
-      if (imageEl.complete) {
-        processLazyPreloader(swiper, imageEl);
-      }
-    });
+    // [...swiper.el.querySelectorAll('[loading="lazy"]')].forEach(imageEl => {
+    //   if (imageEl.complete) {
+    //     processLazyPreloader(swiper, imageEl);
+    //   }
+    // });
     swiper.updateSize();
     swiper.updateSlides();
     swiper.updateProgress();
@@ -3786,19 +3786,19 @@ class Swiper {
 
     // Attach events
     swiper.attachEvents();
-    const lazyElements = [...swiper.el.querySelectorAll('[loading="lazy"]')];
-    if (swiper.isElement) {
-      lazyElements.push(...swiper.hostEl.querySelectorAll('[loading="lazy"]'));
-    }
-    lazyElements.forEach(imageEl => {
-      if (imageEl.complete) {
-        processLazyPreloader(swiper, imageEl);
-      } else {
-        imageEl.addEventListener('load', e => {
-          processLazyPreloader(swiper, e.target);
-        });
-      }
-    });
+    // const lazyElements = [...swiper.el.querySelectorAll('[loading="lazy"]')];
+    // if (swiper.isElement) {
+    //   lazyElements.push(...swiper.hostEl.querySelectorAll('[loading="lazy"]'));
+    // }
+    // lazyElements.forEach(imageEl => {
+    //   if (imageEl.complete) {
+    //     processLazyPreloader(swiper, imageEl);
+    //   } else {
+    //     imageEl.addEventListener('load', e => {
+    //       processLazyPreloader(swiper, e.target);
+    //     });
+    //   }
+    // });
     preload(swiper);
 
     // Init Flag

We can add if( preloadEnabled ) props around this behavior

Alternative

No response

Additional context

No response

Validations

  • Follow our Code of Conduct
  • Read the docs.
  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Would you like to open a PR for this feature?

  • I'm willing to open a PR
@nolimits4web
Copy link
Owner

t0ggles-create swiper

Copy link

t0ggles bot commented Jun 17, 2024

Task nolimits4web/SWIPER-138 was created

t0ggles task SWIPER-138

@t0ggles t0ggles bot added the t0ggles Linked to the t0ggles task label Jun 17, 2024
@PerpetualWar
Copy link

I can confirm this patch for my use case as well. When having multiple instances of swiper on a page, will lead to blocking of the main thread, which is a major issue.

This should be prioritized , if possible, as swiper is useless in list rendering scenario (50 instances in my case).

@PerpetualWar
Copy link

Hm, after more testing Im not sure this fix alone helps with these performance issues.

@Xenossolitarius
Copy link
Author

@PerpetualWar if you are using tailwind removing the classes from swiper component helps a lot. Its an undocumended behavior that swiper emits all the classes prefixed with swiper.

@PerpetualWar
Copy link

@Xenossolitarius I do not use tailwind or having classes on swiper component.

I have actually dealt with this by avoiding rendering those swipers if they are not visible in viewport. Works great for me , cause I have 3-4 swipers visible in the list at any given time, and perf increase is massive.

@Xenossolitarius
Copy link
Author

@PerpetualWar made this, hope it gets approved
#7638

@nolimits4web
Copy link
Owner

Any examples with the issue?

@Xenossolitarius
Copy link
Author

I should probably make a stackblitz but overall I am not interested into coupling my lazy loading logic which adheres to counting elements on the screen and registering them to actually pull images when business rules apply to the case. When i want to show 100 slides and 5 rows of swipers and virtual slides are out of option bcs i want to use slidesPerPage auto bcs of layout it just produces a 1 sec swiper recalculation lag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request t0ggles Linked to the t0ggles task
Projects
None yet
Development

No branches or pull requests

3 participants