Embed Block: Fix Aspect Ratio Classes #29641

packages/block-library/src/embed/edit.js

@@ -4,6 +4,7 @@
 import {
 	createUpgradedEmbedBlock,
 	getClassNames,
+	removeAspectRatioClasses,
 	fallback,
 	getEmbedInfoByProvider,
 	getMergedAttributesWithPreview,
@@ -99,16 +100,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 = () => {
@@ -136,21 +135,20 @@ const EmbedEdit = ( props ) => {
 		setURL( newURL );
 		setIsEditingURL( false );
 		setAttributes( { url: newURL } );
-	}, [ preview?.html, attributesUrl ] );
+	}, [ preview?.html, attributesUrl, 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,8 +186,14 @@ const EmbedEdit = ( props ) => {
 							event.preventDefault();
 						}
 
+						// 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.
+						const blockClass = removeAspectRatioClasses(
+							attributes.className
+						);
+
 						setIsEditingURL( false );
-						setAttributes( { url } );
+						setAttributes( { url, className: blockClass } );
 					} }
 					value={ url }
 					cannotEmbed={ cannotEmbed }

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();
 

packages/block-library/src/embed/test/index.js

@@ -17,6 +17,7 @@ import {
 	createUpgradedEmbedBlock,
 	getEmbedInfoByProvider,
 	removeAspectRatioClasses,
+	hasAspectRatioClass,
 } from '../util';
 import { embedInstagramIcon } from '../icons';
 import variations from '../variations';
@@ -101,6 +102,17 @@ describe( 'utils', () => {
 			).toEqual( expected );
 		} );
 	} );
+	describe( 'hasAspectRatioClass', () => {
+		it( 'should return false if an aspect ratio class does not exist', () => {
+			const existingClassNames = 'wp-block-embed is-type-video';
+			expect( hasAspectRatioClass( existingClassNames ) ).toBe( false );
+		} );
+		it( 'should return true if an aspect ratio class exists', () => {
+			const existingClassNames =
+				'wp-block-embed is-type-video wp-embed-aspect-16-9 wp-has-aspect-ratio';
+			expect( hasAspectRatioClass( existingClassNames ) ).toBe( true );
+		} );
+	} );
 	describe( 'removeAspectRatioClasses', () => {
 		it( 'should return the same falsy value as received', () => {
 			const existingClassNames = undefined;

packages/block-library/src/embed/util.js

@@ -152,6 +152,21 @@ export const createUpgradedEmbedBlock = (
 	} );
 };
 
+/**
+ * Determine if the block already has an aspect ratio class applied.
+ *
+ * @param {string} existingClassNames Existing block classes.
+ * @return {boolean} True or false if the classnames contain an aspect ratio class.
+ */
+export const hasAspectRatioClass = ( existingClassNames ) => {
+	if ( ! existingClassNames ) {
+		return false;
+	}
+	return ASPECT_RATIOS.some( ( { className } ) =>
+		existingClassNames.includes( className )
+	);
+};
+
 /**
  * Removes all previously set aspect ratio related classes and return the rest
  * existing class names.
@@ -283,6 +298,13 @@ export const getAttributesFromPreview = memoize(
 			attributes.providerNameSlug = providerNameSlug;
 		}
 
+		// Aspect ratio classes are removed when the embed URL is updated.
+		// If the embed already has an aspect ratio class, that means the URL has not changed.
+		// Which also means no need to regenerate it with getClassNames.
+		if ( hasAspectRatioClass( currentClassNames ) ) {
+			return attributes;
+		}
+
 		attributes.className = getClassNames(
 			html,
 			currentClassNames,
@@ -296,27 +318,26 @@ export const getAttributesFromPreview = memoize(
 /**
  * Returns the attributes derived from the preview, merged with the current attributes.
  *
- * @param {Object}  currentAttributes       The current attributes of the block.
- * @param {Object}  preview                 The preview data.
- * @param {string}  title                   The block's title, e.g. Twitter.
- * @param {boolean} isResponsive            Boolean indicating if the block supports responsive content.
- * @param {boolean} ignorePreviousClassName Determines if the previous className attribute should be ignored when merging.
+ * @param {Object}  currentAttributes The current attributes of the block.
+ * @param {Object}  preview           The preview data.
+ * @param {string}  title             The block's title, e.g. Twitter.
+ * @param {boolean} isResponsive      Boolean indicating if the block supports responsive content.
  * @return {Object} Merged attributes.
  */
 export const getMergedAttributesWithPreview = (
 	currentAttributes,
 	preview,
 	title,
-	isResponsive,
-	ignorePreviousClassName = false
+	isResponsive
 ) => {
 	const { allowResponsive, className } = currentAttributes;
+
 	return {
 		...currentAttributes,
 		...getAttributesFromPreview(
 			preview,
 			title,
-			ignorePreviousClassName ? undefined : className,
+			className,
 			isResponsive,
 			allowResponsive
 		),