Use correct sizes for small images #1252

mukeshpanchal27 · 2024-05-28T11:30:31Z

Summary

For initial work:

Only return a smaller image size if the content or wide size is larger than the image size.
- If content size is 620px and content use thumbnail the the size will use thumbnail 150px image size instead of 620px.
Parse the width value for "px"; for other types( vw, min() ), return the image width according to the default WordPress behaviour.

joemcgill · 2024-05-28T20:27:14Z

Thanks @mukeshpanchal27. It looks like the intent of this PR to constrain the width attribute value by the layout width. Is that correct? Is this related to an existing issue, or should a new issue be created to associate this PR with?

…mage-sizes

github-actions · 2024-06-05T10:05:06Z

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: mukeshpanchal27 <mukesh27@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>

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

mukeshpanchal27 · 2024-06-05T10:20:50Z

@joemcgill The PR is ready for review.

plugins/auto-sizes/hooks.php

joemcgill

After discussion with @mukeshpanchal27 and reviewing this PR in more detail, I've left the following comment on the original issue with suggestions for a different approach that we should take to resolve this issue.

mukeshpanchal27 · 2024-06-24T04:40:10Z

@joemcgill @westonruter The PR is ready for review.

As discussed in this comment, I have updated the PR to add custom attributes to the image and cover blocks. These custom attributes are then removed in the wp_content_img_tag filter, which allows us to get both the alignment and image width.

@joemcgill In this commit, I have removed the multiple parsing and added an early check to skip if the custom attribute is not present.

plugins/auto-sizes/hooks.php

joemcgill

I've tested locally, and this seems to be working as expected for most of the use cases we're trying to solve with this PR. I've left some code quality feedback that would be good to address before merging.

plugins/auto-sizes/hooks.php

joemcgill · 2024-06-24T20:15:19Z

plugins/auto-sizes/tests/improve-sizes-test.php

+		// Disable lazy loading attribute.
+		add_filter( 'wp_img_tag_add_loading_attr', '__return_false' );


This is a risky filter to disable while running these tests, because we'll be missing scenarios where the loading attribute changes the way the markup gets created. I assume you did this to avoid having auto sizes added while running these tests? If so, instead of removing the wp_img_tag_add_loading_attr filter, I would suggest running these without the auto-sizes filter, which seems to be what is causing potential failures here.

Suggested change

// Disable lazy loading attribute.

add_filter( 'wp_img_tag_add_loading_attr', '__return_false' );

// Disable auto sizes in these tests.

remove_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' );

However, since the hooks global won't get reset after this class runs, you'll also need to add this hook back after the class runs, or else it will affect any tests that run afterwards. Ex:

public static function tear_down_after_class(): void { parent::tear_down_after_class(); add_filter( 'wp_content_img_tag', 'auto_sizes_update_content_img_tag' ); }

If there are specific tests where you need to disable lazy-loading, it would be better to modify hooks directly in that test, because then they will automatically get reset during the WP_UnitTestCase_Base::tear_down() method that runs after each test.

Thanks @joemcgill for suggestion. This will not working as the filter is re-added through the_content filter so it show error in tests.

1) Tests_Improve_Sizes::test_image_block_with_full_alignment with data set "Return wideSize 1280px instead of medium size 300px" ('medium') Failed asserting that '<figure class="wp-block-image size-medium"><img loading="lazy" decoding="async" width="300" height="200" src="http://localhost:8889/wp-content/uploads/2024/06/leaves-283-300x200.jpg" alt="" class="wp-image-5" srcset="http://localhost:8889/wp-content/uploads/2024/06/leaves-283-300x200.jpg 300w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-1024x683.jpg 1024w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-768x512.jpg 768w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283.jpg 1080w" sizes="auto, 100vw" /></figure>' contains "sizes="100vw" ". /var/www/html/wp-content/plugins/performance/plugins/auto-sizes/tests/test-improve-sizes.php:53 2) Tests_Improve_Sizes::test_image_block_with_full_alignment with data set "Return wideSize 1280px instead of large size 1024px" ('large') Failed asserting that '<figure class="wp-block-image size-large"><img loading="lazy" decoding="async" width="1024" height="683" src="http://localhost:8889/wp-content/uploads/2024/06/leaves-283-1024x683.jpg" alt="" class="wp-image-5" srcset="http://localhost:8889/wp-content/uploads/2024/06/leaves-283-1024x683.jpg 1024w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-300x200.jpg 300w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-768x512.jpg 768w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283.jpg 1080w" sizes="auto, 100vw" /></figure>' contains "sizes="100vw" ". /var/www/html/wp-content/plugins/performance/plugins/auto-sizes/tests/test-improve-sizes.php:53 3) Tests_Improve_Sizes::test_image_block_with_full_alignment with data set "Return wideSize 1280px instead of full size 1080px" ('full') Failed asserting that '<figure class="wp-block-image size-full"><img loading="lazy" decoding="async" width="1080" height="720" src="http://localhost:8889/wp-content/uploads/2024/06/leaves-283.jpg" alt="" class="wp-image-5" srcset="http://localhost:8889/wp-content/uploads/2024/06/leaves-283.jpg 1080w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-300x200.jpg 300w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-1024x683.jpg 1024w, http://localhost:8889/wp-content/uploads/2024/06/leaves-283-768x512.jpg 768w" sizes="auto, 100vw" /></figure>' contains "sizes="100vw" ". /var/www/html/wp-content/plugins/performance/plugins/auto-sizes/tests/test-improve-sizes.php:53

plugins/auto-sizes/tests/improve-sizes-test.php

mukeshpanchal27 · 2024-06-25T07:09:43Z

@joemcgill @westonruter Thanks for the feedback. I have updated the PR as per your directions. It is now ready for review.

… fix/small-image-sizes * 'trunk' of https://github.com/WordPress/performance: (113 commits) Further refine command explaining type casting Elaborate on comment explaining why type casting is added Add try/catch block around imagecolorsforindex() Fix grammar typos Fix static analysis issues in Test_WebP_Uploads_Image_Edit Eliminate checks for seemingly impossible false return values Update PHPStan to 1.11.5 Add missing '@' Apply suggestions from code review Update testcase Fix wp:gallery styling not applying Update banner image Extend check code for WordPress 6.6 Harmonize return descriptions Bump Optimization Detective 0.3.1 Log URL metrics group collection to console when WP_DEBUG Include non-intersecting elements in URL metrics Bump version to 0.1.1 Fix logic error for preg_match() return value Add failing test case for a non-background image style ...

…mage-sizes * feature/more-accurate-sizes-attribute:

westonruter

LGTM

joemcgill

This is working as expected. I reverted the changes to the composer.lock file that were added because I think they were unintentional and doesn't seem to be related to this change. If that was an incorrect assumption we can update composer dependencies in a follow-up task.

joemcgill · 2024-06-25T22:06:23Z

plugins/auto-sizes/hooks.php

+
+	return $processor->get_updated_html();
+}
+// Run filter prior to auto sizes "auto_sizes_update_content_img_tag" filter.


Use correct sizes for small images

6d653b2

mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Images no milestone PRs that do not have a defined milestone for release [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin labels May 28, 2024

mukeshpanchal27 self-assigned this May 28, 2024

mukeshpanchal27 requested a review from joemcgill May 28, 2024 11:30

joemcgill linked an issue May 29, 2024 that may be closed by this pull request

Parse width presets for use in sizes values #1251

Closed

Fix merge conflict

e48f1e6

Base automatically changed from fix/1187-accurate-sizes-image-cover-block to feature/more-accurate-sizes-attribute June 5, 2024 04:18

mukeshpanchal27 and others added 2 commits June 5, 2024 09:49

Merge branch 'feature/more-accurate-sizes-attribute' into fix/small-i…

d3b5dc4

…mage-sizes

Add image size tests coverage

868225f

mukeshpanchal27 marked this pull request as ready for review June 5, 2024 10:04

Correct image urls

a6732f4

westonruter reviewed Jun 5, 2024

View reviewed changes

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

joemcgill reviewed Jun 5, 2024

View reviewed changes

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

sstopfer removed the [Focus] Images label Jun 9, 2024

mukeshpanchal27 added 2 commits June 10, 2024 09:23

Update docblock & use str_ends_with

00769a3

Update code that account for resized image width

2656a0d

mukeshpanchal27 requested review from westonruter and joemcgill June 10, 2024 05:01

Update image size classes

b9b75b3

joemcgill mentioned this pull request Jun 11, 2024

Parse width presets for use in sizes values #1251

Closed

joemcgill requested changes Jun 11, 2024

View reviewed changes

mukeshpanchal27 added 3 commits June 12, 2024 13:16

Exclude full and wide alignment from small size codebase

fa9f39f

Fix spell

b254fc5

Revise the approach

93fa153

westonruter reviewed Jun 24, 2024

View reviewed changes

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

plugins/auto-sizes/hooks.php Outdated Show resolved Hide resolved

joemcgill requested changes Jun 24, 2024

View reviewed changes

mukeshpanchal27 added 6 commits June 25, 2024 10:13

Fix phpstan issue

711c500

Fix unit tests

b5a6ded

Load filter prior to auto sizes filter

1f10468

Address review feedbacks

b722402

Address unit tests feedback

3235b9b

Update file name for consistency

6bf672f

mukeshpanchal27 requested review from adamsilverstein, felixarntz, pbearne and spacedmonkey as code owners June 25, 2024 06:14

Fix unit tests

41e118f

mukeshpanchal27 mentioned this pull request Jun 25, 2024

Update PHPStan to 1.11.5 #1318

Merged

Clean-up

fe1442e

mukeshpanchal27 requested review from westonruter and joemcgill June 25, 2024 07:09

westonruter requested a review from thelovekesh as a code owner June 25, 2024 18:06

Merge branch 'feature/more-accurate-sizes-attribute' into fix/small-i…

0a15a38

…mage-sizes * feature/more-accurate-sizes-attribute:

westonruter approved these changes Jun 25, 2024

View reviewed changes

joemcgill added 2 commits June 25, 2024 16:36

Revert changes to composer.lock file

b7117cf

Properly revert composer.lock file

0247988

joemcgill approved these changes Jun 25, 2024

View reviewed changes

joemcgill merged commit 9e4ee2b into feature/more-accurate-sizes-attribute Jun 25, 2024
14 checks passed

joemcgill deleted the fix/small-image-sizes branch June 25, 2024 22:17

westonruter removed the no milestone PRs that do not have a defined milestone for release label Jul 10, 2024

westonruter added this to the auto-sizes 1.1.0 milestone Jul 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use correct sizes for small images #1252

Use correct sizes for small images #1252

mukeshpanchal27 commented May 28, 2024 •

edited

Loading

joemcgill commented May 28, 2024

github-actions bot commented Jun 5, 2024 •

edited

Loading

mukeshpanchal27 commented Jun 5, 2024

joemcgill left a comment

mukeshpanchal27 commented Jun 24, 2024

joemcgill left a comment

joemcgill Jun 24, 2024

mukeshpanchal27 Jun 25, 2024

mukeshpanchal27 commented Jun 25, 2024

westonruter left a comment

joemcgill left a comment

joemcgill Jun 25, 2024

		// Disable lazy loading attribute.
		add_filter( 'wp_img_tag_add_loading_attr', '__return_false' );

Use correct sizes for small images #1252

Use correct sizes for small images #1252

Conversation

mukeshpanchal27 commented May 28, 2024 • edited Loading

Summary

joemcgill commented May 28, 2024

github-actions bot commented Jun 5, 2024 • edited Loading

mukeshpanchal27 commented Jun 5, 2024

joemcgill left a comment

Choose a reason for hiding this comment

mukeshpanchal27 commented Jun 24, 2024

joemcgill left a comment

Choose a reason for hiding this comment

joemcgill Jun 24, 2024

Choose a reason for hiding this comment

mukeshpanchal27 Jun 25, 2024

Choose a reason for hiding this comment

mukeshpanchal27 commented Jun 25, 2024

westonruter left a comment

Choose a reason for hiding this comment

joemcgill left a comment

Choose a reason for hiding this comment

joemcgill Jun 25, 2024

Choose a reason for hiding this comment

mukeshpanchal27 commented May 28, 2024 •

edited

Loading

github-actions bot commented Jun 5, 2024 •

edited

Loading