Skip to content
This repository was archived by the owner on Mar 15, 2024. It is now read-only.

Compress images u2fkey/yubikey using avif/webp - #1232

Merged
Hinton merged 3 commits into
bitwarden:masterfrom
simon04:avif-webp
Jan 10, 2022
Merged

Compress images u2fkey/yubikey using avif/webp#1232
Hinton merged 3 commits into
bitwarden:masterfrom
simon04:avif-webp

Conversation

@simon04

@simon04 simon04 commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

@CLAassistant

CLAassistant commented Oct 12, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Hinton

Hinton commented Oct 20, 2021

Copy link
Copy Markdown
Member

Hi @simon04,

Thanks for the PR. I noticed there's a fair bit of quality loss especially for the avif images.

I think it's worth aiming for a lossless or higher quality of compression for the images even if they end up being slightly larger. We also have a few other images which could benefit from compression.

I also noticed the original images were removed but still keept in the picture. While webp has fairly good support https://caniuse.com/?search=webp, we should still retain the jpg as a last fallback.

@simon04

simon04 commented Dec 5, 2021

Copy link
Copy Markdown
Contributor Author

Sorry for the long silence. Today, I've restored the original JPEG files and improved the quality of the AVIF files.

# Conflicts:
#	src/app/accounts/two-factor.component.html
#	src/connectors/webauthn.html
#	webpack.config.js
@Hinton

Hinton commented Jan 10, 2022

Copy link
Copy Markdown
Member

@simon04 I've rebased this with the prettier changes. added alternatives for u2fkey-mobile and tweaked the avif and webp quality slightly. I believe this is ready to be merged now. Since I've pushed commits to this PR I will get a second reviewer to verify everything looks good.

I also took the time and compressed the original jpegs in #1374.

Thanks for the contribution!

@Hinton
Hinton requested a review from djsmith85 January 10, 2022 11:01

@djsmith85 djsmith85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking good

@Hinton
Hinton merged commit 45284ee into bitwarden:master Jan 10, 2022
@Hinton Hinton mentioned this pull request Jan 10, 2022
8 tasks
Hinton added a commit that referenced this pull request Jan 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants