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 does not work properly when used in a horizontally scrolling container #32774

Closed
agusterodin opened this issue Dec 23, 2021 · 21 comments · Fixed by #33290 or #33933
Closed

Next Image does not work properly when used in a horizontally scrolling container #32774

agusterodin opened this issue Dec 23, 2021 · 21 comments · Fixed by #33290 or #33933
Assignees
Labels
Image (next/image) Related to Next.js Image Optimization.

Comments

@agusterodin
Copy link

What version of Next.js are you using?

12.0.7

What version of Node.js are you using?

16.13.0

What browser are you using?

Chrome (Desktop and Android)

What operating system are you using?

macOS Monterey and Android 12

How are you deploying your application?

Vercel (broken when developing locally with yarn dev as well)

Describe the Bug

There are two issues present:

I am using Next Image inside of a horizontally scrolling container and the lazyBoundary prop isn't working + on Android Chrome images will permanently disappear once scrolled away from.

Firstly, here is a video screenshot of the network tab in Chrome on desktop. Notice that images do not load until the image is literally in the viewport despite lazyBoundary being set to an arbitrarily high value (in this case 9000px). From what I understand from the docs, the syntax is similar to that of the CSS margin property. This means that I should be able to specify both a vertical and horizontal value via shorthand (ex: 500px 1000px), correct?

Screen.Recording.2021-12-23.at.2.13.01.PM.mov

Secondly (and way more critically) on Chrome for Android I experience the same lazyBoundary issue but also experience images completely disappearing after scrolling away from them and back to them.

Next.Image.Horizontal.Scroll.Bug.mov

This issue is present when deploying to Vercel as well as developing locally.

Expected Behavior

The lazyBoundary property to be respected in a horizontal container and for images not to disappear on mobile

To Reproduce

https://github.com/agusterodin/next-image-horizontal-scroll-bug-reproduction

Present when running the project via yarn dev AND when deployed to Vercel.

@agusterodin agusterodin added the bug Issue was opened via the bug report template. label Dec 23, 2021
@balazsorban44 balazsorban44 added Image (next/image) Related to Next.js Image Optimization. type: needs investigation labels Dec 26, 2021
@nwongx
Copy link

nwongx commented Dec 29, 2021

I am using nextjs(12.0.3), chrome( 96.0.4664.110 ) and macOS(12.0.1). I have the same issue but in a vertical scroll for both development (yarn dev) and production (yarn build && yarn start) environment. The UI of mine is an Image component on top of a list of component which contains few Image components. The images in each component in the list are the same. (I have tried both static images and images that are loaded from url.)

It happens when I first load in the page and scroll closely to the bottom (While scrolling down to the bottom, a warning "The resource ${image url} was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it has an appropriate as value and it is preloaded intentionally." shows in console.), and back to the top . The Image component on the top and part of images in components nearby the top of the list are disappear. However, when I drag the area where those disappear images are, I can see the image like we drag the image as usual. Also, when I resize the browser window, those disappear images appear again and do not disappear even I scroll to the bottom.

Another investigation, when I remove the image component in the list of component, the image on the top does not disappear when I scroll to the bottom.

@styfle
Copy link
Member

styfle commented Jan 14, 2022

Thanks for reporting this issue! I was able to reproduce lazyBoundary issue here but I haven't reproduce the Android specific bug.

For the lazyBoundary issue, we need to determine the lazyRoot. I think we'll need to recursively walking up the DOM starting from the <img> element up to the first scrollable element and use that as the root for the IntersectionObserver. (related PR)

@agusterodin
Copy link
Author

Do you have an Android device on hand to test?

Just checked again on Android for Chrome version 97.0.4692.87 (latest version as of today) on Google Pixel 5 and issue is still present.

A quick place to test this is at the image gallery at the bottom of this page (right before contact us section)
https://divtechnologies-2bchlqplm-jeffrosen.vercel.app/

the minimal reproduction I provided above also still recreates the issue on Chrome for Android (updated Next to latest version and checked today again).

@agusterodin
Copy link
Author

Not sure if this helps either but it seems like scrolling away from the container entirely and then back seems to make the broken images reappear

screen-20220114-113606.mp4

@styfle styfle linked a pull request Jan 20, 2022 that will close this issue
1 task
@styfle
Copy link
Member

styfle commented Jan 27, 2022

The horizontal scrolling can be achieved by upgrading to the latest version of Next.js (12.0.9) and setting the lazyRoot property to the scroll parent ref.

The Android bug where images disappear is only affecting Chrome (Firefox Mobile works fine) so the bug has been filed here: https://bugs.chromium.org/p/chromium/issues/detail?id=1291415

@agusterodin
Copy link
Author

agusterodin commented Jan 29, 2022

Hey, I really appreciate the addition of lazyRoot.

Unfortunately when I tried to implement it, the images would always just load immediately during page load, even if I am not close to the content and have lazyBoundary set to an arbitrarily low value. I updated my minimal reproduction to demonstrate this issue. Is my implementation incorrect?

https://github.com/agusterodin/next-image-horizontal-scroll-bug-reproduction

https://github.com/agusterodin/next-image-horizontal-scroll-bug-reproduction/blob/030c205db83d83e355ddddeaaeaedbd9ec569aab/components/ImageGallery.tsx

Screen.Recording.2022-01-29.at.5.39.16.PM.mov

@11koukou
Copy link
Contributor

Hello, I can confirm that indeed there is an issue with lazyloading in the latest version. However it seems that it's not relative to the "lazyroot" addition since I can reproduce this bug without defining the lazyroot prop at all, even if I remove completely the code of this PR. It is something that I had noticed as well after #33474 PR however didn't pay much attention since I thought it was my next cache not being cleared.
I can verify that with version 12.0.8 everything works as intended , adding the exact piece of code for lazyroot. Please review #33474 PR again.
Thanks

@11koukou
Copy link
Contributor

@styfle @balazsorban44

I've been trying all day to fix this if possible but with no luck. Observations:

When lazyRoot prop is used all the images indeed load at once no matter what lazyBoundary is set (just like setting loading='eager'). Digging down the code it seems that my latest addition of useEffect in the useIntersection.tsx file causes that.
The above take place in a testpage with 4 Images one after another inside a scrolling container.

However things change when the test page contains many thumbnail images and there even without the lazyRoot property, loading is unpredictable and many times again everything is being loaded at once (eagerly).

I confirm again that everything works fine if the code from this PR is added in any version prior to #33474 commit. Cross-checked with multiple tests in various pages and properties.

@styfle
Copy link
Member

styfle commented Jan 31, 2022

@hjaber
Copy link
Contributor

hjaber commented Feb 10, 2022

@styfle Thanks for being so fast with updates/fixes but still having trouble lazyLoading images just off screen with next v12.0.10.

Code Sandbox
Github
Deployed on Vercel

@11koukou
Copy link
Contributor

@styfle Thanks for being so fast with updates/fixes but still having trouble lazyLoading images just off screen with next v12.0.10.

Code Sandbox Github Deployed on Vercel

Hello hjaber,
Please try installing next with "npm install next@canary" and verify that it's working, or wait for the next version release

@hjaber
Copy link
Contributor

hjaber commented Feb 10, 2022

@styfle Thanks for being so fast with updates/fixes but still having trouble lazyLoading images just off screen with next v12.0.10.
Code Sandbox Github Deployed on Vercel

Hello hjaber, Please try installing next with "npm install next@canary" and verify that it's working, or wait for the next version release

updated to 12.0.11-canary.11 and still not lazy loading appropriately. Thanks for following up!

@balazsorban44
Copy link
Member

Using the reproduction on https://lazy-root.vercel.app/, I cannot see the issue either. 🤔 It seems to be lazyloading correctly. Do note the bug in Chrome Android #32774 (comment)

@hjaber
Copy link
Contributor

hjaber commented Feb 10, 2022

Am I misinterpreting the lazy loading from inspector? I have used desktop Safari, Chrome, & Brave. In the screen cast below on Safari, the images do not load despite the first box having a lazyBoundary="1000px". The images only load when in view? https://lazy-root.vercel.app/ is on "next": "^12.0.11-canary.11"

lazyRootSafari.mov

@11koukou
Copy link
Contributor

Am I misinterpreting the lazy loading from inspector? I have used desktop Safari, Chrome, & Brave. In the screen cast below on Safari, the images do not load despite the first box having a lazyBoundary="1000px". The images only load when in view? https://lazy-root.vercel.app/ is on "next": "^12.0.11-canary.11"
lazyRootSafari.mov

Hello again,
Could you please post the code of your example? Maybe it is that you are not using the lazyRoot property in order to set the correct parent scrolling container rbut in any case code would be useful for any conclusion possible.
Thanks

@hjaber
Copy link
Contributor

hjaber commented Feb 10, 2022

Am I misinterpreting the lazy loading from inspector? I have used desktop Safari, Chrome, & Brave. In the screen cast below on Safari, the images do not load despite the first box having a lazyBoundary="1000px". The images only load when in view? https://lazy-root.vercel.app/ is on "next": "^12.0.11-canary.11"
lazyRootSafari.mov

Hello again, Could you please post the code of your example? Maybe it is that you are not using the lazyRoot property in order to set the correct parent scrolling container rbut in any case code would be useful for any conclusion possible. Thanks

Sure - all should be on next": "^12.0.11-canary.11"

Code Sandbox
Github
Deployed on Vercel

@styfle
Copy link
Member

styfle commented Feb 10, 2022

It looks like it works when the ref is on a <div> but not another component like a <Card>.

PR: https://github.com/hjaber/lazyRoot/pull/1
Demo: https://lazy-root-git-fork-styfle-fix-div-hjaber.vercel.app

@hjaber
Copy link
Contributor

hjaber commented Feb 10, 2022

It looks like it works when the ref is on a <div> but not another component like a <Card>.

PR: hjaber/lazyRoot#1 Demo: https://lazy-root-git-fork-styfle-fix-div-hjaber.vercel.app

Thanks for reviewing and navigating through chakra UI component library, your PR is working to lazy load perfectly.

The <Card> component rendered a <div> so I didn't think to test.

🙏🏽

@11koukou
Copy link
Contributor

Using refs on components causes react to throw warning. Anyway this is a nice example showing why someone should be using Typescript.. ;)

@styfle
Copy link
Member

styfle commented Feb 11, 2022

Alternatively, you can wrap you component with React.forwardRef().

I'll update the docs so this is clear #34241

kodiakhq bot pushed a commit that referenced this issue Feb 11, 2022
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Image (next/image) Related to Next.js Image Optimization.
Projects
None yet
6 participants