[Commands]: Add group/ungroup commands only when eligible (#53988)

* [Commands]: Add group/ungroup commands only when eligible * fix commands prefix and callback close * pass through client ids in useConvertToGroupButtonProps * address feedback
WordPress · Sep 4, 2023 · c7ff22a · c7ff22a
1 parent b1e6bf5
commit c7ff22a
Show file tree

Hide file tree

Showing 6 changed files with 129 additions and 67 deletions.
diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md
@@ -1077,6 +1077,19 @@ _Returns_
 
 -   `boolean`: Whether block is first in multi-selection.
 
+### isGroupable
+
+Indicates if the provided blocks(by client ids) are groupable. We need to have at least one block, have a grouping block name set and be able to remove these blocks.
+
+_Parameters_
+
+-   _state_ `Object`: Global application state.
+-   _clientIds_ `string[]`: Block client ids. If not passed the selected blocks client ids will be used.
+
+_Returns_
+
+-   `boolean`: True if the blocks are groupable.
+
 ### isLastBlockChangePersistent
 
 Returns true if the most recent block change is be considered persistent, or false otherwise. A persistent change is one committed by BlockEditorProvider via its `onChange` callback, in addition to `onInput`.
@@ -1141,6 +1154,19 @@ _Returns_
 
 -   `boolean`: Whether user is typing.
 
+### isUngroupable
+
+Indicates if a block is ungroupable. A block is ungroupable if it is a single grouping block with inner blocks. If a block has an `ungroup` transform, it is also ungroupable, without the requirement of being the default grouping block. Additionally a block can only be ungrouped if it has inner blocks and can be removed.
+
+_Parameters_
+
+-   _state_ `Object`: Global application state.
+-   _clientId_ `string`: Client Id of the block. If not passed the selected block's client id will be used.
+
+_Returns_
+
+-   `boolean`: True if the block is ungroupable.
+
 ### isValidTemplate
 
 Returns whether the blocks matches the template or not.

diff --git a/packages/block-editor/src/components/block-settings-menu-controls/index.js b/packages/block-editor/src/components/block-settings-menu-controls/index.js
@@ -29,19 +29,15 @@ const BlockSettingsMenuControlsSlot = ( {
 	clientIds = null,
 	__unstableDisplayLocation,
 } ) => {
-	const { selectedBlocks, selectedClientIds, canRemove } = useSelect(
+	const { selectedBlocks, selectedClientIds } = useSelect(
 		( select ) => {
-			const {
-				getBlockNamesByClientId,
-				getSelectedBlockClientIds,
-				canRemoveBlocks,
-			} = select( blockEditorStore );
+			const { getBlockNamesByClientId, getSelectedBlockClientIds } =
+				select( blockEditorStore );
 			const ids =
 				clientIds !== null ? clientIds : getSelectedBlockClientIds();
 			return {
 				selectedBlocks: getBlockNamesByClientId( ids ),
 				selectedClientIds: ids,
-				canRemove: canRemoveBlocks( ids ),
 			};
 		},
 		[ clientIds ]
@@ -55,8 +51,7 @@ const BlockSettingsMenuControlsSlot = ( {
 	const convertToGroupButtonProps =
 		useConvertToGroupButtonProps( selectedClientIds );
 	const { isGroupable, isUngroupable } = convertToGroupButtonProps;
-	const showConvertToGroupButton =
-		( isGroupable || isUngroupable ) && canRemove;
+	const showConvertToGroupButton = isGroupable || isUngroupable;
 
 	return (
 		<Slot

diff --git a/packages/block-editor/src/components/block-toolbar/block-toolbar-menu.native.js b/packages/block-editor/src/components/block-toolbar/block-toolbar-menu.native.js
@@ -60,7 +60,6 @@ const BlockActionsMenu = ( {
 	rootClientId,
 	selectedBlockClientId,
 	selectedBlockPossibleTransformations,
-	canRemove,
 	// Dispatch.
 	createSuccessNotice,
 	convertToRegularBlocks,
@@ -106,8 +105,7 @@ const BlockActionsMenu = ( {
 		selectedBlockClientId ? [ selectedBlockClientId ] : []
 	);
 	const { isGroupable, isUngroupable } = convertToGroupButtonProps;
-	const showConvertToGroupButton =
-		( isGroupable || isUngroupable ) && canRemove;
+	const showConvertToGroupButton = isGroupable || isUngroupable;
 	const convertToGroupButtons = useConvertToGroupButtons( {
 		...convertToGroupButtonProps,
 	} );
@@ -348,7 +346,6 @@ export default compose(
 			getSelectedBlockClientIds,
 			canInsertBlockType,
 			getTemplateLock,
-			canRemoveBlock,
 		} = select( blockEditorStore );
 		const block = getBlock( clientId );
 		const blockName = getBlockName( clientId );
@@ -385,7 +382,6 @@ export default compose(
 		const selectedBlockPossibleTransformations = selectedBlock
 			? getBlockTransformItems( selectedBlock, rootClientId )
 			: EMPTY_BLOCK_LIST;
-		const canRemove = canRemoveBlock( selectedBlockClientId );
 
 		const isReusableBlockType = block ? isReusableBlock( block ) : false;
 		const reusableBlock = isReusableBlockType
@@ -411,7 +407,6 @@ export default compose(
 			rootClientId,
 			selectedBlockClientId,
 			selectedBlockPossibleTransformations,
-			canRemove,
 		};
 	} ),
 	withDispatch(

diff --git a/...block-editor/src/components/convert-to-group-buttons/use-convert-to-group-button-props.js b/...block-editor/src/components/convert-to-group-buttons/use-convert-to-group-button-props.js
@@ -34,55 +34,28 @@ export default function useConvertToGroupButtonProps( selectedClientIds ) {
 	return useSelect(
 		( select ) => {
 			const {
-				getBlockRootClientId,
 				getBlocksByClientId,
-				canInsertBlockType,
 				getSelectedBlockClientIds,
+				isUngroupable,
+				isGroupable,
 			} = select( blockEditorStore );
 			const { getGroupingBlockName, getBlockType } =
 				select( blocksStore );
 			const clientIds = selectedClientIds?.length
 				? selectedClientIds
 				: getSelectedBlockClientIds();
-			const groupingBlockName = getGroupingBlockName();
-
-			const rootClientId = clientIds?.length
-				? getBlockRootClientId( clientIds[ 0 ] )
-				: undefined;
-
-			const groupingBlockAvailable = canInsertBlockType(
-				groupingBlockName,
-				rootClientId
-			);
-
 			const blocksSelection = getBlocksByClientId( clientIds );
-			const isSingleBlockSelected = blocksSelection.length === 1;
 			const [ firstSelectedBlock ] = blocksSelection;
-			// A block is ungroupable if it is a single grouping block with inner blocks.
-			// If a block has an `ungroup` transform, it is also ungroupable, without the
-			// requirement of being the default grouping block.
-			// Do we have a single grouping Block selected and does that group have inner blocks?
-			const isUngroupable =
-				isSingleBlockSelected &&
-				( firstSelectedBlock.name === groupingBlockName ||
-					getBlockType( firstSelectedBlock.name )?.transforms
-						?.ungroup ) &&
-				!! firstSelectedBlock.innerBlocks.length;
-
-			// Do we have
-			// 1. Grouping block available to be inserted?
-			// 2. One or more blocks selected
-			const isGroupable =
-				groupingBlockAvailable && blocksSelection.length;
-
+			const _isUngroupable =
+				clientIds.length === 1 && isUngroupable( clientIds[ 0 ] );
 			return {
 				clientIds,
-				isGroupable,
-				isUngroupable,
+				isGroupable: isGroupable( clientIds ),
+				isUngroupable: _isUngroupable,
 				blocksSelection,
-				groupingBlockName,
+				groupingBlockName: getGroupingBlockName(),
 				onUngroup:
-					isUngroupable &&
+					_isUngroupable &&
 					getBlockType( firstSelectedBlock.name )?.transforms
 						?.ungroup,
 			};

diff --git a/packages/block-editor/src/components/use-block-commands/index.js b/packages/block-editor/src/components/use-block-commands/index.js
@@ -112,12 +112,18 @@ export const useTransformCommands = () => {
 };
 
 const useActionsCommands = () => {
-	const { clientIds } = useSelect( ( select ) => {
-		const { getSelectedBlockClientIds } = select( blockEditorStore );
+	const { clientIds, isUngroupable, isGroupable } = useSelect( ( select ) => {
+		const {
+			getSelectedBlockClientIds,
+			isUngroupable: _isUngroupable,
+			isGroupable: _isGroupable,
+		} = select( blockEditorStore );
 		const selectedBlockClientIds = getSelectedBlockClientIds();
 
 		return {
 			clientIds: selectedBlockClientIds,
+			isUngroupable: _isUngroupable(),
+			isGroupable: _isGroupable(),
 		};
 	}, [] );
 	const {
@@ -194,20 +200,7 @@ const useActionsCommands = () => {
 		canMoveBlocks( clientIds, rootClientId ) &&
 		getBlockCount( rootClientId ) !== 1;
 
-	const commands = [
-		{
-			name: 'ungroup',
-			label: __( 'Ungroup' ),
-			callback: onUngroup,
-			icon: ungroup,
-		},
-		{
-			name: 'Group',
-			label: __( 'Group' ),
-			callback: onGroup,
-			icon: group,
-		},
-	];
+	const commands = [];
 	if ( canInsertDefaultBlock ) {
 		commands.push(
 			{
@@ -262,7 +255,22 @@ const useActionsCommands = () => {
 			icon: move,
 		} );
 	}
-
+	if ( isUngroupable ) {
+		commands.push( {
+			name: 'ungroup',
+			label: __( 'Ungroup' ),
+			callback: onUngroup,
+			icon: ungroup,
+		} );
+	}
+	if ( isGroupable ) {
+		commands.push( {
+			name: 'Group',
+			label: __( 'Group' ),
+			callback: onGroup,
+			icon: group,
+		} );
+	}
 	return {
 		isLoading: false,
 		commands: commands.map( ( command ) => ( {

diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js
@@ -2941,3 +2941,68 @@ export const getBlockEditingMode = createRegistrySelector(
 			return parentMode === 'contentOnly' ? 'default' : parentMode;
 		}
 );
+
+/**
+ * Indicates if a block is ungroupable.
+ * A block is ungroupable if it is a single grouping block with inner blocks.
+ * If a block has an `ungroup` transform, it is also ungroupable, without the
+ * requirement of being the default grouping block.
+ * Additionally a block can only be ungrouped if it has inner blocks and can
+ * be removed.
+ *
+ * @param {Object} state    Global application state.
+ * @param {string} clientId Client Id of the block. If not passed the selected block's client id will be used.
+ * @return {boolean} True if the block is ungroupable.
+ */
+export const isUngroupable = createRegistrySelector(
+	( select ) =>
+		( state, clientId = '' ) => {
+			const _clientId = clientId || getSelectedBlockClientId( state );
+			if ( ! _clientId ) {
+				return false;
+			}
+			const { getGroupingBlockName } = select( blocksStore );
+			const block = getBlock( state, _clientId );
+			const groupingBlockName = getGroupingBlockName();
+			const _isUngroupable =
+				block &&
+				( block.name === groupingBlockName ||
+					getBlockType( block.name )?.transforms?.ungroup ) &&
+				!! block.innerBlocks.length;
+
+			return _isUngroupable && canRemoveBlock( state, _clientId );
+		}
+);
+
+/**
+ * Indicates if the provided blocks(by client ids) are groupable.
+ * We need to have at least one block, have a grouping block name set and
+ * be able to remove these blocks.
+ *
+ * @param {Object}   state     Global application state.
+ * @param {string[]} clientIds Block client ids. If not passed the selected blocks client ids will be used.
+ * @return {boolean} True if the blocks are groupable.
+ */
+export const isGroupable = createRegistrySelector(
+	( select ) =>
+		( state, clientIds = EMPTY_ARRAY ) => {
+			const { getGroupingBlockName } = select( blocksStore );
+			const groupingBlockName = getGroupingBlockName();
+			const _clientIds = clientIds?.length
+				? clientIds
+				: getSelectedBlockClientIds( state );
+			const rootClientId = _clientIds?.length
+				? getBlockRootClientId( state, _clientIds[ 0 ] )
+				: undefined;
+			const groupingBlockAvailable = canInsertBlockType(
+				state,
+				groupingBlockName,
+				rootClientId
+			);
+			const _isGroupable = groupingBlockAvailable && _clientIds.length;
+			return (
+				_isGroupable &&
+				canRemoveBlocks( state, _clientIds, rootClientId )
+			);
+		}
+);