-
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
feat(gatsby-remark-images): set wrapperStyle as a function to enable dynamic css #12060
Conversation
option.addAspectRatio adds the aspect ratio of the image as `flex: value;` to the wrapperStyle
Thank you for opening a PR! 💖 I'm not sure we want to introduce this functionality as this issue lies on the app level and not gatsby itself. I would suggest making the wrapperStyle a function which gets the image information so it can be used to export a dynamic css string.
unsure what the rest of @gatsbyjs/core thinks. |
Not sure if this belongs in @CanRau Have you tried styling the images using |
I like that 👍 UPDATE not yet sure why but it's only running the function on one image (first probably) then using that value for all others 🤔 so they all end up with the same aspectRatio
I need the image dimensions so it's not sufficient as far as I know. And as |
like @wardpeet suggested, instead of hard coding the aspectRatio stuff we allow the user to provide a function which receives all necessary information
@CanRau If I need to take a look why you can't get it executed more than once, just let me know. |
@wardpeet thank you I already figured it out and fixed it, the new commits reflect all my changes EDIT: let me know if the readme isn't clear enough or should follow another style, or any other feedback/changes 😉 |
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.
This looks solid! Thank you for your patience.
Yay 🎉 😁 glad you like it, thanks for approving, and thanks to me traveling the last ~2 weeks 😉 |
Published in [email protected] |
Thanks for posting the release version, very useful!! 👍 🙏 |
I'm not entirely sure about this, I'm aware of the origin #3987 yet this PR I made proofs the opposite #12060 Only thing I can think of right now that browser APIs get stringified, couldn't verify this so far, this #3834 is suggesting they get stringified and it's the reason the note was added to the docs in the first place.. Couldn't just leave my confusion alone and thought doing it in a PR might be useful if it's actually not valid anymore so it can be merged directly, otherwise feel free to reject it of course 👍
Description
User can provide a function as
wrapperStyle
which receives thefluidResult
containing image information like theaspectRatio
which allows dynamic and customizable image stylingOld Description
Providing the new option
addAspectRatio
to the package settings, this PR will add the already available aspect ratio of the image to the wrapperStyle as flex property. Which allows images of different aspect ratios to adjust their sizes to match heights of siblings in adisplay: flex;
containerlike this
instead of this
live example: https://www.gaiama.org/15 (you'll have to scroll a little bit for images with differing aspect ratios)
I'm of course open to feedback and changes or other approaches 👍
IMHO it's a small change no one would ever notice not using it, but would help me a lot and maybe even open others the possibility of (in my opinion) beautiful inline image galleries 😀
I thought about a plugin running after
gatsby-remark-images
as well, yet that would mean recalculating the aspect ratios which are already present here.By the way I'm currently running a small custom remark-plugin afterwards to wrap multiple images in the same paragraph in a flex enabled container. Link to old version where I overcomplicated things a little 🤦♂️https://github.com/GaiAma/gaiama.org/blob/master/packages/gatsby-remark-wrap-images/index.js so lines 31-41 is not necessary anymore.. new approach will be to just wrap images in a div with a customizable class, I could package that one up of course
Wish y'all a great monday 🙏