From d40a3964cf6061415829f057122a7fe1dc33a461 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 4 Jul 2024 16:07:36 +0800 Subject: [PATCH] Image block: Ensure the `wp-image-123` class and `data-id` attributes work with block bindings / pattern overrides (#63013) * Update the image block id classname via dynamic render when it mismatches the attribute value * Also update data-id so that it uses the id attribute * Adjust comment * Fix PHP lint * Only add `data-id` attribute to the image block when used in a gallery * Use multiple array keys to check for gallery parent * Change variable names * Add e2e test * Try alternative approach * Revert "Try alternative approach" This reverts commit ad46c2d9bd325eec3d8d44113dcbce3933982de0. * Add extra test for standalone image * Only apply binding values for data-id attribute and wp-image classname when a binding is applied * Make it more succinct * Fix linting issues --------- Co-authored-by: Mario Santos Co-authored-by: talldan Co-authored-by: SantosGuillamot Co-authored-by: cbravobernal Co-authored-by: kevin940726 Co-authored-by: ndiego Co-authored-by: jeherve Co-authored-by: perezcarreno --- packages/block-library/src/image/index.php | 31 +++- .../editor/various/pattern-overrides.spec.js | 134 ++++++++++++++++++ 2 files changed, 160 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 7fe05b2209c812..584318b51d196f 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -28,12 +28,33 @@ function render_block_core_image( $attributes, $content, $block ) { return ''; } + $has_id_binding = isset( $attributes['metadata']['bindings']['id'] ) && isset( $attributes['id'] ); + + // Ensure the `wp-image-id` classname on the image block supports block bindings. + if ( $has_id_binding ) { + // If there's a mismatch with the 'wp-image-' class and the actual id, the id was + // probably overridden by block bindings. Update it to the correct value. + // See https://github.com/WordPress/gutenberg/issues/62886 for why this is needed. + $id = $attributes['id']; + $image_classnames = $p->get_attribute( 'class' ); + $class_with_binding_value = "wp-image-$id"; + if ( is_string( $image_classnames ) && ! str_contains( $image_classnames, $class_with_binding_value ) ) { + $image_classnames = preg_replace( '/wp-image-(\d+)/', $class_with_binding_value, $image_classnames ); + $p->set_attribute( 'class', $image_classnames ); + } + } + + // For backwards compatibility, the data-id html attribute is only set for + // image blocks nested in a gallery. Detect if the image is in a gallery by + // checking the data-id attribute. + // See the `block_core_gallery_data_id_backcompatibility` function. if ( isset( $attributes['data-id'] ) ) { - // Adds the data-id="$id" attribute to the img element to provide backwards - // compatibility for the Gallery Block, which now wraps Image Blocks within - // innerBlocks. The data-id attribute is added in a core/gallery - // `render_block_data` hook. - $p->set_attribute( 'data-id', $attributes['data-id'] ); + // If there's a binding for the `id`, the `id` attribute is used for the + // value, since `data-id` does not support block bindings. + // Else the `data-id` is used for backwards compatibility, since + // third parties may be filtering its value. + $data_id = $has_id_binding ? $attributes['id'] : $attributes['data-id']; + $p->set_attribute( 'data-id', $data_id ); } $link_destination = isset( $attributes['linkDestination'] ) ? $attributes['linkDestination'] : 'none'; diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 721cea8d527169..5e529043ec3ed6 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -18,6 +18,7 @@ test.describe( 'Pattern Overrides', () => { test.afterEach( async ( { requestUtils } ) => { await requestUtils.deleteAllBlocks(); + await requestUtils.deleteAllMedia(); } ); test.afterAll( async ( { requestUtils } ) => { @@ -795,6 +796,139 @@ test.describe( 'Pattern Overrides', () => { expect( patternInnerBlocks[ 0 ].attributes.link ).toBe( undefined ); } ); + test( 'image block classname and data-id attributes contain the correct media ids when used in a gallery', async ( { + admin, + editor, + page, + requestUtils, + } ) => { + // Upload two images, one for the original pattern, one for the override. + const { id: originalImageId, source_url: originalImageSrc } = + await requestUtils.uploadMedia( + path.resolve( + process.cwd(), + 'test/e2e/assets/10x10_e2e_test_image_z9T8jK.png' + ) + ); + const { id: overrideImageId, source_url: overrideImageSrc } = + await requestUtils.uploadMedia( + path.resolve( + process.cwd(), + 'test/e2e/assets/1024x768_e2e_test_image_size.jpeg' + ) + ); + const overrideName = 'test'; + + // Might be overkill, but check that the ids are actually different. + expect( overrideImageId ).not.toBe( originalImageId ); + + // Create a pattern with a gallery that has a single image with pattern overrides enabled. + // It has media that is not yet uploaded. + const { id } = await requestUtils.createBlock( { + title: 'Pattern', + content: ` +
+
+
+`, + status: 'publish', + } ); + + // Insert the pattern into a new post, overriding the image via the pattern block attributes. + await admin.createNewPost(); + const imageAlt = 'Overridden Image'; + await editor.insertBlock( { + name: 'core/block', + attributes: { + ref: id, + content: { + [ overrideName ]: { + id: overrideImageId, + url: overrideImageSrc, + alt: imageAlt, + }, + }, + }, + } ); + + // Check the image attributes on the frontend. + const postId = await editor.publishPost(); + await page.goto( `/?p=${ postId }` ); + const imageBlock = page.getByAltText( imageAlt ); + await expect( imageBlock ).toHaveAttribute( + 'data-id', + `${ overrideImageId }` + ); + await expect( imageBlock ).toHaveAttribute( + 'class', + `wp-image-${ overrideImageId }` + ); + } ); + + test( 'image block classname contains the correct media id and has no data-id attribute when used as a standalone image', async ( { + admin, + editor, + page, + requestUtils, + } ) => { + // Upload two images, one for the original pattern, one for the override. + const { id: originalImageId, source_url: originalImageSrc } = + await requestUtils.uploadMedia( + path.resolve( + process.cwd(), + 'test/e2e/assets/10x10_e2e_test_image_z9T8jK.png' + ) + ); + const { id: overrideImageId, source_url: overrideImageSrc } = + await requestUtils.uploadMedia( + path.resolve( + process.cwd(), + 'test/e2e/assets/1024x768_e2e_test_image_size.jpeg' + ) + ); + const overrideName = 'test'; + + // Might be overkill, but check that the ids are actually different. + expect( overrideImageId ).not.toBe( originalImageId ); + + // Create a pattern with a gallery that has a single image with pattern overrides enabled. + // It has media that is not yet uploaded. + const { id } = await requestUtils.createBlock( { + title: 'Pattern', + content: ` +
+`, + status: 'publish', + } ); + + // Insert the pattern into a new post, overriding the image via the pattern block attributes. + await admin.createNewPost(); + const imageAlt = 'Overridden Image'; + await editor.insertBlock( { + name: 'core/block', + attributes: { + ref: id, + content: { + [ overrideName ]: { + id: overrideImageId, + url: overrideImageSrc, + alt: imageAlt, + }, + }, + }, + } ); + + // Check the image attributes on the frontend. + const postId = await editor.publishPost(); + await page.goto( `/?p=${ postId }` ); + const imageBlock = page.getByAltText( imageAlt ); + await expect( imageBlock ).not.toHaveAttribute( 'data-id' ); + await expect( imageBlock ).toHaveAttribute( + 'class', + `wp-image-${ overrideImageId }` + ); + } ); + test( 'blocks with the same name should be synced', async ( { page, admin,