Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gatsby-image): add matchMedia to fix wrong aspect ratio and dimensions in art-directed image arrays #19887

Merged
merged 6 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions packages/gatsby-image/src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,22 @@ const setupImages = (
}

describe(`<Image />`, () => {
const OLD_MATCH_MEDIA = window.matchMedia
pvdz marked this conversation as resolved.
Show resolved Hide resolved
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()
Expand Down Expand Up @@ -204,6 +220,66 @@ describe(`<Image />`, () => {
expect(console.warn).toBeCalled()
})

it(`should select the correct mocked image of fluid 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(
<Image
backgroundColor
className={`fluidArtDirectedImage`}
style={{ display: `inline` }}
title={`Title for the image`}
alt={`Alt text for the image`}
crossOrigin={`anonymous`}
fluid={tripleFluidImageShapeMock}
itemProp={`item-prop-for-the-image`}
placeholderStyle={{ color: `red` }}
placeholderClassName={`placeholder`}
/>
)
const aspectPreserver = container.querySelector(`div div div`)
expect(aspectPreserver.getAttribute(`style`)).toEqual(
expect.stringMatching(/padding-bottom: 20%/)
)
})

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(
<Image
backgroundColor
className={`fixedArtDirectedImage`}
style={{ display: `inline` }}
title={`Title for the image`}
alt={`Alt text for the image`}
crossOrigin={`anonymous`}
fixed={tripleFixedImageShapeMock}
itemProp={`item-prop-for-the-image`}
placeholderStyle={{ color: `red` }}
placeholderClassName={`placeholder`}
/>
)
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()
Expand Down
55 changes: 52 additions & 3 deletions packages/gatsby-image/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,67 @@ 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.
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @return {boolean}
timhagn marked this conversation as resolved.
Show resolved Hide resolved
*/
const hasArtDirectionSupport = (props, prop) =>
props[prop] &&
Array.isArray(props[prop]) &&
props[prop].some(image => typeof image.media !== `undefined`)
pvdz marked this conversation as resolved.
Show resolved Hide resolved

/**
* Checks for fluid or fixed Art direction support.
* @param props {object} The props to check for art-direction support.
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @return {boolean}
*/
const hasArtDirectionArray = props =>
hasArtDirectionSupport(props, `fluid`) ||
hasArtDirectionSupport(props, `fixed`)
timhagn marked this conversation as resolved.
Show resolved Hide resolved

/**
* Tries to detect if a media query matches the current viewport.
* @param media {string} A media query string.
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @return {*|boolean}
timhagn marked this conversation as resolved.
Show resolved Hide resolved
*/
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`
* @param {{fluid: {src: string}[], fixed: {src: string}[]}} args
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @return {string}
*/
const getImageSrcKey = ({ fluid, fixed }) => {
const data = (fluid && fluid[0]) || (fixed && fixed[0])
const data = getCurrentSrcData({ fluid, fixed })
timhagn marked this conversation as resolved.
Show resolved Hide resolved

return data.src
}

/**
* Returns the current src - if possible with art-direction support.
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @param fluid {object} Fluid Image (Array) if existent.
* @param fixed {object} Fixed Image (Array) if existent.
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @param index {number} The index of the image to return (default: 0).
timhagn marked this conversation as resolved.
Show resolved Hide resolved
* @return {*}
timhagn marked this conversation as resolved.
Show resolved Hide resolved
*/
const getCurrentSrcData = ({ fluid, fixed }, index = 0) => {
timhagn marked this conversation as resolved.
Show resolved Hide resolved
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({})
Expand Down Expand Up @@ -416,7 +465,7 @@ class Image extends React.Component {

if (fluid) {
const imageVariants = fluid
const image = imageVariants[0]
const image = getCurrentSrcData({ fluid })
timhagn marked this conversation as resolved.
Show resolved Hide resolved

return (
<Tag
Expand Down Expand Up @@ -516,7 +565,7 @@ class Image extends React.Component {

if (fixed) {
const imageVariants = fixed
const image = imageVariants[0]
const image = getCurrentSrcData({ fixed })
timhagn marked this conversation as resolved.
Show resolved Hide resolved

const divStyle = {
position: `relative`,
Expand Down