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

POC - Pass group block alignment context to image block #1704

109 changes: 93 additions & 16 deletions plugins/auto-sizes/includes/improve-calculate-sizes.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ function auto_sizes_filter_image_tag( $content, array $parsed_block, WP_Block $b
if ( ! is_string( $content ) ) {
return '';
}

$processor = new WP_HTML_Tag_Processor( $content );
$has_image = $processor->next_tag( array( 'tag_name' => 'IMG' ) );

Expand All @@ -99,11 +100,13 @@ function auto_sizes_filter_image_tag( $content, array $parsed_block, WP_Block $b
* @param string $size The image size data.
*/
$filter = static function ( $sizes, $size ) use ( $block ) {
$id = $block->attributes['id'] ?? 0;
$alignment = $block->attributes['align'] ?? '';
$width = $block->attributes['width'] ?? '';
$id = $block->attributes['id'] ?? 0;
$alignment = $block->attributes['align'] ?? '';
$width = $block->attributes['width'] ?? '';
$is_parent_block = $block->context['is_parent_block'] ?? false;
$ancestor_block_align = $block->context['ancestor_block_align'] ?? '';

return auto_sizes_calculate_better_sizes( (int) $id, (string) $size, (string) $alignment, (string) $width );
return auto_sizes_calculate_better_sizes( (int) $id, (string) $size, (string) $alignment, (string) $width, (bool) $is_parent_block, (string) $ancestor_block_align );
};

// Hook this filter early, before default filters are run.
Expand Down Expand Up @@ -135,50 +138,124 @@ function auto_sizes_filter_image_tag( $content, array $parsed_block, WP_Block $b
/**
* Modifies the sizes attribute of an image based on layout context.
*
* @param int $id The image id.
* @param string $size The image size data.
* @param string $align The image alignment.
* @param string $resize_width Resize image width.
* @since n.e.x.t
*
* @param int $id The image id.
* @param string $size The image size data.
* @param string $align The image alignment.
* @param string $resize_width Resize image width.
* @param bool $is_parent_block Check if image block has parent block.
* @param string $ancestor_block_align The ancestor block alignment.
* @return string The sizes attribute value.
*/
function auto_sizes_calculate_better_sizes( int $id, string $size, string $align, string $resize_width ): string {
$sizes = '';
function auto_sizes_calculate_better_sizes( int $id, string $size, string $align, string $resize_width, bool $is_parent_block, string $ancestor_block_align ): string {
$image = wp_get_attachment_image_src( $id, $size );

if ( false === $image ) {
return $sizes;
return '';
}

// Retrieve width from the image tag itself.
$image_width = '' !== $resize_width ? (int) $resize_width : $image[1];

if ( $is_parent_block ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think a simpler way to do this is to compare the $ancestor_block_align value to the block's align attribute value, and use whichever is smaller in a single call to auto_sizes_get_sizes_by_block_alignments().

Copy link
Member Author

Choose a reason for hiding this comment

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

See #1704 (comment) it get issue for single image block.

if ( 'full' === $ancestor_block_align && 'full' === $align ) {
return auto_sizes_get_sizes_by_block_alignments( $align, $image_width, true );
} elseif ( 'full' !== $ancestor_block_align && 'full' === $align ) {
return auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width, true );
} elseif ( 'full' !== $ancestor_block_align ) {
$parent_block_alignment_width = auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width );
$block_alignment_width = auto_sizes_get_sizes_by_block_alignments( $align, $image_width );
if ( (int) $parent_block_alignment_width < (int) $block_alignment_width ) {
return sprintf( '(max-width: %1$s) 100vw, %1$s', $parent_block_alignment_width );
} else {
return sprintf( '(max-width: %1$s) 100vw, %1$s', $block_alignment_width );
}
}
}

return auto_sizes_get_sizes_by_block_alignments( $align, $image_width, true );
}

/**
* Generates the `sizes` attribute value based on block information.
*
* @since n.e.x.t
*
* @param string $alignment The alignment.
* @param int $image_width The image width.
* @param bool $print_sizes Print the sizes attribute. Default is false.
* @return string The sizes attribute value.
*/
function auto_sizes_get_sizes_by_block_alignments( string $alignment, int $image_width, bool $print_sizes = false ): string {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand changing the return type based on the $print_sizes parameter. If needed, it would be better to break this into two functions if necessary: one that calculates the width value, and another that formats the string for the sizes attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially, I considered separating them, but since the two functions are similar, I combined them into a single function by adding an extra parameter.

Copy link
Member

Choose a reason for hiding this comment

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

+1 to separating them. Using a boolean to change function behavior is a code smell.

Ideally a separate function should take the return value of this function and format it, like @joemcgill suggested.

$sizes = '';

$layout = wp_get_global_settings( array( 'layout' ) );

// Handle different alignment use cases.
switch ( $align ) {
switch ( $alignment ) {
case 'full':
$sizes = '100vw';
break;

case 'wide':
if ( array_key_exists( 'wideSize', $layout ) ) {
$sizes = sprintf( '(max-width: %1$s) 100vw, %1$s', $layout['wideSize'] );
$sizes = $layout['wideSize'];
}
break;

case 'left':
case 'right':
case 'center':
$sizes = sprintf( '(max-width: %1$dpx) 100vw, %1$dpx', $image_width );
$sizes = auto_sizes_get_width( '', $image_width );
break;

default:
if ( array_key_exists( 'contentSize', $layout ) ) {
$width = auto_sizes_get_width( $layout['contentSize'], $image_width );
$sizes = sprintf( '(max-width: %1$s) 100vw, %1$s', $width );
$sizes = auto_sizes_get_width( $layout['contentSize'], $image_width );
}
break;
}

if ( $print_sizes ) {
$sizes = 'full' === $alignment ? $sizes : sprintf( '(max-width: %1$s) 100vw, %1$s', $sizes );
}

return $sizes;
}

/**
* Filters the context keys that a block type uses.
*
* @since n.e.x.t
*
* @param array<string> $uses_context Array of registered uses context for a block type.
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
* @param WP_Block_Type $block_type The full block type object.
* @return array<string> The filtered context keys used by the block type.
*/
function auto_sizes_allowed_uses_context_for_image_blocks( array $uses_context, WP_Block_Type $block_type ): array {
if ( 'core/image' === $block_type->name ) {
// Use array_values to reset the array keys after merging.
return array_values( array_unique( array_merge( $uses_context, array( 'is_parent_block', 'ancestor_block_align' ) ) ) );
}
return $uses_context;
}
add_filter( 'get_block_type_uses_context', 'auto_sizes_allowed_uses_context_for_image_blocks', 10, 2 );
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Modifies the block context during rendering to blocks.
*
* @since n.e.x.t
*
* @param array<string, mixed> $context Current block context.
* @param array<string, mixed> $block The block being rendered.
* @return array<string, mixed> Modified block context.
*/
function auto_sizes_modify_render_block_context( array $context, array $block ): array {
if ( 'core/group' === $block['blockName'] || 'core/columns' === $block['blockName'] ) {
$context['is_parent_block'] = true;
$context['ancestor_block_align'] = $block['attrs']['align'] ?? '';
}
Copy link
Member

Choose a reason for hiding this comment

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

Rather than passing both of these through as context, I think it would be better to just save the current alignment as context that can be observed by the child block during rendering.

In fact, I did a quick test of removing the is_parent_block context and all the tests still pass, so it doesn't seem necessary.

diff --git a/plugins/auto-sizes/includes/improve-calculate-sizes.php b/plugins/auto-sizes/includes/improve-calculate-sizes.php
index 072effdf..6a6f9e81 100644
--- a/plugins/auto-sizes/includes/improve-calculate-sizes.php
+++ b/plugins/auto-sizes/includes/improve-calculate-sizes.php
@@ -103,10 +103,9 @@ function auto_sizes_filter_image_tag( $content, array $parsed_block, WP_Block $b
 			$id                   = $block->attributes['id'] ?? 0;
 			$alignment            = $block->attributes['align'] ?? '';
 			$width                = $block->attributes['width'] ?? '';
-			$is_parent_block      = $block->context['is_parent_block'] ?? false;
 			$ancestor_block_align = $block->context['ancestor_block_align'] ?? '';
 
-			return auto_sizes_calculate_better_sizes( (int) $id, (string) $size, (string) $alignment, (string) $width, (bool) $is_parent_block, (string) $ancestor_block_align );
+			return auto_sizes_calculate_better_sizes( (int) $id, (string) $size, (string) $alignment, (string) $width, (string) $ancestor_block_align );
 		};
 
 		// Hook this filter early, before default filters are run.
@@ -144,11 +143,10 @@ function auto_sizes_filter_image_tag( $content, array $parsed_block, WP_Block $b
  * @param string $size                 The image size data.
  * @param string $align                The image alignment.
  * @param string $resize_width         Resize image width.
- * @param bool   $is_parent_block      Check if image block has parent block.
  * @param string $ancestor_block_align The ancestor block alignment.
  * @return string The sizes attribute value.
  */
-function auto_sizes_calculate_better_sizes( int $id, string $size, string $align, string $resize_width, bool $is_parent_block, string $ancestor_block_align ): string {
+function auto_sizes_calculate_better_sizes( int $id, string $size, string $align, string $resize_width, string $ancestor_block_align ): string {
 	$image = wp_get_attachment_image_src( $id, $size );
 
 	if ( false === $image ) {
@@ -158,19 +156,18 @@ function auto_sizes_calculate_better_sizes( int $id, string $size, string $align
 	// Retrieve width from the image tag itself.
 	$image_width = '' !== $resize_width ? (int) $resize_width : $image[1];
 
-	if ( $is_parent_block ) {
-		if ( 'full' === $ancestor_block_align && 'full' === $align ) {
-			return auto_sizes_get_sizes_by_block_alignments( $align, $image_width, true );
-		} elseif ( 'full' !== $ancestor_block_align && 'full' === $align ) {
-			return auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width, true );
-		} elseif ( 'full' !== $ancestor_block_align ) {
-			$parent_block_alignment_width = auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width );
-			$block_alignment_width        = auto_sizes_get_sizes_by_block_alignments( $align, $image_width );
-			if ( (int) $parent_block_alignment_width < (int) $block_alignment_width ) {
-				return sprintf( '(max-width: %1$s) 100vw, %1$s', $parent_block_alignment_width );
-			} else {
-				return sprintf( '(max-width: %1$s) 100vw, %1$s', $block_alignment_width );
-			}
+	// Handle different alignment use cases.
+	if ( 'full' === $ancestor_block_align && 'full' === $align ) {
+		return auto_sizes_get_sizes_by_block_alignments( $align, $image_width, true );
+	} elseif ( 'full' !== $ancestor_block_align && 'full' === $align ) {
+		return auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width, true );
+	} elseif ( 'full' !== $ancestor_block_align ) {
+		$parent_block_alignment_width = auto_sizes_get_sizes_by_block_alignments( $ancestor_block_align, $image_width );
+		$block_alignment_width        = auto_sizes_get_sizes_by_block_alignments( $align, $image_width );
+		if ( (int) $parent_block_alignment_width < (int) $block_alignment_width ) {
+			return sprintf( '(max-width: %1$s) 100vw, %1$s', $parent_block_alignment_width );
+		} else {
+			return sprintf( '(max-width: %1$s) 100vw, %1$s', $block_alignment_width );
 		}
 	}
 
@@ -236,7 +233,7 @@ function auto_sizes_get_sizes_by_block_alignments( string $alignment, int $image
 function auto_sizes_allowed_uses_context_for_image_blocks( array $uses_context, WP_Block_Type $block_type ): array {
 	if ( 'core/image' === $block_type->name ) {
 		// Use array_values to reset the array keys after merging.
-		return array_values( array_unique( array_merge( $uses_context, array( 'is_parent_block', 'ancestor_block_align' ) ) ) );
+		return array_values( array_unique( array_merge( $uses_context, array( 'ancestor_block_align' ) ) ) );
 	}
 	return $uses_context;
 }
@@ -253,7 +250,6 @@ add_filter( 'get_block_type_uses_context', 'auto_sizes_allowed_uses_context_for_
  */
 function auto_sizes_modify_render_block_context( array $context, array $block ): array {
 	if ( 'core/group' === $block['blockName'] || 'core/columns' === $block['blockName'] ) {
-		$context['is_parent_block']      = true;
 		$context['ancestor_block_align'] = $block['attrs']['align'] ?? '';
 	}
 	return $context;

Copy link
Member Author

Choose a reason for hiding this comment

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

If i didn't check the is_parent_block then it will break the existing test for Image block only.

It because it consider $ancestor_block_align alignment as None and return image size so instead on 100vw for image it return sizes="(max-width: 300px) 100vw, 300px" for Medium size image.

return $context;
}
add_filter( 'render_block_context', 'auto_sizes_modify_render_block_context', 10, 2 );
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
129 changes: 119 additions & 10 deletions plugins/auto-sizes/tests/test-improve-calculate-sizes.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,38 +390,147 @@ public function test_no_image(): void {
* Test that the layout property of a group block is passed by context to the image block.
*
* @group test
*
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
* @dataProvider data_ancestor_and_image_block_alignment
*
* @param string $ancestor_block_alignment Ancestor block alignment.
* @param string $image_block_alignment Image block alignment.
* @param string $expected Expected output.
*/
public function test_ancestor_layout_is_passed_by_context(): void {
public function test_ancestor_layout_is_passed_by_context( string $ancestor_block_alignment, string $image_block_alignment, string $expected ): void {
$block_content = $this->get_group_block_markup(
$this->get_image_block_markup( self::$image_id, 'large', 'full' )
$this->get_image_block_markup( self::$image_id, 'large', $image_block_alignment ),
array(
'align' => $ancestor_block_alignment,
)
);

$result = apply_filters( 'the_content', $block_content );

$this->assertStringContainsString( 'sizes="(max-width: 620px) 100vw, 620px" ', $result );
$this->assertStringContainsString( $expected, $result );
}

/**
* Data provider.
*
* @return array<string, array<int, bool|string>> The ancestor and image alignments.
*/
public function data_ancestor_and_image_block_alignment(): array {
joemcgill marked this conversation as resolved.
Show resolved Hide resolved
return array(
// Parent default alignment.
'Return contentSize 620px, parent block default alignment, image block default alignment' => array(
'',
'',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return contentSize 620px, parent block default alignment, image block wide alignment' => array(
'',
'wide',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return contentSize 620px, parent block default alignment, image block full alignment' => array(
'',
'full',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return contentSize 620px, parent block default alignment, image block left alignment' => array(
'',
'left',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return contentSize 620px, parent block default alignment, image block center alignment' => array(
'',
'center',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return contentSize 620px, parent block default alignment, image block right alignment' => array(
'',
'right',
'sizes="(max-width: 620px) 100vw, 620px" ',
),

// Parent wide alignment.
'Return contentSize 620px, parent block wide alignment, image block default alignment' => array(
'wide',
'',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return wideSize 1280px, parent block wide alignment, image block wide alignment' => array(
'wide',
'wide',
'sizes="(max-width: 1280px) 100vw, 1280px" ',
),
'Return wideSize 1280px, parent block wide alignment, image block full alignment' => array(
'wide',
'full',
'sizes="(max-width: 1280px) 100vw, 1280px" ',
),
'Return image size 1024px, parent block wide alignment, image block left alignment' => array(
'wide',
'left',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),
'Return image size 1024px, parent block wide alignment, image block center alignment' => array(
'wide',
'center',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),
'Return image size 1024px, parent block wide alignment, image block right alignment' => array(
'wide',
'right',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),

// Parent full alignment.
'Return contentSize 620px, parent block full alignment, image block default alignment' => array(
'full',
'',
'sizes="(max-width: 620px) 100vw, 620px" ',
),
'Return wideSize 1280px, parent block full alignment, image block wide alignment' => array(
'full',
'wide',
'sizes="(max-width: 1280px) 100vw, 1280px" ',
),
'Return full size, parent block full alignment, image block full alignment' => array(
'full',
'full',
'sizes="100vw" ',
),
'Return image size 1024px, parent block full alignment, image block left alignment' => array(
'full',
'left',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),
'Return image size 1024px, parent block full alignment, image block center alignment' => array(
'full',
'center',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),
'Return image size 1024px, parent block full alignment, image block right alignment' => array(
'full',
'right',
'sizes="(max-width: 1024px) 100vw, 1024px" ',
),
);
}

/**
* Helper to generate image block markup.
*
* @param int $attachment_id Attachment ID.
* @param string $size Optional. Image size. Default 'full'.
* @param string $align Optional. Image alignment. Default null.
* @param string $align Optional. Image alignment. Default null.
* @return string Image block markup.
*/
public function get_image_block_markup( int $attachment_id, string $size = 'full', string $align = null ): string {
$image_url = wp_get_attachment_image_url( $attachment_id, $size );

$atts = wp_parse_args(
array(
'id' => $attachment_id,
'sizeSlug' => $size,
'align' => $align,
),
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
array(
'id' => $attachment_id,
'sizeSlug' => 'large',
'sizeSlug' => $size,
'align' => $align,
'linkDestination' => 'none',
)
);
Expand Down