feat(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support #12681
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update fork with upstream changes
This was referenced Mar 19, 2019
wardpeet
reviewed
Mar 20, 2019
wardpeet
reviewed
Mar 20, 2019
|
this is great! |
wardpeet
reviewed
Mar 25, 2019
|
I updated the code a bit to accept an external ref as well. I'm going to test this that it doesn't break anything and get this merged tomorrow! 🎉 Thanks for all the hard work. |
|
Love it! Thanks for the improvements. 👍 |
|
pushed the latest fix, ref handling is ugly but changing forwardRef inside gatsby-image would be a breaking change. |
wardpeet
approved these changes
Mar 26, 2019
|
Sounds great! Thanks for guiding it over the finish line. |
wardpeet
changed the title
wardpeet
changed the title
|
Awesome to see this merged in! 🎉 Is it possible to append the docs for gatsby-image about this new feature if it hasn't already? @ooloth |
ooloth
added a commit
that referenced
this pull request
Mar 26, 2019
Add instructions for using `gatsby-image/withIEPolyfill` as implemented by #12681.
|
Absolutely! I just submitted PR #12869, which adds instructions to the Feel free to improve it as needed. |
marcysutton
pushed a commit
that referenced
this pull request
Mar 29, 2019
* Update README.md Add instructions for using `gatsby-image/withIEPolyfill` as implemented by #12681. * Update packages/gatsby-image/README.md Prompt the user to include an alt attribute, even if empty, for accessibility. Co-Authored-By: ooloth <[email protected]> * Add line breaks Trying to guess what issues Prettier is seeing while my computer is away for repairs... * chore: format
johno
pushed a commit
to jlengstorf/gatsby
that referenced
this pull request
Apr 10, 2019
* Update README.md Add instructions for using `gatsby-image/withIEPolyfill` as implemented by gatsbyjs#12681. * Update packages/gatsby-image/README.md Prompt the user to include an alt attribute, even if empty, for accessibility. Co-Authored-By: ooloth <[email protected]> * Add line breaks Trying to guess what issues Prettier is seeing while my computer is away for repairs... * chore: format
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR will allow users who want
object-fit/object-positionsupport in IE to simplyimport Img from "gatsby-image/withIEPolyfill"instead of importing directly from"gatsby-image".Please feel free to review the PR and let me know if the implementation can be improved.
The
/withIEPolyfillversion ofgatsby-imagecurrently does the following:object-fit/positionCSS properties.a. If yes, no polyfill is loaded.
b. If no, the
object-fit-imagespolyfill is imported and called.gatsby-imagein a component that passes the newobjectFitandobjectPositionprop values to the polyfill implementation (which requires a weirdfont-familyhack).New props
To make the implementation simple, I've added new
objectFitandobjectPositionprops that will need to be used to pass the corresponding values to the polyfill. This is to avoid a scenario where a user attempts to set these values in a way we can't pass to the polyfill (e.g. via an external CSS stylesheet) and doesn't understand why the polyfill isn't working. Let me know if this API can be improved.Loading polyfill in /withIEPolyfill/index.js
I opted to load the
object-fit-imagespolyfill directly inwithIEPolyfill/index.jsrather than ingatsby-browser.js. If anyone knows of a better way to approach this, please let me know and feel free to make improvements.Polyfill repo has been archived
I was surprised to see that the
object-fit-imagesrepo has recently been archived by its owner. It still works, but if anyone is concerned about this or knows of a reliable alternative polyfill, please let me know.Docs to be updated once implementation is finalized
Once we've settled on the API for this
/withIEPolyfillscomponent, a brief explanation will need to be added to thegatsby-imagedocs. I held off on adding one for now in case we end up changing the API.Related Issues
Fixes #4021.