close
The Wayback Machine - https://web.archive.org/web/20221028133227/https://github.com/WordPress/performance/pull/537
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

Jump to bottom

Add checkbox to Settings > Media to control whether to generate JPEG in addition to WebP #537

Merged
merged 34 commits into from Oct 7, 2022
Merged

Add checkbox to Settings > Media to control whether to generate JPEG in addition to WebP #537

merged 34 commits into from Oct 7, 2022

Conversation

mehulkaklotar
Copy link
Member

@mehulkaklotar mehulkaklotar commented Sep 19, 2022 •
edited

Summary

This PR is dependent on the issue #524 and PR #527

PR introduces

  • WordPress setting generate_webp_and_jpeg -> boolean value
  • Changes default filter output for webp_uploads_upload_image_mime_transforms to image/jpeg and image/webp if the generate_webp_and_jpeg is true

Remaining work(After #524 is merged)

Fixes #525

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@mehulkaklotar mehulkaklotar added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Images Issues related to the Images focus area [Module] WebP Uploads Issues for the WebP Uploads module labels Sep 19, 2022
@mehulkaklotar mehulkaklotar added this to the 1.6.0 milestone Sep 19, 2022
mehulkaklotar
mehulkaklotar commented Oct 5, 2022
View changes
Copy link
Member Author

@mehulkaklotar mehulkaklotar left a comment

@mukeshpanchal27 Changes looks good here. We should add some tests for the new setting.

modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
@mehulkaklotar mehulkaklotar marked this pull request as ready for review Oct 5, 2022
felixarntz
felixarntz requested changes Oct 5, 2022
View changes
Copy link
Member

@felixarntz felixarntz left a comment

@mehulkaklotar This is on a good path, some quirks to address though.

modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/image-edit.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/image-edit.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
tests/modules/images/webp-uploads/load-tests.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/helper.php Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
@felixarntz felixarntz changed the title Enhancement/525 checkbox multi mime setting Add checkbox to Settings > Media to control whether to generate JPEG in addition to WebP Oct 5, 2022
felixarntz
felixarntz reviewed Oct 5, 2022
View changes
Copy link
Member

@felixarntz felixarntz left a comment

@mehulkaklotar One more thing: Please make sure to delete the new option in uninstall.php.

@mehulkaklotar mehulkaklotar requested a review from felixarntz Oct 5, 2022
felixarntz
felixarntz reviewed Oct 6, 2022
View changes
Copy link
Member

@felixarntz felixarntz left a comment

@mehulkaklotar This is almost good to go now. Left mostly minor suggestions, so there are two quirks (one little tricky) to still address.

modules/images/webp-uploads/helper.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
mukeshpanchal27 and others added 8 commits Oct 6, 2022
@mukeshpanchal27 @felixarntz
Update document
23b8102 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27 @felixarntz
Update document
1cd10bc 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27 @felixarntz
Update modules/images/webp-uploads/settings.php
f892ddb 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27 @felixarntz
Update document
f7c3a2a 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27 @felixarntz
Update document
02aaa7d 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27 @felixarntz
Add $args in register_setting
0c3a321 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27
Fix PHPCS and remove doc
c7b7575
@mukeshpanchal27
Update structure and add css
ab65615
@mukeshpanchal27 mukeshpanchal27 requested a review from felixarntz Oct 6, 2022
felixarntz
felixarntz reviewed Oct 6, 2022
View changes
Copy link
Member

@felixarntz felixarntz left a comment

@mukeshpanchal27 @mehulkaklotar One small optimization recommendation here, but other than that this is good to go.

modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/settings.php Outdated Show resolved Hide resolved
mukeshpanchal27 and others added 2 commits Oct 7, 2022
@mukeshpanchal27 @felixarntz
Update add_action
25858c9 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@mukeshpanchal27
Remove global and condition
90cac1f
@mukeshpanchal27 mukeshpanchal27 requested a review from felixarntz Oct 7, 2022
felixarntz
felixarntz approved these changes Oct 7, 2022
View changes
Copy link
Member

@felixarntz felixarntz left a comment

Thanks @mukeshpanchal27, LGTM!

@felixarntz felixarntz merged commit 55213ba into trunk Oct 7, 2022
5 checks passed
@felixarntz felixarntz deleted the enhancement/525-checkbox-multi-mime-setting branch Oct 7, 2022
Reviewers

@felixarntz felixarntz

@mukeshpanchal27 mukeshpanchal27

@adamsilverstein adamsilverstein

@mitogh mitogh

@jjgrainger jjgrainger

Assignees
No one assigned
Labels
[Focus] Images Issues related to the Images focus area [Module] WebP Uploads Issues for the WebP Uploads module [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.

Add checkbox to Media screen to enable multi MIME type output (WebP and JPEG)
3 participants
@mehulkaklotar @felixarntz @mukeshpanchal27