Add widget id to blocks in the widgets screen

packages/block-editor/src/components/provider/test/use-block-sync.js

@@ -1,3 +1,8 @@
+/**
+ * WordPress dependencies
+ */
+import { registerBlockType } from '@wordpress/blocks';
+
 /**
  * External dependencies
  */
@@ -19,6 +24,15 @@ const TestWrapper = withRegistryProvider( ( props ) => {
 } );
 
 describe( 'useBlockSync hook', () => {
+	beforeAll( () => {
+		registerBlockType( 'test/test-block', {
+			title: 'Test block',
+			attributes: {
+				foo: { type: 'number' },
+			},
+		} );
+	} );
+
 	afterEach( () => {
 		jest.clearAllMocks();
 	} );
@@ -101,7 +115,12 @@ describe( 'useBlockSync hook', () => {
 		);
 
 		const testBlocks = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 		await act( async () => {
 			root.update(
@@ -131,7 +150,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 		let root;
 		let registry;
@@ -282,7 +306,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 
 		await act( async () => {
@@ -325,7 +354,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 
 		let registry;
@@ -352,7 +386,14 @@ describe( 'useBlockSync hook', () => {
 
 		expect( replaceInnerBlocks ).not.toHaveBeenCalled();
 		expect( onChange ).toHaveBeenCalledWith(
-			[ { clientId: 'a', innerBlocks: [], attributes: { foo: 2 } } ],
+			[
+				{
+					name: 'test/test-block',
+					clientId: 'a',
+					innerBlocks: [],
+					attributes: { foo: 2 },
+				},
+			],
 			{ selectionEnd: {}, selectionStart: {} }
 		);
 		expect( onInput ).not.toHaveBeenCalled();

packages/block-editor/src/components/provider/use-block-sync.js

@@ -63,8 +63,6 @@ import { cloneBlock } from '@wordpress/blocks';
  *                                change has been made in the block-editor blocks
  *                                for the given clientId. When this is called,
  *                                controlling sources do not become dirty.
- * @param {boolean} props.__unstableCloneValue Whether or not to clone each of
- *                                the blocks provided in the value prop.
  */
 export default function useBlockSync( {
 	clientId = null,
@@ -73,7 +71,6 @@ export default function useBlockSync( {
 	selectionEnd: controlledSelectionEnd,
 	onChange = noop,
 	onInput = noop,
-	__unstableCloneValue = true,
 } ) {
 	const registry = useRegistry();
 
@@ -101,12 +98,9 @@ export default function useBlockSync( {
 		if ( clientId ) {
 			setHasControlledInnerBlocks( clientId, true );
 			__unstableMarkNextChangeAsNotPersistent();
-			let storeBlocks = controlledBlocks;
-			if ( __unstableCloneValue ) {
-				storeBlocks = controlledBlocks.map( ( block ) =>
-					cloneBlock( block )
-				);
-			}
+			const storeBlocks = controlledBlocks.map( ( block ) =>
+				cloneBlock( block )
+			);
 			if ( subscribed.current ) {
 				pendingChanges.current.incoming = storeBlocks;
 			}

packages/blocks/src/api/factory.js

@@ -4,7 +4,6 @@
 import { v4 as uuid } from 'uuid';
 import {
 	every,
-	reduce,
 	castArray,
 	findIndex,
 	isObjectLike,
@@ -31,7 +30,7 @@ import {
 	getBlockTypes,
 	getGroupingBlockName,
 } from './registration';
-import { normalizeBlockType } from './utils';
+import { normalizeBlockType, sanitizeBlockAttributes } from './utils';
 
 /**
  * Returns a block object given its type and attributes.
@@ -43,40 +42,7 @@ import { normalizeBlockType } from './utils';
  * @return {Object} Block object.
  */
 export function createBlock( name, attributes = {}, innerBlocks = [] ) {
-	// 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.` );
-	}
-
-	// Ensure attributes contains only values defined by block type, and merge
-	// default values for missing attributes.
-	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 sanitizedAttributes = sanitizeBlockAttributes( name, attributes );
 
 	const clientId = uuid();
 
@@ -134,13 +100,15 @@ export function createBlocksFromInnerBlocksTemplate(
 export function cloneBlock( block, mergeAttributes = {}, newInnerBlocks ) {
 	const clientId = uuid();
 
+	const sanitizedAttributes = sanitizeBlockAttributes( block.name, {
+		...block.attributes,
+		...mergeAttributes,
+	} );
+
 	return {
 		...block,
 		clientId,
-		attributes: {
-			...block.attributes,
-			...mergeAttributes,
-		},
+		attributes: sanitizedAttributes,
 		innerBlocks:
 			newInnerBlocks ||
 			block.innerBlocks.map( ( innerBlock ) => cloneBlock( innerBlock ) ),

packages/blocks/src/api/utils.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { every, has, isFunction, isString, startCase } from 'lodash';
+import { every, has, isFunction, isString, startCase, reduce } from 'lodash';
 import { default as tinycolor, mostReadable } from 'tinycolor2';
 
 /**
@@ -246,3 +246,46 @@ export function getAccessibleBlockLabel(
 		title
 	);
 }
+
+/**
+ * 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 sanitizeBlockAttributes( 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;
+		},
+		{}
+	);
+}

packages/e2e-tests/specs/widgets/adding-widgets.test.js

@@ -306,6 +306,86 @@ describe( 'Widgets screen', () => {
 		}
 	` );
 	} );
+
+	it( 'Should duplicate the widgets', async () => {
+		const firstWidgetArea = await page.$(
+			'[aria-label="Block: Widget Area"][role="group"]'
+		);
+
+		const addParagraphBlock = await getParagraphBlockInGlobalInserter();
+		await addParagraphBlock.click();
+
+		const firstParagraphBlock = await firstWidgetArea.$(
+			'[data-block][data-type="core/paragraph"][aria-label^="Empty block"]'
+		);
+		await page.keyboard.type( 'First Paragraph' );
+
+		// Trigger the toolbar to appear.
+		await page.keyboard.down( 'Shift' );
+		await page.keyboard.press( 'Tab' );
+		await page.keyboard.up( 'Shift' );
+
+		const blockToolbar = await page.waitForSelector(
+			'[role="toolbar"][aria-label="Block tools"]'
+		);
+		const moreOptionsButton = await blockToolbar.$(
+			'button[aria-label="Options"]'
+		);
+		await moreOptionsButton.click();
+
+		const optionsMenu = await page.waitForSelector(
+			'[role="menu"][aria-label="Options"]'
+		);
+		const [ duplicateButton ] = await optionsMenu.$x(
+			'//*[@role="menuitem"][*[text()="Duplicate"]]'
+		);
+		await duplicateButton.click();
+
+		await page.waitForFunction(
+			( paragraph ) =>
+				paragraph.nextSibling &&
+				paragraph.nextSibling.matches( '[data-block]' ),
+			{},
+			firstParagraphBlock
+		);
+		const duplicatedParagraphBlock = await firstParagraphBlock.evaluateHandle(
+			( paragraph ) => paragraph.nextSibling
+		);
+
+		const firstParagraphBlockClientId = await firstParagraphBlock.evaluate(
+			( node ) => node.dataset.block
+		);
+		const duplicatedParagraphBlockClientId = await duplicatedParagraphBlock.evaluate(
+			( node ) => node.dataset.block
+		);
+
+		expect( firstParagraphBlockClientId ).not.toBe(
+			duplicatedParagraphBlockClientId
+		);
+		expect(
+			await duplicatedParagraphBlock.evaluate(
+				( node ) => node.textContent
+			)
+		).toBe( 'First Paragraph' );
+		expect(
+			await duplicatedParagraphBlock.evaluate(
+				( node ) => node === document.activeElement
+			)
+		).toBe( true );
+
+		await saveWidgets();
+		const serializedWidgetAreas = await getSerializedWidgetAreas();
+		expect( serializedWidgetAreas ).toMatchInlineSnapshot( `
+		Object {
+		  "sidebar-1": "<div class=\\"widget widget_block\\"><div class=\\"widget-content\\">
+		<p>First Paragraph</p>
+		</div></div>
+		<div class=\\"widget widget_block\\"><div class=\\"widget-content\\">
+		<p>First Paragraph</p>
+		</div></div>",
+		}
+	` );
+	} );
 } );
 
 async function saveWidgets() {

packages/edit-widgets/src/blocks/legacy-widget/edit/index.js

@@ -192,12 +192,10 @@ function LegacyWidgetEdit( {
 
 export default withSelect( ( select, { clientId, attributes } ) => {
 	const { widgetClass, referenceWidgetName } = attributes;
-	let widgetId = select( editWidgetsStore ).getWidgetIdForClientId(
-		clientId
-	);
+	let widgetId = attributes.__internalWidgetId;
 	const widget = select( editWidgetsStore ).getWidget( widgetId );
-	const widgetArea = select( editWidgetsStore ).getWidgetAreaForClientId(
-		clientId
+	const widgetArea = select( editWidgetsStore ).getWidgetAreaForWidgetId(
+		widgetId
 	);
 	const editorSettings = select( 'core/block-editor' ).getSettings();
 	const { availableLegacyWidgets } = editorSettings;

packages/edit-widgets/src/blocks/widget-area/edit/inner-blocks.js

@@ -16,11 +16,6 @@ export default function WidgetAreaInnerBlocks() {
 			onChange={ onChange }
 			templateLock={ false }
 			renderAppender={ InnerBlocks.DefaultBlockAppender }
-			// HACK: The widget editor relies on a mapping of block client IDs
-			// to widget IDs. We therefore instruct `useBlockSync` to not clone
-			// the blocks it receives which would change the block client IDs`.
-			// See https://github.com/WordPress/gutenberg/issues/27173.
-			__unstableCloneValue={ false }
 		/>
 	);
 }