Enables Block Grouping/UnGrouping using core/group (#14908)

* Experiment with rudity mentory mechanic to allow Block Grouping

Tried a few approaches via `insertBlock/removeBlocks` and even `replaceBlocks` but nothing preserved the undo history apart from this rather brute force method.

* Migrate to `core/group` due to renaming of container Block from `core/section`

* Adds conditionals to hide Group btn if selection is only a single `core/group` Block

* Adds transform and updates Group button implementation to mirror

Adds a `from` transform to the `core/group` Block. Currently this only works for `core/paragraph` but will need to work for all Block types. Updates the convert button to utilise `switchToBlockType` which invokes the same functionality as used in Block transform thereby unifiying the two methods of grouping.

* Adds and applies Group icon

As provided here #14908 (comment)

* Adds editor shortcut for Grouping Blocks

* Add test to check for blocks before attempting replace

* Adds wildcard Block transforms

A major update to the transforms logic to enable wildcard Block transforms.

* Pass Block names into transform callback function - enables dynamic Block creation in wildcard transforms (see `core/group`)
* Add edge cases to Block transformation logic to account for specifying `*` (all) Block types as valid for a transformation
* Remove unwanted test that checks all Blocks are of the same type before allowing a transform on a Multi Block selection

* Reinstate missing createBlock dep after rebase

* Fix to avoid allowing single Group Block to be Grouped

* Update to use more appropriate logical negation operator for comparison

Addresses #14908 (comment)

* Extracts key transform test logic into dedicated methods

Previously hard coded values and difficult to follow logic were making the `switchToBlockType` overly complex. Extracted to well name methods to improve readability and allow to further refactoring.

* Extract method to test for wildcard block transforms

DRYs up code by extracting a function to test for the presence of a wildcard block transform

* Moves logic to allow for wildcard transform into central transform checking method

Previously test to allow wildcard transform to be valid were manually added as edge cases in conditions in predicate functions. This update centralises all logic to test whether a given transform is possible but including the logic that allows wildcard transforms within the main method `isPossibleTransformForSource` which determines whether a given transform is possible.

* Adds UnGrouping mechanic

* Adds UnGroup Icon

* Adds e2e test to cover basic grouping for single and multiple blocks

* Fix edge case with test to detect a single group container Block

* Adds UnGroup keyboard shortcut

* Adds more e2e test coverage

Includes testing grouping via transforms, options menu and keyboard shortcuts

* Adds check for group block availability before displaying grouping UI

Also adds e2e tests to cover this.

* Updates misnamned components

Addresses #14908 (comment)

* Updates to preserve widest width alignment of child block on group container

Previously if one of the child blocks being grouped had a width alignment (eg: wide or full) then the group block did not respect this. This meant that layouts weren’t preserved when grouping. Adds functionality and tests to ensure that when a group is created the widest alignment setting of the child blocks is set on the group block.

* Updates to DRY out tests

* Updates to simplify test setup

Previously API calls were cleaning up blocks. This can be removed because all posts are auto removed before each test is run.

Addresses #14908 (comment)

* Updates to simplify test assertion

Addresses #14908 (comment)

* Combines test cases to simplify and reduce number of test required

Addresses #14908 (comment)

* Updates to combine test for grouping and ungrouping via options menu

Addresses #14908 (comment)

* Adds keyboard shortcut to global keyboard shortcuts modal

* Ensure correct case for group shortcut (ie: lower)

* Add shortcut to Block settings menu dropdown items

* Adds translation context to Group actions in menus

Addresses #14908 (comment)

* Update Block Transforms fixtures to show all Blocks can transform to Group Block

* Updates Keyboard Shortcut test snapshot to include Group/UnGroup

* Fix to ensure Group block accounted for

* Fix Block deletion test failure via keyboard workaround

Due to the addition of the “Group” item into the Block Options toolbar the “Remove Block” item had been removed outside the viewable portion of the Popover (not the popover has a restricted height and allows scrolling to see the additional items if there is insufficient space to display them all).

Pupeteer isn’t able to click on items that are not visible. The simplest work around is to us the keyboard to select the “Remove Block” menu item rather. Trying to scroll a div within Pupeteer is fraught with problems and inconsistencies.

* Fixes Remove Block button helper to be more resilient to change

Previously we relied on the number of tab stops to locate the correct button in the menu. This change uses the actual text of the button to identify it and uses an assertion to ensure that the correct button is explicitly required.

* Rename function to better convey intent

Addresses #14908 (comment)

* Fixes to catch transforms which are invalid for blocks of different types

A check was accidentally removed in `b2af0f2611e2a2bc66c62959dbf483abcbe103a9` which allowed multiple blocks of different types to be considered valid even if they were not. Adds conditional to ensure that unless the transform is a wildcard then we test that all the blocks are of the same type as the first block’s type before considering the transform to be valid.

* Removes redundant snapshots

* Fixes Transforms test to not over-test Group transforms

Previously we tested every block transforming into the Group Block. This was unwanted overhead.

Fixed to only test 1 single Block transforming into Group. Removed redundant snapshots as a result of removing it.

* Fixes e2e test by reinstating helper lost during rebase

* Fix to make Group transform snapshot tests more resilient to change

Now explicity selects paragraph and image blocks to test the Group transform rather than simply testing the first block that transform into a Group. This ensures that if the order of transforms changes in the fixtures the test won’t be accidentally invalidated.

Resolves: #14908

* Updates to DRY out test and reduce test redundancy and performance

Through use of smarter filtering we can get away with a single `it.each()` block which improves perf and removes redundant tests.

Addresses #14908 (comment)

* Adds unit tests to cover new conditionals added for wildcard blocks transforms

* Fixes capitalisation of UnGroup to Ungroup

Resolves #14908 (comment)

* Updates doc block to reflect possible nullable return type

Addresses #14908 (comment)

* Updates Readme with doc update

* Updates to remove unwanted comments

* Updates capitalisatoin of “wildcard”

Addresses #14908 (comment)

* Update comment with more context for future maintainers

Addresses #14908 (comment)

* Updates to remove unwanted level of context on the translation

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>

* Adds tests to cover isWildcardBlockTransform

* Adds tests for isContainerGroupBlock function

Note these test will need updating when we land #15774

* Fixes logic around multi blocks of same type and adds tests

Prevously we had 1 function attempting to test for multiple block selection and checking that the selection was all of the same block type.

This caused bugs within `switchToBlockType` because the logic was confusing. For example, if a selection isn’t a multi block then we don’t need to test that all the blocks are the same.

Separated the two functions and updated conditions in switchToBlockType to reflect this.

Added unit tests to cover two new functions.

* Adds new generator based API signature to Block transforms

Previously the transforms function was pased two arguments

1. attributes
2. innerblocks

This wasn’t extensible and more advanced transformations require more information about the block (eg: name).

To avoid bloating the signature, a progressive enhancement approach is applied whereby if a generator function is passed as the transform then we pass the entire block object to the generator.

This is opt-in only and is backwards compatible with all existing transform functions.

* Adds new apply option method to transform API

Previously we were modifying the existing transform function to conform to the requirements of a new API (ie: receiving the entire block object rather than the individual arguments).

It was decided that introducing a new `apply` option and soft deprecating the old transform option would be preferable. The apply option if provided now takes precedence over the transform option.

This is fully backwards compatible.

See https://wordpress.slack.com/archives/C02QB2JS7/p1559567845087000

* Updates Block Reg docs to cover wildcard transforms

* Updates changelog to document wildcards and transform apply option

* Fix linting error introduce in rebase

* Fixes test util to avoid infinite loops

Previously if the button wasn’t found then the loop would continue forever looking for the button. This would have caused timeouts.

Limits the loop to the number of buttons in the document. Also bails out immediately having found the button.

Resolves #14908 (comment)

* Renames apply to convert to avoid confusion with Func.apply

To avoid potential confusion and overlap with Function.apply, rename to `convert`.

Slack discussion: https://wordpress.slack.com/archives/C02QB2JS7/p1559593099150300?thread_ts=1559571243.134500&cid=C02QB2JS7

MDN Function apply docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply

Resolves: #14908 (comment)

* Fixes unecessary additional var introduced during debugging

* Fix convert API to match established patterns for consistency

Previously `convert` always passed an array of block objects even if there was only a single block.

This is inconsistent with the implementation of the existing `transform` method which passes only a block’s attributes/innerBlocks pair when it is not a multi block.

To retain consistency with the existing `isMultiBlock` paradiagm this updates the API of `convert` to pass a single block object when not in multiblock mode.

* Fixes error in docs

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>

* Fixes doc blocks to match coding style

Resolves #14908 (comment)

* Fixes icon size and color in dropdown for Group/Ungroup

* Updates to remove keyboard shortcuts

Following a discussion it was not possible to achieve a consensus on which shortcuts was most suitable (or indeed whether keyboard shortcuts for this were even a good idea).

As a result this has been descoped from this PR and will be addressed elsewhere. Once merged I will push a new placeholder PR with the foundation for shortcuts in place and others can then amend it.

* Updates snapshot to account for removing keyboard shortcuts

* Removes e2e tests covering keyboard shortcuts

* Fixes unwanted check introduced during debugging

Test for existence of transform is not required and was introduced during debugging. Can be removed.

* Updates to collapse spaces in doc blocks

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>

* Fixes isWildcardBlockTransform to test for Array-ness

Resolves #14908 (comment)

* Fixes incorrect capitalisation of “UnGroup” to “Ungroup”

Addresses #14908 (comment)

* Updates to remove redundant isMultiBlockSelection util function

Addresses #14908 (comment)

* Reinstate missing Ungroup icon

Accidentally got lost during renaming of “UnGroup” to “Ungroup”!

docs/designers-developers/developers/block-api/block-registration.md

@@ -311,6 +311,40 @@ transforms: {
 ```
 {% end %}
 
+In addition to accepting an array of known block types, the `blocks` option also accepts a "wildcard" (`"*"`). This allows for transformations which apply to _all_ block types (eg: all blocks can transform into `core/group`):
+
+{% codetabs %}
+{% ES5 %}
+```js
+transforms: {
+    from: [
+        {
+            type: 'block',
+            blocks: [ '*' ], // wildcard - match any block
+            transform: function( attributes, innerBlocks ) {
+                // transform logic here
+            },
+        },
+    ],
+},
+```
+{% ESNext %}
+```js
+transforms: {
+    from: [
+        {
+            type: 'block',
+            blocks: [ '*' ], // wildcard - match any block
+            transform: ( attributes, innerBlocks ) => {
+                // transform logic here
+            },
+        },
+    ],
+},
+```
+{% end %}
+
+
 A block with innerBlocks can also be transformed from and to another block with innerBlocks.
 
 {% codetabs %}

packages/block-editor/src/components/block-actions/index.js

@@ -8,13 +8,15 @@ import { castArray, first, last, every } from 'lodash';
  */
 import { compose } from '@wordpress/compose';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { cloneBlock, hasBlockSupport } from '@wordpress/blocks';
+import { cloneBlock, hasBlockSupport, switchToBlockType } from '@wordpress/blocks';
 
 function BlockActions( {
 	onDuplicate,
 	onRemove,
 	onInsertBefore,
 	onInsertAfter,
+	onGroup,
+	onUngroup,
 	isLocked,
 	canDuplicate,
 	children,
@@ -24,6 +26,8 @@ function BlockActions( {
 		onRemove,
 		onInsertAfter,
 		onInsertBefore,
+		onGroup,
+		onUngroup,
 		isLocked,
 		canDuplicate,
 	} );
@@ -65,6 +69,7 @@ export default compose( [
 			multiSelect,
 			removeBlocks,
 			insertDefaultBlock,
+			replaceBlocks,
 		} = dispatch( 'core/block-editor' );
 
 		return {
@@ -107,6 +112,39 @@ export default compose( [
 					insertDefaultBlock( {}, rootClientId, lastSelectedIndex + 1 );
 				}
 			},
+			onGroup() {
+				if ( ! blocks.length ) {
+					return;
+				}
+
+				// Activate the `transform` on `core/group` which does the conversion
+				const newBlocks = switchToBlockType( blocks, 'core/group' );
+
+				if ( ! newBlocks ) {
+					return;
+				}
+				replaceBlocks(
+					clientIds,
+					newBlocks
+				);
+			},
+
+			onUngroup() {
+				if ( ! blocks.length ) {
+					return;
+				}
+
+				const innerBlocks = blocks[ 0 ].innerBlocks;
+
+				if ( ! innerBlocks.length ) {
+					return;
+				}
+
+				replaceBlocks(
+					clientIds,
+					innerBlocks
+				);
+			},
 		};
 	} ),
 ] )( BlockActions );

packages/block-library/src/group/index.js

@@ -2,6 +2,7 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
+import { createBlock } from '@wordpress/blocks';
 
 /**
  * Internal dependencies
@@ -25,6 +26,45 @@ export const settings = {
 		anchor: true,
 		html: false,
 	},
+
+	transforms: {
+		from: [
+			{
+				type: 'block',
+				isMultiBlock: true,
+				blocks: [ '*' ],
+				convert( blocks ) {
+					// Avoid transforming a single `core/group` Block
+					if ( blocks.length === 1 && blocks[ 0 ].name === 'core/group' ) {
+						return;
+					}
+
+					const alignments = [ 'wide', 'full' ];
+
+					// Determine the widest setting of all the blocks to be grouped
+					const widestAlignment = blocks.reduce( ( result, block ) => {
+						const { align } = block.attributes;
+						return alignments.indexOf( align ) > alignments.indexOf( result ) ? align : result;
+					}, undefined );
+
+					// Clone the Blocks to be Grouped
+					// Failing to create new block references causes the original blocks
+					// to be replaced in the switchToBlockType call thereby meaning they
+					// are removed both from their original location and within the
+					// new group block.
+					const groupInnerBlocks = blocks.map( ( block ) => {
+						return createBlock( block.name, block.attributes, block.innerBlocks );
+					} );
+
+					return createBlock( 'core/group', {
+						align: widestAlignment,
+					}, groupInnerBlocks );
+				},
+			},
+
+		],
+	},
+
 	edit,
 	save,
 };

packages/blocks/CHANGELOG.md

@@ -3,6 +3,8 @@
 ### New Feature
 
 - Added a default implementation for `save` setting in `registerBlockType` which saves no markup in the post content.
+- Added wildcard block transforms which allows for transforming all/any blocks in another block.
+- Added `convert()` method option to `transforms` definition. It receives complete block object(s) as it's argument(s). It is now preferred over the older `transform()` (note that `transform()` is still fully supported).
 
 ## 6.1.0 (2019-03-06)
 

packages/blocks/README.md

@@ -679,7 +679,7 @@ _Parameters_
 
 _Returns_
 
--   `Array`: Array of blocks.
+-   `?Array`: Array of blocks or null.
 
 <a name="synchronizeBlocksWithTemplate" href="#synchronizeBlocksWithTemplate">#</a> **synchronizeBlocksWithTemplate**
 

packages/blocks/src/api/factory.js

@@ -11,6 +11,7 @@ import {
 	filter,
 	first,
 	flatMap,
+	has,
 	uniq,
 	isFunction,
 	isEmpty,
@@ -117,26 +118,41 @@ const isPossibleTransformForSource = ( transform, direction, blocks ) => {
 		return false;
 	}
 
-	// If multiple blocks are selected, only multi block transforms are allowed.
+	// If multiple blocks are selected, only multi block transforms
+	// or wildcard transforms are allowed.
 	const isMultiBlock = blocks.length > 1;
-	const isValidForMultiBlocks = ! isMultiBlock || transform.isMultiBlock;
+	const firstBlockName = first( blocks ).name;
+	const isValidForMultiBlocks = isWildcardBlockTransform( transform ) || ! isMultiBlock || transform.isMultiBlock;
 	if ( ! isValidForMultiBlocks ) {
 		return false;
 	}
 
+	// Check non-wildcard transforms to ensure that transform is valid
+	// for a block selection of multiple blocks of different types
+	if ( ! isWildcardBlockTransform( transform ) && ! every( blocks, { name: firstBlockName } ) ) {
+		return false;
+	}
+
 	// Only consider 'block' type transforms as valid.
 	const isBlockType = transform.type === 'block';
 	if ( ! isBlockType ) {
 		return false;
 	}
 
-	// Check if the transform's block name matches the source block only if this is a transform 'from'.
+	// Check if the transform's block name matches the source block (or is a wildcard)
+	// only if this is a transform 'from'.
 	const sourceBlock = first( blocks );
-	const hasMatchingName = direction !== 'from' || transform.blocks.indexOf( sourceBlock.name ) !== -1;
+	const hasMatchingName = direction !== 'from' || transform.blocks.indexOf( sourceBlock.name ) !== -1 || isWildcardBlockTransform( transform );
 	if ( ! hasMatchingName ) {
 		return false;
 	}
 
+	// Don't allow single 'core/group' blocks to be transformed into
+	// a 'core/group' block.
+	if ( ! isMultiBlock && isContainerGroupBlock( sourceBlock.name ) && isContainerGroupBlock( transform.blockName ) ) {
+		return false;
+	}
+
 	// If the transform has a `isMatch` function specified, check that it returns true.
 	if ( isFunction( transform.isMatch ) ) {
 		const attributes = transform.isMultiBlock ? blocks.map( ( block ) => block.attributes ) : sourceBlock.attributes;
@@ -171,7 +187,9 @@ const getBlockTypesForPossibleFromTransforms = ( blocks ) => {
 
 			return !! findTransform(
 				fromTransforms,
-				( transform ) => isPossibleTransformForSource( transform, 'from', blocks )
+				( transform ) => {
+					return isPossibleTransformForSource( transform, 'from', blocks );
+				}
 			);
 		},
 	);
@@ -199,7 +217,9 @@ const getBlockTypesForPossibleToTransforms = ( blocks ) => {
 	// filter all 'to' transforms to find those that are possible.
 	const possibleTransforms = filter(
 		transformsTo,
-		( transform ) => isPossibleTransformForSource( transform, 'to', blocks )
+		( transform ) => {
+			return transform && isPossibleTransformForSource( transform, 'to', blocks );
+		}
 	);
 
 	// Build a list of block names using the possible 'to' transforms.
@@ -212,6 +232,45 @@ const getBlockTypesForPossibleToTransforms = ( blocks ) => {
 	return blockNames.map( ( name ) => getBlockType( name ) );
 };
 
+/**
+ * Determines whether transform is a "block" type
+ * and if so whether it is a "wildcard" transform
+ * ie: targets "any" block type
+ *
+ * @param {Object} t the Block transform object
+ *
+ * @return {boolean} whether transform is a wildcard transform
+ */
+export const isWildcardBlockTransform = ( t ) => t && t.type === 'block' && Array.isArray( t.blocks ) && t.blocks.includes( '*' );
+
+/**
+ * Determines whether the given Block is the core Block which
+ * acts as a container Block for other Blocks as part of the
+ * Grouping mechanics
+ *
+ * @param  {string} name the name of the Block to test against
+ *
+ * @return {boolean} whether or not the Block is the container Block type
+ */
+export const isContainerGroupBlock = ( name ) => name === 'core/group';
+
+/**
+ * Determines whether the provided Blocks are of the same type
+ * (eg: all `core/paragraph`).
+ *
+ * @param  {Array}  blocksArray the Block definitions
+ *
+ * @return {boolean} whether or not the given Blocks pass the criteria
+ */
+export const isBlockSelectionOfSameType = ( blocksArray = [] ) => {
+	if ( ! blocksArray.length ) {
+		return false;
+	}
+	const sourceName = blocksArray[ 0 ].name;
+
+	return every( blocksArray, [ 'name', sourceName ] );
+};
+
 /**
  * Returns an array of block types that the set of blocks received as argument
  * can be transformed into.
@@ -225,12 +284,6 @@ export function getPossibleBlockTransformations( blocks ) {
 		return [];
 	}
 
-	const sourceBlock = first( blocks );
-	const isMultiBlock = blocks.length > 1;
-	if ( isMultiBlock && ! every( blocks, { name: sourceBlock.name } ) ) {
-		return [];
-	}
-
 	const blockTypesForFromTransforms = getBlockTypesForPossibleFromTransforms( blocks );
 	const blockTypesForToTransforms = getBlockTypesForPossibleToTransforms( blocks );
 
@@ -313,30 +366,34 @@ export function getBlockTransforms( direction, blockTypeOrName ) {
  * @param {Array|Object} blocks Blocks array or block object.
  * @param {string}       name   Block name.
  *
- * @return {Array} Array of blocks.
+ * @return {?Array} Array of blocks or null.
  */
 export function switchToBlockType( blocks, name ) {
 	const blocksArray = castArray( blocks );
 	const isMultiBlock = blocksArray.length > 1;
 	const firstBlock = blocksArray[ 0 ];
 	const sourceName = firstBlock.name;
 
-	if ( isMultiBlock && ! every( blocksArray, ( block ) => ( block.name === sourceName ) ) ) {
+	// Unless it's a `core/group` Block then for multi block selections
+	// check that all Blocks are of the same type otherwise
+	// we can't run a conversion
+	if ( ! isContainerGroupBlock( name ) && isMultiBlock && ! isBlockSelectionOfSameType( blocksArray ) ) {
 		return null;
 	}
 
 	// Find the right transformation by giving priority to the "to"
 	// transformation.
 	const transformationsFrom = getBlockTransforms( 'from', name );
 	const transformationsTo = getBlockTransforms( 'to', sourceName );
+
 	const transformation =
 		findTransform(
 			transformationsTo,
-			( t ) => t.type === 'block' && t.blocks.indexOf( name ) !== -1 && ( ! isMultiBlock || t.isMultiBlock )
+			( t ) => t.type === 'block' && ( ( isWildcardBlockTransform( t ) ) || t.blocks.indexOf( name ) !== -1 ) && ( ! isMultiBlock || t.isMultiBlock )
 		) ||
 		findTransform(
 			transformationsFrom,
-			( t ) => t.type === 'block' && t.blocks.indexOf( sourceName ) !== -1 && ( ! isMultiBlock || t.isMultiBlock )
+			( t ) => t.type === 'block' && ( ( isWildcardBlockTransform( t ) ) || t.blocks.indexOf( sourceName ) !== -1 ) && ( ! isMultiBlock || t.isMultiBlock )
 		);
 
 	// Stop if there is no valid transformation.
@@ -345,11 +402,18 @@ export function switchToBlockType( blocks, name ) {
 	}
 
 	let transformationResults;
+
 	if ( transformation.isMultiBlock ) {
-		transformationResults = transformation.transform(
-			blocksArray.map( ( currentBlock ) => currentBlock.attributes ),
-			blocksArray.map( ( currentBlock ) => currentBlock.innerBlocks )
-		);
+		if ( has( transformation, 'convert' ) ) {
+			transformationResults = transformation.convert( blocksArray );
+		} else {
+			transformationResults = transformation.transform(
+				blocksArray.map( ( currentBlock ) => currentBlock.attributes ),
+				blocksArray.map( ( currentBlock ) => currentBlock.innerBlocks ),
+			);
+		}
+	} else if ( has( transformation, 'convert' ) ) {
+		transformationResults = transformation.convert( firstBlock );
 	} else {
 		transformationResults = transformation.transform( firstBlock.attributes, firstBlock.innerBlocks );
 	}