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

Re-introduce lazy loading for pictures #2367

Closed
tlylt opened this issue Sep 10, 2023 · 4 comments · Fixed by #2450
Closed

Re-introduce lazy loading for pictures #2367

tlylt opened this issue Sep 10, 2023 · 4 comments · Fixed by #2450

Comments

@tlylt
Copy link
Contributor

tlylt commented Sep 10, 2023

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

#2365, #1626, #2364

What is the area that this feature belongs to?

Author Usability, Reader Usability

Is your feature request related to a problem? Please describe.

See related issues for context.

TLDR: scrolling to anchor does not work as expected when there are lazily loaded images, hence we will remove existing lazy loading support.

Describe the solution you'd like

Add an option to specify eager/lazy loading of images,
https://developer.mozilla.org/en-US/docs/Web/Performance/Lazy_loading

Need to ensure scrolling still works.

Describe alternatives you've considered

No response

Additional context

No response

@yucheng11122017
Copy link
Contributor

Hi I've done some research on this

  1. One way to fix this issue is to call another scrollIntoView when the images have been loaded. But this would need to be done for every single lazy loaded image which is not ideal - https://stackoverflow.com/questions/68467475/lazy-loading-conflicting-with-scrollto-anchor-to-id-scroll-stops-halfway-throu
  2. Another way is to call two animations (basically the same as the first but only 2 animations). When the first is ending, hopefully the images would have loaded and hence the second animation would result in correct positioning https://stackoverflow.com/questions/63197942/scrollintoview-not-working-properly-with-lazy-image-load
  3. Both method 1 and method 2 are extremely hacky. I think generally the better way is the user to input the height and width so that scrollIntoView can compute properly and hence doesn't get pushed down. https://www.reddit.com/r/webdev/comments/13kcpko/how_to_fix_jscss_scrolling_down_in_the_page_with/

@LamJiuFong LamJiuFong self-assigned this Mar 4, 2024
@LamJiuFong
Copy link
Contributor

Hi I've done some research too, the layout shift happens because the browser does not know how much space should be reserved for the image, leading to layout shifts when the image is fully loaded.

It seems like the solution that people generally use is specifying the height and width of the image, same as @yucheng11122017's third point. It allows the browser to reserve the correct space for the image before it is loaded.

If we add the option of lazy/eager loading, maybe we can ask the users to input the height and width manually if they want lazy loading (and warn them about the consequences if they don't do so)? But I am not sure if it will be troublesome for them since not all users knows the exact height and width that they want

What do y'all think?

@kaixin-hc
Copy link
Contributor

kaixin-hc commented Mar 5, 2024

I agree with your proposal @LamJiuFong - thanks to you and @yucheng11122017 for investigating!

I think instead of having to specify eager loading, we can have the users manually specify lazy loading if they want it. Then where we list in the documentation that lazy loading is supported, we can include that they should specify height and width manually to ensure scrolling to header works. Perhaps we can have an informational log pop up (similar to broken internal links) if this value is not specified.

(I think even when exact desired height/width is not known - like resizing the image on different sized screens - specifying desired height and width will be enough for the browser to reserve the correct space)

Do take this up if you are interested!

@LamJiuFong
Copy link
Contributor

Thank you @kaixin-hc for your suggestions! I will try to tackle this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants