From c3c380ac2161c75f410874cc2d3e1e21114c8454 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Fri, 19 Apr 2024 18:47:08 -0400 Subject: [PATCH 1/9] Update image handling --- static/js/formatters-internal.js | 138 ++++----------- tests/static/js/formatters-internal/image.js | 175 ++++++++++++------- 2 files changed, 148 insertions(+), 165 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 75474835a..66d0b8b3d 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -311,110 +311,52 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs if (!(Object.prototype.toString.call(image).indexOf('Object') > 0)) { throw new Error("Expected parameter of type Map"); } - if ((typeof desiredSize !== 'string') || (desiredSize == null)) { + if (typeof desiredSize !== 'string') { throw new Error(`Object of type string expected. Got ${typeof desiredSize}.`); } if (desiredSize.indexOf('x') === -1) { throw new Error("Invalid desired size"); } - if ((typeof atLeastAsLarge !== 'boolean') || (atLeastAsLarge == null)) { + if (typeof atLeastAsLarge !== 'boolean') { throw new Error(`Object of type boolean expected. Got ${typeof atLeastAsLarge}.`); } - const isEuImage = image.url.includes('eu.mktgcdn.com'); - - const dynamicUrl = isEuImage - ? _getEuImageDynamicUrl(image, desiredSize, atLeastAsLarge) - : _getUsImageDynamicUrl(image.url, desiredSize, atLeastAsLarge); - - return Object.assign( - {}, - image, - { - url: dynamicUrl.replace('http://', 'https://') + let url; + try { + url = new URL(image.url); + let urlPath = url.pathname; + if (url.pathname.match('^\/p')) { + urlPath = _removePhotoImageUrlExtension(urlPath); } - ); -} -/** - * Given a US image url, returns the dynamic url. - * - * @param {string} imageUrl Image's url (e.g. - * 'https://dynl.mktgcdn.com/p/ldMLwj1JkN94-2pwh6CjR_OMy4KnexHJCfZhPAZCbi0/196x400.jpg', - * 'https://a.mktgcdn.com/p/ldMLwj1JkN94-2pwh6CjR_OMy4KnexHJCfZhPAZCbi0/196x400.jpg') - * @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 {string} A dynamic url (e.g. 'https://dynl.mktgcdn.com/p/ldMLwj1JkN94-2pwh6CjR_OMy4KnexHJCfZhPAZCbi0/200x1.jpg') - */ -function _getUsImageDynamicUrl(imageUrl, desiredSize, atLeastAsLarge) { - const [urlWithoutExtension, extension] = _splitStringOnIndex(imageUrl, imageUrl.lastIndexOf('.')); - const [urlBeforeDimensions, dimensions] = _splitStringOnIndex(urlWithoutExtension, urlWithoutExtension.lastIndexOf('/') + 1); - const fullSizeDims = dimensions.split('x'); + const hostname = url.hostname.replace(/^[a-z]+\./, 'dyn.'); + const formatOptionsString = _getImageFormatOptions(desiredSize, atLeastAsLarge, image.width, image.height); - let desiredWidth, desiredHeight; - let desiredDims = desiredSize.split('x'); + return Object.assign( + {}, + image, + { + url: `https://${hostname}${urlPath}${formatOptionsString}`, + } + ); - if (desiredDims[0] !== '') { - desiredWidth = Number.parseInt(desiredDims[0]); - if (Number.isNaN(desiredWidth)) { - throw new Error("Invalid width specified"); - } - } else { - desiredWidth = atLeastAsLarge ? 1 : Number.parseInt(fullSizeDims[0]); + } catch (error) { + throw new Error(`Error processing image url ${image.url}: ${error}`); } - - if (desiredDims[1] !== '') { - desiredHeight = Number.parseInt(desiredDims[1]); - if (Number.isNaN(desiredHeight)) { - throw new Error("Invalid height specified"); - } - } else { - desiredHeight = atLeastAsLarge ? 1 : Number.parseInt(fullSizeDims[1]); - } - - const urlWithDesiredDims = urlBeforeDimensions + desiredWidth + 'x' + desiredHeight + extension; - - return atLeastAsLarge - ? _replaceUrlHost(urlWithDesiredDims, 'dynl.mktgcdn.com') - : _replaceUrlHost(urlWithDesiredDims, 'dynm.mktgcdn.com'); } /** - * Given an EU image url, returns the dynamic url. + * Construct the format options string with given parameters. * - * @param {Object} image The image object. (e.g. - * { - * url: 'https://a.eu.mktgcdn.com/f/0/FLVfkpR1IwpWrWDuyNYCJWVYIDfPO6x1QSztXozMIzo.jpg', - * sourceUrl: 'https://a.mktgcdn.com/p/UN9RPhz0V9D8bNZ3XfNpkGnAk6ikFhVmgvntlBjVyMA/1200x675.jpg', - * width: 1200, - * height: 675, - * }) * @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 {string} A dynamic url (e.g. 'https://dyn.eu.mktgcdn.com/f/0/FLVfkpR1IwpWrWDuyNYCJWVYIDfPO6x1QSztXozMIzo.jpg/width=200,fit=contain') + * @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') */ -function _getEuImageDynamicUrl(image, desiredSize, atLeastAsLarge) { - let fullSizeWidth, fullSizeHeight; - if (image.width) { - fullSizeWidth = image.width; - } - if (image.height) { - fullSizeHeight = image.height; - } - - if (image.sourceUrl && (!fullSizeWidth || !fullSizeHeight)) { - const [urlWithoutExtension, _] = _splitStringOnIndex(image.sourceUrl, image.sourceUrl.lastIndexOf('.')); - const [__, dimensions] = _splitStringOnIndex(urlWithoutExtension, urlWithoutExtension.lastIndexOf('/') + 1); - const fullSizeDims = dimensions.split('x'); - - fullSizeWidth = Number.parseInt(fullSizeDims[0]); - fullSizeHeight = Number.parseInt(fullSizeDims[1]); - } - +function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, fullSizeHeight) { let desiredDims = desiredSize.split('x'); let formatOptions = []; @@ -442,33 +384,21 @@ function _getEuImageDynamicUrl(image, desiredSize, atLeastAsLarge) { formatOptions.push(`fit=${atLeastAsLarge ? 'cover' : 'contain'}`); - const urlWithOptions = image.url + `/${formatOptions.join(',')}`; - - return _replaceUrlHost(urlWithOptions, 'dyn.eu.mktgcdn.com'); -} - -/** - * Splits a string into two parts at the specified index. - * - * @param {string} str The string to be split - * @param {number} index The index at which to split the string - * @returns {Array} The two parts of the string after splitting - */ -function _splitStringOnIndex(str, index) { - return [str.slice(0, index), str.slice(index)]; + return `/${formatOptions.join(',')}`; } /** - * Replaces the current host of a url with the specified host. + * Given a photo image url path, remove the trailing extension. * - * @param {string} url The url whose host is to be changed - * @param {string} host The new host to change to - * @returns {string} The url updated with the specified host + * @param {string} imageUrlPath Image's url path (e.g. + * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg') + * @returns {string} A canonicalized image url path (e.g. + * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs') */ -function _replaceUrlHost(url, host) { - const splitUrl = url.split('://'); - const urlAfterHost = splitUrl[1].slice(splitUrl[1].indexOf('/')); - return splitUrl[0] + '://' + host + urlAfterHost; +function _removePhotoImageUrlExtension(imageUrlPath) { + return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+)|(\/)$/, ''); } /** diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index b56e7db10..0b534bac0 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -1,106 +1,159 @@ import Formatters from 'static/js/formatters.js'; describe('image formatter', () => { - const usUrl = 'https://a.mktgcdn.com/p/1024x768.jpg'; - const euUrl = 'https://dyn.eu.mktgcdn.com/f/0/FOO.jpg'; - const usImg = {url: usUrl}; - const euImg = { - url: euUrl, + const photoUrl = 'https://a.mktgcdn.com/p/FOO/1024x768.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'; + + const photoImg = { + url: photoUrl, + width: 1024, + height: 768, + }; + + const oldFileImg = { + url: oldFileUrl, + width: 1024, + height: 768, + }; + + const newFileImg = { + url: newFileUrl, + width: 1024, + height: 768, + }; + + const euFileImg = { + url: euFileUrl, width: 1024, height: 768, - sourceUrl: 'https://a.mktgcdn.com/p/FOO/1024x768.jpg', } describe('when choosing the smallest image over threshold', () => { it('By default chooses the smallest image with width >= 200', () => { - const usImageUrl = Formatters.image(usImg).url; - expect(usImageUrl).toEqual('https://dynl.mktgcdn.com/p/200x1.jpg'); - const euImageUrl = Formatters.image(euImg).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=200,fit=cover'); + const photoImgUrl = Formatters.image(photoImg).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=200,fit=cover'); + const oldFileImgUrl = Formatters.image(oldFileImg).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=200,fit=cover'); + const newFileImgUrl = Formatters.image(newFileImg).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=200,fit=cover'); + const euFileImgUrl = Formatters.image(euFileImg).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=200,fit=cover'); }); it('Can restrict the dimensions by width', () => { - const usImageUrl = Formatters.image(usImg, '601x').url; - expect(usImageUrl).toEqual('https://dynl.mktgcdn.com/p/601x1.jpg'); - const euImageUrl = Formatters.image(euImg, '601x').url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,fit=cover'); + const photoImgUrl = Formatters.image(photoImg, '601x').url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,fit=cover'); + const oldFileImgUrl = Formatters.image(oldFileImg, '601x').url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,fit=cover'); + const newFileImgUrl = Formatters.image(newFileImg, '601x').url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,fit=cover'); + const euFileImgUrl = Formatters.image(euFileImg, '601x').url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,fit=cover'); }); it('Can restrict the dimensions by height', () => { - const usImageUrl = Formatters.image(usImg, 'x338').url; - expect(usImageUrl).toEqual('https://dynl.mktgcdn.com/p/1x338.jpg'); - const euImageUrl = Formatters.image(euImg, 'x338').url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/height=338,fit=cover'); + const photoImgUrl = Formatters.image(photoImg, 'x338').url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/height=338,fit=cover'); + const oldFileImgUrl = Formatters.image(oldFileImg, 'x338').url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/height=338,fit=cover'); + const newFileImgUrl = Formatters.image(newFileImg, 'x338').url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/height=338,fit=cover'); + const euFileImgUrl = Formatters.image(euFileImg, 'x338').url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/height=338,fit=cover'); }); it('Can restrict by both dimensions', () => { - const usImageUrl = Formatters.image(usImg, '601x338').url; - expect(usImageUrl).toEqual('https://dynl.mktgcdn.com/p/601x338.jpg'); - const euImageUrl = Formatters.image(euImg, '601x338').url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=338,fit=cover'); + const photoImgUrl = Formatters.image(photoImg, '601x338').url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,height=338,fit=cover'); + const oldFileImgUrl = Formatters.image(oldFileImg, '601x338').url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,height=338,fit=cover'); + const newFileImgUrl = Formatters.image(newFileImg, '601x338').url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,height=338,fit=cover'); + const euFileImgUrl = Formatters.image(euFileImg, '601x338').url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=338,fit=cover'); }); it('returns the smallest image when no dimensions given', () => { - const usImageUrl = Formatters.image(usImg, 'x').url; - expect(usImageUrl).toEqual('https://dynl.mktgcdn.com/p/1x1.jpg'); - const euImageUrl = Formatters.image(euImg, 'x').url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + const photoImgUrl = Formatters.image(photoImg, 'x').url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=cover'); + const oldFileImgUrl = Formatters.image(oldFileImg, 'x').url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + const newFileImgUrl = Formatters.image(newFileImg, 'x').url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=cover'); + const euFileImgUrl = Formatters.image(euFileImg, 'x').url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); }); }); describe('when choosing the biggest image under threshold', () => { it('Can restrict the dimensions by width', () => { - const usImageUrl = Formatters.image(usImg, '601x', false).url; - expect(usImageUrl).toEqual('https://dynm.mktgcdn.com/p/601x768.jpg'); - const euImageUrl = Formatters.image(euImg, '601x', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=768,fit=contain'); + const photoImgUrl = Formatters.image(photoImg, '601x', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,height=768,fit=contain'); + const oldFileImgUrl = Formatters.image(oldFileImg, '601x', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,height=768,fit=contain'); + const newFileImgUrl = Formatters.image(newFileImg, '601x', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,height=768,fit=contain'); + const euFileImgUrl = Formatters.image(euFileImg, '601x', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=768,fit=contain'); }); it('Can restrict the dimensions by height', () => { - const usImageUrl = Formatters.image(usImg, 'x338', false).url; - expect(usImageUrl).toEqual('https://dynm.mktgcdn.com/p/1024x338.jpg'); - const euImageUrl = Formatters.image(euImg, 'x338', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=338,fit=contain'); + const photoImgUrl = Formatters.image(photoImg, 'x338', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,height=338,fit=contain'); + const oldFileImgUrl = Formatters.image(oldFileImg, 'x338', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,height=338,fit=contain'); + const newFileImgUrl = Formatters.image(newFileImg, 'x338', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,height=338,fit=contain'); + const euFileImgUrl = Formatters.image(euFileImg, 'x338', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=338,fit=contain'); }); it('Can restrict by both dimensions', () => { - const usImageUrl = Formatters.image(usImg, '999x338', false).url; - expect(usImageUrl).toEqual('https://dynm.mktgcdn.com/p/999x338.jpg'); - const euImageUrl = Formatters.image(euImg, '999x338', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=999,height=338,fit=contain'); + const photoImgUrl = Formatters.image(photoImg, '999x338', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=999,height=338,fit=contain'); + const oldFileImgUrl = Formatters.image(oldFileImg, '999x338', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=999,height=338,fit=contain'); + const newFileImgUrl = Formatters.image(newFileImg, '999x338', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=999,height=338,fit=contain'); + const euFileImgUrl = Formatters.image(euFileImg, '999x338', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=999,height=338,fit=contain'); }); it('return the largest image when no dimensions given', () => { - const usImageUrl = Formatters.image(usImg, 'x', false).url; - expect(usImageUrl).toEqual('https://dynm.mktgcdn.com/p/1024x768.jpg'); - const euImageUrl = Formatters.image(euImg, 'x', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); + const photoImgUrl = Formatters.image(photoImg, 'x', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,height=768,fit=contain'); + const oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); + const newFileImgUrl = Formatters.image(newFileImg, 'x', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,height=768,fit=contain'); + const euFileImgUrl = Formatters.image(euFileImg, 'x', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); }); }); - describe('when image is served from EU with no dimensions specified', () => { + describe('when image has no dimensions specified', () => { it('when choosing the biggest image under threshold, use the width and height on the image object if exists', () => { - const euImageUrl = Formatters.image( - { url: euUrl, - width: 1024, - height: 768, - sourceUrl: 'https://a.mktgcdn.com/p/FOO/516x384.jpg', - }, 'x', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); - }); - - it('when choosing the biggest image under threshold, use dimensions from the sourceUrl if width/height does not exist', () => { - const euImageUrl = Formatters.image( - { url: euUrl, - width: 1024, - sourceUrl: 'https://a.mktgcdn.com/p/FOO/516x384.jpg' - }, 'x', false).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=516,height=384,fit=contain'); + const photoImgUrl = Formatters.image({url: photoUrl, width: 1024}, 'x', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,fit=contain'); + const oldFileImgUrl = Formatters.image({url: oldFileUrl, width: 1024}, 'x', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,fit=contain'); + const newFileImgUrl = Formatters.image({url: newFileUrl, width: 1024}, 'x', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,fit=contain'); + const euFileImgUrl = Formatters.image({url: euFileUrl, width: 1024}, 'x', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,fit=contain'); }); it('when choosing the smallest image over threshold, omit width/height if can\'t parse it from the image object', () => { - const euImageUrl = Formatters.image({url: euUrl}, 'x', true).url; - expect(euImageUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + const photoImgUrl = Formatters.image({url: photoUrl}, 'x', true).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=cover'); + const oldFileImgUrl = Formatters.image({url: oldFileUrl}, 'x', true).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + const newFileImgUrl = Formatters.image({url: newFileUrl}, 'x', true).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=cover'); + const euFileImgUrl = Formatters.image({url: euFileUrl}, 'x', true).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); }); }); }); \ No newline at end of file From 292e9a0916f53bc8124a1b6100e8b184e38ddbc5 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Mon, 22 Apr 2024 15:09:15 -0400 Subject: [PATCH 2/9] Update behavior --- static/js/formatters-internal.js | 49 ++-- test-site/scripts/build.sh | 4 +- tests/static/js/formatters-internal/image.js | 226 +++++++++---------- 3 files changed, 149 insertions(+), 130 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 66d0b8b3d..daaabbb03 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -358,31 +358,54 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs */ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, fullSizeHeight) { let desiredDims = desiredSize.split('x'); - let formatOptions = []; + const desiredWidthProvided = desiredDims[0] !== '' && desiredDims[0] !== '1'; + const desiredHeightProvided = desiredDims[1] !== '' && desiredDims[1] !== '1'; - if (desiredDims[0] !== '') { - const desiredWidth = Number.parseInt(desiredDims[0]); + // both dimensions are not provided, return original image + if (!desiredWidthProvided && !desiredHeightProvided) { + return ''; + } + + const originalRatio = + (!!fullSizeWidth && !!fullSizeHeight) ? (fullSizeWidth / fullSizeHeight) : undefined; + let desiredWidth; + let desiredHeight; + let formatOptions = ['fit=contain']; + if (desiredWidthProvided) { + desiredWidth = Number.parseInt(desiredDims[0]); if (Number.isNaN(desiredWidth)) { throw new Error("Invalid width specified"); } - - formatOptions.push(`width=${desiredWidth}`); - } else if (!atLeastAsLarge && fullSizeWidth) { - formatOptions.push(`width=${fullSizeWidth}`); } - - if (desiredDims[1] !== '') { - const desiredHeight = Number.parseInt(desiredDims[1]); + if (desiredHeightProvided) { + desiredHeight = Number.parseInt(desiredDims[1]); if (Number.isNaN(desiredHeight)) { throw new Error("Invalid height specified"); } + } + // only width is provided + if (desiredWidthProvided && !desiredHeightProvided) { + formatOptions.push(`width=${desiredWidth}`); + + return `/${formatOptions.join(',')}`; + } + + // only height is provided + if (!desiredWidthProvided && desiredHeightProvided) { formatOptions.push(`height=${desiredHeight}`); - } else if (!atLeastAsLarge && fullSizeHeight) { - formatOptions.push(`height=${fullSizeHeight}`); + + return `/${formatOptions.join(',')}`; } - formatOptions.push(`fit=${atLeastAsLarge ? 'cover' : 'contain'}`); + // 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))}`); + } else { + formatOptions.push(`width=${desiredWidth}`); + formatOptions.push(`height=${desiredHeight}`); + } return `/${formatOptions.join(',')}`; } diff --git a/test-site/scripts/build.sh b/test-site/scripts/build.sh index 839e858a6..acd51844e 100755 --- a/test-site/scripts/build.sh +++ b/test-site/scripts/build.sh @@ -7,4 +7,6 @@ set_working_dir_to_test_site () { set_working_dir_to_test_site -npx jambo build && npx grunt webpack +npx jambo build +chmod u+x ./public/static/node_modules/esbuild/bin/esbuild +npx grunt webpack diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 0b534bac0..20a50b2cb 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -1,159 +1,153 @@ import Formatters from 'static/js/formatters.js'; describe('image formatter', () => { - const photoUrl = 'https://a.mktgcdn.com/p/FOO/1024x768.jpg'; + const photoUrl = 'https://a.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'; const photoImg = { url: photoUrl, - width: 1024, - height: 768, + width: 2000, + height: 1000, }; const oldFileImg = { url: oldFileUrl, - width: 1024, - height: 768, + width: 2000, + height: 1000, }; const newFileImg = { url: newFileUrl, - width: 1024, - height: 768, + width: 2000, + height: 1000, }; const euFileImg = { url: euFileUrl, - width: 1024, - height: 768, + width: 2000, + height: 1000, } - describe('when choosing the smallest image over threshold', () => { - it('By default chooses the smallest image with width >= 200', () => { - const photoImgUrl = Formatters.image(photoImg).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=200,fit=cover'); - const oldFileImgUrl = Formatters.image(oldFileImg).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=200,fit=cover'); - const newFileImgUrl = Formatters.image(newFileImg).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=200,fit=cover'); - const euFileImgUrl = Formatters.image(euFileImg).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=200,fit=cover'); - }); - - it('Can restrict the dimensions by width', () => { - const photoImgUrl = Formatters.image(photoImg, '601x').url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,fit=cover'); - const oldFileImgUrl = Formatters.image(oldFileImg, '601x').url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,fit=cover'); - const newFileImgUrl = Formatters.image(newFileImg, '601x').url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,fit=cover'); - const euFileImgUrl = Formatters.image(euFileImg, '601x').url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,fit=cover'); + describe('when both dimensions are specified', () => { + it('if can get original ratio and atLeastAsLarge is true, 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; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000,height=500'); + const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', true).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1500'); + const euFileImgUrl = Formatters.image(euFileImg, '3000x4000', true).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=8000,height=4000'); }); - it('Can restrict the dimensions by height', () => { - const photoImgUrl = Formatters.image(photoImg, 'x338').url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/height=338,fit=cover'); - const oldFileImgUrl = Formatters.image(oldFileImg, 'x338').url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/height=338,fit=cover'); - const newFileImgUrl = Formatters.image(newFileImg, 'x338').url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/height=338,fit=cover'); - const euFileImgUrl = Formatters.image(euFileImg, 'x338').url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/height=338,fit=cover'); + it('if can get original ratio and atLeastAsLarge is false, 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'); + 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'); + const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,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'); }); - it('Can restrict by both dimensions', () => { - const photoImgUrl = Formatters.image(photoImg, '601x338').url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,height=338,fit=cover'); - const oldFileImgUrl = Formatters.image(oldFileImg, '601x338').url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,height=338,fit=cover'); - const newFileImgUrl = Formatters.image(newFileImg, '601x338').url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,height=338,fit=cover'); - const euFileImgUrl = Formatters.image(euFileImg, '601x338').url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=338,fit=cover'); + it('if can\'t get original ratio and atLeastAsLarge is true, 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; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200,height=500'); + const newFileImgUrl = Formatters.image({url: newFileUrl}, '3000x2000', true).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=2000'); + const euFileImgUrl = Formatters.image({url: euFileUrl}, '3000x3000', true).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); }); - it('returns the smallest image when no dimensions given', () => { - const photoImgUrl = Formatters.image(photoImg, 'x').url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=cover'); - const oldFileImgUrl = Formatters.image(oldFileImg, 'x').url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=cover'); - const newFileImgUrl = Formatters.image(newFileImg, 'x').url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=cover'); - const euFileImgUrl = Formatters.image(euFileImg, 'x').url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + it('if can\'t get original ratio and atLeastAsLarge is true, 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; + 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; + 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; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); }); }); - describe('when choosing the biggest image under threshold', () => { - it('Can restrict the dimensions by width', () => { - const photoImgUrl = Formatters.image(photoImg, '601x', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=601,height=768,fit=contain'); - const oldFileImgUrl = Formatters.image(oldFileImg, '601x', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=601,height=768,fit=contain'); - const newFileImgUrl = Formatters.image(newFileImg, '601x', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=601,height=768,fit=contain'); - const euFileImgUrl = Formatters.image(euFileImg, '601x', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=601,height=768,fit=contain'); - }); - - it('Can restrict the dimensions by height', () => { - const photoImgUrl = Formatters.image(photoImg, 'x338', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,height=338,fit=contain'); - const oldFileImgUrl = Formatters.image(oldFileImg, 'x338', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,height=338,fit=contain'); - const newFileImgUrl = Formatters.image(newFileImg, 'x338', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,height=338,fit=contain'); - const euFileImgUrl = Formatters.image(euFileImg, 'x338', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=338,fit=contain'); - }); - - it('Can restrict by both dimensions', () => { - const photoImgUrl = Formatters.image(photoImg, '999x338', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=999,height=338,fit=contain'); - const oldFileImgUrl = Formatters.image(oldFileImg, '999x338', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=999,height=338,fit=contain'); - const newFileImgUrl = Formatters.image(newFileImg, '999x338', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=999,height=338,fit=contain'); - const euFileImgUrl = Formatters.image(euFileImg, '999x338', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=999,height=338,fit=contain'); + describe('when image has only one dimension specified', () => { + 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; + 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; + 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; + 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; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000'); }); - it('return the largest image when no dimensions given', () => { - const photoImgUrl = Formatters.image(photoImg, 'x', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,height=768,fit=contain'); - const oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); - const newFileImgUrl = Formatters.image(newFileImg, 'x', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,height=768,fit=contain'); - const euFileImgUrl = Formatters.image(euFileImg, 'x', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,height=768,fit=contain'); + 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; + 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; + 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; + 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; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=2000'); }); }); describe('when image has no dimensions specified', () => { - it('when choosing the biggest image under threshold, use the width and height on the image object if exists', () => { - const photoImgUrl = Formatters.image({url: photoUrl, width: 1024}, 'x', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/width=1024,fit=contain'); - const oldFileImgUrl = Formatters.image({url: oldFileUrl, width: 1024}, 'x', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/width=1024,fit=contain'); - const newFileImgUrl = Formatters.image({url: newFileUrl, width: 1024}, 'x', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/width=1024,fit=contain'); - const euFileImgUrl = Formatters.image({url: euFileUrl, width: 1024}, 'x', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/width=1024,fit=contain'); + it('By default chooses the smallest image with width >= 200', () => { + const photoImgUrl = Formatters.image(photoImg).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=200'); + const oldFileImgUrl = Formatters.image(oldFileImg).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200'); + const newFileImgUrl = Formatters.image(newFileImg).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=200'); + const euFileImgUrl = Formatters.image(euFileImg).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200'); }); - it('when choosing the smallest image over threshold, omit width/height if can\'t parse it from the image object', () => { - const photoImgUrl = Formatters.image({url: photoUrl}, 'x', true).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=cover'); - const oldFileImgUrl = Formatters.image({url: oldFileUrl}, 'x', true).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=cover'); - const newFileImgUrl = Formatters.image({url: newFileUrl}, 'x', true).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=cover'); - const euFileImgUrl = Formatters.image({url: euFileUrl}, 'x', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=cover'); + it('do not transform image regardless of atLeastAsLarge\'s', () => { + let photoImgUrl = Formatters.image(photoImg, 'x', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO'); + let oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg'); + let newFileImgUrl = Formatters.image(newFileImg, 'x', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg'); + let euFileImgUrl = Formatters.image(euFileImg, 'x', false).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg'); + + photoImgUrl = Formatters.image(photoImg, 'x', true).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO'); + oldFileImgUrl = Formatters.image(oldFileImg, 'x', true).url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg'); + newFileImgUrl = Formatters.image(newFileImg, 'x', true).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg'); + euFileImgUrl = Formatters.image(euFileImg, 'x', true).url; + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg'); }); }); }); \ No newline at end of file From 16c6f985415063408560a48f8d4ac545c96a4387 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Tue, 23 Apr 2024 10:39:31 -0400 Subject: [PATCH 3/9] Addressing comments --- static/js/formatters-internal.js | 2 +- test-site/scripts/build.sh | 3 +- tests/static/js/formatters-internal/image.js | 45 +++++++++++++------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index daaabbb03..12f6e9a56 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -329,7 +329,7 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs urlPath = _removePhotoImageUrlExtension(urlPath); } - const hostname = url.hostname.replace(/^[a-z]+\./, 'dyn.'); + const hostname = url.hostname.replace(/^(a|dynl|dynm)\./, 'dyn.'); const formatOptionsString = _getImageFormatOptions(desiredSize, atLeastAsLarge, image.width, image.height); return Object.assign( diff --git a/test-site/scripts/build.sh b/test-site/scripts/build.sh index acd51844e..cbdc19864 100755 --- a/test-site/scripts/build.sh +++ b/test-site/scripts/build.sh @@ -7,6 +7,5 @@ set_working_dir_to_test_site () { set_working_dir_to_test_site -npx jambo build -chmod u+x ./public/static/node_modules/esbuild/bin/esbuild +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 20a50b2cb..52eee5d5d 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://a.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'; @@ -30,7 +30,18 @@ describe('image formatter', () => { height: 1000, } - describe('when both dimensions are specified', () => { + describe('when both dimensions are specified and atLeastAsLarge is true', () => { + it('atLeastAsLarge is default to true if not provided by caller', () => { + const photoImgUrl = Formatters.image(photoImg, '1000x200').url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=500'); + const oldFileImgUrl = Formatters.image(oldFileImg, '200x500').url; + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000,height=500'); + const newFileImgUrl = Formatters.image(newFileImg, '3000x1000').url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1500'); + const euFileImgUrl = Formatters.image(euFileImg, '3000x4000').url; + 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', () => { const photoImgUrl = Formatters.image(photoImg, '1000x200', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=500'); @@ -42,17 +53,6 @@ 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 false, 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'); - 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'); - const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,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'); - }); - it('if can\'t get original ratio and atLeastAsLarge is true, 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'); @@ -63,8 +63,21 @@ describe('image formatter', () => { const euFileImgUrl = Formatters.image({url: euFileUrl}, '3000x3000', true).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); }); + }); - it('if can\'t get original ratio and atLeastAsLarge is true, use desired dimensions', () => { + 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', () => { + const photoImgUrl = Formatters.image(photoImg, '1000x200', false).url; + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,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'); + const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,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'); + }); + + it('if can\'t get original ratio and atLeastAsLarge is false, 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; @@ -97,7 +110,7 @@ describe('image formatter', () => { 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', () => { + 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; @@ -119,7 +132,7 @@ describe('image formatter', () => { }); describe('when image has no dimensions specified', () => { - it('By default chooses the smallest image with width >= 200', () => { + it('by default chooses the smallest image with width >= 200', () => { const photoImgUrl = Formatters.image(photoImg).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=200'); const oldFileImgUrl = Formatters.image(oldFileImg).url; From 07039f04f8d873d0d8701347fb2013f673ac5fd0 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Thu, 25 Apr 2024 12:11:27 -0400 Subject: [PATCH 4/9] 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'); }); }); From b2a35ba31eb4837669673c88b21c0cf71ddcc35d Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Thu, 25 Apr 2024 18:13:05 -0400 Subject: [PATCH 5/9] More comments --- static/js/formatters-internal.js | 40 +++++++++----------- tests/static/js/formatters-internal/image.js | 16 ++++---- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 15a0e962f..132f9a918 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -366,17 +366,15 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full let desiredDims = desiredSize.split('x'); const hasDesiredWidth = desiredDims[0] !== ''; const hasDesiredHeight = desiredDims[1] !== ''; + let formatOptions = ['fit=contain']; // both dimensions are not provided, return original image if (!hasDesiredWidth && !hasDesiredHeight) { - return ''; + return `/${formatOptions.join(',')}`; } - const originalRatio = - (!!fullSizeWidth && !!fullSizeHeight) ? (fullSizeWidth / fullSizeHeight) : undefined; let desiredWidth; let desiredHeight; - let formatOptions = ['fit=contain']; if (hasDesiredWidth) { desiredWidth = Number.parseInt(desiredDims[0]); if (Number.isNaN(desiredWidth)) { @@ -390,31 +388,29 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full } } - // only width is provided - if (hasDesiredWidth && !hasDesiredHeight) { - formatOptions.push(`width=${desiredWidth}`); + const originalRatio = + (!!fullSizeWidth && !!fullSizeHeight) ? (fullSizeWidth / fullSizeHeight) : undefined; - return `/${formatOptions.join(',')}`; - } + // both dimensions are provided + if (hasDesiredWidth && hasDesiredHeight && 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)); - // only height is provided - if (!hasDesiredWidth && hasDesiredHeight) { - formatOptions.push(`height=${desiredHeight}`); + formatOptions.push(`width=${width}`); + formatOptions.push(`height=${height}`); return `/${formatOptions.join(',')}`; } - // both dimensions are provided - 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 { + if (hasDesiredWidth) { formatOptions.push(`width=${desiredWidth}`); + } + + if (hasDesiredHeight) { formatOptions.push(`height=${desiredHeight}`); } diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index dedd3a004..902b9f975 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -145,22 +145,22 @@ describe('image formatter', () => { it('do not transform image regardless of atLeastAsLarge\'s', () => { let photoImgUrl = Formatters.image(photoImg, 'x', false).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO'); + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain'); let oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg'); + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain'); let newFileImgUrl = Formatters.image(newFileImg, 'x', false).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg'); + 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'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain'); photoImgUrl = Formatters.image(photoImg, 'x', true).url; - expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO'); + expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain'); oldFileImgUrl = Formatters.image(oldFileImg, 'x', true).url; - expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg'); + expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain'); newFileImgUrl = Formatters.image(newFileImg, 'x', true).url; - expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg'); + expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain'); euFileImgUrl = Formatters.image(euFileImg, 'x', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain'); }); }); }); \ No newline at end of file From 9744b8e73a7bacd500f780260c068486700b9715 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Fri, 26 Apr 2024 13:12:17 -0400 Subject: [PATCH 6/9] Add more tests --- static/js/formatters-internal.js | 8 +++-- tests/static/js/formatters-internal/image.js | 31 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 132f9a918..746287a6b 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -422,10 +422,14 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full * * @param {string} imageUrlPath Image's url path (e.g. * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg/', - * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg') + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/225x225.jpg/', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/1.0000/225x225.jpg') * @returns {string} A canonicalized image url path (e.g. * '/p/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs', - * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs') + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs', + * '/p-sandbox/mFsjqWGQEOMQGNoNIcnq61JtdSGiCs/1.0000' respectively) */ function _removePhotoImageUrlExtension(imageUrlPath) { return imageUrlPath.replace(/(\/[0-9]+x[0-9]+\.[a-z]+(\/)*)|(\/)$/, ''); diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 902b9f975..6fa22d3db 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -2,6 +2,7 @@ import Formatters from 'static/js/formatters.js'; describe('image formatter', () => { const photoUrl = 'https://dynm.mktgcdn.com/p/FOO/2000x1000.jpg/'; + const photoWithPaddingUrl = 'https://dynm.mktgcdn.com/p/FOO/1.0000/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'; @@ -12,6 +13,12 @@ describe('image formatter', () => { height: 1000, }; + const photoWithPaddingImg = { + url: photoWithPaddingUrl, + width: 2000, + height: 1000, + }; + const oldFileImg = { url: oldFileUrl, width: 2000, @@ -34,6 +41,8 @@ describe('image formatter', () => { it('atLeastAsLarge is default to true if not provided by caller', () => { const photoImgUrl = Formatters.image(photoImg, '1000x200').url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=500'); + const photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '1000x200').url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=1000,height=500'); const oldFileImgUrl = Formatters.image(oldFileImg, '200x500').url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000,height=500'); const newFileImgUrl = Formatters.image(newFileImg, '3000x1000').url; @@ -45,6 +54,8 @@ describe('image formatter', () => { 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 photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '1000x200', true).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=1000,height=500'); const oldFileImgUrl = Formatters.image(oldFileImg, '200x500', true).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=1000,height=500'); const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', true).url; @@ -56,6 +67,8 @@ describe('image formatter', () => { 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 photoWithPaddingImgUrl = Formatters.image({url: photoWithPaddingUrl}, '1000x200', true).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', true).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', true).url; @@ -69,6 +82,8 @@ describe('image formatter', () => { 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=400,height=200'); + const photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '1000x200', false).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/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=100'); const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; @@ -80,6 +95,8 @@ describe('image formatter', () => { 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 photoWithPaddingImgUrl = Formatters.image({url: photoWithPaddingUrl}, '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; 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; @@ -93,6 +110,8 @@ 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 photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '1000x', false).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=1000'); 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; @@ -102,6 +121,8 @@ describe('image formatter', () => { photoImgUrl = Formatters.image(photoImg, '1000x', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000'); + photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, '1000x', true).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=1000'); 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; @@ -113,6 +134,8 @@ describe('image formatter', () => { 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 photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, 'x500', false).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,height=500'); 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; @@ -122,6 +145,8 @@ describe('image formatter', () => { photoImgUrl = Formatters.image(photoImg, 'x500', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,height=500'); + photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, 'x500', true).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,height=500'); 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; @@ -135,6 +160,8 @@ describe('image formatter', () => { it('by default chooses the smallest image with width >= 200', () => { const photoImgUrl = Formatters.image(photoImg).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=200'); + const photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain,width=200'); const oldFileImgUrl = Formatters.image(oldFileImg).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200'); const newFileImgUrl = Formatters.image(newFileImg).url; @@ -146,6 +173,8 @@ describe('image formatter', () => { it('do not transform image regardless of atLeastAsLarge\'s', () => { 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; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain'); let oldFileImgUrl = Formatters.image(oldFileImg, 'x', false).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain'); let newFileImgUrl = Formatters.image(newFileImg, 'x', false).url; @@ -155,6 +184,8 @@ describe('image formatter', () => { photoImgUrl = Formatters.image(photoImg, 'x', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain'); + photoWithPaddingImgUrl = Formatters.image(photoWithPaddingImg, 'x', true).url; + expect(photoWithPaddingImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/1.0000/fit=contain'); oldFileImgUrl = Formatters.image(oldFileImg, 'x', true).url; expect(oldFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/0/FOO.jpg/fit=contain'); newFileImgUrl = Formatters.image(newFileImg, 'x', true).url; From 40214cee5670db2338f5db04079899a7befab9a2 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Fri, 26 Apr 2024 18:02:41 -0400 Subject: [PATCH 7/9] More comments --- static/js/formatters-internal.js | 55 ++++++++---------- tests/static/js/formatters-internal/image.js | 60 +++++++------------- 2 files changed, 46 insertions(+), 69 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index 746287a6b..e067dcc6f 100644 --- a/static/js/formatters-internal.js +++ b/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 ('x') * @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,35 +366,27 @@ 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)); @@ -400,17 +394,16 @@ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, full ? 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]+(\/)?)|(\/)$/, ''); } /** diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 6fa22d3db..50fb15107 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/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(); + }); + }); }); \ No newline at end of file From 9f56d4c911600c99fe112770d6bde9c168738515 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Fri, 26 Apr 2024 20:09:05 -0400 Subject: [PATCH 8/9] More comments --- static/js/formatters-internal.js | 5 +++-- tests/static/js/formatters-internal/image.js | 12 ++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/static/js/formatters-internal.js b/static/js/formatters-internal.js index e067dcc6f..143329756 100644 --- a/static/js/formatters-internal.js +++ b/static/js/formatters-internal.js @@ -294,7 +294,8 @@ export function joinList(list, separator) { * 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): + * If only one dimension is provided in desiredSize (e.g. "200x", which is also the default, or + * "200x0"): * - 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). * @@ -365,7 +366,7 @@ export function image(simpleOrComplexImage = {}, desiredSize = '200x', atLeastAs * (e.g. 'height=200,width=100,fit=contain') */ function _getImageFormatOptions(desiredSize, atLeastAsLarge, fullSizeWidth, fullSizeHeight) { - let desiredDims = desiredSize.split('x'); + const desiredDims = desiredSize.split('x'); const desiredWidth = desiredDims[0] ? Number.parseInt(desiredDims[0]) : 0; if (Number.isNaN(desiredWidth)) { throw new Error("Invalid width specified"); diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 50fb15107..55bd81626 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -75,11 +75,11 @@ describe('image formatter', () => { expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain,width=1000,height=200'); 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, height: 100}, '200x500', false).url; + const oldFileImgUrl = Formatters.image({url: oldFileUrl, width: undefined}, '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, 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, height: undefined}, '3000x3000', false).url; + const euFileImgUrl = Formatters.image({url: euFileUrl, width: 0, height: 0}, '3000x3000', false).url; expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); }); }); @@ -177,5 +177,13 @@ describe('image formatter', () => { it('throws an error', () => { expect(() => {Formatters.image(photoImg, 'ax')}).toThrowError(); }); + + it('throws an error', () => { + expect(() => {Formatters.image(photoImg, 'xa')}).toThrowError(); + }); + + it('throws an error', () => { + expect(() => {Formatters.image(photoImg, '')}).toThrowError(); + }); }); }); \ No newline at end of file From 9aaed14e430d8f70d603478bfb4e1fbf55489fe5 Mon Sep 17 00:00:00 2001 From: Emily Zhang Date: Fri, 26 Apr 2024 20:34:53 -0400 Subject: [PATCH 9/9] More comments --- tests/static/js/formatters-internal/image.js | 42 +++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/static/js/formatters-internal/image.js b/tests/static/js/formatters-internal/image.js index 55bd81626..7e78141d3 100644 --- a/tests/static/js/formatters-internal/image.js +++ b/tests/static/js/formatters-internal/image.js @@ -5,7 +5,7 @@ describe('image formatter', () => { const photoWithPaddingUrl = 'https://dynm.mktgcdn.com/p/FOO/1.0000/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'; + const euFileUrl = 'https://a.eu.mktgcdn.com/f/FOO.jpg'; const defaultSize = {width: 2000, height: 1000}; @@ -26,7 +26,7 @@ describe('image formatter', () => { const newFileImgUrl = Formatters.image(newFileImg, '3000x1000').url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1500'); const euFileImgUrl = Formatters.image(euFileImg, '3000x4000').url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=8000,height=4000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=8000,height=4000'); }); it('if can get original ratio, preserve ratio and cover the specified space', () => { @@ -39,7 +39,7 @@ describe('image formatter', () => { const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', true).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1500'); const euFileImgUrl = Formatters.image(euFileImg, '3000x4000', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=8000,height=4000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=8000,height=4000'); }); it('if can\'t get original ratio, use desired dimensions', () => { @@ -52,7 +52,7 @@ describe('image formatter', () => { const newFileImgUrl = Formatters.image({url: newFileUrl}, '3000x2000', true).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=2000'); const euFileImgUrl = Formatters.image({url: euFileUrl}, '3000x3000', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=3000'); }); }); @@ -67,7 +67,7 @@ describe('image formatter', () => { const newFileImgUrl = Formatters.image(newFileImg, '3000x1000', false).url; 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=1500'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=1500'); }); it('if can\'t get original ratio, use desired dimensions', () => { @@ -80,7 +80,7 @@ describe('image formatter', () => { 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, width: 0, height: 0}, '3000x3000', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000,height=3000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000,height=3000'); }); }); @@ -95,7 +95,7 @@ describe('image formatter', () => { 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, '3000x', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/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'); @@ -106,7 +106,7 @@ describe('image formatter', () => { 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, '3000x', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=3000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=3000'); }); it('can restrict the dimensions by height, regardless of atLeastAsLarge', () => { @@ -119,7 +119,7 @@ describe('image formatter', () => { 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, 'x2000', false).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=2000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/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'); @@ -130,7 +130,7 @@ describe('image formatter', () => { 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, 'x2000', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,height=2000'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,height=2000'); }); }); @@ -145,7 +145,7 @@ describe('image formatter', () => { const newFileImgUrl = Formatters.image(newFileImg).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain,width=200'); const euFileImgUrl = Formatters.image(euFileImg).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain,width=200'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain,width=200'); }); it('do not transform image regardless of atLeastAsLarge', () => { @@ -158,7 +158,7 @@ describe('image formatter', () => { 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'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain'); photoImgUrl = Formatters.image(photoImg, 'x', true).url; expect(photoImgUrl).toEqual('https://dyn.mktgcdn.com/p/FOO/fit=contain'); @@ -169,21 +169,25 @@ describe('image formatter', () => { newFileImgUrl = Formatters.image(newFileImg, 'x', true).url; expect(newFileImgUrl).toEqual('https://dyn.mktgcdn.com/f/FOO.jpg/fit=contain'); euFileImgUrl = Formatters.image(euFileImg, 'x', true).url; - expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/0/FOO.jpg/fit=contain'); + expect(euFileImgUrl).toEqual('https://dyn.eu.mktgcdn.com/f/FOO.jpg/fit=contain'); }); }); describe('when desiredSize is not parseable', () => { - it('throws an error', () => { - expect(() => {Formatters.image(photoImg, 'ax')}).toThrowError(); + it('if width is invalid, throws an error', () => { + expect(() => {Formatters.image(photoImg, 'ax')}).toThrow( + 'Error processing image url https://dynm.mktgcdn.com/p/FOO/2000x1000.jpg/: ' + + 'Error: Invalid width specified'); }); - it('throws an error', () => { - expect(() => {Formatters.image(photoImg, 'xa')}).toThrowError(); + it('if height is invalid, throws an error', () => { + expect(() => {Formatters.image(photoImg, 'xa')}).toThrow( + 'Error processing image url https://dynm.mktgcdn.com/p/FOO/2000x1000.jpg/: ' + + 'Error: Invalid height specified'); }); - it('throws an error', () => { - expect(() => {Formatters.image(photoImg, '')}).toThrowError(); + it('if desiredSize is "", throws an error', () => { + expect(() => {Formatters.image(photoImg, '')}).toThrow('Invalid desired size'); }); }); }); \ No newline at end of file