Merge pull request #665 from shopgate/PWA-1932-zoom-slider-issues

Fixed a slider issue with swiping through zoomed slides
shopgate · May 22, 2019 · 4fc3d02 · 4fc3d02
2 parents 89ee91d + 1144333
commit 4fc3d02
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 50 deletions.
diff --git a/libraries/commerce/product/subscriptions/index.js b/libraries/commerce/product/subscriptions/index.js
@@ -11,6 +11,7 @@ import fetchProductOptions from '../actions/fetchProductOptions';
 import { productImageFormats } from '../collections';
 import {
   productWillEnter$,
+  galleryWillEnter$,
   productReceived$,
   cachedProductReceived$,
   productRelationsReceived$,
@@ -42,6 +43,11 @@ function product(subscribe) {
     dispatch(fetchProductShipping(id));
   });
 
+  subscribe(galleryWillEnter$, ({ action, dispatch }) => {
+    const { productId } = action.route.params;
+    dispatch(fetchProductImages(hex2bin(productId), productImageFormats.getAllUniqueFormats()));
+  });
+
   subscribe(processProduct$, ({ action, dispatch }) => {
     const {
       id,

diff --git a/libraries/commerce/product/subscriptions/index.spec.js b/libraries/commerce/product/subscriptions/index.spec.js
@@ -5,7 +5,9 @@ import showModal from '@shopgate/pwa-common/actions/modal/showModal';
 import { historyPop } from '@shopgate/pwa-common/actions/router';
 import expireProductById from '../action-creators/expireProductById';
 import subscription from './index';
-import { productRelationsReceived$, visibleProductNotFound$ } from '../streams';
+import fetchProductImages from '../actions/fetchProductImages';
+import { galleryWillEnter$, productRelationsReceived$, visibleProductNotFound$ } from '../streams';
+import { productImageFormats } from '../collections';
 import { ERROR_PRODUCT, RECEIVE_PRODUCT_CACHED } from '../constants';
 
 const mockedGetProductsById = jest.fn();
@@ -15,6 +17,7 @@ jest.mock('@shopgate/pwa-common/actions/router', () => ({
 }));
 jest.mock('../actions/fetchProductsById', () => (...args) => mockedGetProductsById(...args));
 jest.mock('../action-creators/expireProductById', () => jest.fn());
+jest.mock('../actions/fetchProductImages', () => jest.fn());
 
 describe('Product subscription', () => {
   const subscribe = jest.fn();
@@ -26,13 +29,42 @@ describe('Product subscription', () => {
   });
 
   it('should subscribe', () => {
-    expect(subscribe).toHaveBeenCalledTimes(6);
+    expect(subscribe).toHaveBeenCalledTimes(7);
+  });
+
+  describe('galleryWillEnter$', () => {
+    let galleryWillEnter$Subscription;
+    beforeAll(() => {
+      ([, galleryWillEnter$Subscription] = subscribe.mock.calls);
+    });
+
+    it('should dispatch fetchProductImages when called', () => {
+      const formats = [{ height: 1024, width: 1024 }];
+      productImageFormats.set('GROUP', formats);
+      const [stream, callback] = galleryWillEnter$Subscription;
+      expect(stream === galleryWillEnter$).toBe(true);
+
+      const action = {
+        route: {
+          params: {
+            productId: '31333337',
+          },
+        },
+      };
+
+      callback({
+        dispatch,
+        action,
+      });
+
+      expect(fetchProductImages).toHaveBeenCalledWith('1337', formats);
+    });
   });
 
   describe('productRelationsReceived$', () => {
     let productRelationsReceived$Subscription;
     beforeAll(() => {
-      ([,,,, productRelationsReceived$Subscription] = subscribe.mock.calls);
+      ([, , , , , productRelationsReceived$Subscription] = subscribe.mock.calls);
     });
 
     it('should dispatch fetchProductsById when called', () => {
@@ -66,7 +98,7 @@ describe('Product subscription', () => {
     let visibleProductNotFound$Subscription;
 
     beforeAll(() => {
-      ([,,, visibleProductNotFound$Subscription] = subscribe.mock.calls);
+      ([, , , , visibleProductNotFound$Subscription] = subscribe.mock.calls);
     });
 
     it('should call subscription on correct stream', () => {

diff --git a/libraries/common/components/Swiper/index.jsx b/libraries/common/components/Swiper/index.jsx
@@ -5,7 +5,7 @@ import cls from 'classnames';
 import IDSwiper from 'react-id-swiper';
 import { Pagination, Navigation, Autoplay, Zoom } from 'swiper/dist/js/swiper.esm';
 import SwiperItem from './components/SwiperItem';
-import { container, innerContainer, bullet, bulletActive } from './styles';
+import { container, innerContainer, zoomFix, bullet, bulletActive } from './styles';
 
 /**
  * The basic swiper component.
@@ -42,55 +42,40 @@ function Swiper(props) {
     // Only update the instance, when it differs from the current one.
     if (instance !== null && instance !== swiper) {
       setSwiper(instance);
-    }
-  };
-
-  /**
-   * Removes all event listeners from the swiper instance.
-   */
-  const removeEventListeners = () => {
-    if (swiper !== null) {
-      swiper.off('slideChange');
-      swiper.off('transitionEnd');
-      swiper.off('beforeDestroy');
-    }
-  };
-
-  useEffect(() => {
-    if (swiper !== null) {
-      removeEventListeners();
 
-      if (loop) {
-        // Recreate the loop on prop updates to avoid duplicated slides from the last slide set.
-        swiper.loopCreate();
-      }
-
-      swiper.update();
-      swiper.on('slideChange', () => onSlideChange(swiper.realIndex));
-      swiper.on('transitionEnd', () => {
+      instance.on('slideChange', () => onSlideChange(instance.realIndex));
+      instance.on('transitionEnd', () => {
         // In loop mode the Swiper duplicates elements, which are not in the virtual DOM
         if (loop) {
-          const autoplayRunning = swiper.autoplay.running;
-          const previousIndex = swiper.activeIndex;
+          const autoplayRunning = instance.autoplay.running;
+          const previousIndex = instance.activeIndex;
 
           // Skip duplicated elements
-          if (swiper.activeIndex < 1) {
-            swiper.slideToLoop(children.length - 1, 0);
-          } else if (swiper.activeIndex > children.length) {
-            swiper.slideToLoop(0, 0);
+          if (instance.activeIndex < 1) {
+            instance.slideToLoop(children.length - 1, 0);
+          } else if (instance.activeIndex > children.length) {
+            instance.slideToLoop(0, 0);
           }
 
-          if (autoplayRunning && swiper.activeIndex !== previousIndex) {
+          if (autoplayRunning && instance.activeIndex !== previousIndex) {
             // Restart the autoplay when it was active before the auto slide.
-            swiper.autoplay.start();
+            instance.autoplay.start();
           }
         }
       });
-      swiper.on('beforeDestroy', () => removeEventListeners());
     }
+  };
 
-    return () => removeEventListeners();
-  }, [swiper, children]);
+  useEffect(() => {
+    if (swiper !== null) {
+      if (loop) {
+        // Recreate the loop on prop updates to avoid duplicated slides from the last slide set.
+        swiper.loopCreate();
+      }
+
+      swiper.update();
+    }
+  }, [children]);
 
   const useFraction = (maxIndicators && maxIndicators < children.length);
   const paginationType = useFraction ? 'fraction' : 'bullets';
@@ -111,7 +96,7 @@ function Swiper(props) {
 
   const params = {
     modules: [Pagination, Navigation, Autoplay, Zoom],
-    containerClass: cls(innerContainer, classNames.container),
+    containerClass: cls(innerContainer, classNames.container, { [zoomFix]: zoom }),
     autoplay: autoPlay ? {
       delay: interval,
     } : false,

diff --git a/libraries/common/components/Swiper/styles.js b/libraries/common/components/Swiper/styles.js
@@ -15,6 +15,15 @@ export const innerContainer = css({
   },
 }).toString();
 
+/**
+ * Prevents a visible shrink animation of swiped out slides which where in a zoomed state before.
+ */
+export const zoomFix = css({
+  ' .swiper-slide': {
+    overflow: 'hidden',
+  },
+}).toString();
+
 export const wrapper = css({
   flexShrink: 0,
 });

diff --git a/themes/theme-gmd/pages/ProductGallery/components/Content/__snapshots__/spec.jsx.snap b/themes/theme-gmd/pages/ProductGallery/components/Content/__snapshots__/spec.jsx.snap
@@ -85,7 +85,7 @@ exports[`<ProductGallery.Content> page should pass initialSlide prop 1`] = `
               allowSlideNext={true}
               allowSlidePrev={true}
               autoplay={false}
-              containerClass="css-1n5i2c5 css-sa13d4"
+              containerClass="css-1n5i2c5 css-sa13d4 css-qb469q"
               freeMode={false}
               getSwiper={[Function]}
               initialSlide={3}
@@ -230,7 +230,7 @@ exports[`<ProductGallery.Content> page should pass initialSlide prop 1`] = `
               }
             >
               <div
-                className="css-1n5i2c5 css-sa13d4"
+                className="css-1n5i2c5 css-sa13d4 css-qb469q"
               >
                 <div
                   className="swiper-wrapper"
@@ -372,7 +372,7 @@ exports[`<ProductGallery.Content> page should render Swiper with images 1`] = `
               allowSlideNext={true}
               allowSlidePrev={true}
               autoplay={false}
-              containerClass="css-1n5i2c5 css-sa13d4"
+              containerClass="css-1n5i2c5 css-sa13d4 css-qb469q"
               freeMode={false}
               getSwiper={[Function]}
               initialSlide={0}
@@ -517,7 +517,7 @@ exports[`<ProductGallery.Content> page should render Swiper with images 1`] = `
               }
             >
               <div
-                className="css-1n5i2c5 css-sa13d4"
+                className="css-1n5i2c5 css-sa13d4 css-qb469q"
               >
                 <div
                   className="swiper-wrapper"

diff --git a/themes/theme-ios11/pages/ProductGallery/components/Content/__snapshots__/spec.jsx.snap b/themes/theme-ios11/pages/ProductGallery/components/Content/__snapshots__/spec.jsx.snap
@@ -84,7 +84,7 @@ exports[`<ProductGallery.Content> page should pass initialSlide prop 1`] = `
               allowSlideNext={true}
               allowSlidePrev={true}
               autoplay={false}
-              containerClass="css-1n5i2c5 css-sa13d4"
+              containerClass="css-1n5i2c5 css-sa13d4 css-qb469q"
               freeMode={false}
               getSwiper={[Function]}
               initialSlide={3}
@@ -228,7 +228,7 @@ exports[`<ProductGallery.Content> page should pass initialSlide prop 1`] = `
               }
             >
               <div
-                className="css-1n5i2c5 css-sa13d4"
+                className="css-1n5i2c5 css-sa13d4 css-qb469q"
               >
                 <div
                   className="swiper-wrapper"
@@ -369,7 +369,7 @@ exports[`<ProductGallery.Content> page should render Swiper with images 1`] = `
               allowSlideNext={true}
               allowSlidePrev={true}
               autoplay={false}
-              containerClass="css-1n5i2c5 css-sa13d4"
+              containerClass="css-1n5i2c5 css-sa13d4 css-qb469q"
               freeMode={false}
               getSwiper={[Function]}
               initialSlide={0}
@@ -513,7 +513,7 @@ exports[`<ProductGallery.Content> page should render Swiper with images 1`] = `
               }
             >
               <div
-                className="css-1n5i2c5 css-sa13d4"
+                className="css-1n5i2c5 css-sa13d4 css-qb469q"
               >
                 <div
                   className="swiper-wrapper"