From 7c09de3d501104e9858614555036324fc67c21a4 Mon Sep 17 00:00:00 2001 From: Tyler Bailey Date: Tue, 17 May 2022 16:12:00 -0400 Subject: [PATCH 1/4] Check if aspect ratio class exists before regenerating. Resets if the URL has changed. --- packages/block-library/src/embed/edit.js | 9 ++++++++- packages/block-library/src/embed/util.js | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/embed/edit.js b/packages/block-library/src/embed/edit.js index c3e664385809c1..838c8258ad58a6 100644 --- a/packages/block-library/src/embed/edit.js +++ b/packages/block-library/src/embed/edit.js @@ -4,6 +4,7 @@ import { createUpgradedEmbedBlock, getClassNames, + removeAspectRatioClasses, fallback, getEmbedInfoByProvider, getMergedAttributesWithPreview, @@ -188,8 +189,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 } diff --git a/packages/block-library/src/embed/util.js b/packages/block-library/src/embed/util.js index 35bf4c5d2966e2..7ed23b676c8257 100644 --- a/packages/block-library/src/embed/util.js +++ b/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. @@ -311,6 +326,14 @@ export const getMergedAttributesWithPreview = ( ignorePreviousClassName = false ) => { const { allowResponsive, className } = currentAttributes; + + // 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. + if ( hasAspectRatioClass( className ) ) { + return currentAttributes; + } + return { ...currentAttributes, ...getAttributesFromPreview( From 7c99c0e6847d197d9817a13cefdbff9038f6f348 Mon Sep 17 00:00:00 2001 From: Tyler Bailey Date: Fri, 24 Feb 2023 14:54:42 -0500 Subject: [PATCH 2/4] Check for aspect ratio class from preview attributes. Remove ignorePreviousClassNames parameters. --- packages/block-library/src/embed/edit.js | 19 +++++++--------- packages/block-library/src/embed/util.js | 28 +++++++++++------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/packages/block-library/src/embed/edit.js b/packages/block-library/src/embed/edit.js index 838c8258ad58a6..28902020c75e8e 100644 --- a/packages/block-library/src/embed/edit.js +++ b/packages/block-library/src/embed/edit.js @@ -100,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 = () => { @@ -137,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 ) { diff --git a/packages/block-library/src/embed/util.js b/packages/block-library/src/embed/util.js index 7ed23b676c8257..609a46293666e8 100644 --- a/packages/block-library/src/embed/util.js +++ b/packages/block-library/src/embed/util.js @@ -298,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, @@ -311,35 +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; - // 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. - if ( hasAspectRatioClass( className ) ) { - return currentAttributes; - } - return { ...currentAttributes, ...getAttributesFromPreview( preview, title, - ignorePreviousClassName ? undefined : className, + className, isResponsive, allowResponsive ), From 80fac253e0e8af7c18d53e96a57a4c9b03abd8d3 Mon Sep 17 00:00:00 2001 From: Tyler Bailey Date: Fri, 24 Feb 2023 14:55:05 -0500 Subject: [PATCH 3/4] Add unit test for hasAspectRatioClass --- packages/block-library/src/embed/test/index.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/block-library/src/embed/test/index.js b/packages/block-library/src/embed/test/index.js index 9ef7082f913570..1ae85105ad7c3b 100644 --- a/packages/block-library/src/embed/test/index.js +++ b/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; From 2b6a1b454a3a8e63bb3735bae3940eab316d87a9 Mon Sep 17 00:00:00 2001 From: Tyler Bailey Date: Fri, 3 Mar 2023 16:24:27 -0500 Subject: [PATCH 4/4] Implement patch file for native embed block. --- .../block-library/src/embed/edit.native.js | 46 +++++++++++-------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/packages/block-library/src/embed/edit.native.js b/packages/block-library/src/embed/edit.native.js index 109478461c0b3e..eec991c7b2037b 100644 --- a/packages/block-library/src/embed/edit.native.js +++ b/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();