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: image transitions #56

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

BlackDahlia313
Copy link

@BlackDahlia313 BlackDahlia313 commented Nov 27, 2024

1.mp4

πŸ”— Linked issue

#51

❓ Type of change

  • ✨ New feature (a non-breaking change that adds functionality)

πŸ“š Description

This adds optional transitions between BlurHash or ThumbHash via a new prop. It uses the Canvas API for compatibility and performance. This resolves the linked issue.

Copy link

netlify bot commented Nov 27, 2024

βœ… Deploy Preview for unlazy ready!

Name Link
πŸ”¨ Latest commit b802f67
πŸ” Latest deploy log https://app.netlify.com/sites/unlazy/deploys/674896dac89c7600083f4659
😎 Deploy Preview https://deploy-preview-56--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.

@johannschopplich
Copy link
Owner

Interesting! Thanks for the PR.

Copy link
Owner

@johannschopplich johannschopplich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if it was AI or you, but you removed a lot of features and comments. Please revert these changes and only implement the new transition feature without adding breaking changes.

Also: please add the documentation for the new feature.

packages/core/src/lazyLoad.ts Show resolved Hide resolved
packages/core/src/lazyLoad.ts Show resolved Hide resolved
packages/core/src/lazyLoad.ts Outdated Show resolved Hide resolved
packages/core/src/lazyLoad.ts Show resolved Hide resolved
packages/core/src/lazyLoad.ts Show resolved Hide resolved
packages/core/src/lazyLoad.ts Outdated Show resolved Hide resolved
@johannschopplich johannschopplich changed the title Add Transitions! feat: image transitions Nov 28, 2024
@BlackDahlia313
Copy link
Author

Hey @johannschopplich so sorry about that! I was doing a lot of testing from before I moved from css to canvas and I had forgot to revert things back when doing that (making changes in all packages trying to get css to work before reverting to canvas)

I also will bring back the comments. I did have a bot check my code for any vulnerabilities and I didn't even notice it removed all the dang comments when I did that.

I'll get a new commit out soon.

@BlackDahlia313
Copy link
Author

Also: please add the documentation for the new feature.

I had already updated the readme.md and the examples and totally didn't realize there was a docs directory. Getting this part done now and I think we will be all set! :)

@johannschopplich
Copy link
Owner

Thanks a lot for your feedback!

@BlackDahlia313
Copy link
Author

Thanks a lot for your feedback!

No Thank you! this is one of my first bigger contributions and it means a lot to get proper feedback on what I'm doing.

@BlackDahlia313
Copy link
Author

@johannschopplich LGTM unless you notice anything else!

@johannschopplich
Copy link
Owner

Thanks for the PR! Unfortunately I haven't had time to review it yet. πŸ₯²

@BlackDahlia313
Copy link
Author

Thanks for the PR! Unfortunately I haven't had time to review it yet. πŸ₯²

Hey no rush at all! My project isn't launching till closer to end of Q1. Take your time :)

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.

3 participants