Accurate sizes: Get accurate sizes base on ancestor block context #1818
Conversation
mukeshpanchal27
added
[Type] Feature
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## add/ancestor-block-context #1818 +/- ##
==============================================================
+ Coverage 69.41% 69.59% +0.18%
==============================================================
Files 86 86
Lines 7006 7018 +12
==============================================================
+ Hits 4863 4884 +21
+ Misses 2143 2134 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@joemcgill This one ready for your review. |
|
The PR is ready for internal review. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
What‘s with the failing tests? |
In 3a51aed it will skip failed tests for WordPress versions lower than 6.8 |
| * | ||
| * @return array<string, array<int, string>> The ancestor and image alignments. | ||
| */ | ||
| public function data_image_block_with_parent_columns_and_its_parent_group_block(): array { |
There was a problem hiding this comment.
Each of the tests has the exact same sizes attribute: sizes="(max-width: 413px) 100vw, 413px".
Is this expected?
There was a problem hiding this comment.
Yes, this is expected as it end up to the default alignment any of it's parents or image alignment as default alignment. In bba4997, I added more variations of alignment options.
There was a problem hiding this comment.
@mukeshpanchal27 This looks really good, just a few questions.
| // Convert to float for better precision. | ||
| $layout_width = (float) $layout_width * $container_relative_width; | ||
| $layout_width = sprintf( '%dpx', (int) $layout_width ); | ||
| } |
There was a problem hiding this comment.
Curious: Why does the wide alignment not take into consideration the image's own width, but left, right, center, and default do?
There was a problem hiding this comment.
It's because when we set the wide alignment option for the image block, it does not allow setting the Width and Height options for the image. So, we don't need to consider that option for wide alignment.
| // First remove 'px' from width. | ||
| $layout_width = str_replace( 'px', '', $layout_width ); | ||
| // Convert to float for better precision. | ||
| $layout_width = (float) $layout_width * $container_relative_width; | ||
| $layout_width = sprintf( '%dpx', min( (int) $layout_width, $image_width ) ); | ||
| } else { | ||
| $layout_width = sprintf( 'min(%1$s, %2$spx)', $layout_width, $image_width ); |
There was a problem hiding this comment.
Is it allowed to have relative values like % in the sizes attribute? Or what is the rationale for having this min() value here?
There was a problem hiding this comment.
See #1737 for context why it was added.
Summary
See #1511