-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby-image): add matchMedia to fix wrong aspect ratio and dimensions in art-directed image arrays #19887
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've left some comments. Please have a look :)
change wording according to review Co-Authored-By: Peter van der Zee <209817+pvdz@users.noreply.github.com>
Hi @pvdz, thanks for the reviews, gonna continue attending them now. I just took the Best, Tim. #done Adapted everything to your reviews / change requests and commented on the other two : ). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement, thank you!
What's left is mostly about typing.
Hi @pvdz, thanks again for the reviews and sorry that I didn't get back to you earlier, was just fully booked last week and got me a stomach bug over the weekend : (. Best, Tim. |
Hi @pvdz, while having a short break of both my bug & work and looking through your requests, I finally realize, why most of Best, Tim. |
Hi again, hope I now id change all comments and requests accordingly : ). Best, Tim. |
Mkay. The starters_validate failing seems to have nothing to do with this PR, any ideas? Best, Tim. EDIT: Oh, I forgot to merge in the other changes from upstream % ). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now everything should be according to your requests - else I'm just stripping out the JSDocs ; ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing all the issues and for creating this PR in the first place :D
Thanks @pvdz! Hope I didn't get the JSDocs too wrong ^^. Glad something from Best, Tiim. EDIT: Smiley split corrected; ). |
Description
This PR introduces a
matchMedia()
query forfluid
orfixed
art-directed arrays.This prevents retrieving the wrong image (the first from the art-direction stack ain't always the correct one) when ascertaining its
aspectRatio
orwidth & height
respectively.It adds tests for
fluid
orfixed
art-directed stacks as well.Of course this sadly only works during rendering in the browser, and falls back to the default first image from the array, but at least in the browser the correct image is chosen ; ).
Related Issues
Addresses #15189
Addresses a few of the pain points in ##13395
(And probably a few other issues ; )
Best,
Tim.
P.S.: Works quite well for me in
gatsby-backrgound-image
; ).