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
Changes from 11 commits
51c543b
a5a100b
64739e2
aeb6d7b
9418d37
91a04e2
824de6d
5cb002f
6635d42
f1083a8
17a4030
e26be69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -33,6 +34,98 @@ const parseShortcodeIds = ( ids ) => { | |
return ids.split( ',' ).map( ( id ) => parseInt( id, 10 ) ); | ||
}; | ||
|
||
/** | ||
* Third party block plugins don't have an easy way to detect if the | ||
* innerBlocks version of the Gallery is running when they run a | ||
* 3rdPartyBlock -> GalleryBlock transform so this tranform filter | ||
* will handle this. Once the innerBlocks version is the default | ||
* in a core release, this could be deprecated and removed after | ||
* plugin authors have been given time to update transforms. | ||
* | ||
* @typedef {Object} Attributes | ||
* @typedef {Object} Block | ||
* @property {Attributes} attributes The attributes of the block. | ||
* @param {Block} block The transformed block. | ||
* @return {Block} The transformed block. | ||
*/ | ||
function updateThirdPartyTransformToGallery( block ) { | ||
const settings = select( blockEditorStore ).getSettings(); | ||
if ( | ||
settings.__unstableGalleryWithImageBlocks && | ||
block.name === 'core/gallery' && | ||
block.attributes?.images.length > 0 | ||
) { | ||
const innerBlocks = block.attributes.images.map( | ||
( { url, id, alt } ) => { | ||
return createBlock( 'core/image', { | ||
url, | ||
id: id ? parseInt( id, 10 ) : null, | ||
alt, | ||
sizeSlug: block.attributes.sizeSlug, | ||
linkDestination: block.attributes.linkDestination, | ||
} ); | ||
} | ||
); | ||
|
||
delete block.attributes.ids; | ||
delete block.attributes.images; | ||
block.innerBlocks = innerBlocks; | ||
} | ||
|
||
return block; | ||
} | ||
addFilter( | ||
'blocks.switchToBlockType.transformedBlock', | ||
'core/gallery/update-third-party-transform-to', | ||
updateThirdPartyTransformToGallery | ||
); | ||
|
||
/** | ||
* Third party block plugins don't have an easy way to detect if the | ||
* innerBlocks version of the Gallery is running when they run a | ||
* GalleryBlock -> 3rdPartyBlock transform so this transform filter | ||
* will handle this. Once the innerBlocks version is the default | ||
* in a core release, this could be deprecated and removed after | ||
* plugin authors have been given time to update transforms. | ||
* | ||
* @typedef {Object} Attributes | ||
* @typedef {Object} Block | ||
* @property {Attributes} attributes The attributes of the block. | ||
* @param {Block} toBlock The block to transform to. | ||
* @param {Block[]} fromBlocks The blocks to transform from. | ||
* @return {Block} The transformed block. | ||
*/ | ||
function updateThirdPartyTransformFromGallery( toBlock, fromBlocks ) { | ||
const settings = select( blockEditorStore ).getSettings(); | ||
const galleryBlock = fromBlocks.find( | ||
( transformedBlock ) => | ||
transformedBlock.name === 'core/gallery' && | ||
transformedBlock.innerBlocks.length > 0 && | ||
! transformedBlock.attributes.images?.length > 0 && | ||
! toBlock.name.includes( 'core/' ) | ||
); | ||
|
||
if ( settings.__unstableGalleryWithImageBlocks && galleryBlock ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the There's a chance the user might add a gallery block while the experiment is active, but then deactivate it and try to transform the block. I think even once the experiment has been deactivated, the saved block will still have inner blocks. The transform would still have to convert from those inner blocks regardless of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
True, nice catch, thanks. I have removed this check from the |
||
const images = galleryBlock.innerBlocks.map( | ||
( { attributes: { url, id, alt } } ) => ( { | ||
url, | ||
id: id ? parseInt( id, 10 ) : null, | ||
alt, | ||
} ) | ||
); | ||
const ids = images.map( ( { id } ) => id ); | ||
galleryBlock.attributes.images = images; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only works because this method is called twice, so on the second time around the mutated block data is used for the actual transformation. Not sure if this is behaviour that is safe to exploit in this way or not - given this block object is only transient and disappears after the transformation maybe it is ok to mutate it like this? |
||
galleryBlock.attributes.ids = ids; | ||
} | ||
|
||
return toBlock; | ||
} | ||
addFilter( | ||
'blocks.switchToBlockType.transformedBlock', | ||
'core/gallery/update-third-party-transform-from', | ||
updateThirdPartyTransformFromGallery | ||
); | ||
|
||
const transforms = { | ||
from: [ | ||
{ | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 workwindow. __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
andsubscribe
, 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.