Update Image Formatter To Work With All Image Url Formats

static/js/formatters-internal.js

@@ -290,21 +290,23 @@ export function joinList(list, separator) {
  *     the original image ratio (e.g. if original image is 100x100, returns an image of 200x200).
  *     Note that if we can't get the original image ratio from the image object, the behavior would
  *     be similar to when atLeastAsLarge = false.
- *   - if atLeastAsLarge = false, returns an image that is as large in at least one dimension while
- *     preserving the original image ratio (e.g. if original image is 100x100, returns an image of
- *     100x100).
+ *   - if atLeastAsLarge = false, returns an image that is contained by the desiredSize, while
+ *     preserving the aspect ratio. In other words, both dimensions will either match or be smaller
+ *     than the desired dimensions (e.g. if original image is 100x100, returns an image of 100x100).
  *
  * If only one dimension is provided in desiredSize (e.g. "200x", which is also the default):
  *   - returns an image that matches this dimension while preserving ratio of the original image
  *     (e.g. if original image is 100x100, returned image would be 200x200).
  *
- * If "" is provided as the desiredSize, returns an image in the original size.
+ * If "", "x", "0x0", or a string with unexpected format is provided as the desiredSize, returns an
+ * image in the original size.
  *
  * @param {Object} simpleOrComplexImage An image object with a url
  * @param {string} desiredSize The desired size of the image ('<width>x<height>')
  * @param {boolean} atLeastAsLarge Whether the image should be at least as large as the desired
- *                                 size in both dimensions or at least one dimension.
- * @returns {Object} An object with a url for dynamic
+ *                                 size in both dimensions or at most as large as the desired size
+ *                                 in both dimensions.
+ * @returns {Object} An object with a dynamic url
  */
 export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAsLarge = true) {
   let image = simpleOrComplexImage.image || simpleOrComplexImage;
@@ -364,53 +366,44 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs
  */
 function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, fullSizeHeight) {
   let desiredDims = desiredSize.split('x');
-  const hasDesiredWidth = desiredDims[0] !== '';
-  const hasDesiredHeight = desiredDims[1] !== '';
-  let formatOptions = ['fit=contain'];
+  const desiredWidth = desiredDims[0] ? Number.parseInt(desiredDims[0]) : 0;
+  if (Number.isNaN(desiredWidth)) {
+    throw new Error("Invalid width specified");
+  }
+  const desiredHeight = desiredDims[1] ? Number.parseInt(desiredDims[1]) : 0;
+  if (Number.isNaN(desiredHeight)) {
+    throw new Error("Invalid height specified");
+  }
+
+  const formatOptions = ['fit=contain'];
 
   // both dimensions are not provided, return original image
-  if (!hasDesiredWidth && !hasDesiredHeight) {
+  if (!desiredWidth && !desiredHeight) {
     return `/${formatOptions.join(',')}`;
   }
 
-  let desiredWidth;
-  let desiredHeight;
-  if (hasDesiredWidth) {
-    desiredWidth = Number.parseInt(desiredDims[0]);
-    if (Number.isNaN(desiredWidth)) {
-      throw new Error("Invalid width specified");
-    }
-  }
-  if (hasDesiredHeight) {
-    desiredHeight = Number.parseInt(desiredDims[1]);
-    if (Number.isNaN(desiredHeight)) {
-      throw new Error("Invalid height specified");
-    }
-  }
-
   const originalRatio =
     (!!fullSizeWidth && !!fullSizeHeight) ? (fullSizeWidth / fullSizeHeight) : undefined;
 
   // both dimensions are provided
-  if (hasDesiredWidth && hasDesiredHeight && originalRatio) {
+  if (desiredWidth && desiredHeight && originalRatio) {
     const width = atLeastAsLarge
       ? Math.max(desiredWidth, Math.round(desiredHeight * originalRatio))
       : Math.min(desiredWidth, Math.round(desiredHeight * originalRatio));
     const height = atLeastAsLarge
       ? Math.max(desiredHeight, Math.round(desiredWidth / originalRatio))
       : Math.min(desiredHeight, Math.round(desiredWidth / originalRatio));
 
-    formatOptions.push(`width=${width}`);
-    formatOptions.push(`height=${height}`);
+    formatOptions.push(`width=${width}`, `height=${height}`);
 
     return `/${formatOptions.join(',')}`;
   }
 
-  if (hasDesiredWidth) {
+  if (desiredWidth) {
     formatOptions.push(`width=${desiredWidth}`);
   }
 
-  if (hasDesiredHeight) {
+  if (desiredHeight) {
     formatOptions.push(`height=${desiredHeight}`);
   }
 
@@ -432,7 +425,7 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full
  *   '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/1.0000' respectively)
  */
 function _removePhotoImageUrlExtension(imageUrlPath) {
-  return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+(\/)*)|(\/)$/, '');
+  return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+(\/)?)|(\/)$/, '');
 }
 
 /**

tests/static/js/formatters-internal/image.js

@@ -7,35 +7,13 @@ describe('image formatter', () => {
   const newFileUrl = 'https://a.mktgcdn.com/f/FOO.jpg';
   const euFileUrl = 'https://a.eu.mktgcdn.com/f/0/FOO.jpg';
 
-  const photoImg = {
-    url: photoUrl,
-    width: 2000,
-    height: 1000,
-  };
-
-  const photoWithPaddingImg = {
-    url: photoWithPaddingUrl,
-    width: 2000,
-    height: 1000,
-  };
-
-  const oldFileImg = {
-    url: oldFileUrl,
-    width: 2000,
-    height: 1000,
-  };
-
-  const newFileImg = {
-    url: newFileUrl,
-    width: 2000,
-    height: 1000,
-  };
-
-  const euFileImg = {
-    url: euFileUrl,
-    width: 2000,
-    height: 1000,
-  }
+  const defaultSize = {width: 2000, height: 1000};
+
+  const photoImg = {...defaultSize, url: photoUrl};
+  const photoWithPaddingImg = {...defaultSize, url: photoWithPaddingUrl};
+  const oldFileImg = {...defaultSize, url: oldFileUrl};
+  const newFileImg = {...defaultSize, url: newFileUrl};
+  const euFileImg = {...defaultSize, url: euFileUrl}
 
   describe('when both dimensions are specified and atLeastAsLarge is true', () => {
     it('atLeastAsLarge is default to true if not provided by caller', () => {
@@ -93,15 +71,15 @@ describe('image formatter', () => {
     });
 
     it('if can\'t get original ratio, use desired dimensions', () => {
-      const photoImgUrl = Formatters.image({url: photoUrl}, '1000x200', false).url;
+      const photoImgUrl = Formatters.image({url: photoUrl, width: 100}, '1000x200', false).url;
       expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=200');
-      const photoWithPaddingImgUrl = Formatters.image({url: photoWithPaddingUrl}, '1000x200', false).url;
+      const photoWithPaddingImgUrl = Formatters.image({url: photoWithPaddingUrl, width: 0}, '1000x200', false).url;
       expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=1000,height=200');
-      const oldFileImgUrl = Formatters.image({url: oldFileUrl}, '200x500', false).url;
+      const oldFileImgUrl = Formatters.image({url: oldFileUrl, height: 100}, '200x500', false).url;
       expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200,height=500');
-      const newFileImgUrl = Formatters.image({url: newFileUrl}, '3000x2000', false).url;
+      const newFileImgUrl = Formatters.image({url: newFileUrl, height: 0}, '3000x2000', false).url;
       expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=2000');
-      const euFileImgUrl = Formatters.image({url: euFileUrl}, '3000x3000', false).url;
+      const euFileImgUrl = Formatters.image({url: euFileUrl, height: undefined}, '3000x3000', false).url;
       expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000');
     });
   });
@@ -170,14 +148,14 @@ describe('image formatter', () => {
       expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200');
     });
 
-    it('do not transform image regardless of atLeastAsLarge\'s', () => {
+    it('do not transform image regardless of atLeastAsLarge', () => {
       let photoImgUrl = Formatters.image(photoImg, 'x', false).url;
       expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain');
-      let photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, 'x', false).url;
+      let photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '0x0', false).url;
       expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain');
-      let oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url;
+      let oldFileImgUrl = Formatters.image(oldFileImg, '0x', false).url;
       expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain');
-      let newFileImgUrl = Formatters.image(newFileImg, 'x', false).url;
+      let newFileImgUrl = Formatters.image(newFileImg, 'x0', false).url;
       expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain');
       let euFileImgUrl = Formatters.image(euFileImg, 'x', false).url;
       expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain');
@@ -194,4 +172,10 @@ describe('image formatter', () => {
       expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain');
     });
   });
+
+  describe('when desiredSize is not parseable', () => {
+    it('throws an error', () => {
+      expect(() => {Formatters.image(photoImg, 'ax')}).toThrowError();
+    });
+  });
 });