Review comments.

Author: Sepand Parhami

src/img-dimensions.ts

@@ -42,52 +42,52 @@
   * @param naturalSize The natural dimensions of the image.
   * @param containerSize The size of the container we want to render the image
   *     in.
-  * @param toMin If we whould cap the smaller dimension of the image to fit the
+  * @param toMin If we should cap the smaller dimension of the image to fit the
   *     container (`object-fit: cover`) or the larger dimension 
   *     (`object-fit: contain`).
   * @return The Size that the image should be rendered as.
   */
 function constrain(
     naturalSize: Size, containerSize: Size, toMin: boolean): Size {
-  const {width, height} = containerSize;
-  const elAspectRatio = width / height;
+  const elAspectRatio = containerSize.width / containerSize.height;
   const naturalAspectRatio = naturalSize.width / naturalSize.height;
 
   if (naturalAspectRatio > elAspectRatio !== toMin) {
     return {
-      width: height * naturalAspectRatio,
-      height,
+      width: containerSize.height * naturalAspectRatio,
+      height: containerSize.height,
     };
   }
 
   return {
-    width,
-    height: width / naturalAspectRatio,
+    width: containerSize.width,
+    height: containerSize.width / naturalAspectRatio,
   };
 }
 
-function getDimensionsForCover(
+function getDimensionsForObjectFitCover(
     naturalSize: Size, containerSize: Size): Size {
   return constrain(naturalSize, containerSize, false);
 }
 
-function getDimensionsForContain(
+function getDimensionsForObjectFitContain(
     naturalSize: Size, containerSize: Size): Size {
   return constrain(naturalSize, containerSize, true);
 }
 
-function getDimensionsForFill(containerSize: Size): Size {
+function getDimensionsForObjectFitFill(containerSize: Size): Size {
   return containerSize;
 }
 
-function getDimensionsForNone(naturalSize: Size): Size {
+function getDimensionsForObjectFitNone(naturalSize: Size): Size {
   return naturalSize;
 }
 
-function getDimensionsForScaleDown(
+function getDimensionsForObjectFitScaleDown(
     naturalSize: Size, containerSize: Size): Size {
-  const noneSize = getDimensionsForNone(naturalSize);
-  const containSize = getDimensionsForContain(naturalSize, containerSize);
+  const noneSize = getDimensionsForObjectFitNone(naturalSize);
+  const containSize = getDimensionsForObjectFitContain(
+      naturalSize, containerSize);
 
   // Since both have the same aspect ratio, we can simply take the smaller
   // dimension for both.
@@ -114,15 +114,15 @@ export function getRenderedDimensions(
 
   switch(objectFit) {
     case 'cover':
-      return getDimensionsForCover(naturalSize, containerSize);
+      return getDimensionsForObjectFitCover(naturalSize, containerSize);
     case 'contain': 
-      return getDimensionsForContain(naturalSize, containerSize);
+      return getDimensionsForObjectFitContain(naturalSize, containerSize);
     case 'fill':
-      return getDimensionsForFill(containerSize);
+      return getDimensionsForObjectFitFill(containerSize);
     case 'none':
-      return getDimensionsForNone(naturalSize);
+      return getDimensionsForObjectFitNone(naturalSize);
     case 'scale-down':
-      return getDimensionsForScaleDown(naturalSize, containerSize);
+      return getDimensionsForObjectFitScaleDown(naturalSize, containerSize);
     default:
       throw new Error(`object-fit: ${objectFit} not supported`);
   }

src/imitation-img.ts renamed to src/intermdediate-img.ts

@@ -14,6 +14,13 @@
  * limitations under the License.
  */
 
+/**
+ * @fileoverview This file is used to create an image for use in an `<img>` to
+ * `<img>` animation. It is implemented in a way to allow for animating the
+ * cropping of  the rendered image. Once the animation is completed, the
+ * intermediate image can be removed to show the destination `<img>` instead.
+ */
+
 import {Size, getRenderedDimensions} from './img-dimensions.js';
 
 /**
@@ -29,7 +36,7 @@ import {Size, getRenderedDimensions} from './img-dimensions.js';
  *    provided if already measured.
  * @return The replacement container along with structural information.
  */
-export function createImitationImg(
+export function createItermediateImg(
   srcImg: HTMLImageElement,
   srcImgRect: ClientRect = srcImg.getBoundingClientRect(),
   imageDimensions: Size = getRenderedDimensions(srcImg, srcImgRect),

src/test-img-dimensions.ts

@@ -148,7 +148,7 @@ describe('getDimensions', () => {
 
       const {width, height} = getRenderedDimensions(img, dimensions(2, 2));
       expect(width).to.equal(2);
-      expect(height).to.equal(1.5);
+      expect(height).to.equal(3/2);
     });
   });
 });

src/test-imitation-img.ts renamed to src/test-intermediate-img.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {createImitationImg} from './imitation-img';
+import {createItermediateImg} from './intermdediate-img';
 import {imgLoadPromise} from './testing/utils';
 
 const {expect} = chai;
@@ -55,7 +55,7 @@ describe('createImitationImg', () => {
 
     describe('the scaleElement', () => {
       it('should size correctly', async () => {
-        const {scaleElement} = createImitationImg(srcImg);
+        const {scaleElement} = createItermediateImg(srcImg);
         testContainer.appendChild(scaleElement);
 
         const {width, height} = scaleElement.getBoundingClientRect();
@@ -68,7 +68,7 @@ describe('createImitationImg', () => {
       let imgElement;
 
       beforeEach(() => {
-        const {img, scaleElement} = createImitationImg(srcImg);
+        const {img, scaleElement} = createItermediateImg(srcImg);
         imgElement = img;
         testContainer.appendChild(scaleElement);
         scaleElement.style.position = 'fixed';
@@ -107,7 +107,7 @@ describe('createImitationImg', () => {
         let imgElement;
 
         beforeEach(() => {
-          const {img, scaleElement} = createImitationImg(srcImg);
+          const {img, scaleElement} = createItermediateImg(srcImg);
           imgElement = img;
           testContainer.appendChild(scaleElement);
           scaleElement.style.position = 'fixed';
@@ -145,7 +145,7 @@ describe('createImitationImg', () => {
         let imgElement;
 
         beforeEach(() => {
-          const {img, scaleElement} = createImitationImg(srcImg);
+          const {img, scaleElement} = createItermediateImg(srcImg);
           imgElement = img;
           testContainer.appendChild(scaleElement);
           scaleElement.style.position = 'fixed';

src/testing/animation-test-controller.ts

@@ -129,10 +129,10 @@ const styleText = elementSelectors
  * this must be re-created each time, using cloneNode(true) will not work.
  * @return An HTMLStyleElement that stops all animations on all Elements.
  */
-const createStopperStyle = function(): HTMLStyleElement {
-  const stopperStyle = document.createElement('style');
-  stopperStyle.appendChild(document.createTextNode(styleText));
-  return stopperStyle;
+const creatseAnimationPauseStyle = function(): HTMLStyleElement {
+  const styleElement = document.createElement('style');
+  styleElement.appendChild(document.createTextNode(styleText));
+  return styleElement;
 }
 
 /**
@@ -148,9 +148,9 @@ function after(obj: Object, prop: string, callback: Function) {
 
   const old = obj[prop];
   obj[prop] = function() {
-    const retn = old.apply(this, arguments);
-    callback(retn);
-    return retn;
+    const returnValue = old.apply(this, arguments);
+    callback(returnValue);
+    return returnValue;
   }
 }
 
@@ -179,7 +179,7 @@ function attachStyleElement(root: Document|ShadowRoot) {
     return;
   }
 
-  const style = createStopperStyle();
+  const style = creatseAnimationPauseStyle();
 
   if (root == document) {
     root.head!.appendChild(style);

src/testing/test-animation-test-controller.ts

@@ -28,19 +28,17 @@ describe('Animation test controller', () => {
   before(() => {
     const style = document.createElement('style');
     style.textContent = `
-      .animate {
+      .animate,
+      .animate-after::after,
+      .animate-before::before,
+      .animate-backdrop::backdrop  {
         animation: 1000ms foo infinite;
       }
 
-      .delayed {
-        animation-delay: 125ms;
-      }
-
-      .animate-before::before {
-        animation: 1000ms foo infinite;
-      }
-
-      .delayed-before::before {
+      .delayed,
+      .delayed-after::after,
+      .delayed-before::before,
+      .delayed-backdrop::backdrop {
         animation-delay: 125ms;
       }
 
@@ -68,7 +66,7 @@ describe('Animation test controller', () => {
   });
 
   describe('document', () => {
-    it('should stop animations on existing Elements', () => {
+    it('should pause animations on existing Elements', () => {
       const el = document.createElement('div');
       el.className = 'animate';
 
@@ -78,7 +76,7 @@ describe('Animation test controller', () => {
       expect(getComputedStyle(el).animationPlayState).to.equal('paused');
     });
 
-    it('should stop animations on newly added Elements', () => {
+    it('should pause animations on newly added Elements', () => {
       const el = document.createElement('div');
       el.className = 'animate';
 
@@ -88,14 +86,34 @@ describe('Animation test controller', () => {
       expect(getComputedStyle(el).animationPlayState).to.equal('paused');
     });
 
-    it('should stop animations on psuedo elements', () => {
+    it('should pause animations on pseudo after', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-after';
+
+      setupAnimations();
+      container.appendChild(el);
+
+      expect(getComputedStyle(el, '::after').animationPlayState).to.equal('paused');
+    });
+
+    it('should pause animations on pseudo before', () => {
       const el = document.createElement('div');
       el.className = 'animate-before';
 
       setupAnimations();
       container.appendChild(el);
 
-      expect(getComputedStyle(el, ':before').animationPlayState).to.equal('paused');
+      expect(getComputedStyle(el, '::before').animationPlayState).to.equal('paused');
+    });
+
+    it('should pause animations on pseudo backdrop', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-backdrop';
+
+      setupAnimations();
+      container.appendChild(el);
+
+      expect(getComputedStyle(el, '::backdrop').animationPlayState).to.equal('paused');
     });
 
     it('should resume animations', () => {
@@ -131,18 +149,64 @@ describe('Animation test controller', () => {
       expect(getComputedStyle(el).animationDelay).to.equal('0.025s');
     });
 
-    it('should offset a delayed animation on a psuedo element', () => {
+    it('should offset a delayed animation on a pseudo after', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-after delayed-after';
+
+      container.appendChild(el);
+      setupAnimations();
+
+      offset(100);
+      expect(getComputedStyle(el, '::after').animationDelay).to.equal('0.025s');
+    });
+
+    it('should offset a delayed animation on a pseudo before', () => {
       const el = document.createElement('div');
       el.className = 'animate-before delayed-before';
 
       container.appendChild(el);
       setupAnimations();
 
       offset(100);
-      expect(getComputedStyle(el, ':before').animationDelay).to.equal('0.025s');
+      expect(getComputedStyle(el, '::before').animationDelay).to.equal('0.025s');
+    });
+
+    it('should offset a delayed animation on a pseudo backdrop', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-backdrop delayed-backdrop';
+
+      container.appendChild(el);
+      setupAnimations();
+
+      offset(100);
+      expect(getComputedStyle(el, '::backdrop').animationDelay).to.equal('0.025s');
     });
 
     it('should offset multiple times', () => {
+      const el = document.createElement('div');
+      el.className = 'animate delayed';
+
+      container.appendChild(el);
+      setupAnimations();
+
+      offset(100);
+      offset(300);
+      expect(getComputedStyle(el).animationDelay).to.equal('-0.175s');
+    });
+
+    it('should offset multiple times on pseudo after', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-after delayed-after';
+
+      container.appendChild(el);
+      setupAnimations();
+
+      offset(100);
+      offset(300);
+      expect(getComputedStyle(el, '::after').animationDelay).to.equal('-0.175s');
+    });
+    
+    it('should offset multiple times on pseudo before', () => {
       const el = document.createElement('div');
       el.className = 'animate-before delayed-before';
 
@@ -151,7 +215,19 @@ describe('Animation test controller', () => {
 
       offset(100);
       offset(300);
-      expect(getComputedStyle(el).animationDelay).to.equal('-0.3s');
+      expect(getComputedStyle(el, '::before').animationDelay).to.equal('-0.175s');
+    });
+
+    it('should offset multiple times on pseudo backdrop', () => {
+      const el = document.createElement('div');
+      el.className = 'animate-backdrop delayed-backdrop';
+
+      container.appendChild(el);
+      setupAnimations();
+
+      offset(100);
+      offset(300);
+      expect(getComputedStyle(el, '::backdrop').animationDelay).to.equal('-0.175s');
     });
   });
 
@@ -160,7 +236,7 @@ describe('Animation test controller', () => {
       return;
     }
 
-    it('should stop animations on existing Elements', () => {
+    it('should pause animations on existing Elements', () => {
       const outer = document.createElement('div');
       const sr = outer.attachShadow({ mode: 'closed' });
       const el = document.createElement('div');
@@ -173,7 +249,7 @@ describe('Animation test controller', () => {
       expect(getComputedStyle(el).animationPlayState).to.equal('paused');
     });
 
-    it('should stop animations on newly added Elements', () => {
+    it('should pause animations on newly added Elements', () => {
       const outer = document.createElement('div');
       const sr = outer.attachShadow({ mode: 'closed' });
       const el = document.createElement('div');