🚀 [Story performance] Move aspect-ratio logic to CSS for styling (#36061

) * Aspect-ratio logic in CSS * Started setting up tests * Fixed tests * Updated code
ampproject · Oct 14, 2021 · 6b53dc6 · 6b53dc6
1 parent 79ce7e8
commit 6b53dc6
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 144 deletions.
diff --git a/examples/amp-story/grid-layer-presets.html b/examples/amp-story/grid-layer-presets.html
@@ -75,7 +75,7 @@
   </head>
 
   <body>
-    <amp-story standalone title="Grid layer presets showcase">
+    <amp-story standalone title="Grid layer presets showcase" supports-landscape>
       <amp-story-page id="cover">
         <amp-story-grid-layer template="fill">
           <amp-img src="https://images.unsplash.com/photo-1522072629756-c9274e34ef5e?w=1000&q=80"

diff --git a/extensions/amp-story/1.0/amp-story-grid-layer.js b/extensions/amp-story/1.0/amp-story-grid-layer.js
@@ -11,8 +11,11 @@
  */
 
 import {AmpStoryBaseLayer} from './amp-story-base-layer';
-import {StateProperty, getStoreService} from './amp-story-store-service';
-import {assertDoesNotContainDisplay, px, setStyles} from '#core/dom/style';
+import {
+  assertDoesNotContainDisplay,
+  setImportantStyles,
+  setStyles,
+} from '#core/dom/style';
 import {isPrerenderActivePage} from './prerender-active-page';
 import {scopedQuerySelectorAll} from '#core/dom/query';
 
@@ -43,12 +46,6 @@ const SUPPORTED_CSS_GRID_ATTRIBUTES_SELECTOR = Object.keys(
   .map((key) => `[${key}]`)
   .join(',');
 
-/**
- * The attribute name for grid layer presets.
- * @private @const {string}
- */
-const PRESET_ATTRIBUTE_NAME = 'preset';
-
 /**
  * @typedef {{
  *  aspect-ratio: string,
@@ -57,20 +54,6 @@ const PRESET_ATTRIBUTE_NAME = 'preset';
  */
 export let PresetDetails;
 
-/**
- * The attributes that will be applied for each preset.
- * @private @const {!Object<string, !PresetDetails>}
- */
-const GRID_LAYER_PRESET_DETAILS = {
-  '2021-background': {
-    'aspect-ratio': '69:116',
-    'scaling-factor': 1.142,
-  },
-  '2021-foreground': {
-    'aspect-ratio': '69:116',
-  },
-};
-
 /**
  * Grid layer template templating system.
  */
@@ -83,86 +66,29 @@ export class AmpStoryGridLayer extends AmpStoryBaseLayer {
   /** @param {!AmpElement} element */
   constructor(element) {
     super(element);
-
-    /** @private {?{horiz: number, vert: number}} */
-    this.aspectRatio_ = null;
-
-    /** @private {number} */
-    this.scalingFactor_ = 1;
   }
 
   /** @override */
   buildCallback() {
     super.buildCallback();
-    this.applyResponsivenessPresets_();
+    this.applyAspectRatioAttributes_();
     this.setOwnCssGridStyles_();
     this.setDescendentCssGridStyles_();
-    this.initializeListeners_();
-  }
-
-  /**
-   * Applies the attributes to the layer from the preset specified in the [preset] attribute.
-   * @private
-   */
-  applyResponsivenessPresets_() {
-    if (!this.element.hasAttribute(PRESET_ATTRIBUTE_NAME)) {
-      return;
-    }
-    const preset = this.element.getAttribute(PRESET_ATTRIBUTE_NAME);
-    const presetDetails = GRID_LAYER_PRESET_DETAILS[preset];
-    if (!presetDetails) {
-      return;
-    }
-    Object.entries(presetDetails).forEach((keyValue) =>
-      this.element.setAttribute(keyValue[0], keyValue[1])
-    );
-  }
-
-  /** @private */
-  initializeListeners_() {
-    const aspectRatio = this.element.getAttribute('aspect-ratio');
-    const scalingFactorFloat = parseFloat(
-      this.element.getAttribute('scaling-factor')
-    );
-    if (scalingFactorFloat && scalingFactorFloat > 0) {
-      this.scalingFactor_ = scalingFactorFloat;
-    }
-    if (aspectRatio) {
-      const aspectRatioSplits = aspectRatio.split(':');
-      const horiz = parseInt(aspectRatioSplits[0], 10);
-      const vert = parseInt(aspectRatioSplits[1], 10);
-      if (horiz > 0 && vert > 0) {
-        this.aspectRatio_ = {horiz, vert};
-        const storeService = getStoreService(this.win);
-        storeService.subscribe(
-          StateProperty.PAGE_SIZE,
-          this.updatePageSize_.bind(this),
-          true /* callToInitialize */
-        );
-      }
-    }
   }
 
   /**
-   * @param {?{width: number, height: number}} pageSize
+   * Grab the aspect-ratio attribute and apply to CSS variable as a fraction.
    * @private
    */
-  updatePageSize_(pageSize) {
-    if (!pageSize) {
+  applyAspectRatioAttributes_() {
+    if (!this.element.hasAttribute('aspect-ratio')) {
       return;
     }
-    const {height: vh, width: vw} = pageSize;
-    const {horiz, vert} = this.aspectRatio_;
-    const width = Math.min(vw, (vh * horiz) / vert);
-    const height = Math.min(vh, (vw * vert) / horiz);
-    if (width > 0 && height > 0) {
-      this.getVsync().mutate(() => {
-        setStyles(this.element, {
-          '--i-amphtml-story-layer-width': px(width * this.scalingFactor_),
-          '--i-amphtml-story-layer-height': px(height * this.scalingFactor_),
-        });
-      });
-    }
+    setImportantStyles(this.element, {
+      '--aspect-ratio': this.element
+        .getAttribute('aspect-ratio')
+        .replace(':', '/'),
+    });
   }
 
   /**

diff --git a/extensions/amp-story/1.0/amp-story.css b/extensions/amp-story/1.0/amp-story.css
@@ -409,6 +409,23 @@ amp-story-grid-layer .i-amphtml-embedded-component::after {
   height: var(--i-amphtml-story-layer-height, 100%);
   font-size: calc(var(--i-amphtml-story-layer-height, 100vh) / 10);
   margin-left: calc((var(--story-page-vw, 1%) * 100 - var(--i-amphtml-story-layer-width, 100%)) * 0.5); /* Set margin to 1/2 of page-width - layer-width to center layer */
+
+  --i-amphtml-aspect-ratio-float: calc(var(--aspect-ratio)) !important; /* Avoid fractions, which don't work with compilers that remove parenthesis */
+  --i-amphtml-story-page-width: calc(100 * var(--story-page-vw, 1vw)) !important;
+  --i-amphtml-story-page-height: calc(100 * var(--story-page-vh, 1vh)) !important;
+  --i-amphtml-story-unscaled-width: min(var(--i-amphtml-story-page-width), calc(var(--i-amphtml-story-page-height) * var(--i-amphtml-aspect-ratio-float))) !important;
+  --i-amphtml-story-unscaled-height: min(var(--i-amphtml-story-page-height), calc(var(--i-amphtml-story-page-width) / var(--i-amphtml-aspect-ratio-float))) !important;
+  --i-amphtml-story-layer-width: calc(var(--i-amphtml-story-unscaled-width) * var(--scaling-factor, 1)) !important;
+  --i-amphtml-story-layer-height: calc(var(--i-amphtml-story-unscaled-height) * var(--scaling-factor, 1)) !important;
+}
+
+[preset="2021-foreground"] {
+  --aspect-ratio: 69/116 !important;
+}
+
+[preset="2021-background"] {
+  --aspect-ratio: 69/116 !important;
+  --scaling-factor: 1.142 !important;
 }
 
 /**

diff --git a/extensions/amp-story/1.0/test/test-amp-story-grid-layer.js b/extensions/amp-story/1.0/test/test-amp-story-grid-layer.js
@@ -5,6 +5,8 @@ import {AmpStoryPage} from '../amp-story-page';
 import {MediaType} from '../media-pool';
 import {Services} from '#service';
 import {registerServiceBuilder} from '../../../../src/service-helpers';
+import {setStyles} from '#core/dom/style';
+import {CSS} from '../../../../build/amp-story-1.0.css';
 
 describes.realWin('amp-story-grid-layer', {amp: true}, (env) => {
   let win;
@@ -56,6 +58,16 @@ describes.realWin('amp-story-grid-layer', {amp: true}, (env) => {
     env.sandbox.stub(page, 'mutateElement').callsFake((fn) => fn());
 
     grid = new AmpStoryGridLayer(gridLayerEl);
+
+    // Add CSS styles and set page width/height to 1000px.
+    const styles = env.win.document.createElement('style');
+    styles.textContent = CSS;
+    env.win.document.head.appendChild(styles);
+
+    setStyles(gridLayerEl, {
+      '--story-page-vw': `10px`,
+      '--story-page-vh': `10px`,
+    });
   });
 
   afterEach(() => {
@@ -70,79 +82,39 @@ describes.realWin('amp-story-grid-layer', {amp: true}, (env) => {
   }
 
   it('should set the vertical aspect-ratio', async () => {
-    gridLayerEl.setAttribute('aspect-ratio', '9:16');
+    gridLayerEl.setAttribute('aspect-ratio', '10:16');
     await buildGridLayer();
 
-    storeService.dispatch(Action.SET_PAGE_SIZE, {width: 1000, height: 1000});
-
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-width'),
-        10
-      )
-    ).to.equal(562);
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-height'),
-        10
-      )
-    ).to.equal(1000);
+    expect(gridLayerEl.offsetWidth).to.equal(625);
+    expect(gridLayerEl.offsetHeight).to.equal(1000);
   });
 
   it('should set the horizontal aspect-ratio', async () => {
-    gridLayerEl.setAttribute('aspect-ratio', '16:9');
+    gridLayerEl.setAttribute('aspect-ratio', '16:10');
     await buildGridLayer();
 
-    storeService.dispatch(Action.SET_PAGE_SIZE, {width: 1000, height: 1000});
-
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-width'),
-        10
-      )
-    ).to.equal(1000);
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-height'),
-        10
-      )
-    ).to.equal(562);
+    expect(gridLayerEl.offsetWidth).to.equal(1000);
+    expect(gridLayerEl.offsetHeight).to.equal(625);
   });
 
   it('should use the scaling factor to set the size of the layer', async () => {
+    setStyles(gridLayerEl, {
+      '--aspect-ratio': '1/2',
+      '--scaling-factor': 1.5,
+    });
     gridLayerEl.setAttribute('aspect-ratio', '1:2');
-    gridLayerEl.setAttribute('scaling-factor', '1.5');
-    await buildGridLayer();
-
-    storeService.dispatch(Action.SET_PAGE_SIZE, {width: 1000, height: 1000});
 
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-width'),
-        10
-      )
-    ).to.equal(750);
-    expect(
-      parseInt(
-        gridLayerEl.style.getPropertyValue('--i-amphtml-story-layer-height'),
-        10
-      )
-    ).to.equal(1500);
+    expect(gridLayerEl.offsetWidth).to.equal(750);
+    expect(gridLayerEl.offsetHeight).to.equal(1500);
   });
 
   it('should apply the aspect-ratio attribute from the responsiveness preset', async () => {
     gridLayerEl.setAttribute('preset', '2021-foreground');
-    await buildGridLayer();
 
     storeService.dispatch(Action.SET_PAGE_SIZE, {width: 1000, height: 1000});
 
-    expect(gridLayerEl.getAttribute('aspect-ratio')).to.equal('69:116');
-  });
-
-  it('should not add aspect-ratio attribute if preset passed is incorrect', async () => {
-    gridLayerEl.setAttribute('preset', 'wrong-preset');
-    await buildGridLayer();
-
-    expect(gridLayerEl.hasAttribute('aspect-ratio')).to.be.false;
+    expect(
+      getComputedStyle(gridLayerEl).getPropertyValue('--aspect-ratio')
+    ).to.equal('69/116');
   });
 });