Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gallery block: Add a filter to automatically convert transforms to and from 3rd party blocks #33861

28 changes: 28 additions & 0 deletions packages/block-library/src/gallery/transforms.js
Expand Up @@ -10,6 +10,7 @@ import { createBlock } from '@wordpress/blocks';
import { createBlobURL } from '@wordpress/blob';
import { select } from '@wordpress/data';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { addFilter } from '@wordpress/hooks';

/**
* Internal dependencies
Expand All @@ -33,6 +34,33 @@ const parseShortcodeIds = ( ids ) => {
return ids.split( ',' ).map( ( id ) => parseInt( id, 10 ) );
};

function transformV1FormatFromThirdPartyBlocks( block ) {
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
const settings = select( blockEditorStore ).getSettings();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the global "select", "dispatch" and "subscribe" is ideally something we should avoid. It breaks when we use several block editors in one page because this is going to get the settings for the default store only which might not be the right editor we're targeting here.

I do remember that we have a very old issue somewhere related to providing access to the block editor settings in transforms, but it's unsolved at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a related discussion #14755

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking more here, it's seem the only reason we need setting here is to check whether the experimental feature is enabled or not. __unstableGalleryWithImageBlocks. Maybe this is not a block editor setting but more a global WP setting in which case a global JS variable could work window. __unstableGalleryWithImageBlocks.

That said, the current approach in this PR will work exactly like a global variable because it will only target the global store. I think we all should be aware of the limitation of select dispatch and subscribe, maybe even have a lint rule forbidding their usage unless explicitly skipped but maybe it's fine here in the end. We should able to remove this check soon.

if (
settings.__unstableGalleryWithImageBlocks &&
block.name === 'core/gallery' &&
block.attributes?.images
) {
const innerBlocks = block.attributes.images.map( ( image ) => {
return createBlock( 'core/image', {
url: image.url,
id: image.id,
} );
} );
ramonjd marked this conversation as resolved.
Show resolved Hide resolved

delete block.attributes.ids;
delete block.attributes.images;
block.innerBlocks = innerBlocks;
}

return block;
}
addFilter(
'blocks.switchToBlockType.transformedBlock',
'core.transformthirdpartygalleries',
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
transformV1FormatFromThirdPartyBlocks
);

const transforms = {
from: [
{
Expand Down