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

NextJS: Allow disabling next/image lazy loading #21909

Merged

Conversation

martinnabhan
Copy link
Contributor

What I did

next/image lazy loads images by default. When using Storybook (and especially Chromatic) we usually want images below the fold to load as well so they show up in snapshots. This pull request sets next/image's loading property to eager which disables lazy loading.

How to test

  1. Create a story using next/image and place an image below the fold
  2. Open Storybook in your browser
  3. Access the story and make sure the image gets loaded even if it's below the fold (or use Chromatic to snapshot the story and make sure images below the fold are loaded and visible)

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@valentinpalkovic
Copy link
Contributor

@martinnabhan Thank you so much for your contribution.

Your PR would introduce a breaking change where people might rely on lazy loading behavior. Instead, this option should be configurable. Let's add it to the parameters.nextjs object instead, where people like you could define the default loading behaviour.

@martinnabhan
Copy link
Contributor Author

I thought that might be the way to go! I'll see if I can turn it into a parameter some time this week.

@martinnabhan martinnabhan marked this pull request as draft April 5, 2023 06:35
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch 2 times, most recently from 6ab3a72 to e06f67e Compare April 13, 2023 05:51
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch from e06f67e to fc49e62 Compare April 13, 2023 06:37
@martinnabhan martinnabhan marked this pull request as ready for review April 13, 2023 06:55
@valentinpalkovic valentinpalkovic added the ci:daily Run the CI jobs that normally run in the daily job. label Apr 17, 2023
@martinnabhan martinnabhan force-pushed the next-image-disable-lazy-loading branch from fc49e62 to 0d64367 Compare April 17, 2023 07:07
@martinnabhan martinnabhan changed the title Disable next/image lazy loading Allow disabling next/image lazy loading Apr 17, 2023
@martinnabhan
Copy link
Contributor Author

@valentinpalkovic Thank you for approving! What's the status on this? build-sandboxes seems to be failing, but it doesn't seem to be related to this pull request.

@valentinpalkovic valentinpalkovic merged commit a333319 into storybookjs:next Apr 25, 2023
@shilman shilman added maintenance User-facing maintenance tasks and removed feature request labels Jul 4, 2023
@shilman shilman changed the title Allow disabling next/image lazy loading NextJS: Allow disabling next/image lazy loading Jul 4, 2023
@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jul 4, 2023
@shilman shilman mentioned this pull request Jul 4, 2023
18 tasks
shilman pushed a commit that referenced this pull request Jul 4, 2023
…oading

Allow disabling next/image lazy loading
@github-actions github-actions bot mentioned this pull request Jul 5, 2023
27 tasks
@JReinhold JReinhold added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:daily Run the CI jobs that normally run in the daily job. maintenance User-facing maintenance tasks nextjs patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants