-
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
gatsby-image browser support #4021
Comments
Oh hmmm yeah — we should definitely polyfill gatsby-image for IE. object-fit-images looks great from a quick look. |
I've just come across this issue on a project that has (sadly) about 15% IE11 usage. I'm happy to jump in and take a crack at adding the polyfill - just wanted to check in here and see if there's any WIP or other thinking around this. Thanks! |
plus one - got the same problem |
Hey all! I also noticed this issue. Here's how you can fix it: npm i intersection-observer object-fit-images // gatsby-browser.js
exports.onClientEntry = () => {
// NOTE: Don't polyfill Promise here (Gatsby already includes a Promise polyfill)
// IntersectionObserver polyfill for gatsby-image (Safari, IE)
if (typeof window.IntersectionObserver === `undefined`) {
require(`intersection-observer`)
console.log(`👍 IntersectionObserver is polyfilled`)
}
// Object-fit/Object-position polyfill for gatsby-image (IE)
const testImg = document.createElement(`img`)
if (
typeof testImg.style.objectFit === `undefined` ||
typeof testImg.style.objectPosition === `undefined`
) {
require(`object-fit-images`)()
console.log(`👍 Object-fit/Object-position are polyfilled`)
}
} The However, the 'object-fit-images` polyfill requires that you add the following CSS to your image's styles: font-family: "object-fit: VALUE_HERE; object-position: VALUE_HERE" You can either add this font-family declaration manually to each image, or create a wrapper for // Img.js (a gatsby-image wrapper)
const Img = props => {
// Construct font-family declaration for object-fit-images
const objFit = props.objFit ? props.objFit : `cover`
const objPosition = props.objPosition ? props.objPosition : `50% 50%`
const fontFamily = `"object-fit: ${objFit}; object-position: ${objPosition}"`
const imgStyle = {
objectFit: objFit,
objectPosition: objPosition,
fontFamily: fontFamily
}
return (
<Image
sizes={props.sizes}
alt={props.alt}
className={props.className}
style={props.style}
outerWrapperClassName={props.outerWrapperClassName}
imgStyle={{ ...imgStyle }}
position={props.position || `relative`}
backgroundColor={props.backgroundColor || `transparent`}
Tag={props.Tag || `div`}
/>
)
}
/*
*
* Imports & Exports
*
*/
import React from 'react'
import Image from 'gatsby-image'
export default Img Hope that helps! |
Thanks for sharing @ooloth! 🤗 After copy-pasta-ing your fix, more specifically the gatsby-image wrapper component, I noticed that Here's my quick fix for that: const Img = props => {
// Construct font-family declaration for object-fit-images
const objFit = props.objFit ? props.objFit : `cover`
const objPosition = props.objPosition ? props.objPosition : `50% 50%`
const fontFamily = `"object-fit: ${objFit}; object-position: ${objPosition}"`
const polyfillStyles = {
objectFit: objFit,
objectPosition: objPosition,
fontFamily: fontFamily,
}
return (
<Image
sizes={props.sizes}
resolutions={props.resolutions}
alt={props.alt}
className={props.className}
style={props.style}
outerWrapperClassName={props.outerWrapperClassName}
imgStyle={{ ...props.imgStyle, ...polyfillStyles }}
position={props.position || `relative`}
backgroundColor={props.backgroundColor || `transparent`}
Tag={props.Tag || `div`}
/>
)
} |
@fk Awesome! Thanks for the code review. 😎 |
To simplify this a bit further, we can pass all props through except the ones we actually need: const Img = ({ objFit = `cover`, objPosition = `50% 50%`, ...props }) =>(
<Image
{...props}
imgStyle={{
...props.imgStyle,
objectFit: objFit,
objectPosition: objPosition,
fontFamily: `"object-fit: ${objFit}; object-position: ${objPosition}"`,
}}
/>
) This should allow usage of this wrapper exactly the same as I feel like it probably makes sense to roll this into |
oh, if that's all the polyfill is then by all means, let's add it into the component directly. |
@KyleAMathews There are also two dependencies we'd need to add to |
Hmmm so the two together are 3.7kb That seems a bit much to add to every user of gatsby-image? Perhaps just document well this for people who need to support older IEs? |
@KyleAMathews Would dynamic import remove that extra size for modern browsers? exports.onClientEntry = async () => {
// NOTE: Don't polyfill Promise here (Gatsby already includes a Promise polyfill)
// IntersectionObserver polyfill for gatsby-image (Safari, IE)
if (typeof window.IntersectionObserver === `undefined`) {
await import(`intersection-observer`)
console.log(`👍 IntersectionObserver is polyfilled`)
}
// Object-fit/Object-position polyfill for gatsby-image (IE)
const testImg = document.createElement(`img`)
if (
typeof testImg.style.objectFit === `undefined` ||
typeof testImg.style.objectPosition === `undefined`
) {
await import(`object-fit-images`)()
console.log(`👍 Object-fit/Object-position are polyfilled`)
}
} (Please note that I haven't tried this and I'm not 100% sure if I've got the syntax right.) |
Oh yeah! That'd totally work. No reason to await though (also await isn't well-supported in browsers or babel so best still to avoid it in frontend) |
In Gatsby v2, will Babel 7 automatically polyfill I'm currently loading the (I assume CSS polyfills like |
@jlengstorf @KyleAMathews dynamic import would be really great so that everyone gets out of the box support for legacy browsers. Since we already have the solution let's fix this soon? |
@kakadiadarpan seems reasonable to me. Does anyone have time to implement this? We're effectively looking to add the solution I proposed in #4021 (comment), but without the |
@jlengstorf Sure! I’m happy to take this one. I’ve been using this solution on my end (in gatsby-browser) and it would be nice to bake it in. |
Quick Gatsby plugin authoring question for the group! I'm following the contributing instructions here and my local updates to the Although everything appears to be wired up correctly, the For example, the following console log does not appear on the test site: // gatsby/packages/gatsby-image/gatsby-browser.js
// same as: test-site/node_modules/gatsby-image/gatsby-browser.js
"use strict"
exports.onClientEntry = () => {
console.log(`Working`)
} Is there a trick to getting |
For |
@pieh Thank you! Such a simple fix. 👍 |
Has this been added to gatsby-image? If not what's the preferred solution for IE11? |
Not quite yet! (The PR is in progress.) In the meantime,
exports.onClientEntry = () => {
// IntersectionObserver polyfill for gatsby-image (Safari, IE)
if (typeof window.IntersectionObserver === `undefined`) {
import(`intersection-observer`)
console.log(`👍 IntersectionObserver is polyfilled`)
}
// Object-fit/Object-position polyfill for gatsby-image (IE)
const testImg = document.createElement(`img`)
if (
typeof testImg.style.objectFit === `undefined` ||
typeof testImg.style.objectPosition === `undefined`
) {
import(`object-fit-images`)()
console.log(`👍 Object-fit/Object-position are polyfilled`)
}
}
import Image from 'gatsby-image'
const Img = ({ objFit = `cover`, objPosition = `50% 50%`, ...props }) => (
<Image
{...props}
imgStyle={{
...props.imgStyle,
objectFit: objFit,
objectPosition: objPosition,
fontFamily: `"object-fit: ${objFit}; object-position: ${objPosition}"`,
}}
/>
) |
https://gist.github.com/mik-laj/d9455f4543c71376c9b2220f3437f5de |
For folks copying from here, The following works as an alternative.
|
Hiya! This issue has gone quiet. Spooky quiet. 👻 We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open! Thanks for being a part of the Gatsby community! 💪💜 |
How's the PR going, is there anything we can help with? I just ran into this myself, so very interested to see what we do here. IE11 is still very much in use for Windows users. |
Hi @marcysutton! The PR has gone a bit quiet:
At this point, it's unclear to me whether adding the It may be cleaner to simply add a page to the docs that explains how to wrap A documentation-based approach has the advantage of keeping the I'm interested in hearing your thoughts! |
That's a really interesting idea — we could actually ship that as part of gatsby-image so all you'd need to do is |
Would it be possible to check the |
I really like the idea of offering it by default, so if we can support |
Alrighty, I was finally able to carve out some time to resolve this puppy! I've created a new PR (#12681) that allows users who want Thanks to @KyleAMathews and @jlengstorf for suggesting this approach. Please feel free to review the PR and let me know if the implementation can be improved in any way. |
…t-fit/position support (#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 #4021. Co-authored-by: Ward Peeters <[email protected]>
@ooloth great work! Have you observed any issue in production using the polyfill? Are there any performance issues on using it? Do you know any websites that are using it? I am wondering if gatsbyjs.org is using it as I can see the images in IE11. |
@PolGuixe Thanks!
Nope! Please let me know if you do.
Not that I've encountered.
I've used the
The gatsbyjs.org site isn't using it (you can verify this by searching the The |
Description
gatsby-image
usesobject-fit/object-position
which are not supported in IE.https://www.gatsbyjs.org/docs/browser-support/ states support for IE 9+.
Some discussion about the issue and a workaround can be seen here: #2470 (comment)
Expected behavior
a) Either a note on
gatsby-image
that includes a "blessed" solution for IE,b) an
object-fit/object-position
polyfill ingatsby-image
.I'd be happy to open a pull request for either of these options (not sure what including the polyfill directly would look like), but maybe adding the note about object-fit-images would be enough?
The text was updated successfully, but these errors were encountered: