FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add WebP support in site health by kirtangajjar · Pull Request #141 · WordPress/performance · GitHub

Add WebP support in site health - #141

Merged
adamsilverstein merged 11 commits into
WordPress:trunkfrom
kirtangajjar:feature/130-webp-support-site-health
Feb 24, 2022
Merged

Add WebP support in site health#141
adamsilverstein merged 11 commits into
WordPress:trunkfrom
kirtangajjar:feature/130-webp-support-site-health

Conversation

kirtangajjar commented Feb 1, 2022
edited
Loading

Copy link
Copy Markdown
Member

Summary

Fixes #130.

Relevant technical choices

Adds a module in site health to check for WebP support and shows a warning if support is not present.
If either GD or Imagick has WebP support, the WebP support warning is not shown.

The code of checking support in GD and Imagick is same as how it's shown in site health info's media handling section.

Screenshots

kirtangajjar marked this pull request as draft February 1, 2022 17:00
kirtangajjar changed the title WIP Add WebP support in site health [WIP] Add WebP support in site health Feb 1, 2022

pbearne commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

it's nice to see the output But most users won't know what to look for
I feel we need to add a "score" as to how good the support is.
ie a nice green 100% support if all is good and messages, if support expected/desired, is missing

kirtangajjar commented Feb 3, 2022
edited
Loading

Copy link
Copy Markdown
Member Author

@pbearne Can you look at the screenshot again? Earlier I added a screenshot in the last line to indicate where I copied the code of checking WebP support from, but I saw how it can be mistaken as the output of the code, so I have uploaded the actual screenshot of output.

Do you still feel we need to show percentage-wise support? I think now since the user has a clear picture to see that they have an action to take here, I feel we don't need to show partial support indication.

kirtangajjar changed the title [WIP] Add WebP support in site health Add WebP support in site health Feb 4, 2022
kirtangajjar marked this pull request as ready for review February 4, 2022 12:38

mitogh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks good to me, let's wait on other people to chime in case they have any additional feedback.

Thank you for putting this together 🥇 🙇

mitogh added [Focus] Site Health [Type] Feature A new feature within an existing module labels Feb 7, 2022

Copy link
Copy Markdown

I wouldn't call it critical. Kick it down to recommendations.

Copy link
Copy Markdown
Member

Nice work @kirtangajjar - code looks good, I'll give this a test.

Copy link
Copy Markdown
Member

This worked well in my testing!

After testing though, I feel like the notice belongs under "recommended improvements" instead of "critical issues". Having WebP support is nice to have, it isn't really critical.

kirtangajjar and others added 2 commits February 14, 2022 14:21
Co-authored-by: Adam Silverstein <adamsilverstein@earthboundhosting.com>

Copy link
Copy Markdown
Member Author

@adamsilverstein Done. LMK if you find any additional changes.

adamsilverstein added this to the 1.0.0-beta.1 milestone Feb 22, 2022
Co-authored-by: Adam Silverstein <adamsilverstein@earthboundhosting.com>

Copy link
Copy Markdown
Member

@kirtangajjar I left one additional tiny suggestion on the text, then we can merge this. Thanks for your work here!

Copy link
Copy Markdown
Member Author

@adamsilverstein I've committed your suggestion. Thanks for giving it a second look!

kirtangajjar requested a review from mitogh February 23, 2022 13:27

Copy link
Copy Markdown
Member

👍🏼 Looks good, nice work @kirtangajjar - going to go ahead and merge this. We'll get another chance to review the exact wording when we propose this feature for core.

adamsilverstein merged commit bd87192 into WordPress:trunk Feb 24, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Feature A new feature within an existing module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose WebP support under Site Health status tab

6 participants


Back | FazBrowse Home | New Git URL