✨ [Amp story panning media] Support portrait images (#36053)

* Handle container aspect ratio. * Update template. * Measure element instead of page. * Remove whitespace. * Revert template change. * Update test
ampproject · Sep 15, 2021 · 41d08bb · 41d08bb
1 parent cfb102a
commit 41d08bb
Show file tree

Hide file tree

Showing 5 changed files with 164 additions and 20 deletions.
diff --git a/examples/amp-story/amp-story-panning-media.html b/examples/amp-story/amp-story-panning-media.html
@@ -29,6 +29,7 @@
       publisher="AMP Story"
       publisher-logo-src="example.com/logo.png"
       poster-portrait-src="example.com/poster.jpg"
+      supports-landscape
     >
       <!-- coordinates example -->
       <amp-story-page id="painting-1">
@@ -265,6 +266,105 @@ <h3>x="-50%" y="-50%" zoom="2" lock-bounds</h3>
         </amp-story-grid-layer>
       </amp-story-page>
 
+      <!-- Portrait image example -->
+      <amp-story-page id="painting-portrait-0">
+        <amp-story-grid-layer template="fill">
+          <amp-story-panning-media
+            layout="fill"
+            x="0"
+            y="0"
+            zoom="1"
+          >
+            <amp-img
+              layout="fill"
+              width="1356"
+              height="2334"
+              src="img/van-coords.jpg"
+            ></amp-img>
+          </amp-story-panning-media>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="fill">
+          <div class="text-wrapper">
+
+            <h3>x="0%" y="0" zoom="1"</h3>
+          </div>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
+      <amp-story-page id="painting-portrait-1">
+        <amp-story-grid-layer template="fill">
+          <amp-story-panning-media
+            layout="fill"
+            x="-50%"
+            y="50%"
+            zoom="1"
+          >
+            <amp-img
+              layout="fill"
+              width="1356"
+              height="2334"
+              src="img/van-coords.jpg"
+            ></amp-img>
+          </amp-story-panning-media>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="fill">
+          <div class="text-wrapper">
+
+            <h3>x="-50%" y="50%" zoom="1"</h3>
+          </div>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
+      <amp-story-page id="painting-portrait-2">
+        <amp-story-grid-layer template="fill">
+          <amp-story-panning-media
+            layout="fill"
+            x="0"
+            y="0"
+            zoom="1"
+            lock-bounds
+          >
+            <amp-img
+              layout="fill"
+              width="1356"
+              height="2334"
+              src="img/van-coords.jpg"
+            ></amp-img>
+          </amp-story-panning-media>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="fill">
+          <div class="text-wrapper">
+
+            <h3>x="0" y="0" zoom="1" lock-bounds</h3>
+          </div>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
+      <amp-story-page id="painting-portrait-3">
+        <amp-story-grid-layer template="fill">
+          <amp-story-panning-media
+            layout="fill"
+            x="-50%"
+            y="50%"
+            zoom="1"
+            lock-bounds
+          >
+            <amp-img
+              layout="fill"
+              width="1356"
+              height="2334"
+              src="img/van-coords.jpg"
+            ></amp-img>
+          </amp-story-panning-media>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="fill">
+          <div class="text-wrapper">
+
+            <h3>x="-50%" y="50%" zoom="1" lock-bounds</h3>
+          </div>
+        </amp-story-grid-layer>
+      </amp-story-page>
+
       <!-- multiple per page example -->
       <amp-story-page id="stars-1">
         <amp-story-grid-layer template="fill">

diff --git a/examples/amp-story/img/van-coords.jpg b/examples/amp-story/img/van-coords.jpg
diff --git a/extensions/amp-story-panning-media/0.1/amp-story-panning-media.css b/extensions/amp-story-panning-media/0.1/amp-story-panning-media.css
@@ -17,11 +17,11 @@ amp-story-panning-media {
 }
 
 amp-story-panning-media amp-img {
-backface-visibility: hidden !important;
+  backface-visibility: hidden !important;
 }
+
 amp-story-grid-layer[template="fill"] amp-story-panning-media amp-img img {
-  /* override for layout=fill. This centers the image without clipping it */
+  /* override for layout=fill */
   position: relative !important;
   display: block !important;
-  height: 100% !important;
 }
diff --git a/extensions/amp-story-panning-media/0.1/amp-story-panning-media.js b/extensions/amp-story-panning-media/0.1/amp-story-panning-media.js
@@ -72,7 +72,7 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
     this.animateTo_ = {};
 
     /** @private {?{width: number, height: number}} */
-    this.pageSize_ = null;
+    this.elementSize_ = null;
 
     /** @private {?panningMediaPositionDef} Current animation state. */
     this.animationState_ = {};
@@ -120,12 +120,7 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
         const imgEl = dev().assertElement(this.element_.querySelector('img'));
         // Remove layout="fill" classes so image is not clipped.
         imgEl.classList = '';
-        // Centers the amp-img horizontally. The image does not load if this is done in CSS.
-        // TODO(#31515): Handle base zoom of aspect ratio wider than image
-        setImportantStyles(this.ampImgEl_, {
-          left: 'auto',
-          right: 'auto',
-        });
+        this.setImageCenteringStyles_();
       })
       .catch(() => user().error(TAG, 'Failed to load the amp-img.'));
   }
@@ -134,8 +129,12 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
   initializeListeners_() {
     this.storeService_.subscribe(
       StateProperty.PAGE_SIZE,
-      (pageSize) => {
-        this.pageSize_ = pageSize;
+      () => {
+        this.elementSize_ = {
+          width: this.element_./*OK*/ offsetWidth,
+          height: this.element_./*OK*/ offsetHeight,
+        };
+        this.setImageCenteringStyles_();
         this.setAnimateTo_();
         this.animate_();
       },
@@ -191,7 +190,7 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
    */
   getMaxBounds_() {
     // Calculations to clamp image to edge of container.
-    const {height: containerHeight, width: containerWidth} = this.pageSize_;
+    const {height, width} = this.elementSize_;
 
     const ampImgWidth = this.ampImgEl_.getAttribute('width');
     const ampImgHeight = this.ampImgEl_.getAttribute('height');
@@ -201,15 +200,18 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
         '"lock-bounds" requires "width" and "height" to be set on the amp-img child.'
       );
     }
-    // TODO(#31515): When aspect ratio is portrait, containerWidth will be used for this.
-    const percentScaledToFitViewport = containerHeight / ampImgHeight;
+
+    const containerRatio = width / height;
+    const imageRatio = ampImgWidth / ampImgHeight;
+    const percentScaledToFitViewport =
+      containerRatio < imageRatio ? height / ampImgHeight : width / ampImgWidth;
+
     const scaledImageWidth = percentScaledToFitViewport * ampImgWidth;
     const scaledImageHeight = percentScaledToFitViewport * ampImgHeight;
 
-    const widthFraction =
-      1 - containerWidth / (scaledImageWidth * this.animateTo_.zoom);
+    const widthFraction = 1 - width / (scaledImageWidth * this.animateTo_.zoom);
     const heightFraction =
-      1 - containerHeight / (scaledImageHeight * this.animateTo_.zoom);
+      1 - height / (scaledImageHeight * this.animateTo_.zoom);
 
     return {
       horizontal: DISTANCE_TO_CENTER_EDGE_PERCENT * widthFraction,
@@ -269,6 +271,49 @@ export class AmpStoryPanningMedia extends AMP.BaseElement {
     requestAnimationFrame(nextFrame);
   }
 
+  /**
+   * Centers the amp-img horizontally or vertically based on aspect ratio.
+   * The img element does not load if this is done in CSS.
+   * @private
+   */
+  setImageCenteringStyles_() {
+    const imgEl = this.element_.querySelector('img');
+    if (!imgEl) {
+      return;
+    }
+    const {height, width} = this.elementSize_;
+    const containerRatio = width / height;
+    const ampImgWidth = this.ampImgEl_.getAttribute('width');
+    const ampImgHeight = this.ampImgEl_.getAttribute('height');
+    const imageRatio = ampImgWidth / ampImgHeight;
+
+    this.mutateElement(() => {
+      if (containerRatio < imageRatio) {
+        setImportantStyles(this.ampImgEl_, {
+          left: 'auto',
+          right: 'auto',
+          top: '0',
+          bottom: '0',
+        });
+        setImportantStyles(imgEl, {
+          width: 'auto',
+          height: '100%',
+        });
+      } else {
+        setImportantStyles(this.ampImgEl_, {
+          left: '0',
+          right: '0',
+          top: 'auto',
+          bottom: 'auto',
+        });
+        setImportantStyles(imgEl, {
+          width: '100%',
+          height: 'auto',
+        });
+      }
+    });
+  }
+
   /**
    * @private
    * @param {!Object<string, string>} panningMediaState

diff --git a/extensions/amp-story-panning-media/0.1/test/test-amp-story-panning-media.js b/extensions/amp-story-panning-media/0.1/test/test-amp-story-panning-media.js
@@ -101,10 +101,9 @@ describes.realWin(
       );
     });
 
-    it('calculates transform with lock-bounds', async () => {
+    it('calculates zoom with lock-bounds', async () => {
       const attributes = {
         'group-id': 'group-1',
-        'y': '50%',
         'zoom': 0.2,
         'lock-bounds': '',
       };