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

Ideal image flashing when navigating back and forward #10214

Open
7 tasks done
Legend-Master opened this issue Jun 12, 2024 · 7 comments
Open
7 tasks done

Ideal image flashing when navigating back and forward #10214

Legend-Master opened this issue Jun 12, 2024 · 7 comments
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@Legend-Master
Copy link

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

Image flashing when navigating back and forward using Ideal Image

A possible fix would be to use browser native image lazy loading <img loading="lazy"> instead of react-ideal-image (also posted in https://docusaurus.io/feature-requests/p/use-browser-lazy-loading-in-plugin-ideal-image)

Swizzling code I'm currently using to avoid this problem
export default function IdealImage(props: Props): JSX.Element {
    const { img, ...propsRest } = props

    // In dev env just use regular img with original file
    if (typeof img === 'string' || 'default' in img) {
  	  return (
  		  // eslint-disable-next-line jsx-a11y/alt-text
  		  <img src={typeof img === 'string' ? img : img.default} {...propsRest} />
  	  )
    }

    return (
  	  <img
  		  height={img.src.height ?? 100}
  		  width={img.src.width ?? 100}
  		  src={img.src.src}
  		  style={{
  			  backgroundImage: `url(${img.preSrc})`,
  			  backgroundRepeat: 'no-repeat',
  			  backgroundSize: 'cover',
  			  width: '100%',
  			  height: 'auto',
  		  }}
  		  loading="lazy"
  		  {...propsRest}
  	  />
    )
}

Reproducible demo

No response

Steps to reproduce

  1. Go to https://docusaurus.io/docs/playground
  2. Navigate to another page
  3. Navigate back

Expected behavior

No flashing

Actual behavior

Flashing

ideal-image-flashing.mp4

image

Your environment

  • Public source code:
  • Public site URL: https://docusaurus.io/docs/playground
  • Docusaurus version used:
  • Environment name and version (e.g. Chrome 89, Node.js 16.4):
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS):

Self-service

  • I'd be willing to fix this bug myself.
@Legend-Master Legend-Master added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jun 12, 2024
@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Jun 21, 2024
@Josh-Cena
Copy link
Collaborator

I'm not sure we want to do anything by default. You should probably not optimize images that are in the initial viewport—these kinds of optimizations are for images that are not initially visible. Ideal image also does much more than lazy loading—there's low-quality placeholder, etc.

@Legend-Master
Copy link
Author

Legend-Master commented Jul 19, 2024

I'm not sure we want to do anything by default. You should probably not optimize images that are in the initial viewport

This is not necessarily about images inside the initial viewport, you can scroll down and see a image and navigate to another page and then go back, and the images flashes

Ideal image also does much more than lazy loading—there's low-quality placeholder, etc.

Low quality placeholder can be done with just a background image, load the image with different sizes based on the screen size can be done using srcset and other things like refuse to load the already compressed image (normally < 100KB) on low bandwidth (which often fails to detect) is to be honest not appreciated

@Josh-Cena
Copy link
Collaborator

I'm with you there. I would personally suggest not using ideal image, but not sure if we would go as far as deprecating the plugin.

@slorber
Copy link
Collaborator

slorber commented Jul 19, 2024

This plugin is quite old and difficult to maintain and reason about, with external dependencies being unmaintained forks.

You can still use it in its current state, but we'll probably have to create a new image plugin from scratch sooner or later, it will be easier than upgrading the current one to modern std.

@Legend-Master
Copy link
Author

Thanks, I'm thinking about if we can give it an option to use the browser native lazy load and srcset, we still want the plugin to do the image generation for multiple sizes though, this way we don't rely on that unmaintained library and the upgrade shouldn't be too hard?

@slorber
Copy link
Collaborator

slorber commented Jul 19, 2024

If you want to submit a PR you can. I'm not sure to understand what you have in mind and don't really plan to work on this in the immediate future.

@Legend-Master
Copy link
Author

I mean we keep the other parts, just add an option to replace

return (
<ReactIdealImage
{...propsRest}
height={img.src.height ?? 100}
width={img.src.width ?? 100}
placeholder={{lqip: img.preSrc}}
src={img.src.src}
srcSet={img.src.images.map((image) => ({
...image,
src: image.path,
}))}
getMessage={getMessage}
/>
);
to something like

<img
  height={img.src.height ?? 100}
  width={img.src.width ?? 100}
  src={img.src.src}
  srcSet={...}
  style={{
    backgroundImage: `url(${img.preSrc})`,
    backgroundRepeat: 'no-repeat',
    backgroundSize: 'cover',
    width: '100%',
    height: 'auto',
  }}
  loading="lazy"
  {...propsRest}
/>

If you're open to the idea I can take a look and try to make a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants