From 07039f04f8d873d0d8701347fb2013f673ac5fd0 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Thu, 25 Apr 2024 12:11:27 -0400 Subject: [PATCH] Remove special handling for '1', update both dimensions provided & atLeastAsLarge case --- static/js/formatters-internal.js | 65 ++++++++++++-------- test-site/scripts/build.sh | 1 - tests/static/js/formatters-internal/image.js | 34 +++++----- 3 files changed, 55 insertions(+), 45 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 12f6e9a56..15a0e962f 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -283,22 +283,28 @@ export function joinList(list, separator) { } /** - * Given an image object with a url, changes the url to use dynamic thumbnailer and https. + * Given an image object with a url, changes the url to be dynamic and use https. * - * Note: A dynamic thumbnailer url generated with atLeastAsLarge = true returns an image that is - * at least as large in one dimension of the desired size. In other words, the returned image will - * be at least as large, and as close as possible to, the largest image that is contained within a - * box of the desired size dimensions. + * If both dimensions are provided in desiredSize (e.g. "200x100"): + * - if atLeastAsLarge = true, returns an image that covers the desired space while preserving + * 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, the dynamic thumbnailer url will give the largest image that is - * smaller than the desired size in both dimensions. + * 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. * * @param {Object} simpleOrComplexImage An image object with a url * @param {string} desiredSize The desired size of the image ('x') * @param {boolean} atLeastAsLarge Whether the image should be at least as large as the desired - * size in one dimension or smaller than the desired size in both - * dimensions. - * @returns {Object} An object with a url for dynamic thumbnailer + * size in both dimensions or at least one dimension. + * @returns {Object} An object with a url for dynamic */ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAsLarge = true) { let image = simpleOrComplexImage.image || simpleOrComplexImage; @@ -350,19 +356,19 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs * * @param {string} desiredSize The desired size of the image ('x') * @param {boolean} atLeastAsLarge Whether the image should be at least as large as the desired - * size in one dimension or smaller than the desired size in both - * dimensions. - * @param {number?} fullSizeWidth The full size width of the original image - * @param {number?} fullSizeHeight The full size height of the original image - * @returns {string} A string representing the format options (e.g. 'height=200,width=100,fit=cover') + * size in both dimensions or at least one dimension. + * @param {number?} fullSizeWidth The full size width of the original image if exists + * @param {number?} fullSizeHeight The full size height of the original image if exists + * @returns {string} A string representing the format options + * (e.g. 'height=200,width=100,fit=contain') */ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, fullSizeHeight) { let desiredDims = desiredSize.split('x'); - const desiredWidthProvided = desiredDims[0] !== '' && desiredDims[0] !== '1'; - const desiredHeightProvided = desiredDims[1] !== '' && desiredDims[1] !== '1'; + const hasDesiredWidth = desiredDims[0] !== ''; + const hasDesiredHeight = desiredDims[1] !== ''; // both dimensions are not provided, return original image - if (!desiredWidthProvided && !desiredHeightProvided) { + if (!hasDesiredWidth && !hasDesiredHeight) { return ''; } @@ -371,13 +377,13 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full let desiredWidth; let desiredHeight; let formatOptions = ['fit=contain']; - if (desiredWidthProvided) { + if (hasDesiredWidth) { desiredWidth = Number.parseInt(desiredDims[0]); if (Number.isNaN(desiredWidth)) { throw new Error("Invalid width specified"); } } - if (desiredHeightProvided) { + if (hasDesiredHeight) { desiredHeight = Number.parseInt(desiredDims[1]); if (Number.isNaN(desiredHeight)) { throw new Error("Invalid height specified"); @@ -385,23 +391,28 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full } // only width is provided - if (desiredWidthProvided && !desiredHeightProvided) { + if (hasDesiredWidth && !hasDesiredHeight) { formatOptions.push(`width=${desiredWidth}`); return `/${formatOptions.join(',')}`; } // only height is provided - if (!desiredWidthProvided && desiredHeightProvided) { + if (!hasDesiredWidth && hasDesiredHeight) { formatOptions.push(`height=${desiredHeight}`); return `/${formatOptions.join(',')}`; } // both dimensions are provided - if (atLeastAsLarge && !!originalRatio) { - formatOptions.push(`width=${Math.max(desiredWidth, Math.round(desiredHeight * originalRatio))}`); - formatOptions.push(`height=${Math.max(desiredHeight, Math.round(desiredWidth / originalRatio))}`); + if (!!originalRatio) { + if (atLeastAsLarge) { + formatOptions.push(`width=${Math.max(desiredWidth, Math.round(desiredHeight * originalRatio))}`); + formatOptions.push(`height=${Math.max(desiredHeight, Math.round(desiredWidth / originalRatio))}`); + } else { + formatOptions.push(`width=${Math.min(desiredWidth, Math.round(desiredHeight * originalRatio))}`); + formatOptions.push(`height=${Math.min(desiredHeight, Math.round(desiredWidth / originalRatio))}`); + } } else { formatOptions.push(`width=${desiredWidth}`); formatOptions.push(`height=${desiredHeight}`); @@ -414,14 +425,14 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full * Given a photo image url path, remove the trailing extension. * * @param {string} imageUrlPath Image's url path (e.g. - * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg', + * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg/', * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg') * @returns {string} A canonicalized image url path (e.g. * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs', * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs') */ function _removePhotoImageUrlExtension(imageUrlPath) { - return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+)|(\/)$/, ''); + return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+(\/)*)|(\/)$/, ''); } /** diff --git a/test-site/scripts/build.sh b/test-site/scripts/build.sh index cbdc19864..839e858a6 100755 --- a/test-site/scripts/build.sh +++ b/test-site/scripts/build.sh @@ -8,4 +8,3 @@ set_working_dir_to_test_site () { set_working_dir_to_test_site npx jambo build && npx grunt webpack -npx grunt webpack diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 52eee5d5d..dedd3a004 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -1,7 +1,7 @@ import Formatters from 'static/js/formatters.js'; describe('image formatter', () => { - const photoUrl = 'https://dynm.mktgcdn.com/p/FOO/2000x1000.jpg'; + const photoUrl = 'https://dynm.mktgcdn.com/p/FOO/2000x1000.jpg/'; const oldFileUrl = 'https://a.mktgcdn.com/f/0/FOO.jpg'; const newFileUrl = 'https://a.mktgcdn.com/f/FOO.jpg'; const euFileUrl = 'https://a.eu.mktgcdn.com/f/0/FOO.jpg'; @@ -42,7 +42,7 @@ describe('image formatter', () => { expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=8000,height=4000'); }); - it('if can get original ratio and atLeastAsLarge is true, preserve ratio and cover the specified space', () => { + it('if can get original ratio, preserve ratio and cover the specified space', () => { const photoImgUrl = Formatters.image(photoImg, '1000x200', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=500'); const oldFileImgUrl = Formatters.image(oldFileImg, '200x500', true).url; @@ -53,7 +53,7 @@ describe('image formatter', () => { expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=8000,height=4000'); }); - it('if can\'t get original ratio and atLeastAsLarge is true, use desired dimensions', () => { + it('if can\'t get original ratio, use desired dimensions', () => { const photoImgUrl = Formatters.image({url: photoUrl}, '1000x200', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=200'); const oldFileImgUrl = Formatters.image({url: oldFileUrl}, '200x500', true).url; @@ -66,18 +66,18 @@ describe('image formatter', () => { }); describe('when both dimensions are specified and atLeastAsLarge is false', () => { - it('if can get original ratio and atLeastAsLarge is false, preserve ratio and return the largest image within the space', () => { + it('if can get original ratio, preserve ratio and return the largest image within the space', () => { const photoImgUrl = Formatters.image(photoImg, '1000x200', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=200'); + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=400,height=200'); const oldFileImgUrl = Formatters.image(oldFileImg, '200x500', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200,height=500'); + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200,height=100'); const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1000'); + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=2000,height=1000'); const euFileImgUrl = Formatters.image(euFileImg, '3000x4000', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=4000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=1500'); }); - it('if can\'t get original ratio and atLeastAsLarge is false, use desired dimensions', () => { + it('if can\'t get original ratio, use desired dimensions', () => { const photoImgUrl = Formatters.image({url: photoUrl}, '1000x200', false).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=200'); const oldFileImgUrl = Formatters.image({url: oldFileUrl}, '200x500', false).url; @@ -93,40 +93,40 @@ describe('image formatter', () => { it('can restrict the dimensions by width, regardless of atLeastAsLarge', () => { let photoImgUrl = Formatters.image(photoImg, '1000x', false).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000'); - let oldFileImgUrl = Formatters.image(oldFileImg, '1000x1', false).url; + let oldFileImgUrl = Formatters.image(oldFileImg, '1000x', false).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000'); let newFileImgUrl = Formatters.image(newFileImg, '3000x', false).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000'); - let euFileImgUrl = Formatters.image(euFileImg, '3000x1', false).url; + let euFileImgUrl = Formatters.image(euFileImg, '3000x', false).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000'); photoImgUrl = Formatters.image(photoImg, '1000x', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000'); - oldFileImgUrl = Formatters.image(oldFileImg, '1000x1', true).url; + oldFileImgUrl = Formatters.image(oldFileImg, '1000x', true).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000'); newFileImgUrl = Formatters.image(newFileImg, '3000x', true).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000'); - euFileImgUrl = Formatters.image(euFileImg, '3000x1', true).url; + euFileImgUrl = Formatters.image(euFileImg, '3000x', true).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000'); }); it('can restrict the dimensions by height, regardless of atLeastAsLarge', () => { let photoImgUrl = Formatters.image(photoImg, 'x500', false).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,height=500'); - let oldFileImgUrl = Formatters.image(oldFileImg, '1x500', false).url; + let oldFileImgUrl = Formatters.image(oldFileImg, 'x500', false).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=500'); let newFileImgUrl = Formatters.image(newFileImg, 'x2000', false).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,height=2000'); - let euFileImgUrl = Formatters.image(euFileImg, '1x2000', false).url; + let euFileImgUrl = Formatters.image(euFileImg, 'x2000', false).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=2000'); photoImgUrl = Formatters.image(photoImg, 'x500', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,height=500'); - oldFileImgUrl = Formatters.image(oldFileImg, '1x500', true).url; + oldFileImgUrl = Formatters.image(oldFileImg, 'x500', true).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=500'); newFileImgUrl = Formatters.image(newFileImg, 'x2000', true).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,height=2000'); - euFileImgUrl = Formatters.image(euFileImg, '1x2000', true).url; + euFileImgUrl = Formatters.image(euFileImg, 'x2000', true).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=2000'); }); });