From 267f99671a2b032b151ba09607e11178965ef6f6 Mon Sep 17 00:00:00 2001 From: Cathy Zhu Date: Fri, 26 Jan 2018 12:11:05 -0800 Subject: [PATCH] Handle excluding elements from lightbox --- .../0.1/amp-lightbox-viewer.js | 34 +++++++++--- .../0.1/service/lightbox-manager-impl.js | 53 ++++++++++++++----- 2 files changed, 68 insertions(+), 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 index 9a96dd0458182..fdeb7155dbaf7 100644 --- 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 index 520dfb66ad76a..8c64222a4e6ad 100644 --- 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 + * }} */ +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>} */ - 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} + * @private {!Object} */ - 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} + */ + 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); }