Address review comments.

Author: Sepand Parhami

src/img-dimensions.ts

@@ -36,9 +36,16 @@
  }
 
  /**
-  * Constrains the size of the image to the given width an height. This either
+  * Constrains the size of the image to the given width and height. This either
   * caps the width or the height depending on the aspect ratio of original img
   * and if we want to have the smaller or larger dimension fit the container.
+  * @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
+  *     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 {

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

@@ -26,11 +26,11 @@ import {getRenderedDimensions} from './img-dimensions.js';
  * @param srcImgRect
  * @return The replacement container along with structural information.
  */
-export function createReplacement(
+export function createImitationImg(
   srcImg: HTMLImageElement,
   srcImgRect: ClientRect = srcImg.getBoundingClientRect(),
 ): {
-  translateElement,
+  translateElement: HTMLElement,
   scaleElement: HTMLElement,
   counterScaleElement: HTMLElement,
   img: HTMLImageElement,

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

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-import {createReplacement} from './replacement-img.js';
-import {imgLoadPromise} from './testing/utils.js';
+import {createImitationImg} from './imitation-img';
+import {imgLoadPromise} from './testing/utils';
 
 const {expect} = chai;
 const fourByThreeUri = 'data:image/gif;base64,R0lGODdhBAADAIAAAP///////ywAAAAABAADAAACA4SPVgA7';
@@ -27,7 +27,7 @@ async function updateImg(img, fit, src) {
   await imgLoadPromise(img);
 }
 
-describe('createReplacement', () => {
+describe('createImitationImg', () => {
   let testContainer;
   let img;
 
@@ -51,7 +51,7 @@ describe('createReplacement', () => {
 
     describe('the scaleElement', () => {
       it('should size correctly', async () => {
-        const {scaleElement} = createReplacement(img);
+        const {scaleElement} = createImitationImg(img);
         testContainer.appendChild(scaleElement);
 
         const {width, height} = scaleElement.getBoundingClientRect();
@@ -65,7 +65,7 @@ describe('createReplacement', () => {
       let replacementImg;
 
       beforeEach(() => {
-        const replacement = createReplacement(img);
+        const replacement = createImitationImg(img);
         scaleElement = replacement.scaleElement;
         replacementImg = replacement.img;
         testContainer.appendChild(scaleElement);
@@ -100,7 +100,7 @@ describe('createReplacement', () => {
         let replacementImg;
 
         beforeEach(() => {
-          const replacement = createReplacement(img);
+          const replacement = createImitationImg(img);
           const scaleElement = replacement.scaleElement;
           replacementImg = replacement.img;
           testContainer.appendChild(scaleElement);
@@ -134,7 +134,7 @@ describe('createReplacement', () => {
         let replacementImg;
 
         beforeEach(() => {
-          const replacement = createReplacement(img);
+          const replacement = createImitationImg(img);
           const scaleElement = replacement.scaleElement;
           replacementImg = replacement.img;
           testContainer.appendChild(scaleElement);

src/testing/utils.ts

@@ -14,6 +14,10 @@
  * limitations under the License.
  */
 
+ /**
+  * @param img An img to wait for.
+  * @return A Promise that resolves once the image has loaded.
+  */
  export async function imgLoadPromise(img: HTMLImageElement): Promise<void> {
   if (img.complete) {
     return;

src/transform-img/README.md

@@ -14,10 +14,20 @@ Each of these are implemented in seprate file.
 ## Implementation
 
 In order to have the animation perform well, including on older devices, only
-CPU accelrated animatable properties. As a result, the 'crop' portion of the
+CPU accelerated animatable properties. As a result, the 'crop' portion of the
 animation is implemented by scaling up a cropping container (with 
 `overflow: hidden`) and counteracting the scaling with an inverse scale on a
-child container.
+child container. This cannot be done easily with a CSS timing function, so a
+stylesheet with keyframes is generated for the crop scale / counteracting scale
+values.
+
+The code attempts to generate enough keyframes to keep any errors due to
+interpolation during the animation low, but at the same time avoid doing more
+work than necessary.
+
+Both the scaling of the image itself as well as the translate can be done
+directly with a CSS timing function, so those two operations do not generate
+keyframes.
 
 The animations are implemented by taking the larger size and animating using
 that as a reference point for scaling. That is, when the transition is where

src/transform-img/crop-animation.ts

@@ -35,10 +35,10 @@ interface Scale {
 const numSamples = 20;
 
 /**
- * Interpolates a value x% between b and a.
+ * Interpolates a value x% between a and b.
  */
 function interpolate(a: number, b: number, x: number): number {
-  return b + x * (a - b);
+  return a + x * (b - a);
 }
 
 /**
@@ -75,8 +75,8 @@ function generateCropKeyframes({
     // The output percentage at this point.
     const py = getBezierCurveValue(curve.y1, curve.y2, t);
     const keyframePercentage = px * 100;
-    const scaleX = interpolate(endScale.x, startScale.x, py);
-    const scaleY = interpolate(endScale.y, startScale.y, py);
+    const scaleX = interpolate(startScale.x, endScale.x, py);
+    const scaleY = interpolate(startScale.y, endScale.y, py);
     const counterScaleX = 1 / scaleX;
     const counterScaleY = 1 / scaleY;
 

src/transform-img/transform-img.ts

@@ -16,11 +16,16 @@
 
 import {Curve} from '../bezier-curve-utils.js';
 import {getRenderedDimensions} from '../img-dimensions.js';
-import {createReplacement} from '../replacement-img.js';
+import {createImitationImg} from '../imitation-img.js';
 import {prepareCropAnimation} from './crop-animation.js';
 import {prepareScaleAnimation} from './scale-animation.js';
 import {prepareTranslateAnimation} from './translate-animation.js';
 
+/**
+ * @see https://developer.mozilla.org/en-US/docs/Web/CSS/single-transition-timing-function#ease-in-out
+ */
+const EASE_IN_OUT = {x1: 0.42, y1: 0, x2: 0.58, y2: 1};
+
 /**
  * A counter that makes sure the keyframes names are unique.
  */
@@ -39,33 +44,34 @@ function getKeyframesPrefix(namespace: string): string {
 }
 
 /**
- * Prepares an animation from one image to another. Creates a a temporary
- * replacement image that is transitioned between the two images.
+ * Prepares an animation from one image to another. Creates a temporary
+ * transition image that is transitioned between the position, size and crop of
+ * two images.
  * @param options
  * @param options.transitionContainer The container to place the transition
- *    image in. Defaults to document.body.
+ *    image in. This could be useful if you need to place the transition in a
+ *    container with a specific `z-index` to appear on top of other elements in
+ *    the page. It can also be useful if you want to have the transition stay
+ *    within the `ShadowRoot` of a component. Defaults to document.body.
  * @param options.styleContainer The container to place the generated
  *    stylesheet in. Defaults to document.head.
  * @param options.srcImg The image to transition from.
  * @param options.targetImg The image to transition to.
  * @param options.srcImgRect The ClientRect for where the transition should
- *    start from. Defaults to the current rect for srcImg.
+ *    start from. Specifying this could be useful if you need to hide
+ *    (`display: none`) the container for `srcImg` in order to layout a page
+ *    containing `targetImg`. In that case, you can capture the rect for
+ *    `srcImg` up front and then pass it. Defaults to the current rect for
+ *    srcImg.
  * @param options.targetImgRect The ClientRect for where the transition should
- *    end at. Defaults to the current rect for targetImg.
+ *    end at. Specifying this could be useful if you have not had a chance to
+ *    perform the layout for the target yet (e.g. in a `display: none`
+ *    container), but you know where on the screen it will go. Defaults to the
+ *    current rect for targetImg.
  * @param options.curve Control points for a Bezier curve to use for the
  *    animation.
  * @param options.styles Styles to apply to the transitioning Elements. This
  *    should include animationDuration. It might also include animationDelay.
- * @param options.translateCurve Control points for a Bezier curve to use for
- *    the translation portion of animation. Defaults to `curve`.
- * @param options.translateStyles Styles to apply to the translating Elements.
- *    This should include animationDuration. It might also include
- *    animationDelay. Defaults to `styles`.
- * @param options.scaleCurve Control points for a Bezier curve to use for
- *    the translation portion of animation. Defaults to `curve`.
- * @param options.scaleStyles Styles to apply to the scaling Elements. This
- *    should include animationDuration. It might also include animationDelay.
- *    Defaults to `styles`.
  * @param options.keyframesNamespace A namespace to use for the generated
  *    keyframes to ensure they do not clash with existing keyframes.
  */
@@ -76,12 +82,8 @@ export function prepareImageAnimation({
   targetImg,
   srcImgRect = srcImg.getBoundingClientRect(),
   targetImgRect = targetImg.getBoundingClientRect(),
-  curve,
+  curve = EASE_IN_OUT,
   styles,
-  translateCurve = curve,
-  translateStyles = styles,
-  scaleCurve = curve,
-  scaleStyles = styles,
   keyframesNamespace = 'img-transform',
 } : {
   transitionContainer: HTMLElement,
@@ -90,12 +92,8 @@ export function prepareImageAnimation({
   targetImg: HTMLImageElement,
   srcImgRect?: ClientRect,
   targetImgRect?: ClientRect,
-  curve: Curve,
+  curve?: Curve,
   styles: Object,
-  translateCurve?: Curve,
-  translateStyles?: Object,
-  scaleCurve?: Curve,
-  scaleStyles?: Object,
   keyframesNamespace?: string,
 }) : {
   applyAnimation: () => void,
@@ -118,7 +116,7 @@ export function prepareImageAnimation({
     img,
     imgWidth: largerImgWidth,
     imgHeight: largerImgHeight,
-  } = createReplacement(largerImg, largerRect);
+  } = createImitationImg(largerImg, largerRect);
   const {
     width: smallerImgWidth,
     height: smallerImgHeight,
@@ -138,8 +136,8 @@ export function prepareImageAnimation({
     element: translateElement,
     largerRect,
     smallerRect,
-    curve: translateCurve,
-    styles: translateStyles,
+    curve,
+    styles,
     keyframesPrefix,
     toLarger,
   });
@@ -149,8 +147,8 @@ export function prepareImageAnimation({
     largerImgHeight,
     smallerImgWidth,
     smallerImgHeight,
-    curve: scaleCurve,
-    styles: scaleStyles,
+    curve,
+    styles,
     keyframesPrefix,
     toLarger,
   });

src/transform-img/translate-animation.ts

@@ -16,6 +16,14 @@
 
 import {Curve, curveToString} from '../bezier-curve-utils.js';
 
+/**
+ * Prepares a translation animation, assuming that `smallerRect` will be scaled
+ * up to `largerRect` and adjusting the positions to account for the scaling.
+ * This function sets up the animation by setting the appropriate style
+ * properties on the desired Element. The returned style text needs to be
+ * inserted for the animation to run.
+ * @return CSS style text to perform the aniamtion.
+ */
 export function prepareTranslateAnimation({
   element,
   largerRect,
@@ -40,7 +48,9 @@ export function prepareTranslateAnimation({
   // We need to calculate the left/top, but account for the fact the
   // container will be a different size due to scaling.
   const deltaWidth = (endRect.width - startRect.width);
-  const deltaHeight = (endRect.height - startRect.height)
+  const deltaHeight = (endRect.height - startRect.height);
+  // The larger rect will always have a scaling of 1:1, so we do not need to
+  // modify the position if our start/end is th larger rect.
   const startLeft = startRect.left - (toLarger ? deltaWidth / 2 : 0);
   const startTop = startRect.top - (toLarger ? deltaHeight / 2 : 0);
   const endLeft = endRect.left + (toLarger ? 0 : deltaWidth / 2);