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

[gatsby-remark-images] allow user-specified max-width to overwrite presentationWidth #15952

Closed
wants to merge 3 commits into from

Conversation

birjj
Copy link
Contributor

@birjj birjj commented Jul 21, 2019

Description

Currently gatsby-remark-images enforces a max-width on each image equal to the presentationWidth returned by gatsby-plugin-sharp. Any user-specified max-width in the wrapperStyle is simply ignored.
Use case for custom max-width is described in #15578

This PR adds support for specifying a max-width in the wrapperStyle which will be used instead of the presentationWidth.

Related Issues

Fixes #15578

…sentationWidth

This is useful when you e.g. want to use non-pixel units for max-width
Fix #15578
@birjj birjj requested a review from a team as a code owner July 21, 2019 11:40
rawHTML = `
<span
class="${imageWrapperClass}"
style="position: relative; display: block; margin-left: auto; margin-right: auto; ${
imageCaption ? `` : wrapperStyle
} max-width: ${presentationWidth}px;"
} ${shouldApplyMaxWidth ? `max-width: ${presentationWidth}px;` : ``}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving max-width before ${imageCaption ? : wrapperStyle}``? So maxWidth from user supplied `wrapperStyle` would overwrite default one.

This seems fragile - we have 2 different conditions relying on imageCaption and if we ever change this condition in future, it's very easy to miss it in one of the places

@wardpeet
Copy link
Contributor

ping @birjolaxew any chance you can resolve this comment?

@birjj
Copy link
Contributor Author

birjj commented Sep 1, 2019

@wardpeet @pieh

Due to some changes in my personal life I unfortunately won't be updating this PR (although I agree with your suggestion, @pieh). If anyone feels like opening a new one for this issue, feel free.

@birjj birjj closed this Sep 1, 2019
@pieh
Copy link
Contributor

pieh commented Sep 2, 2019

@birjolaxew I hope this means change of focus and not any problems happening in your personal life! Even tho we didn't get to finish this changeset - I'd like to thank you for opening this PR ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gatsby-remark-images] Can't specify max-width
3 participants