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(ideal-image): support webp & avif formats #6051

Closed
wants to merge 5 commits into from
Closed

Conversation

Josh-Cena
Copy link
Collaborator

@Josh-Cena Josh-Cena commented Dec 4, 2021

Motivation

See #6043 (comment). Luckily sharp supports both WebP and AVIF, so it's a good opportunity to enable them.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Converted one image to WebP for dogfooding.

Blocking PRs

Currently build will fail because blocked by slorber/responsive-loader#2

@Josh-Cena Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Dec 4, 2021
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 4, 2021
@Josh-Cena Josh-Cena added status: blocked This issue is blocked by another issue or external dep and can't be pushed further. and removed status: blocked This issue is blocked by another issue or external dep and can't be pushed further. labels Dec 4, 2021
@Josh-Cena Josh-Cena marked this pull request as draft December 7, 2021 15:05
@Josh-Cena Josh-Cena marked this pull request as ready for review December 22, 2021 02:40
@Josh-Cena
Copy link
Collaborator Author

Interesting, the ideal image loader doesn't seem to upsize images from 640 to 1030... The previous build error was because our ideal image max width is 1030, but I resized the image to 640. I would need to locally test with #6043 to see if that's only a quirk with WebP

@Josh-Cena
Copy link
Collaborator Author

react-ideal-image disables using WebP in SSR (probably because at that time browsers didn't implement this). However nowadays most browsers do: https://caniuse.com/?search=webp.

I think we should also allow passing multiple source sets to the ideal image component because the component is able to pick different sources.

@Josh-Cena Josh-Cena added the status: blocked This issue is blocked by another issue or external dep and can't be pushed further. label Dec 23, 2021
@RDIL
Copy link
Contributor

RDIL commented Jan 6, 2022

It wasn't even partially supported in Safari until 2020, but I think its widely supported enough to include.

@matkoch
Copy link
Contributor

matkoch commented Jun 19, 2022

Could this be merged now since slorber/responsive-loader#2 was merged?

@matkoch
Copy link
Contributor

matkoch commented Jun 19, 2022

Btw – since it wasn't mentioned here yet – AVIF doesn't seem to be supported on iOS and in macOS Safari.

@Josh-Cena
Copy link
Collaborator Author

@matkoch The library we use under the hood doesn't support webp yet. See comments above.

@korhox
Copy link

korhox commented Aug 1, 2023

Hello!

It's been a bit over a year since last update on this. Is this PR still going to happen on v2 or is it waiting for v3?

Best regards

@slorber
Copy link
Collaborator

slorber commented Aug 2, 2023

Is this PR still going to happen on v2 or is it waiting for v3?

This feature is not a top-priority for me. It won't be part of v3 initial release but can eventually come in a minor release once we have a working implementation, which we currently have not.

It's been a bit over a year since last update on this.

What's your point here? 🤔

If you need this feature fast, the best way is to submit a PR to add support that works. So far all attempts have failed.


Closing in favor of #8686, more advanced, less stale PR

@slorber slorber closed this Aug 2, 2023
@Josh-Cena Josh-Cena deleted the jc/lqip-webp branch August 2, 2023 17:39
@korhox
Copy link

korhox commented Aug 3, 2023

Thank you for the update! I'll make an PR if I come up with something. How ever I don't have much experience yet, so I would not hold my breath.

What's your point here? 🤔

Sorry if my message came out wrong (I'm not native english speaker). I was just pointing out that this PR has been inactive and I'm not sure what the current status of it is, so I thought that I should should ask.

As a side note, thanks for the great software. I definetely will contribute somehow if I find out something I can do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior. status: blocked This issue is blocked by another issue or external dep and can't be pushed further.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants