Skip to content

Commit

Permalink
Embed Block: Fix Aspect Ratio Classes #29641 (#41141)
Browse files Browse the repository at this point in the history 
* Check if aspect ratio class exists before regenerating. Resets if the URL has changed.

* Check for aspect ratio class from preview attributes. Remove ignorePreviousClassNames parameters.

* Add unit test for hasAspectRatioClass

* Implement patch file for native embed block.
  • Loading branch information
@TylerB24890
TylerB24890 authored Mar 8, 2023
1 parent c5d76f8 commit d19dbd6
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 38 deletions.
28 changes: 16 additions & 12 deletions packages/block-library/src/embed/edit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import {
createUpgradedEmbedBlock,
getClassNames,
removeAspectRatioClasses,
fallback,
getEmbedInfoByProvider,
getMergedAttributesWithPreview,
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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 }
Expand Down
46 changes: 28 additions & 18 deletions packages/block-library/src/embed/edit.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import {
createUpgradedEmbedBlock,
getClassNames,
removeAspectRatioClasses,
fallback,
getEmbedInfoByProvider,
getMergedAttributesWithPreview,
Expand Down Expand Up @@ -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 = () => {
Expand All @@ -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 ) {
Expand All @@ -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();

Expand Down
12 changes: 12 additions & 0 deletions packages/block-library/src/embed/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
createUpgradedEmbedBlock,
getEmbedInfoByProvider,
removeAspectRatioClasses,
hasAspectRatioClass,
} from '../util';
import { embedInstagramIcon } from '../icons';
import variations from '../variations';
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 29 additions & 8 deletions packages/block-library/src/embed/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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
),
Expand Down

0 comments on commit d19dbd6

Please sign in to comment.