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

feat: update sizes attribute on image resize with new value #15

Merged
merged 8 commits into from
Jul 19, 2023

Conversation

felixranesberger
Copy link
Contributor

πŸ”— Linked issue

fixes: #14

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSDoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

When resizing the browser window, some image sizes may change.

Currently the sizes attribute of the image is not updated, when a resize occurs.
This pull request adds this feature to the library.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Jul 7, 2023

βœ… Deploy Preview for unlazy ready!

Name Link
πŸ”¨ Latest commit f714a41
πŸ” Latest deploy log https://app.netlify.com/sites/unlazy/deploys/64b791df8069c5000888e6f2
😎 Deploy Preview https://deploy-preview-15--unlazy.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

packages/core/src/lazyLoad.ts Outdated Show resolved Hide resolved
packages/core/src/lazyLoad.ts Outdated Show resolved Hide resolved
packages/core/src/lazyLoad.ts Outdated Show resolved Hide resolved
packages/core/src/utils/index.ts Outdated Show resolved Hide resolved
packages/core/src/utils/index.ts Outdated Show resolved Hide resolved
@johannschopplich
Copy link
Owner

Hey there, thanks for providing a PR! Please have a look at my comments. πŸ™‚

@johannschopplich
Copy link
Owner

Thanks for the updates. I'll a look at it later.

@johannschopplich
Copy link
Owner

johannschopplich commented Jul 19, 2023

Hey there! Thanks for the MR. I've introduced a global updateSizesOnResize option. It's false by default and required opt-in. This is just to keep breaking changes at minimum.
Does this still suit your use-case? πŸ™‚

@johannschopplich johannschopplich merged commit 264e5ef into johannschopplich:main Jul 19, 2023
7 checks passed
@felixranesberger
Copy link
Contributor Author

Does this still suit your use-case? πŸ™‚

Yeah, this works just fine ✌️

Have you seen the pagespeed / debounce review comment I added a few days ago?

@johannschopplich
Copy link
Owner

No, haven't seen that comment. Would you mind posting it again?

@felixranesberger
Copy link
Contributor Author

Screenshot 2023-07-19 at 09 51 55

@johannschopplich
Copy link
Owner

johannschopplich commented Jul 19, 2023

Ah, I see. You haven't posted our comment. It's till in pending state, I presume. :)

Edit: I think debounce is OK for now. For Lighthouse scores, the browser won't resize the window. At least that's the latest info I've got. We should be fine.

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

Successfully merging this pull request may close these issues.

Img sizes attribute not updating on image resize
2 participants