From 8cdf1a45c9da1192f3668425aafa628bcd5cc61f Mon Sep 17 00:00:00 2001 From: Tim Hagn Date: Sat, 30 Nov 2019 15:51:30 +0100 Subject: [PATCH 1/5] fix query for wrong aspect ratio --- packages/gatsby-image/src/__tests__/index.js | 46 +++++++++++++++++ packages/gatsby-image/src/index.js | 53 +++++++++++++++++++- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-image/src/__tests__/index.js b/packages/gatsby-image/src/__tests__/index.js index 78809a8485a9e..c4f33f3cacb8a 100644 --- a/packages/gatsby-image/src/__tests__/index.js +++ b/packages/gatsby-image/src/__tests__/index.js @@ -119,6 +119,22 @@ const setupImages = ( } describe(``, () => { + const OLD_MATCH_MEDIA = window.matchMedia + beforeEach(() => { + window.matchMedia = jest.fn(media => + media === `only screen and (min-width: 1024px)` + ? { + matches: true, + } + : { + matches: false, + } + ) + }) + afterEach(() => { + window.matchMedia = OLD_MATCH_MEDIA + }) + it(`should render fixed size images`, () => { const component = setup() expect(component).toMatchSnapshot() @@ -204,6 +220,36 @@ describe(``, () => { expect(console.warn).toBeCalled() }) + it(`should select the correct mocked image of variants provided.`, () => { + const tripleFluidImageShapeMock = fluidImagesShapeMock.concat({ + aspectRatio: 5, + src: `test_image_4.jpg`, + srcSet: `third other srcSet`, + srcSetWebp: `third other srcSetWebp`, + sizes: `(max-width: 1920px) 100vw, 1920px`, + base64: `string_of_base64`, + media: `only screen and (min-width: 1024px)`, + }) + const { container } = render( + {`Alt + ) + const aspectPreserver = container.querySelector(`div div div`) + expect(aspectPreserver.getAttribute(`style`)).toEqual( + expect.stringMatching(/padding-bottom: 20%/) + ) + }) + it(`should call onLoad and onError image events`, () => { const onLoadMock = jest.fn() const onErrorMock = jest.fn() diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index c1a9ce2bc50bc..feeb5f918a475 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -48,6 +48,34 @@ const convertProps = props => { return convertedProps } +/** + * Checks if fluid or fixed are art-direction arrays. + * @param props {object} The props to check for images. + * @param prop {string} Check for fluid or fixed. + * @return {boolean} + */ +export const hasArtDirectionSupport = (props, prop) => + props[prop] && + Array.isArray(props[prop]) && + props[prop].some(image => typeof image.media !== `undefined`) + +/** + * Checks for fluid or fixed Art direction support. + * @param props {object} The props to check for art-direction support. + * @return {boolean} + */ +export const hasArtDirectionArray = props => + hasArtDirectionSupport(props, `fluid`) || + hasArtDirectionSupport(props, `fixed`) + +/** + * Tries to detect if a media query matches the current viewport. + * @param media {string} A media query string. + * @return {*|boolean} + */ +export const matchesMedia = ({ media }) => + media ? isBrowser && window.matchMedia(media).matches : false + /** * Find the source of an image to use as a key in the image cache. * Use `the first image in either `fixed` or `fluid` @@ -55,11 +83,32 @@ const convertProps = props => { * @return {string} */ const getImageSrcKey = ({ fluid, fixed }) => { - const data = (fluid && fluid[0]) || (fixed && fixed[0]) + const data = getCurrentSrcData({ fluid, fixed }) return data.src } +/** + * Returns the current src - if possible with art-direction support. + * @param fluid {object} Fluid Image (Array) if existent. + * @param fixed {object} Fixed Image (Array) if existent. + * @param index {number} The index of the image to return (default: 0). + * @return {*} + */ +export const getCurrentSrcData = ({ fluid, fixed }, index = 0) => { + const currentData = fluid || fixed + if (isBrowser && hasArtDirectionArray({ fluid, fixed })) { + // Do we have an image for the current Viewport? + const foundMedia = currentData.findIndex(matchesMedia) + if (foundMedia !== -1) { + return currentData[foundMedia] + } + return currentData[index] + } + // Else return the selected image. + return currentData[index] +} + // Cache if we've seen an image before so we don't bother with // lazy-loading & fading in on subsequent mounts. const imageCache = Object.create({}) @@ -416,7 +465,7 @@ class Image extends React.Component { if (fluid) { const imageVariants = fluid - const image = imageVariants[0] + const image = getCurrentSrcData({ fluid }) return ( Date: Sat, 30 Nov 2019 16:26:02 +0100 Subject: [PATCH 2/5] exchange fixed[0] to getCurrentSrcData({ fixed }) to get correct dimensions --- packages/gatsby-image/src/__tests__/index.js | 32 +++++++++++++++++++- packages/gatsby-image/src/index.js | 10 +++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/gatsby-image/src/__tests__/index.js b/packages/gatsby-image/src/__tests__/index.js index c4f33f3cacb8a..a3868ccee55c9 100644 --- a/packages/gatsby-image/src/__tests__/index.js +++ b/packages/gatsby-image/src/__tests__/index.js @@ -220,7 +220,7 @@ describe(``, () => { expect(console.warn).toBeCalled() }) - it(`should select the correct mocked image of variants provided.`, () => { + it(`should select the correct mocked image of fluid variants provided.`, () => { const tripleFluidImageShapeMock = fluidImagesShapeMock.concat({ aspectRatio: 5, src: `test_image_4.jpg`, @@ -250,6 +250,36 @@ describe(``, () => { ) }) + it(`should select the correct mocked image of fixed variants provided.`, () => { + const tripleFixedImageShapeMock = fixedImagesShapeMock.concat({ + width: 1024, + height: 768, + src: `test_image_4.jpg`, + srcSet: `third other srcSet`, + srcSetWebp: `third other srcSetWebp`, + base64: `string_of_base64`, + media: `only screen and (min-width: 1024px)`, + }) + const { container } = render( + {`Alt + ) + const aspectPreserver = container.querySelector(`div div`) + expect(aspectPreserver.getAttribute(`style`)).toEqual( + expect.stringMatching(/width: 1024px; height: 768px;/) + ) + }) + it(`should call onLoad and onError image events`, () => { const onLoadMock = jest.fn() const onErrorMock = jest.fn() diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index feeb5f918a475..324b73820611a 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -54,7 +54,7 @@ const convertProps = props => { * @param prop {string} Check for fluid or fixed. * @return {boolean} */ -export const hasArtDirectionSupport = (props, prop) => +const hasArtDirectionSupport = (props, prop) => props[prop] && Array.isArray(props[prop]) && props[prop].some(image => typeof image.media !== `undefined`) @@ -64,7 +64,7 @@ export const hasArtDirectionSupport = (props, prop) => * @param props {object} The props to check for art-direction support. * @return {boolean} */ -export const hasArtDirectionArray = props => +const hasArtDirectionArray = props => hasArtDirectionSupport(props, `fluid`) || hasArtDirectionSupport(props, `fixed`) @@ -73,7 +73,7 @@ export const hasArtDirectionArray = props => * @param media {string} A media query string. * @return {*|boolean} */ -export const matchesMedia = ({ media }) => +const matchesMedia = ({ media }) => media ? isBrowser && window.matchMedia(media).matches : false /** @@ -95,7 +95,7 @@ const getImageSrcKey = ({ fluid, fixed }) => { * @param index {number} The index of the image to return (default: 0). * @return {*} */ -export const getCurrentSrcData = ({ fluid, fixed }, index = 0) => { +const getCurrentSrcData = ({ fluid, fixed }, index = 0) => { const currentData = fluid || fixed if (isBrowser && hasArtDirectionArray({ fluid, fixed })) { // Do we have an image for the current Viewport? @@ -565,7 +565,7 @@ class Image extends React.Component { if (fixed) { const imageVariants = fixed - const image = imageVariants[0] + const image = getCurrentSrcData({ fixed }) const divStyle = { position: `relative`, From a94d0c087d9f6087c88bb0f395e57047d086305d Mon Sep 17 00:00:00 2001 From: Tim Hagn Date: Mon, 2 Dec 2019 12:25:33 +0100 Subject: [PATCH 3/5] Update packages/gatsby-image/src/index.js change wording according to review Co-Authored-By: Peter van der Zee <209817+pvdz@users.noreply.github.com> --- packages/gatsby-image/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index 324b73820611a..a825211f157f2 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -89,7 +89,7 @@ const getImageSrcKey = ({ fluid, fixed }) => { } /** - * Returns the current src - if possible with art-direction support. + * Returns the current src - Preferably with art-direction support. * @param fluid {object} Fluid Image (Array) if existent. * @param fixed {object} Fixed Image (Array) if existent. * @param index {number} The index of the image to return (default: 0). From e670a07bb3d910317ddece6530c786ede49fc1cc Mon Sep 17 00:00:00 2001 From: Tim Hagn Date: Mon, 2 Dec 2019 20:57:37 +0100 Subject: [PATCH 4/5] change functions according to reviews by @pvdz --- packages/gatsby-image/src/index.js | 45 +++++++++++------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index a825211f157f2..31f95974e2157 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -50,31 +50,22 @@ const convertProps = props => { /** * Checks if fluid or fixed are art-direction arrays. - * @param props {object} The props to check for images. - * @param prop {string} Check for fluid or fixed. + * + * @param currentData {array} The props to check for images. * @return {boolean} */ -const hasArtDirectionSupport = (props, prop) => - props[prop] && - Array.isArray(props[prop]) && - props[prop].some(image => typeof image.media !== `undefined`) - -/** - * Checks for fluid or fixed Art direction support. - * @param props {object} The props to check for art-direction support. - * @return {boolean} - */ -const hasArtDirectionArray = props => - hasArtDirectionSupport(props, `fluid`) || - hasArtDirectionSupport(props, `fixed`) +const hasArtDirectionSupport = currentData => + currentData && + Array.isArray(currentData) && + currentData.some(image => typeof image.media !== `undefined`) /** * Tries to detect if a media query matches the current viewport. * @param media {string} A media query string. - * @return {*|boolean} + * @return {boolean} */ const matchesMedia = ({ media }) => - media ? isBrowser && window.matchMedia(media).matches : false + media ? isBrowser && !!window.matchMedia(media).matches : false /** * Find the source of an image to use as a key in the image cache. @@ -83,30 +74,26 @@ const matchesMedia = ({ media }) => * @return {string} */ const getImageSrcKey = ({ fluid, fixed }) => { - const data = getCurrentSrcData({ fluid, fixed }) + const data = fluid ? getCurrentSrcData(fluid) : getCurrentSrcData(fixed) return data.src } /** * Returns the current src - Preferably with art-direction support. - * @param fluid {object} Fluid Image (Array) if existent. - * @param fixed {object} Fixed Image (Array) if existent. - * @param index {number} The index of the image to return (default: 0). + * @param currentData {array} The fluid or fixed image array. * @return {*} */ -const getCurrentSrcData = ({ fluid, fixed }, index = 0) => { - const currentData = fluid || fixed - if (isBrowser && hasArtDirectionArray({ fluid, fixed })) { +const getCurrentSrcData = currentData => { + if (isBrowser && hasArtDirectionSupport(currentData)) { // Do we have an image for the current Viewport? const foundMedia = currentData.findIndex(matchesMedia) if (foundMedia !== -1) { return currentData[foundMedia] } - return currentData[index] } - // Else return the selected image. - return currentData[index] + // Else return the first image. + return currentData[0] } // Cache if we've seen an image before so we don't bother with @@ -465,7 +452,7 @@ class Image extends React.Component { if (fluid) { const imageVariants = fluid - const image = getCurrentSrcData({ fluid }) + const image = getCurrentSrcData(fluid) return ( Date: Thu, 12 Dec 2019 14:31:10 +0100 Subject: [PATCH 5/5] adapt jsdocs to change requests --- packages/gatsby-image/src/index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index 31f95974e2157..cb3753855b194 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -51,17 +51,17 @@ const convertProps = props => { /** * Checks if fluid or fixed are art-direction arrays. * - * @param currentData {array} The props to check for images. + * @param currentData {{media?: string}[]} The props to check for images. * @return {boolean} */ const hasArtDirectionSupport = currentData => - currentData && + !!currentData && Array.isArray(currentData) && currentData.some(image => typeof image.media !== `undefined`) /** * Tries to detect if a media query matches the current viewport. - * @param media {string} A media query string. + * @property media {{media?: string}} A media query string. * @return {boolean} */ const matchesMedia = ({ media }) => @@ -70,7 +70,7 @@ const matchesMedia = ({ media }) => /** * Find the source of an image to use as a key in the image cache. * Use `the first image in either `fixed` or `fluid` - * @param {{fluid: {src: string}[], fixed: {src: string}[]}} args + * @param {{fluid: {src: string, media?: string}[], fixed: {src: string, media?: string}[]}} args * @return {string} */ const getImageSrcKey = ({ fluid, fixed }) => { @@ -81,8 +81,8 @@ const getImageSrcKey = ({ fluid, fixed }) => { /** * Returns the current src - Preferably with art-direction support. - * @param currentData {array} The fluid or fixed image array. - * @return {*} + * @param currentData {{media?: string}[]} The fluid or fixed image array. + * @return {{src: string, media?: string}} */ const getCurrentSrcData = currentData => { if (isBrowser && hasArtDirectionSupport(currentData)) {