Embed Block: Fix Aspect Ratio Classes #29641

[TylerB24890]

What?

This Pull Request ensures the aspect ratio classes remain the same across Block Editor reloads. If an editor adds a custom Aspect Ratio class to a YouTube video, it should retain that class until the embed is changed.

Why?

Aspect Ratio classes (i.e. wp-embed-aspect-16-9) are added automatically to YouTube Videos (and potentially other embeds.) When this aspect ratio class is manually changed (to, for example, wp-embed-aspect-4-3) the video aspect ratio would change as expected and save to the block, but when the block editor reloads the classes are reverted to the original aspect ratio despite what is saved in the Block Attributes.

Resolves #29641.
Resolves #41813.
Resolves #45540.
Resolves #46627.

How?

When the Embed URL is added or changed the Aspect Ratio classes are reset using the existing removeAspectRatioClasses() method. This action fires in the onSubmit action of the URL input field.

When the Preview view is activated, we check if the Block already has a valid Aspect Ratio class applied. If it does, do not regenerate the classes and retain the classes that have already been set. If it does not have an Aspect Ratio class this can be considered a new embed, so the ratio classes are regenerated.

A new utility method hasAspectRatioClass( existingClassNames ) has been added to determine if a block already has an Aspect Ratio class.

Testing Instructions

1. Open the Block Editor in any post type
2. Add the Embed Block
3. Add a valid YouTube Embed Link
   • Confirm the Aspect Ratio classes are generated and added to the "Classes" input under the "Advanced" panel.
4. Change the Aspect Ratio class to another valid Aspect Ratio class
   • If the aspect ratio class is wp-embed-aspect-16-9 change it to wp-embed-aspect-4-3 (and vice-versa)
   • Verify the video Aspect Ratio has changed in the editor.
5. Update the page.
   • Do not refresh the block editor page yet.
6. View the frontend of the page in a new tab and confirm the new aspect ratio is applied to the block.
7. Revisit the Block Editor tab and refresh the page.
8. Confirm the Embed Block retains the custom applied Aspect Ratio.
9. Click the Edit icon to replace the Embed URL.
10. Either replace the Embed URL with a new video or simply click "Embed" again.
11. Confirm the Aspect Ratio classes were reset (or changed to the correct/original)

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @TylerB24890! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[Mamaduka]

Sorry for the ping, @fluiddot. Since you recently worked on Embed block fixes, maybe you can have a look at this PR 🙇

[fluiddot]

Sorry for the ping, @fluiddot. Since you recently worked on Embed block fixes, maybe you can have a look at this PR 🙇

Hey @Mamaduka, thanks for the ping 😊. I'll be happy to check these changes but I'd like to note that although I worked on fixes related to the Embed block, they were mainly related to the native version which is the platform I focus on as part of the Gutenberg Mobile project. Being this said, we might eventually ping someone else with higher context on the web version of the Embed block.

[Mamaduka]

That sounds good, @fluiddot.

P.S. I think I could also use a refresher on the Embed block 😄

[fluiddot]

@TylerB24890 I've tested the changes and work as expected 🎊 . However, I added a couple of comments that would be great if you could take a look at them, before proceeding to approve the PR, thanks 🙇 .

[fluiddot]

I tested the last changes using the Testing Instructions outlined in the PR and confirmed that work as expected 🎊.

I also tried the mobile native version and looks like we should apply the same changes to match the same behavior between platforms. @TylerB24890 I created the following patch file in case you'd like to apply it to this PR, otherwise, I'll be happy to push a commit with the changes. I tested those changes in the mobile native version and confirmed they also work as expected (including the issue solved in #35326).

Patch file

diff --git forkSrcPrefix/packages/block-library/src/embed/edit.native.js forkDstPrefix/packages/block-library/src/embed/edit.native.js
index 109478461c0b3e14975e4f3df2a17523774bf7ea..eec991c7b2037b46e5d0e40c3ab006fbcfb35b5f 100644
--- forkSrcPrefix/packages/block-library/src/embed/edit.native.js
+++ forkDstPrefix/packages/block-library/src/embed/edit.native.js
@@ -4,6 +4,7 @@
 import {
 	createUpgradedEmbedBlock,
 	getClassNames,
+	removeAspectRatioClasses,
 	fallback,
 	getEmbedInfoByProvider,
 	getMergedAttributesWithPreview,
@@ -123,16 +124,14 @@ const EmbedEdit = ( props ) => {
 	/**
 	 * Returns the attributes derived from the preview, merged with the current attributes.
 	 *
-	 * @param {boolean} ignorePreviousClassName Determines if the previous className attribute should be ignored when merging.
 	 * @return {Object} Merged attributes.
 	 */
-	const getMergedAttributes = ( ignorePreviousClassName = false ) =>
+	const getMergedAttributes = () =>
 		getMergedAttributesWithPreview(
 			attributes,
 			preview,
 			title,
-			responsive,
-			ignorePreviousClassName
+			responsive
 		);
 
 	const toggleResponsive = () => {
@@ -159,21 +158,20 @@ const EmbedEdit = ( props ) => {
 		const newURL = url.replace( /\/$/, '' );
 		setIsEditingURL( false );
 		setAttributes( { url: newURL } );
-	}, [ preview?.html, url ] );
+	}, [ preview?.html, url, cannotEmbed, fetching ] );
 
 	// Handle incoming preview.
 	useEffect( () => {
 		if ( preview && ! isEditingURL ) {
-			// When obtaining an incoming preview, we set the attributes derived from
-			// the preview data. In this case when getting the merged attributes,
-			// we ignore the previous classname because it might not match the expected
-			// classes by the new preview.
-			setAttributes( getMergedAttributes( true ) );
+			// When obtaining an incoming preview,
+			// we set the attributes derived from the preview data.
+			const mergedAttributes = getMergedAttributes();
+			setAttributes( mergedAttributes );
 
 			if ( onReplace ) {
 				const upgradedBlock = createUpgradedEmbedBlock(
 					props,
-					getMergedAttributes()
+					mergedAttributes
 				);
 
 				if ( upgradedBlock ) {
@@ -188,13 +186,25 @@ const EmbedEdit = ( props ) => {
 		[ isEditingURL ]
 	);
 
-	const onEditURL = useCallback( ( value ) => {
-		// The order of the following calls is important, we need to update the URL attribute before changing `isEditingURL`,
-		// otherwise the side-effect that potentially replaces the block when updating the local state won't use the new URL
-		// for creating the new block.
-		setAttributes( { url: value } );
-		setIsEditingURL( false );
-	}, [] );
+	const onEditURL = useCallback(
+		( value ) => {
+			// If the embed URL was changed, we need to reset the aspect ratio class.
+			// To do this we have to remove the existing ratio class so it can be recalculated.
+			if ( attributes.url !== value ) {
+				const blockClass = removeAspectRatioClasses(
+					attributes.className
+				);
+				setAttributes( { className: blockClass } );
+			}
+
+			// The order of the following calls is important, we need to update the URL attribute before changing `isEditingURL`,
+			// otherwise the side-effect that potentially replaces the block when updating the local state won't use the new URL
+			// for creating the new block.
+			setAttributes( { url: value } );
+			setIsEditingURL( false );
+		},
+		[ attributes, setAttributes ]
+	);
 
 	const blockProps = useBlockProps();
 

Thanks!

[fluiddot]

As I mentioned here, this PR will also fix the issue #41813 🎊 .

[TylerB24890]

I tested the last changes using the Testing Instructions outlined in the PR and confirmed that work as expected 🎊.

I also tried the mobile native version and looks like we should apply the same changes to match the same behavior between platforms. @TylerB24890 I created the following patch file in case you'd like to apply it to this PR, otherwise, I'll be happy to push a commit with the changes. I tested those changes in the mobile native version and confirmed they also work as expected (including the issue solved in #35326).

Patch file
Thanks!

This is great! Thank you, @fluiddot !

I've applied the patch file in this PR and have re-submitted. Please take a look and let me know if you have any further suggestions or changes!

[fluiddot]

LGTM 🎊 !

I confirm the following test cases work as expected:

• Change the Aspect Ratio class to another valid Aspect Ratio class is preserved (YouTube videos cannot be changed aspect #29641) ✅ .
• Additional CSS class(es) are preserved when changing the aspect ratio or Embed URL (Additional CSS class(es) not loading properly for Embed-block in wp-admin #41813) ✅ .
• No regression introduced related to cut-off in preview when editing URL on mobile (Embed block: Inline preview cut-off when editing URL wordpress-mobile/gutenberg-mobile#4059) ✅.

Thank you very much @TylerB24890 for working on this fix 🙇 !

From my side, the PR is ready to be merged. @Mamaduka since you pinged me originally in the PR, let me know if you'd like to take a look or we can go ahead and merge it, I'll be happy to do it. Thanks!

[Mamaduka]

Thank you, @TylerB24890! The changes look good to me!

@t-hamano, if you have time, do you mind checking if this also fixes the issue reported by you - #45540?

[t-hamano]

Thank you for the great work, @TylerB24890!

This PR would also fix the two problems of #45540 and #46627. In particular, I have tested whether custom classes not related to aspect-ratio are retained, and have confirmed that they are retained no matter what operation is performed.