diff --git a/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js b/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js index 8bce0ae063915..9d643173c70c2 100644 --- a/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js +++ b/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js @@ -12,7 +12,11 @@ import { useSelect } from '@wordpress/data'; import { moreVertical } from '@wordpress/icons'; import { Children, cloneElement, useCallback } from '@wordpress/element'; -import { serialize, store as blocksStore } from '@wordpress/blocks'; +import { + serialize, + store as blocksStore, + __experimentalCloneSanitizedBlock, +} from '@wordpress/blocks'; import { store as keyboardShortcutsStore } from '@wordpress/keyboard-shortcuts'; import { useCopyToClipboard } from '@wordpress/compose'; @@ -33,7 +37,12 @@ const POPOVER_PROPS = { }; function CopyMenuItem( { blocks, onCopy } ) { - const ref = useCopyToClipboard( () => serialize( blocks ), onCopy ); + const ref = useCopyToClipboard( () => { + blocks = blocks.map( ( block ) => + __experimentalCloneSanitizedBlock( block ) + ); + return serialize( blocks ); + }, onCopy ); return { __( 'Copy' ) }; } diff --git a/packages/block-editor/src/components/copy-handler/index.js b/packages/block-editor/src/components/copy-handler/index.js index 0015ea7a9b6ae..9c4e9e9105582 100644 --- a/packages/block-editor/src/components/copy-handler/index.js +++ b/packages/block-editor/src/components/copy-handler/index.js @@ -5,6 +5,7 @@ import { useCallback } from '@wordpress/element'; import { serialize, pasteHandler, + __experimentalCloneSanitizedBlock, store as blocksStore, } from '@wordpress/blocks'; import { @@ -121,7 +122,12 @@ export function useClipboardHandler() { flashBlock( selectedBlockClientIds[ 0 ] ); } notifyCopy( event.type, selectedBlockClientIds ); - const blocks = getBlocksByClientId( selectedBlockClientIds ); + let blocks = getBlocksByClientId( selectedBlockClientIds ); + if ( event.type === 'copy' ) { + blocks = blocks.map( ( block ) => + __experimentalCloneSanitizedBlock( block ) + ); + } const serialized = serialize( blocks ); event.clipboardData.setData( 'text/plain', serialized ); diff --git a/packages/blocks/src/api/factory.js b/packages/blocks/src/api/factory.js index 7937ad98fb330..78d240ce44e5a 100644 --- a/packages/blocks/src/api/factory.js +++ b/packages/blocks/src/api/factory.js @@ -15,6 +15,8 @@ import { isFunction, isEmpty, map, + reduce, + omit, } from 'lodash'; /** @@ -32,7 +34,7 @@ import { } from './registration'; import { normalizeBlockType, - __experimentalSanitizeBlockAttributes, + __experimentalGetBlockAttributesNamesByRole, } from './utils'; /** @@ -45,9 +47,37 @@ import { * @return {Object} Block object. */ export function createBlock( name, attributes = {}, innerBlocks = [] ) { - const sanitizedAttributes = __experimentalSanitizeBlockAttributes( - name, - attributes + // Get the type definition associated with a registered block. + const blockType = getBlockType( name ); + + if ( undefined === blockType ) { + throw new Error( `Block type '${ name }' is not registered.` ); + } + + const sanitizedAttributes = reduce( + blockType.attributes, + ( accumulator, schema, key ) => { + const value = attributes[ key ]; + + if ( undefined !== value ) { + accumulator[ key ] = value; + } else if ( schema.hasOwnProperty( 'default' ) ) { + accumulator[ key ] = schema.default; + } + + if ( [ 'node', 'children' ].indexOf( schema.source ) !== -1 ) { + // Ensure value passed is always an array, which we're expecting in + // the RichText component to handle the deprecated value. + if ( typeof accumulator[ key ] === 'string' ) { + accumulator[ key ] = [ accumulator[ key ] ]; + } else if ( ! Array.isArray( accumulator[ key ] ) ) { + accumulator[ key ] = []; + } + } + + return accumulator; + }, + {} ); const clientId = uuid(); @@ -94,8 +124,9 @@ export function createBlocksFromInnerBlocksTemplate( } /** - * Given a block object, returns a copy of the block object while sanitizing its attributes, + * Given a block object, returns a copy of the block object, * optionally merging new attributes and/or replacing its inner blocks. + * Attributes with the `internal` role are not copied into the new object. * * @param {Object} block Block instance. * @param {Object} mergeAttributes Block attributes. @@ -110,18 +141,20 @@ export function __experimentalCloneSanitizedBlock( ) { const clientId = uuid(); - const sanitizedAttributes = __experimentalSanitizeBlockAttributes( - block.name, + // Merge in new attributes and strip out attributes with the `internal` role, + // which should not be copied during block duplication. + const filteredAttributes = omit( { ...block.attributes, ...mergeAttributes, - } + }, + __experimentalGetBlockAttributesNamesByRole( block.name, 'internal' ) ); return { ...block, clientId, - attributes: sanitizedAttributes, + attributes: filteredAttributes, innerBlocks: newInnerBlocks || block.innerBlocks.map( ( innerBlock ) => diff --git a/packages/blocks/src/api/index.js b/packages/blocks/src/api/index.js index b5ae50093233d..e57b34b4bb9a4 100644 --- a/packages/blocks/src/api/index.js +++ b/packages/blocks/src/api/index.js @@ -141,7 +141,6 @@ export { isValidIcon, getBlockLabel as __experimentalGetBlockLabel, getAccessibleBlockLabel as __experimentalGetAccessibleBlockLabel, - __experimentalSanitizeBlockAttributes, __experimentalGetBlockAttributesNamesByRole, } from './utils'; diff --git a/packages/blocks/src/api/test/factory.js b/packages/blocks/src/api/test/factory.js index 3b4d8671f6e1b..4042f54ea99c8 100644 --- a/packages/blocks/src/api/test/factory.js +++ b/packages/blocks/src/api/test/factory.js @@ -90,17 +90,21 @@ describe( 'block factory', () => { registerBlockType( 'core/test-block', { ...defaultBlockSettings, attributes: { - content: { + childrenContent: { type: 'array', source: 'children', }, + nodeContent: { + source: 'node', + }, }, } ); const block = createBlock( 'core/test-block' ); expect( block.attributes ).toEqual( { - content: [], + childrenContent: [], + nodeContent: [], } ); } ); @@ -177,6 +181,14 @@ describe( 'block factory', () => { expect( block.attributes ).toEqual( {} ); } ); + + it( 'throws error if the block is not registered', () => { + expect( () => { + createBlock( 'core/not-registered-test-block', {} ); + } ).toThrowErrorMatchingInlineSnapshot( + `"Block type 'core/not-registered-test-block' is not registered."` + ); + } ); } ); describe( 'createBlocksFromInnerBlocksTemplate', () => { @@ -428,25 +440,182 @@ describe( 'block factory', () => { } ); describe( '__experimentalCloneSanitizedBlock', () => { - it( 'should sanitize attributes not defined in the block type', () => { + it( 'should not duplicate internal attributes', () => { registerBlockType( 'core/test-block', { ...defaultBlockSettings, attributes: { - align: { + internalAttribute: { + type: 'string', + __experimentalRole: 'internal', + }, + contentAttribute: { + type: 'string', + __experimentalRole: 'content', + }, + attributeWithNoRole: { type: 'string', }, }, } ); const block = createBlock( 'core/test-block', { - notDefined: 'not-defined', + internalAttribute: 'this-should-not-be-copied', + contentAttribute: 'some content', + attributeWithNoRole: 'another attribute', + } ); + + const clonedBlock = __experimentalCloneSanitizedBlock( block, {} ); + + expect( clonedBlock.attributes ).toEqual( { + contentAttribute: 'some content', + attributeWithNoRole: 'another attribute', + } ); + } ); + + it( 'should merge attributes into the existing block', () => { + registerBlockType( 'core/test-block', { + attributes: { + align: { + type: 'string', + }, + isDifferent: { + type: 'boolean', + default: false, + }, + includesDefault: { + type: 'boolean', + default: true, + }, + includesFalseyDefault: { + type: 'number', + default: 0, + }, + content: { + type: 'array', + source: 'children', + }, + defaultContent: { + type: 'array', + source: 'children', + default: 'test', + }, + unknownDefaultContent: { + type: 'array', + source: 'children', + default: 1, + }, + htmlContent: { + source: 'html', + }, + }, + save: noop, + category: 'text', + title: 'test block', } ); + const block = deepFreeze( + createBlock( 'core/test-block', { align: 'left' }, [ + createBlock( 'core/test-block' ), + ] ) + ); const clonedBlock = __experimentalCloneSanitizedBlock( block, { - notDefined2: 'not-defined-2', + isDifferent: true, + htmlContent: 'test', } ); - expect( clonedBlock.attributes ).toEqual( {} ); + expect( clonedBlock.name ).toEqual( block.name ); + expect( clonedBlock.attributes ).toEqual( { + includesDefault: true, + includesFalseyDefault: 0, + align: 'left', + isDifferent: true, + content: [], + defaultContent: [ 'test' ], + unknownDefaultContent: [], + htmlContent: 'test', + } ); + expect( clonedBlock.innerBlocks ).toHaveLength( 1 ); + expect( typeof clonedBlock.clientId ).toBe( 'string' ); + expect( clonedBlock.clientId ).not.toBe( block.clientId ); + } ); + + it( 'should replace inner blocks of the existing block', () => { + registerBlockType( 'core/test-block', { + attributes: { + align: { + type: 'string', + }, + isDifferent: { + type: 'boolean', + default: false, + }, + }, + save: noop, + category: 'text', + title: 'test block', + } ); + const block = deepFreeze( + createBlock( 'core/test-block', { align: 'left' }, [ + createBlock( 'core/test-block', { align: 'right' } ), + createBlock( 'core/test-block', { align: 'left' } ), + ] ) + ); + + const clonedBlock = __experimentalCloneSanitizedBlock( + block, + undefined, + [ createBlock( 'core/test-block' ) ] + ); + + expect( clonedBlock.innerBlocks ).toHaveLength( 1 ); + expect( + clonedBlock.innerBlocks[ 0 ].attributes + ).not.toHaveProperty( 'align' ); + } ); + + it( 'should clone innerBlocks if innerBlocks are not passed', () => { + registerBlockType( 'core/test-block', { + attributes: { + align: { + type: 'string', + }, + isDifferent: { + type: 'boolean', + default: false, + }, + }, + save: noop, + category: 'text', + title: 'test block', + } ); + const block = deepFreeze( + createBlock( 'core/test-block', { align: 'left' }, [ + createBlock( 'core/test-block', { align: 'right' } ), + createBlock( 'core/test-block', { align: 'left' } ), + ] ) + ); + + const clonedBlock = __experimentalCloneSanitizedBlock( block ); + + expect( clonedBlock.innerBlocks ).toHaveLength( 2 ); + expect( clonedBlock.innerBlocks[ 0 ].clientId ).not.toBe( + block.innerBlocks[ 0 ].clientId + ); + expect( clonedBlock.innerBlocks[ 0 ].attributes ).not.toBe( + block.innerBlocks[ 0 ].attributes + ); + expect( clonedBlock.innerBlocks[ 0 ].attributes ).toEqual( + block.innerBlocks[ 0 ].attributes + ); + expect( clonedBlock.innerBlocks[ 1 ].clientId ).not.toBe( + block.innerBlocks[ 1 ].clientId + ); + expect( clonedBlock.innerBlocks[ 1 ].attributes ).not.toBe( + block.innerBlocks[ 1 ].attributes + ); + expect( clonedBlock.innerBlocks[ 1 ].attributes ).toEqual( + block.innerBlocks[ 1 ].attributes + ); } ); } ); diff --git a/packages/blocks/src/api/test/utils.js b/packages/blocks/src/api/test/utils.js index 3ffc1bf5944bc..3cf3f7b1cc51d 100644 --- a/packages/blocks/src/api/test/utils.js +++ b/packages/blocks/src/api/test/utils.js @@ -17,7 +17,6 @@ import { isUnmodifiedDefaultBlock, getAccessibleBlockLabel, getBlockLabel, - __experimentalSanitizeBlockAttributes, __experimentalGetBlockAttributesNamesByRole, } from '../utils'; @@ -215,103 +214,6 @@ describe( 'getAccessibleBlockLabel', () => { } ); } ); -describe( 'sanitizeBlockAttributes', () => { - afterEach( () => { - getBlockTypes().forEach( ( block ) => { - unregisterBlockType( block.name ); - } ); - } ); - - it( 'sanitize block attributes not defined in the block type', () => { - registerBlockType( 'core/test-block', { - attributes: { - defined: { - type: 'string', - }, - }, - title: 'Test block', - } ); - - const attributes = __experimentalSanitizeBlockAttributes( - 'core/test-block', - { - defined: 'defined-attribute', - notDefined: 'not-defined-attribute', - } - ); - - expect( attributes ).toEqual( { - defined: 'defined-attribute', - } ); - } ); - - it( 'throws error if the block is not registered', () => { - expect( () => { - __experimentalSanitizeBlockAttributes( - 'core/not-registered-test-block', - {} - ); - } ).toThrowErrorMatchingInlineSnapshot( - `"Block type 'core/not-registered-test-block' is not registered."` - ); - } ); - - it( 'handles undefined values and default values', () => { - registerBlockType( 'core/test-block', { - attributes: { - hasDefaultValue: { - type: 'string', - default: 'default-value', - }, - noDefaultValue: { - type: 'string', - }, - }, - title: 'Test block', - } ); - - const attributes = __experimentalSanitizeBlockAttributes( - 'core/test-block', - {} - ); - - expect( attributes ).toEqual( { - hasDefaultValue: 'default-value', - } ); - } ); - - it( 'handles node and children sources as arrays', () => { - registerBlockType( 'core/test-block', { - attributes: { - nodeContent: { - source: 'node', - }, - childrenContent: { - source: 'children', - }, - withDefault: { - source: 'children', - default: 'test', - }, - }, - title: 'Test block', - } ); - - const attributes = __experimentalSanitizeBlockAttributes( - 'core/test-block', - { - nodeContent: [ 'test-1', 'test-2' ], - } - ); - - expect( attributes ).toEqual( { - nodeContent: [ 'test-1', 'test-2' ], - childrenContent: [], - withDefault: [ 'test' ], - } ); - } ); -} ); - describe( '__experimentalGetBlockAttributesNamesByRole', () => { beforeAll( () => { registerBlockType( 'core/test-block-1', { diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index 1cc87b237cabd..e087484c91d6f 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { every, has, isFunction, isString, reduce, maxBy } from 'lodash'; +import { every, has, isFunction, isString, maxBy } from 'lodash'; import { colord, extend } from 'colord'; import namesPlugin from 'colord/plugins/names'; import a11yPlugin from 'colord/plugins/a11y'; @@ -239,49 +239,6 @@ export function getAccessibleBlockLabel( ); } -/** - * Ensure attributes contains only values defined by block type, and merge - * default values for missing attributes. - * - * @param {string} name The block's name. - * @param {Object} attributes The block's attributes. - * @return {Object} The sanitized attributes. - */ -export function __experimentalSanitizeBlockAttributes( name, attributes ) { - // Get the type definition associated with a registered block. - const blockType = getBlockType( name ); - - if ( undefined === blockType ) { - throw new Error( `Block type '${ name }' is not registered.` ); - } - - return reduce( - blockType.attributes, - ( accumulator, schema, key ) => { - const value = attributes[ key ]; - - if ( undefined !== value ) { - accumulator[ key ] = value; - } else if ( schema.hasOwnProperty( 'default' ) ) { - accumulator[ key ] = schema.default; - } - - if ( [ 'node', 'children' ].indexOf( schema.source ) !== -1 ) { - // Ensure value passed is always an array, which we're expecting in - // the RichText component to handle the deprecated value. - if ( typeof accumulator[ key ] === 'string' ) { - accumulator[ key ] = [ accumulator[ key ] ]; - } else if ( ! Array.isArray( accumulator[ key ] ) ) { - accumulator[ key ] = []; - } - } - - return accumulator; - }, - {} - ); -} - /** * Filter block attributes by `role` and return their names. * diff --git a/packages/customize-widgets/src/filters/move-to-sidebar.js b/packages/customize-widgets/src/filters/move-to-sidebar.js index 9e7910ce785a6..df11b6ee4942d 100644 --- a/packages/customize-widgets/src/filters/move-to-sidebar.js +++ b/packages/customize-widgets/src/filters/move-to-sidebar.js @@ -55,10 +55,10 @@ const withMoveToSidebarToolbarItem = createHigherOrderComponent( const newSidebarControl = sidebarControls.find( ( sidebarControl ) => sidebarControl.id === sidebarControlId ); - - if ( widgetId ) { + const activeSidebarAdapter = activeSidebarControl.sidebarAdapter; + if ( widgetId && activeSidebarAdapter.getWidget( widgetId ) ) { /** - * If there's a widgetId, move it to the other sidebar. + * If there's an existing widget, move it to the other sidebar. */ const oldSetting = activeSidebarControl.setting; const newSetting = newSidebarControl.setting; @@ -67,15 +67,15 @@ const withMoveToSidebarToolbarItem = createHigherOrderComponent( newSetting( [ ...newSetting(), widgetId ] ); } else { /** - * If there isn't a widgetId, it's most likely a inner block. - * First, remove the block in the original sidebar, + * If there isn't an existing widget with this id, it's most likely an + * inner block. First, remove the block in the original sidebar, * then, create a new widget in the new sidebar and get back its widgetId. */ - const sidebarAdapter = newSidebarControl.sidebarAdapter; + const newSidebarAdapter = newSidebarControl.sidebarAdapter; removeBlock( clientId ); - const addedWidgetIds = sidebarAdapter.setWidgets( [ - ...sidebarAdapter.getWidgets(), + const addedWidgetIds = newSidebarAdapter.setWidgets( [ + ...newSidebarAdapter.getWidgets(), blockToWidget( block ), ] ); // The last non-null id is the added widget's id. diff --git a/packages/customize-widgets/src/index.js b/packages/customize-widgets/src/index.js index 554f19cf05122..ea679beaaaffd 100644 --- a/packages/customize-widgets/src/index.js +++ b/packages/customize-widgets/src/index.js @@ -8,6 +8,7 @@ import { __experimentalRegisterExperimentalCoreBlocks, } from '@wordpress/block-library'; import { + registerInternalWidgetIds, registerLegacyWidgetBlock, registerLegacyWidgetVariations, registerWidgetGroupBlock, @@ -59,6 +60,8 @@ export function initialize( editorName, blockEditorSettings ) { block.name.startsWith( 'core/navigation' ) ); } ); + + registerInternalWidgetIds(); registerCoreBlocks( coreBlocks ); registerLegacyWidgetBlock(); if ( process.env.IS_GUTENBERG_PLUGIN ) { diff --git a/packages/edit-widgets/src/index.js b/packages/edit-widgets/src/index.js index 5495ed11dff9d..0e8b9bf13eae6 100644 --- a/packages/edit-widgets/src/index.js +++ b/packages/edit-widgets/src/index.js @@ -16,6 +16,7 @@ import { } from '@wordpress/block-library'; import { __experimentalFetchLinkSuggestions as fetchLinkSuggestions } from '@wordpress/core-data'; import { + registerInternalWidgetIds, registerLegacyWidgetBlock, registerLegacyWidgetVariations, registerWidgetGroupBlock, @@ -85,7 +86,10 @@ export function initialize( id, settings ) { themeStyles: true, } ); + registerInternalWidgetIds(); + dispatch( blocksStore ).__experimentalReapplyBlockTypeFilters(); + registerCoreBlocks( coreBlocks ); registerLegacyWidgetBlock(); if ( process.env.IS_GUTENBERG_PLUGIN ) { diff --git a/packages/server-side-render/package.json b/packages/server-side-render/package.json index 5c9390431f6b0..065a2d58eb292 100644 --- a/packages/server-side-render/package.json +++ b/packages/server-side-render/package.json @@ -28,7 +28,6 @@ "dependencies": { "@babel/runtime": "^7.16.0", "@wordpress/api-fetch": "file:../api-fetch", - "@wordpress/blocks": "file:../blocks", "@wordpress/components": "file:../components", "@wordpress/compose": "file:../compose", "@wordpress/data": "file:../data", diff --git a/packages/server-side-render/src/server-side-render.js b/packages/server-side-render/src/server-side-render.js index 35338196f6c2b..7fa5426d4d888 100644 --- a/packages/server-side-render/src/server-side-render.js +++ b/packages/server-side-render/src/server-side-render.js @@ -12,7 +12,6 @@ import { __, sprintf } from '@wordpress/i18n'; import apiFetch from '@wordpress/api-fetch'; import { addQueryArgs } from '@wordpress/url'; import { Placeholder, Spinner } from '@wordpress/components'; -import { __experimentalSanitizeBlockAttributes } from '@wordpress/blocks'; export function rendererPath( block, attributes = null, urlQueryArgs = {} ) { return addQueryArgs( `/wp/v2/block-renderer/${ block }`, { @@ -88,20 +87,12 @@ export default function ServerSideRender( props ) { setIsLoading( true ); - const sanitizedAttributes = - attributes && - __experimentalSanitizeBlockAttributes( block, attributes ); - // If httpMethod is 'POST', send the attributes in the request body instead of the URL. // This allows sending a larger attributes object than in a GET request, where the attributes are in the URL. const isPostRequest = 'POST' === httpMethod; - const urlAttributes = isPostRequest - ? null - : sanitizedAttributes ?? null; + const urlAttributes = isPostRequest ? null : attributes ?? null; const path = rendererPath( block, urlAttributes, urlQueryArgs ); - const data = isPostRequest - ? { attributes: sanitizedAttributes ?? null } - : null; + const data = isPostRequest ? { attributes: attributes ?? null } : null; // Store the latest fetch request so that when we process it, we can // check if it is the current request, to avoid race conditions on slow networks. diff --git a/packages/widgets/package.json b/packages/widgets/package.json index e66199099fcae..1b866d8e8295a 100644 --- a/packages/widgets/package.json +++ b/packages/widgets/package.json @@ -29,6 +29,7 @@ "@wordpress/core-data": "file:../core-data", "@wordpress/data": "file:../data", "@wordpress/element": "file:../element", + "@wordpress/hooks": "file:../hooks", "@wordpress/i18n": "file:../i18n", "@wordpress/icons": "file:../icons", "@wordpress/notices": "file:../notices", diff --git a/packages/widgets/src/utils.js b/packages/widgets/src/utils.js index 75b1f3d08140e..50b026ddd615a 100644 --- a/packages/widgets/src/utils.js +++ b/packages/widgets/src/utils.js @@ -1,4 +1,8 @@ // @ts-check +/** + * WordPress dependencies + */ +import { addFilter } from '@wordpress/hooks'; /** * Get the internal widget id from block. @@ -31,3 +35,32 @@ export function addWidgetIdToBlock( block, widgetId ) { }, }; } + +/** + * Filters registered block settings, extending attributes to include + * `__internalWidgetId` if needed. + * + * @param {Object} settings Original block settings. + * + * @return {Object} Updated block settings. + */ +function addInternalWidgetIdAttribute( settings ) { + return { + ...settings, + attributes: { + ...settings.attributes, + __internalWidgetId: { + type: 'string', + __experimentalRole: 'internal', + }, + }, + }; +} + +export function registerInternalWidgetIds() { + addFilter( + 'blocks.registerBlockType', + 'core/widget/addAttributes', + addInternalWidgetIdAttribute + ); +}