Handle excluding elements from lightbox

ampproject · Jan 26, 2018 · 267f996 · 267f996
1 parent 2e31805
commit 267f996
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 19 deletions.
diff --git a/extensions/amp-lightbox-viewer/0.1/amp-lightbox-viewer.js b/extensions/amp-lightbox-viewer/0.1/amp-lightbox-viewer.js
@@ -27,7 +27,10 @@ import {Layout} from '../../../src/layout';
 import {user, dev} from '../../../src/log';
 import {toggle, setStyle} from '../../../src/style';
 import {getData, listen} from '../../../src/event-helper';
-import {LightboxManager} from './service/lightbox-manager-impl';
+import {
+  LightboxManager,
+  LightboxedCarouselMetadataDef,
+} from './service/lightbox-manager-impl';
 import {layoutRectFromDomRect} from '../../../src/layout-rect';
 import {closest, elementByTag, scopedQuerySelector} from '../../../src/dom';
 import * as st from '../../../src/style';
@@ -308,12 +311,6 @@ export class AmpLightboxViewer extends AMP.BaseElement {
   slideChangeHandler_(event) {
     this.currentElemId_ = getData(event)['index'];
     this.updateDescriptionBox_();
-    const sourceCarousel = this.manager_
-        .getCarouselForLightboxGroup(this.currentLightboxGroupId_);
-    if (sourceCarousel) {
-      /**@type {?}*/ (sourceCarousel).implementation_
-          .showSlideWhenReady(this.currentElemId_);
-    }
   }
 
   /**
@@ -906,6 +903,27 @@ export class AmpLightboxViewer extends AMP.BaseElement {
         MAX_TRANSITION_DURATION
     );
   }
+
+  maybeSyncSourceCarousel_() {
+    if (this.manager_.hasCarousel(this.currentLightboxGroupId_)) {
+      const lightboxCarouselMetadata = this.manager_
+          .getCarouselMetadataForLightboxGroup(this.currentLightboxGroupId_);
+
+      let returnSlideIndex = this.currentElemId_;
+
+      lightboxCarouselMetadata.excludedIndexes.some(i => {
+        if (i <= returnSlideIndex) {
+          returnSlideIndex++;
+        } else {
+          return true;
+        }
+      });
+
+      /**@type {?}*/ (lightboxCarouselMetadata.sourceCarousel).implementation_
+          .showSlideWhenReady(returnSlideIndex);
+    }
+  }
+
   /**
    * Closes the lightbox-viewer
    * @return {!Promise}
@@ -920,6 +938,8 @@ export class AmpLightboxViewer extends AMP.BaseElement {
 
     this.cleanupEventListeners_();
 
+    this.maybeSyncSourceCarousel_();
+
     this.vsync_.mutate(() => {
       // If there's gallery, set gallery to display none
       this.container_.removeAttribute('gallery-view');

diff --git a/extensions/amp-lightbox-viewer/0.1/service/lightbox-manager-impl.js b/extensions/amp-lightbox-viewer/0.1/service/lightbox-manager-impl.js
@@ -24,6 +24,7 @@ import {
 } from '../../../../src/dom';
 import {toArray} from '../../../../src/types';
 import {CommonSignals} from '../../../../src/common-signals';
+import {hasOwn, map} from '../../../../src/utils/object';
 
 const LIGHTBOX_ELIGIBLE_TAGS = {
   'amp-img': true,
@@ -46,7 +47,13 @@ const VALIDATION_ERROR_MSG = `lightbox attribute is only supported for the
  *  url: string,
  *  element: !Element
  * }} */
-let LightboxThumbnailDataDef;
+export let LightboxThumbnailDataDef;
+
+/** @typedef {{
+ *  sourceCarousel: !Element,
+ *  excludedIndexes: !Array<number>
+ * }} */
+let LightboxedCarouselMetadataDef;
 
 /**
  * LightboxManager is a document-scoped service responsible for:
@@ -80,9 +87,9 @@ export class LightboxManager {
      * Ordered lists of lightboxable elements according to group
      * @private {!Object<string, !Array<!Element>>}
      */
-    this.lightboxGroups_ = {
+    this.lightboxGroups_ = map({
       default: [],
-    };
+    });
 
     /**
      * Counter tracking number of carousels without ids
@@ -93,9 +100,9 @@ export class LightboxManager {
     /**
      * If the lightbox group is a carousel, this object contains a
      * mapping of the lightbox group id to the carousel element.
-     * @private {!Object<string, !Element>}
+     * @private {!Object<string, !LightboxedCarouselMetadataDef>}
      */
-    this.lightboxSourceCarousels_ = {};
+    this.lightboxSourceCarousels_ = map();
   }
 
   /**
@@ -114,22 +121,35 @@ export class LightboxManager {
    * Returns a reference to the source carousel of the lightbox
    * group if one exists.
    * @param {string} lightboxGroupId
-   * @return {Element|null}
+   * @return {LightboxedCarouselMetadataDef}
    */
-  getCarouselForLightboxGroup(lightboxGroupId) {
-    if (this.lightboxSourceCarousels_.hasOwnProperty(lightboxGroupId)) {
+  getCarouselMetadataForLightboxGroup(lightboxGroupId) {
+    if (hasOwn(this.lightboxSourceCarousels_, lightboxGroupId)) {
       return this.lightboxSourceCarousels_[lightboxGroupId];
     }
     return null;
   }
 
+  /**
+   * Returns a reference to the source carousel of the lightbox
+   * group if one exists.
+   * @param {string} lightboxGroupId
+   * @return {Array<number>}
+   */
+  getLightboxExcludedIndexes(lightboxGroupId) {
+    if (hasOwn(this.lightboxSourceCarousels_, lightboxGroupId)) {
+      return this.lightboxSourceCarousels_[lightboxGroupId].excludedIndexes;
+    }
+    return [];
+  }
+
   /**
    * Returns true if the lightboxGroupId belongs to an amp carousel
    * @param {string} lightboxGroupId
    * @return {boolean}
    */
   hasCarousel(lightboxGroupId) {
-    return this.lightboxSourceCarousels_.hasOwnProperty(lightboxGroupId);
+    return hasOwn(this.lightboxSourceCarousels_, lightboxGroupId);
   }
 
   /**
@@ -185,13 +205,22 @@ export class LightboxManager {
     const lightboxGroupId = carousel.getAttribute('lightbox') ||
     'carousel' + (carousel.getAttribute('id') || this.counter_++);
     if (carousel.getAttribute('type') == 'slides') {
-      this.lightboxSourceCarousels_[lightboxGroupId] = carousel;
+      this.lightboxSourceCarousels_[lightboxGroupId] = map({
+        'sourceCarousel': carousel,
+        'excludedIndexes': [],
+      });
       // TODO (#13011): scroll carousel needs to support goToSlide
       // before we can use it for lightbox, so they currently don't count.
     }
     this.getSlidesFromCarousel_(carousel).then(slides => {
-      slides.forEach(slide => {
-        if (!slide.hasAttribute('lightbox-exclude')) {
+      slides.forEach((slide, index) => {
+        const shouldExcludeSlide = slide.hasAttribute('lightbox-exclude')
+            || (slide.hasAttribute('lightbox')
+                && slide.getAttribute('lightbox') !== lightboxGroupId);
+        if (shouldExcludeSlide) {
+          this.lightboxSourceCarousels_[lightboxGroupId]
+              .excludedIndexes.push(index);
+        } else {
           slide.setAttribute('lightbox', lightboxGroupId);
           this.processBaseLightboxElement_(slide, lightboxGroupId);
         }