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

next/image - onLoad event work incorrect #20368

Closed
therealtx opened this issue Dec 21, 2020 · 11 comments
Closed

next/image - onLoad event work incorrect #20368

therealtx opened this issue Dec 21, 2020 · 11 comments
Labels
bug Issue was opened via the bug report template.

Comments

@therealtx
Copy link

Bug report

Describe the bug

onLoad event on image fires not when user scroll until image, but when page loads, so that's impossible to add any spinner to next/image

To Reproduce

Add next/image component with onLoad event to long page bottom and just reload page, after page load you will see onLoad runs without scroll

Expected behavior

onLoad should work when image on user view port i think, or maybe there is should be onRender method for that.

System information

  • OS: [e.g. macOS, Windows]
  • Browser (if applies) chrome 87.0.4280.88 (Official Build) (64-bit)
  • Version of Next.js: 10.0.3
  • Version of Node.js: 14.6.0
  • Deployment: next dev
@therealtx therealtx added the bug Issue was opened via the bug report template. label Dec 21, 2020
@timneutkens
Copy link
Member

timneutkens commented Dec 22, 2020

onLoad is not supported on next/image.

@therealtx
Copy link
Author

But there is no any method to handle when image start/finish Rendering.

@vendramini
Copy link

I'm trying to implement a simple placeholder, but I can't remove the background's animation without onLoad. What's the reason for not support onLoad? Is it possible to listen to other similar event?

@CitizenBeta
Copy link

I think this deserves a second review by NextJs, currently image loading using next/image is very jarring visually. An onLoad event would allow us to have animated loads and css transitions. Is there a different way we're supposed to do this?

@dbismut
Copy link

dbismut commented Jan 3, 2021

I agree... can't the onLoad props be passed to the underlying object?

@dbismut
Copy link

dbismut commented Jan 10, 2021

It looks like I was wrong as onLoad is indeed passed to the underlying image. But the problem is that the placeholder fires it as well, so it's potentially fired twice.

If anyone's interested, here's my current workaround:

<Image
  src={img.url}
  onLoad={(e) => {
    e.target.src.indexOf('data:image/gif;base64') < 0 && handleLoad()
  }}
 />

We only want to trigger the load handler when the actual image is loaded, hence making sure the source of the target element triggering the event is not base64.

This obviously has limitations but this is my best attempt atm.

Ideally, the lib would expose both onLoad and onVisible callbacks with a prop isPlaceholder passed to the handler.

@nwesthoff
Copy link

Thanks @dbismut! That helps a lot.

One thing to note is that the onLoad event doesn't seem to reliably fire when an image is set to priority. So if anyone is having trouble with images seemingly not loading on page refresh, that might be it.

@pcwa-ahendricks
Copy link

Thanks @dbismut! That helps a lot.

One thing to note is that the onLoad event doesn't seem to reliably fire when an image is set to priority. So if anyone is having trouble with images seemingly not loading on page refresh, that might be it.

I can confirm that onLoad doesn't work with priority prop.

@justinphilpott
Copy link
Contributor

I've just had exactly this issue, has anyone dug into why that onload isn't firing?

@MarcGuiselin
Copy link

Use onLoadingComplete instead of onLoad!

@timneutkens was correct that image components don't support onLoad

An onLoad event isn't always triggered in react because react might bind the event handler after an image has already loaded.

See the following comment in the next/image source:

// See https://stackoverflow.com/q/39777833/266535 for why we use this ref
// handler instead of the img's onLoad attribute.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

10 participants