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

Use jpeg source files as fallback rather than converting to png #9010

Merged
merged 6 commits into from
Nov 8, 2023

Conversation

jasikpark
Copy link
Contributor

@jasikpark jasikpark commented Nov 6, 2023

Changes

  • Add .jpg and .jpeg as image sources that will be represented as themselves in Picture.astro

If JPEG is the source for an image, it is likely that was the intention of the creator of the image - PNGs are more effective at compressing simple shapes with lossless quality, while JPEGs are more effective at providing high-noise images like photographs in an effective file size. (https://web.dev/articles/choose-the-right-image-format#features_of_different_raster_image_formats)

Testing

This is a change in default behavior of an existing api

Docs

/cc @withastro/maintainers-docs for feedback!

Probably worth updating the docs with the defaults?

If JPEG is the source for an image, it is likely that was the intention of the creator of the image - PNGs are more effective at compressing simple shapes with lossless quality, while JPEGs are more effective at providing high-noise images like photographs in an effective file size. (https://web.dev/articles/choose-the-right-image-format#features_of_different_raster_image_formats)
@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 6, 2023
@jasikpark jasikpark marked this pull request as ready for review November 6, 2023 15:05
@florian-lefebvre
Copy link
Member

Not sure about this. I personally don't mind about what the input is, but I want the most optimized output (webp if possible). Can this behavior be updated through the config instead of making it the default?

@jasikpark
Copy link
Contributor Author

jasikpark commented Nov 6, 2023

Not sure about this. I personally don't mind about what the input is, but I want the most optimized output (webp if possible). Can this behavior be updated through the config instead of making it the default?

This is not a change removing webp, it is a change enforcing jpg as the fallback value in the img, if the original source was a jpg, instead of png.

There will still always be a webp generated 👍

Copy link

changeset-bot bot commented Nov 6, 2023

🦋 Changeset detected

Latest commit: fbcc04d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pr: docs A PR that includes documentation for review label Nov 6, 2023
@florian-lefebvre
Copy link
Member

Oh ok! I misunderstood, thanks for clarifying!

Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
Copy link
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

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

Looks great to me! Will merge on merge day tomorrow.

@Princesseuh Princesseuh removed the pr: docs A PR that includes documentation for review label Nov 8, 2023
@Princesseuh Princesseuh merged commit 100b61a into main Nov 8, 2023
4 checks passed
@Princesseuh Princesseuh deleted the add-jpeg-as-a-picture-fallback branch November 8, 2023 09:09
@astrobot-houston astrobot-houston mentioned this pull request Nov 8, 2023
natemoo-re pushed a commit that referenced this pull request Nov 22, 2023
Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants