#2333 Add optional ratio for ressources (#2372)

* #2333 Add optional ratio for ressources * Apply suggestions from code review Co-authored-by: Revanth Kumar Annavarapu <35203638+revanth0212@users.noreply.github.com> * PR changes. * Prettier changes. * Deriving height if not provided in props using ratio. * Taking ratio as an argument in generateUrlFromContainerWidth. * Added new story to images story book. * Fixed styleguide build failure. * Not setting height on image, it will override the src calculation by the browser. * Snapshot updates. Co-authored-by: Tommy Wiebell <twiebell@adobe.com> Co-authored-by: Revanth Kumar Annavarapu <35203638+revanth0212@users.noreply.github.com> Co-authored-by: Revanth Kumar <revanth0212@gmail.com> Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
magento · Jun 1, 2020 · d1a9fa3 · d1a9fa3
1 parent d443b93
commit d1a9fa3
Show file tree

Hide file tree

Showing 14 changed files with 163 additions and 40 deletions.
diff --git a/packages/peregrine/lib/talons/Image/__tests__/useImage.spec.js b/packages/peregrine/lib/talons/Image/__tests__/useImage.spec.js
@@ -6,7 +6,8 @@ import { useImage } from '../useImage';
 const props = {
     onError: jest.fn(),
     onLoad: jest.fn(),
-    widths: new Map().set('default', 50)
+    widths: new Map().set('default', 50),
+    ratio: 4 / 5
 };
 
 const log = jest.fn();
@@ -30,7 +31,8 @@ test('it returns the proper shape', () => {
         handleImageLoad: expect.any(Function),
         hasError: expect.any(Boolean),
         isLoaded: expect.any(Boolean),
-        resourceWidth: expect.any(Number)
+        resourceWidth: expect.any(Number),
+        resourceHeight: expect.any(Number)
     });
 });
 
@@ -85,3 +87,69 @@ describe('resourceWidth', () => {
         );
     });
 });
+
+describe('resourceHeight', () => {
+    test('should return height if not a falsy value', () => {
+        const newProps = {
+            ...props,
+            height: 500
+        };
+
+        createTestInstance(<Component {...newProps} />);
+
+        expect(log).toHaveBeenCalledWith(
+            expect.objectContaining({
+                resourceHeight: 500
+            })
+        );
+    });
+
+    test('should return height set to undefined if ratio is falsy', () => {
+        const newProps = {
+            ...props,
+            height: null,
+            ratio: null
+        };
+
+        createTestInstance(<Component {...newProps} />);
+
+        expect(log).toHaveBeenCalledWith(
+            expect.objectContaining({
+                resourceHeight: undefined
+            })
+        );
+    });
+
+    test('should return height set to undefined if resourceWidth is falsy', () => {
+        const newProps = {
+            ...props,
+            height: null,
+            widths: null
+        };
+
+        createTestInstance(<Component {...newProps} />);
+
+        expect(log).toHaveBeenCalledWith(
+            expect.objectContaining({
+                resourceHeight: undefined
+            })
+        );
+    });
+
+    test('should derive height from resourceWidth and ratio if they are not falsy', () => {
+        const newProps = {
+            ...props,
+            height: null,
+            ratio: 4 / 5,
+            width: 100
+        };
+
+        createTestInstance(<Component {...newProps} />);
+
+        expect(log).toHaveBeenCalledWith(
+            expect.objectContaining({
+                resourceHeight: 500 / 4
+            })
+        );
+    });
+});
diff --git a/packages/peregrine/lib/talons/Image/useImage.js b/packages/peregrine/lib/talons/Image/useImage.js
@@ -8,10 +8,12 @@ export const UNCONSTRAINED_SIZE_KEY = 'default';
  * @param {function} props.onError callback for error of loading image
  * @param {function} props.onLoad callback for load of image
  * @param {number}   props.width the intrinsic width of the image & the width to request for the fallback image for browsers that don't support srcset / sizes.
+ * @param {number}   props.height the intrinsic height of the image & the height to request for the fallback image for browsers that don't support srcset / sizes.
+ * @param {number}   props.ratio is the image width to height ratio. Defaults to `DEFAULT_WIDTH_TO_HEIGHT_RATIO` from `util/images.js`.
  * @param {Map}      props.widths a map of breakpoints to possible widths used to create the img's sizes attribute.
  */
 export const useImage = props => {
-    const { onError, onLoad, width, widths } = props;
+    const { onError, onLoad, width, widths, height, ratio } = props;
     const [isLoaded, setIsLoaded] = useState(false);
     const [hasError, setHasError] = useState(false);
 
@@ -46,11 +48,22 @@ export const useImage = props => {
         return widths.get(UNCONSTRAINED_SIZE_KEY);
     }, [width, widths]);
 
+    const resourceHeight = useMemo(() => {
+        if (height) {
+            return height;
+        } else if (resourceWidth && ratio) {
+            return resourceWidth / ratio;
+        } else {
+            return undefined;
+        }
+    }, [height, ratio, resourceWidth]);
+
     return {
         handleError,
         handleImageLoad,
         hasError,
         isLoaded,
-        resourceWidth
+        resourceWidth,
+        resourceHeight
     };
 };
diff --git a/packages/peregrine/lib/talons/Image/useResourceImage.js b/packages/peregrine/lib/talons/Image/useResourceImage.js
@@ -12,6 +12,7 @@ import { UNCONSTRAINED_SIZE_KEY } from './useImage';
  * @param {string}  props.type - The Magento image type ("image-category" / "image-product"). Used to build the resource URL.
  * @param {number}  props.width - The width to request for the fallback image for browsers that don't support srcset / sizes.
  * @param {Map}     props.widths - The map of breakpoints to possible widths used to create the img's sizes attribute.
+ * @param {number}   props.ratio is the image width to height ratio. Defaults to 4/5.
  */
 export const useResourceImage = props => {
     const {
@@ -21,16 +22,17 @@ export const useResourceImage = props => {
         resource,
         type,
         width,
-        widths
+        widths,
+        ratio
     } = props;
 
     const src = useMemo(() => {
         return generateUrl(resource, type)(width, height);
     }, [generateUrl, height, resource, type, width]);
 
     const srcSet = useMemo(() => {
-        return generateSrcset(resource, type);
-    }, [generateSrcset, resource, type]);
+        return generateSrcset(resource, type, ratio);
+    }, [generateSrcset, resource, type, ratio]);
 
     // Example: 100px
     // Example: (max-width: 640px) 50px, 100px

diff --git a/.../lib/components/CheckoutPage/ItemsReview/__tests__/__snapshots__/itemsReview.spec.js.snap b/.../lib/components/CheckoutPage/ItemsReview/__tests__/__snapshots__/itemsReview.spec.js.snap
@@ -24,7 +24,7 @@ exports[`Snapshot test 1`] = `
           onError={[Function]}
           onLoad={[Function]}
           sizes="100px"
-          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/t/vt12-kh_main_2.jpg?auto=webp&format=pjpg&width=100&fit=cover"
+          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/t/vt12-kh_main_2.jpg?auto=webp&format=pjpg&width=100&height=125&fit=cover"
           srcSet="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/t/vt12-kh_main_2.jpg?auto=webp&format=pjpg&width=40&height=50&fit=cover 40w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/t/vt12-kh_main_2.jpg?auto=webp&format=pjpg&width=80&height=100&fit=cover 80w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/t/vt12-kh_main_2.jpg?auto=webp&format=pjpg&width=160&height=200&fit=cover 160w,
@@ -74,7 +74,7 @@ https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/produc
           onError={[Function]}
           onLoad={[Function]}
           sizes="100px"
-          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/s/vsw02-pe_main_2.jpg?auto=webp&format=pjpg&width=100&fit=cover"
+          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/s/vsw02-pe_main_2.jpg?auto=webp&format=pjpg&width=100&height=125&fit=cover"
           srcSet="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/s/vsw02-pe_main_2.jpg?auto=webp&format=pjpg&width=40&height=50&fit=cover 40w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/s/vsw02-pe_main_2.jpg?auto=webp&format=pjpg&width=80&height=100&fit=cover 80w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/s/vsw02-pe_main_2.jpg?auto=webp&format=pjpg&width=160&height=200&fit=cover 160w,
@@ -124,7 +124,7 @@ https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/produc
           onError={[Function]}
           onLoad={[Function]}
           sizes="100px"
-          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/d/vd01-ll_main_2.jpg?auto=webp&format=pjpg&width=100&fit=cover"
+          src="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/d/vd01-ll_main_2.jpg?auto=webp&format=pjpg&width=100&height=125&fit=cover"
           srcSet="https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/d/vd01-ll_main_2.jpg?auto=webp&format=pjpg&width=40&height=50&fit=cover 40w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/d/vd01-ll_main_2.jpg?auto=webp&format=pjpg&width=80&height=100&fit=cover 80w,
 https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/media/catalog/product/cache/d3ba9f7bcd3b0724e976dc5144b29c7d/v/d/vd01-ll_main_2.jpg?auto=webp&format=pjpg&width=160&height=200&fit=cover 160w,

diff --git a/packages/venia-ui/lib/components/Gallery/__tests__/__snapshots__/gallery.spec.js.snap b/packages/venia-ui/lib/components/Gallery/__tests__/__snapshots__/gallery.spec.js.snap
@@ -24,7 +24,6 @@ exports[`renders if \`items\` is an array of objects 1`] = `
         <img
           alt="Test Product 1"
           className="image loaded"
-          height={375}
           loading="lazy"
           onError={[MockFunction]}
           onLoad={[MockFunction]}
@@ -68,7 +67,6 @@ a.url 2560w"
         <img
           alt="Test Product 2"
           className="image loaded"
-          height={375}
           loading="lazy"
           onError={[MockFunction]}
           onLoad={[MockFunction]}

diff --git a/packages/venia-ui/lib/components/Gallery/__tests__/__snapshots__/item.spec.js.snap b/packages/venia-ui/lib/components/Gallery/__tests__/__snapshots__/item.spec.js.snap
@@ -59,12 +59,11 @@ exports[`renders correctly with valid item data 1`] = `
       <img
         alt="Test Product"
         className="image loaded"
-        height={375}
         loading="lazy"
         onError={[MockFunction]}
         onLoad={[MockFunction]}
         sizes="(max-width: 640px) 300px, 840px"
-        src="/media/catalog/product/foo/bar/pic.png?auto=webp&format=png&width=100&height=375&fit=cover"
+        src="/media/catalog/product/foo/bar/pic.png?auto=webp&format=png&width=100&fit=cover"
         srcSet="/media/catalog/product/foo/bar/pic.png?auto=webp&format=png&width=40&height=50&fit=cover 40w,
 /media/catalog/product/foo/bar/pic.png?auto=webp&format=png&width=80&height=100&fit=cover 80w,
 /media/catalog/product/foo/bar/pic.png?auto=webp&format=png&width=160&height=200&fit=cover 160w,

diff --git a/packages/venia-ui/lib/components/Image/___tests__/__snapshots__/resourceImage.spec.js.snap b/packages/venia-ui/lib/components/Image/___tests__/__snapshots__/resourceImage.spec.js.snap
@@ -4,7 +4,6 @@ exports[`Allows overriding of loading attribute 1`] = `
 <img
   alt="SimpleImage Unit Test"
   className="unit_test_class"
-  height={125}
   loading="eager"
   onError={[MockFunction]}
   onLoad={[MockFunction]}
@@ -26,7 +25,6 @@ exports[`renders correctly 1`] = `
 <img
   alt="SimpleImage Unit Test"
   className="unit_test_class"
-  height={125}
   loading="lazy"
   onError={[MockFunction]}
   onLoad={[MockFunction]}

diff --git a/packages/venia-ui/lib/components/Image/__stories__/image.js b/packages/venia-ui/lib/components/Image/__stories__/image.js
@@ -10,6 +10,9 @@ const stories = storiesOf('Components/Image', module);
 
 const getSrc = text =>
     `https://via.placeholder.com/400x600/0000FF/FFFFFF?text=${encodeURI(text)}`;
+const getSrc19 = text =>
+    `https://via.placeholder.com/480x270/0000FF/FFFFFF?text=${encodeURI(text)}`;
+
 const loadingPlaceholder =
     'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAASsAAACoCAMAAACPKThEAAAAsVBMVEUiOUsiJ0IiNUkiz64i2rYiQlEiOEwiP08lP08qSVUiQVEpRlMaKEIjPE0dL0VKiYExWF9Hgnw8bmwsTFciKkM1X2MbK0MxV19FfnlBd3Jdrpk4ZWg7bW5AdHM6ampMjYJRl4lYo5NjuqJowqdty64gI0AisJgioI0ihHoikoMib2wiFzsiYGMipJAivKAieXMiwaQi6L8ijH8iVlwiHz8iEToiADIhTVYiY2VaqZUj4LlYhDuQAAAEIUlEQVR4nO3ZD1PiOAAF8DTYNG1IWmjaglAUpX8AqUW8c9fv/8Euraurrpx7OyOFm/ebN50KzqTzKAkBQgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA6JvvmLMcI6upDjR3dvyuIP1EFZe+hak1gzQrUiROldTgul1NMDquuLOzK6ttgmd9UqX2prWa02utTlMn9QN3lZoqw3dLWud17Pul0vb4u7+zyP67+2u7BHe9bqzOr66o6Lrlme6eJmU+RlbtGHTVytq8yqV2W8rjSm+td0ve4R3dtt/l6Wm9W6MPfVuta6jnvObc+SDqb6n1ShH/LqRhd5sdJ5VZa0sMzsbh4tVpVYFrslJq0Xyqx4mpolUFGizdrHlHmIxCq/q0qzIlJU9bn4m4eafhf9/F/gf0hp63Ro1eFyqtVd3TsdVam7mv1YVmzzlc3+K7o39LOo96E/wkw+G1as8u0y6+TWYvosjzX9aGwpTZjkjHPmcSZMbOLaJOQs9MxRJk14wu1AeoEUEXEj6UY0nNCkTTBRUZtJm+lEDSOVRnQcsVFATAYJ9xPhh6HvJhcmPLjgxIzqkOb4NHx75M8nUlKqaV13UlZ2tudFksRUxD1iC+YKFgoSuiQJWRCyqAmfhHzm8qkQqeuOvXBshyMnGPBoQCNfzXw69FXqx4OL/sDv+37/wo8v/Xg+6H8fqfMROx/Lq1ReDcPrYXIdBYtotgiGC3e8ENImzo8wz8QjJvbLicdsh93ndXboosxcVVT3Hz/z1JVNRNuV67K3XclJKGeCT10xFW5quvLCEQ9MnruaDkxX/ZH/qqtBfDnqz5+7Ok/51dB96uo6mC2Stiv7VVdtTEW26cq01HblNHHWvcN/ccHi7b5V5V1X4o+6ir+kK2au6vHgN5Yqc73nqaPuysnq1aE/4Opq75BH3JWJutvse5G/itVz9y0ox90VXVWH78o+0a6+HbwrXX870ffg/on2q6jl3rf9cXdlVbeH3+hsrdP7zEBsSbeH/7FD58WeQY+4K+ZkVREftqiGflx9XNZLV/bTHufXrto9jivS113Jn101e5ymqHd7nPngpSv53FW0CD7a4zTxfu3KI9ldJ7+hUc/s2j/+7N7sVjmxOREea+8u0uwKBUsEaSMjwSNhR0JMeJOZDE2mLElpZDJUk1TNUjVNVZqq8VCNhmowVf6MXszIxURemkTiMnLnSTJPgnkYzcXkO5ceMTEbUekwE8ch7bGNbCJVtjnTnXzRQGn9WLI//eIt25usiW6O90/nTe7/JVa2bvIpb7nNu6nK7Amth/zx7IRs3K6qMqjOND0V5lq7/km26/F/EzuR6wQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADgx/wDGcra5uVy7UQAAAABJRU5ErkJggg==';
 
@@ -67,6 +70,43 @@ stories.add(
     )
 );
 
+stories.add(
+    'An Image using a Magento resource with resource constraints with ratio',
+    () => (
+        <div className={classes.container}>
+            <Image
+                alt="An Image using a Magento resource with resource constraints"
+                classes={{ root: classes.root }}
+                resource={getSrc19('480x270')}
+                ratio={16 / 9}
+            />
+        </div>
+    )
+);
+
+stories.add(
+    'An Image using a Magento resource with resource constraints with ratio and widths',
+    () => {
+        const widths = new Map()
+            .set(320, 240)
+            .set(768, 480)
+            .set(1024, 960)
+            .set(UNCONSTRAINED_SIZE_KEY, 1280);
+
+        return (
+            <div className={classes.container}>
+                <Image
+                    alt="An Image using a Magento resource with resource constraints"
+                    classes={{ root: classes.root }}
+                    resource={getSrc19('480x270')}
+                    ratio={16 / 9}
+                    widths={widths}
+                />
+            </div>
+        );
+    }
+);
+
 stories.add('An Image using a Magento resource with sizes', () => {
     const widths = new Map().set(640, 300).set(UNCONSTRAINED_SIZE_KEY, 800);
 

diff --git a/packages/venia-ui/lib/components/Image/image.js b/packages/venia-ui/lib/components/Image/image.js
@@ -10,12 +10,13 @@ import PropTypes, {
 } from 'prop-types';
 import { useImage } from '@magento/peregrine/lib/talons/Image/useImage';
 
-import { mergeClasses } from '../../classify';
-import defaultClasses from './image.css';
 import PlaceholderImage from './placeholderImage';
 import ResourceImage from './resourceImage';
 import SimpleImage from './simpleImage';
+import { mergeClasses } from '../../classify';
+import { DEFAULT_WIDTH_TO_HEIGHT_RATIO } from '../../util/images';
 
+import defaultClasses from './image.css';
 /**
  * The Image component renders a placeholder until the image is loaded.
  *
@@ -29,6 +30,7 @@ import SimpleImage from './simpleImage';
  * @param {string}   props.src the source of the image, ready to use in an img element
  * @param {string}   props.type the Magento image type ("image-category" / "image-product"). Used to build the resource URL.
  * @param {number}   props.width the intrinsic width of the image & the width to request for the fallback image for browsers that don't support srcset / sizes.
+ * @param {number}   props.ratio is the image width to height ratio. Defaults to `DEFAULT_WIDTH_TO_HEIGHT_RATIO` from `util/images.js`.
  * @param {Map}      props.widths a map of breakpoints to possible widths used to create the img's sizes attribute.
  */
 const Image = props => {
@@ -45,22 +47,26 @@ const Image = props => {
         type,
         width,
         widths,
+        ratio,
         ...rest
     } = props;
 
     const talonProps = useImage({
         onError,
         onLoad,
         width,
-        widths
+        widths,
+        height,
+        ratio
     });
 
     const {
         handleError,
         handleImageLoad,
         hasError,
         isLoaded,
-        resourceWidth: talonResourceWidth
+        resourceWidth: talonResourceWidth,
+        resourceHeight: talonResourceHeight
     } = talonProps;
 
     const classes = mergeClasses(defaultClasses, propsClasses);
@@ -75,7 +81,7 @@ const Image = props => {
             className={imageClass}
             handleError={handleError}
             handleLoad={handleImageLoad}
-            height={height}
+            height={talonResourceHeight}
             src={src}
             width={width}
             {...rest}
@@ -86,11 +92,12 @@ const Image = props => {
             className={imageClass}
             handleError={handleError}
             handleLoad={handleImageLoad}
-            height={height}
+            height={talonResourceHeight}
             resource={resource}
             type={type}
             width={talonResourceWidth}
             widths={widths}
+            ratio={ratio}
             {...rest}
         />
     );
@@ -149,11 +156,13 @@ Image.propTypes = {
     src: conditionallyRequiredString,
     type: string,
     width: oneOfType([number, string]),
-    widths: instanceOf(Map)
+    widths: instanceOf(Map),
+    ratio: number
 };
 
 Image.defaultProps = {
-    displayPlaceholder: true
+    displayPlaceholder: true,
+    ratio: DEFAULT_WIDTH_TO_HEIGHT_RATIO
 };
 
 export default Image;