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

The placeholder image in "gatsby-image" does not fade on Safari #8323

Closed
JakeRider opened this issue Sep 19, 2018 · 26 comments
Closed

The placeholder image in "gatsby-image" does not fade on Safari #8323

JakeRider opened this issue Sep 19, 2018 · 26 comments
Labels
type: bug An issue or pull request relating to a bug in Gatsby

Comments

@JakeRider
Copy link

Description

The low-res placeholder image that gatsby-image generates is not fading out on Safari, both on desktop and mobile. You can see it happening at the gatsby-image example site. It becomes an issue when using images with transparency, as you can still see parts of the placeholder.

Steps to reproduce

Use gatsby-image on Safari. https://using-gatsby-image.gatsbyjs.org is a good example, but it seems to affect any site built with Gatsby. My project, the gatsby-image demo site, and the main Gatsby site are all being affected from what I saw.

Expected result

The placeholder should fade out after the main image loads.

Actual result

The placeholder remains opaque, and the main image loads over top of it.

Environment

System:
OS: macOS High Sierra 10.13.6
CPU: x64 Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 10.8.0 - ~/.nvm/versions/node/v10.8.0/bin/node
npm: 6.4.1 - ~/.nvm/versions/node/v10.8.0/bin/npm
Browsers:
Chrome: 68.0.3440.106
Safari: 12.0
npmGlobalPackages:
gatsby-cli: 2.4.0

@stefanprobst
Copy link
Contributor

stefanprobst commented Sep 19, 2018

Not sure if I am misunderstanding the issue, but could you check the css of the placeholder <img> in your browser's devtools? It should look like this:

<div class="gatsby-image-wrapper">
  <img> <!-- CHECK IF THIS HAS opacity: 0 -->
  <picture></picture>
</div>

Also, is this working correctly with another browser?

EDIT: I think I have spotted the issue, working on it.

@JakeRider
Copy link
Author

gatsby

The opacity is still 1. To confirm, everything works fine in Chrome/Firefox, but mobile and desktop Safari are both doing this. If you come to the page from within the site, it works fine, but if you refresh the page, or link to it directly, it doesn't seem to trigger.

@stefanprobst
Copy link
Contributor

Hmm, that's weird - transition-delay is also not updated when state changes. See:

const imagePlaceholderStyle = {
opacity: this.state.imgLoaded ? 0 : 1,
transitionDelay: this.state.imgLoaded ? `0.5s` : `0.25s`,
...imgStyle,
...placeholderStyle,
}

You could check if onLoad gets called by passing it as a prop to <Img onLoad={() => console.log('Image loaded!')} />, or you could check with the React DevTools, if you have it installed in your browser. (Unfortunately I cannot check myself because Safari has no linux version).

Related to the issue that I think I spotted: in Firefox/Chrome does the placeholder actually fade out, or just gets invisible. Because it seems the placeholder img gets a transition-delay but no actual transition.

@stefanprobst
Copy link
Contributor

One more thought: difference between Safari and Firefox/Chrome is that the latter support IntersectionObserver (which is used in gatsby-image), and Safari doesn't.

@JakeRider
Copy link
Author

After having experimented a bit, polyfilling IntersectionObserver seems to solve the issue . It looks like #4021 already has some discussion about adding a polyfill. Would you like me to put together a PR for this & a working transition?

@stefanprobst
Copy link
Contributor

That would be great, thanks!
wrt adding IntersectionObserver: @KyleAMathews ok to add IntersectionObserver polyfill to gatsby-image?

@DSchau
Copy link
Contributor

DSchau commented Sep 19, 2018

I'll let @KyleAMathews answer for himself, but I'm not sure we'd want to include a polyfill explicitly within gatsby-image.

We can (and probably should!) document the usage of such polyfill(s) and mention how to use them, but I don't think I like the idea of an explicit polyfill.

That said, I'm also not 100% sure I'm seeing the issue. The images appear to load fine for me in Safari (12.0 (13606.2.11)) 🤷‍♂️ Am I missing something?

Safari Images

@JakeRider
Copy link
Author

JakeRider commented Sep 19, 2018

The issue is not that the images don’t load, it’s that the placeholder doesn’t fade. It’s not obvious on pictures that don’t have transparency, but it looks terrible if the image does. There’s probably a way to fix it that doesn’t involve a polyfill, but Safari makes up a big enough chunk of the market that it’s worth considering.

Edit: This is what images with transparency look like on Safari, desktop and mobile.
img_0071

As you can see, the placeholder doesn't fade, and bleeds through the actual image as a result.

@stefanprobst
Copy link
Contributor

Maybe we can figure this out together. AFAICS the placeholder <img> has opacity: 1 even though it should be 0 when state.imgLoaded === true.
@JakeRider can you check the <Img> component state in React DevTools? And check if props.onLoad gets called (which should happen in handleImageLoaded which also sets state.imgLoaded: true

@DSchau
Copy link
Contributor

DSchau commented Sep 19, 2018

@JakeRider that example is a lot more helpful! I think I've seen something similar on a site I helped out with. Scroll down to the "We've worked with amazing clients" section.

Note: this is before some of the gatsby-image v2 stuff was shipped, so it's a bit out of date. (the opacity is still 1 here on the placeholder image, which should be the same issue)

I'm game to help figure this out too. @stefanprobst I think if I use the above site's source, we could debug it pretty easily.

@kakadiadarpan kakadiadarpan added the type: bug An issue or pull request relating to a bug in Gatsby label Sep 20, 2018
@stefanprobst
Copy link
Contributor

Sounds good! The issue should also be visible on a simple default starter (link to repo). Could you (or somebody with Safari) check? This is what I see when I manually set the opacity on the placeholder:

issue8323

@stefanprobst
Copy link
Contributor

Possible duplicate of #7539 ?

@DSchau
Copy link
Contributor

DSchau commented Sep 21, 2018

@stefanprobst I wouldn't say that's a duplicate, I would say that (hopefully!) that fixes this issue! 🤞 I can pull it down locally and try in a bit.

@stefanprobst
Copy link
Contributor

Thanks! The actual issue this PR is trying to solve has this comment that looks very similar to what seems to be happening here.

@stefanprobst
Copy link
Contributor

@JakeRider Sorry for getting back at this so late. I have tried to clean-up gatsby-image a bit (esp. removing stuff from state that does not need to live there). This is not a proper PR (yet), but if you're willing to try this out I'd be very interested if it fixes this particular issue. You can install the component directly from my github:

yarn add https://github.com/stefanprobst/gatsby-image-compact

Then adjust your imports from gatsby-image to gatsby-image-compact, the rest should hopefully "just work".

@ThiagoMiranda
Copy link

I'm having the same issue on Chrome 69.0.3497.100 for MacOS as well. Is there a fix for this?
Thanks!
captura de tela 2018-10-09 as 17 09 00

@stefanprobst
Copy link
Contributor

Is there a fix for this?

Not yet. You could try if #7539 (or my test repo above) fixes the issue for you and report back here.

@m-allanson
Copy link
Contributor

This should be fixed in gatsby-image@2.0.24, see #7539 for details! I'm going to close this, please re-open or create a new issue if it's not fixed for you.

@samselikoff
Copy link

This issue perfectly describes a bug I hit, but my google-fu is failing me. I can't find any other traces of this & my project's dependencies are all up to date.

Should I upload a simplified reproduction?

@ooloth
Copy link
Contributor

ooloth commented Dec 13, 2019

@samselikoff Before you spend time on a repro, are you seeing this lack of fading in a version of Safari that supports IntersectionObserver or one that doesn't?

If it doesn't, the first thing you might try is polyfilling IntersectionObserver and seeing if that solves it.

@samselikoff
Copy link

Hm I did try using this technique I believe #10435 but I can double check

@samselikoff
Copy link

samselikoff commented Dec 13, 2019

Also I’m seeing this on latest Safari on my desktop so I don’t think it’s the issue

@ooloth
Copy link
Contributor

ooloth commented Dec 13, 2019

Hmm...actually, yeah: I can confirming I'm also currently seeing this lack of fading on https://using-gatsby-image.gatsbyjs.org in the latest Safari as well.

@samselikoff
Copy link

Yes, exactly. Looks like the placeholder is just disappearing.

@samselikoff
Copy link

Should we open a new issue?

@ooloth
Copy link
Contributor

ooloth commented Dec 13, 2019

Should we open a new issue?

Probably a good idea, with a link to this one in the description for context.

Glad you spotted this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

8 participants