feat(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support by ooloth · Pull Request #12681 · gatsbyjs/gatsby

ooloth · 2019-03-19T20:59:31Z

Description

This PR will allow users who want object-fit/object-position support in IE to simply import 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 /withIEPolyfill version of gatsby-image currently does the following:

Checks if the browser supports the object-fit/position CSS properties.
a. If yes, no polyfill is loaded.
b. If no, the object-fit-images polyfill is imported and called.
Wraps gatsby-image in a component that passes the new objectFit and objectPosition prop values to the polyfill implementation (which requires a weird font-family hack).

New props

To make the implementation simple, I've added new objectFit and objectPosition props 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-images polyfill directly in withIEPolyfill/index.js rather than in gatsby-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-images repo 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 /withIEPolyfills component, a brief explanation will need to be added to the gatsby-image docs. I held off on adding one for now in case we end up changing the API.

Related Issues

Fixes #4021.

Update fork with upstream changes

wardpeet · 2019-03-20T12:04:24Z

this is great!

jonlow

Looks great 💪

wardpeet · 2019-03-25T19:23:26Z

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.

ooloth · 2019-03-25T19:25:05Z

Love it! Thanks for the improvements. 👍

wardpeet · 2019-03-26T12:55:39Z

pushed the latest fix, ref handling is ugly but changing forwardRef inside gatsby-image would be a breaking change.

wardpeet

Let's ship this when tests pass!

ooloth · 2019-03-26T14:11:41Z

Sounds great! Thanks for guiding it over the finish line.

lannonbr · 2019-03-26T14:56:49Z

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

Add instructions for using `gatsby-image/withIEPolyfill` as implemented by #12681.

ooloth · 2019-03-26T15:40:39Z

Absolutely!

I just submitted PR #12869, which adds instructions to the gatsby-image README.

Feel free to improve it as needed.

* 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 <hello@michaeluloth.com> * Add line breaks Trying to guess what issues Prettier is seeing while my computer is away for repairs... * chore: format

* 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 <hello@michaeluloth.com> * Add line breaks Trying to guess what issues Prettier is seeing while my computer is away for repairs... * chore: format

…t-fit/position support (gatsbyjs#12681) ## Description This PR will allow users who want `object-fit/object-position` support in IE to simply `import 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 `/withIEPolyfill` version of `gatsby-image` currently does the following: 1. Checks if the browser supports the `object-fit/position` CSS properties. a. If yes, no polyfill is loaded. b. If no, the `object-fit-images` polyfill is imported and called. 2. Wraps `gatsby-image` in a component that passes the new `objectFit` and `objectPosition` prop values to the polyfill implementation (which requires a weird `font-family` hack). ### New props To make the implementation simple, I've added new `objectFit` and `objectPosition` props that will needed 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-images` polyfill directly in `withIEPolyfill/index.js` rather than in `gatsby-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-images` [repo](https://github.com/bfred-it/object-fit-images) 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 `/withIEPolyfills` component, a brief explanation will need to be added to the `gatsby-image` docs. I held off on adding one for now in case we end up changing the API. ## Related Issues Fixes gatsbyjs#4021. Co-authored-by: Ward Peeters <ward@coding-tech.com>

* 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 <hello@michaeluloth.com> * Add line breaks Trying to guess what issues Prettier is seeing while my computer is away for repairs... * chore: format

ooloth added 2 commits March 19, 2019 15:03

Merge pull request #4 from gatsbyjs/master

90e5d56

Update fork with upstream changes

Add a withIEPolyfill wrapper for gatsby-image

dc909ff

This was referenced Mar 19, 2019

gatsby-image browser support #4021

Closed

Dynamically add IO and object-fit/object-position polyfills to gatsby-image (only when needed) #9451

Closed

wardpeet reviewed Mar 20, 2019

View reviewed changes

Comment thread packages/gatsby-image/withIEPolyfill/index.js Outdated

wardpeet reviewed Mar 20, 2019

View reviewed changes

Comment thread packages/gatsby-image/src/withIEPolyfill/index.js Outdated

ooloth added 2 commits March 20, 2019 19:47

Ignore compiled source file

951c9f9

Apply polyfill to current image only

2fae59e

jonlow approved these changes Mar 25, 2019

View reviewed changes

wardpeet reviewed Mar 25, 2019

View reviewed changes

Comment thread packages/gatsby-image/src/withIEPolyfill/index.js Outdated

wardpeet added 2 commits March 25, 2019 20:18

rm withIEPolyfill

1f9b866

update ref handling to accept an external ref

ef08cee

fix image ref handling in polyfill

0ccf2b8

wardpeet approved these changes Mar 26, 2019

View reviewed changes

Merge branch 'master' into pr/12681

b58b21e

wardpeet changed the title ~~Add gatsby-image/withIEPolyfill export with object-fit/position support~~ chore(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support Mar 26, 2019

wardpeet changed the title ~~chore(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support~~ feat(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support Mar 26, 2019

wardpeet merged commit 4b9b6a1 into gatsbyjs:master Mar 26, 2019

ooloth deleted the topics/gatsby-image-with-ie-polyfill branch March 26, 2019 14:51

ooloth added a commit that referenced this pull request Mar 26, 2019

Update README.md

4bf2405

Add instructions for using `gatsby-image/withIEPolyfill` as implemented by #12681.

ooloth mentioned this pull request Mar 26, 2019

docs(gatsby-image): Add withIEPolyfill documentation #12869

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support#12681

feat(gatsby-image): Add gatsby-image/withIEPolyfill export with object-fit/position support#12681
wardpeet merged 8 commits intogatsbyjs:masterfrom
ooloth:topics/gatsby-image-with-ie-polyfill

ooloth commented Mar 19, 2019 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

wardpeet commented Mar 20, 2019

Uh oh!

jonlow left a comment

Uh oh!

Uh oh!

wardpeet commented Mar 25, 2019

Uh oh!

ooloth commented Mar 25, 2019

Uh oh!

wardpeet commented Mar 26, 2019

Uh oh!

wardpeet left a comment

Uh oh!

ooloth commented Mar 26, 2019

Uh oh!

lannonbr commented Mar 26, 2019

Uh oh!

ooloth commented Mar 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ooloth commented Mar 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

New props

Loading polyfill in /withIEPolyfill/index.js

Polyfill repo has been archived

Docs to be updated once implementation is finalized

Related Issues

Uh oh!

Uh oh!

Uh oh!

wardpeet commented Mar 20, 2019

Uh oh!

jonlow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wardpeet commented Mar 25, 2019

Uh oh!

ooloth commented Mar 25, 2019

Uh oh!

wardpeet commented Mar 26, 2019

Uh oh!

wardpeet left a comment

Choose a reason for hiding this comment

Uh oh!

ooloth commented Mar 26, 2019

Uh oh!

lannonbr commented Mar 26, 2019

Uh oh!

ooloth commented Mar 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ooloth commented Mar 19, 2019 •

edited

Loading