Enable end user opt-in to generate all sizes in fallback format #1689

b1ink0 · 2024-11-21T12:52:17Z

Summary

Relevant technical choices

This PR adds a checkbox users can check right below the "generate fallback images" checkbox (and it would only be enabled when "generate fallback images" checkbox is checked).

When users enable this feature it is notate in the Generator tag so we can later discover how many sites opt into generating all image sizes.

Settings Page Screenshots

Sample outputs for the below custom sizes

add_image_size( 'custom_size_1', 400, 400, true );
add_image_size( 'custom_size_2', 800, 800, true );
add_image_size( 'custom_size_3', 1200, 1200, true );

When the new option is off
When the new option is on

…lity to use added new settings

…izes option

…sizes

github-actions · 2024-11-21T13:01:53Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: b1ink0 <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: adamsilverstein <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

felixarntz

@b1ink0 A bit of feedback here, but overall this looks great!

felixarntz · 2024-11-21T18:15:34Z

plugins/webp-uploads/helper.php

+ *
+ * @return bool True if the option is enabled, false otherwise.
+ */
+function webp_uploads_is_fallback_all_sizes_enabled(): bool {


Maybe a better name would be something like webp_uploads_should_generate_all_fallback_sizes()?

felixarntz · 2024-11-21T18:20:33Z

plugins/webp-uploads/hooks.php

-		$allowed_sizes[ $size ] = ! empty( $size_details['provide_additional_mime_types'] );
+		if ( isset( $size_details['provide_additional_mime_types'] ) ) {
+			// Give priority to the 'provide_additional_mime_types' property.
+			$allowed_sizes[ $size ] = (bool) $size_details['provide_additional_mime_types'];
+		} else {
+			// If 'provide_additional_mime_types' is not set, use the fallback all sizes setting.
+			$allowed_sizes[ $size ] = webp_uploads_is_fallback_all_sizes_enabled();
+		}


Rather than doing this here, I think a more elegant solution would be to decouple the UI control and option from the main logic, by using the webp_uploads_image_sizes_with_additional_mime_type_support filter below. So I think this would be better separately implemented.

It would also allow the code to be a bit more efficient: We should only need to call webp_uploads_is_fallback_all_sizes_enabled() once, not for every size available. If false we can immediately return without changing anything. If true, we go through the array of sizes and set them to true.

I think it's okay not to check the provide_additional_mime_types value directly to see if it's false. It's primary purpose it to enable (not disable) this behavior anyway, and since the setting explicitly says "all", it's fine to do it for all sizes, regardless of what other configuration may be present.

felixarntz · 2024-11-21T18:21:27Z

plugins/webp-uploads/hooks.php

+	if ( webp_uploads_is_fallback_all_sizes_enabled() ) {
+		echo '<meta name="generator" content="webp-uploads-fallback-all-sizes ' . esc_attr( WEBP_UPLOADS_VERSION ) . '">' . "\n";
+	}


Why is that being added? We shouldn't have an extra generator tag for this feature specifically. If we need feature-specific indicators in the frontend, let's discuss that in a separate issue.

In the #1513 there is mention of the adding the generator tag thats why I added it.

Ideally when users enable this feature we should notate that in the Generator tag so we can later discover how many sites opt into generating all image sizes.

Should I remove the generator tag code from this PR?

Yeah, I think so, let's not do it in this PR, as it needs further discussion.

Maybe worth opening an issue to discuss whether/how we want to make more specific features discoverable in the frontend? Such an issue could link to that comment as one example reference.

Removed the generator tag code from this PR.

felixarntz · 2024-11-21T18:21:51Z

plugins/webp-uploads/settings.php

+	// Add a setting to generate fallback images in all sizes including custom sizes.
+	register_setting(
+		'media',
+		'perflab_generate_fallback_all_sizes',


Maybe rename this to perflab_generate_all_fallback_sizes, that reads a bit better.

felixarntz · 2024-11-21T18:22:06Z

plugins/webp-uploads/settings.php

+ *
+ * @since n.e.x.t
+ */
+function webp_uploads_generate_fallback_all_sizes_callback(): void {


See above in terms of naming.

felixarntz · 2024-11-21T18:23:24Z

plugins/webp-uploads/settings.php

+	// Add setting field for generating fallback images in all sizes including custom sizes.
+	add_settings_field(
+		'perflab_generate_fallback_all_sizes',
+		__( 'Output all fallback images', 'webp-uploads' ),


Suggested change

__( 'Output all fallback images', 'webp-uploads' ),

__( 'Generate all fallback image sizes', 'webp-uploads' ),

…ub.com/b1ink0/performance into add/option-to-generate-all-image-sizes

felixarntz

Thanks @b1ink0, LGTM!

Paging @adamsilverstein for additional review.

b1ink0 and others added 4 commits November 14, 2024 17:03

Add settings for generate fallback images in all sizes. Add functiona…

95e7878

…lity to use added new settings

Add generator tag for tracking usage of generate all fallback image s…

8473bb8

…izes option

Merge branch 'WordPress:trunk' into add/option-to-generate-all-image-…

536c799

…sizes

Remove use of empty()

fa34edf

b1ink0 marked this pull request as ready for review November 21, 2024 13:01

b1ink0 requested review from adamsilverstein, felixarntz and westonruter as code owners November 21, 2024 13:01

felixarntz requested changes Nov 21, 2024

View reviewed changes

felixarntz added this to the webp-uploads n.e.x.t milestone Nov 21, 2024

felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) labels Nov 21, 2024

b1ink0 and others added 4 commits November 22, 2024 14:47

Change the function, option, variable names

e8460fe

Merge branch 'trunk' into add/option-to-generate-all-image-sizes

39e6004

Merge branch 'add/option-to-generate-all-image-sizes' of https://gith…

e297363

…ub.com/b1ink0/performance into add/option-to-generate-all-image-sizes

Move logic to separate function

6ddd884

b1ink0 requested a review from felixarntz November 22, 2024 12:22

b1ink0 and others added 2 commits November 25, 2024 19:31

Remove generator tag

b80a9b0

Merge branch 'trunk' into add/option-to-generate-all-image-sizes

d477128

felixarntz approved these changes Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable end user opt-in to generate all sizes in fallback format #1689

Enable end user opt-in to generate all sizes in fallback format #1689

b1ink0 commented Nov 21, 2024

github-actions bot commented Nov 21, 2024

felixarntz left a comment

felixarntz Nov 21, 2024

felixarntz Nov 21, 2024

felixarntz Nov 21, 2024

b1ink0 Nov 22, 2024

felixarntz Nov 23, 2024 •

edited

Loading

b1ink0 Nov 26, 2024

felixarntz Nov 21, 2024

felixarntz Nov 21, 2024

felixarntz Nov 21, 2024

felixarntz left a comment

	__( 'Output all fallback images', 'webp-uploads' ),
	__( 'Generate all fallback image sizes', 'webp-uploads' ),

Enable end user opt-in to generate all sizes in fallback format #1689

Are you sure you want to change the base?

Enable end user opt-in to generate all sizes in fallback format #1689

Conversation

b1ink0 commented Nov 21, 2024

Summary

Relevant technical choices

Settings Page Screenshots

Sample outputs for the below custom sizes

github-actions bot commented Nov 21, 2024

felixarntz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz Nov 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

felixarntz Nov 23, 2024 •

edited

Loading