From f619b7e10341c77bd821215b8be981894ddb0c1e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 14:25:15 +0000 Subject: [PATCH 01/10] Apply refactor from 6.4 branch --- .../src/components/block-rename/index.js | 3 + .../block-rename/is-empty-string.js | 3 + .../src/components/block-rename/modal.js | 115 +++++++++ .../components/block-rename/rename-control.js | 80 ++++++ .../block-rename/style.scss} | 0 .../block-rename/use-block-rename.js | 20 ++ .../block-settings-menu-controls/index.js | 9 + .../block-editor/src/hooks/block-rename-ui.js | 228 ------------------ .../block-editor/src/hooks/block-rename.js | 52 ++++ packages/block-editor/src/hooks/index.js | 3 +- packages/block-editor/src/style.scss | 2 +- 11 files changed, 284 insertions(+), 231 deletions(-) create mode 100644 packages/block-editor/src/components/block-rename/index.js create mode 100644 packages/block-editor/src/components/block-rename/is-empty-string.js create mode 100644 packages/block-editor/src/components/block-rename/modal.js create mode 100644 packages/block-editor/src/components/block-rename/rename-control.js rename packages/block-editor/src/{hooks/block-rename-ui.scss => components/block-rename/style.scss} (100%) create mode 100644 packages/block-editor/src/components/block-rename/use-block-rename.js delete mode 100644 packages/block-editor/src/hooks/block-rename-ui.js create mode 100644 packages/block-editor/src/hooks/block-rename.js diff --git a/packages/block-editor/src/components/block-rename/index.js b/packages/block-editor/src/components/block-rename/index.js new file mode 100644 index 00000000000000..0379893d412ec9 --- /dev/null +++ b/packages/block-editor/src/components/block-rename/index.js @@ -0,0 +1,3 @@ +export { default as BlockRenameControl } from './rename-control'; +export { default as BlockRenameModal } from './modal'; +export { default as useBlockRename } from './use-block-rename'; diff --git a/packages/block-editor/src/components/block-rename/is-empty-string.js b/packages/block-editor/src/components/block-rename/is-empty-string.js new file mode 100644 index 00000000000000..42d88be77b96e5 --- /dev/null +++ b/packages/block-editor/src/components/block-rename/is-empty-string.js @@ -0,0 +1,3 @@ +export default function isEmptyString( testString ) { + return testString?.trim()?.length === 0; +} diff --git a/packages/block-editor/src/components/block-rename/modal.js b/packages/block-editor/src/components/block-rename/modal.js new file mode 100644 index 00000000000000..a1e9193f348fd0 --- /dev/null +++ b/packages/block-editor/src/components/block-rename/modal.js @@ -0,0 +1,115 @@ +/** + * WordPress dependencies + */ +import { + __experimentalHStack as HStack, + __experimentalVStack as VStack, + Button, + TextControl, + Modal, +} from '@wordpress/components'; +import { useInstanceId } from '@wordpress/compose'; +import { __, sprintf } from '@wordpress/i18n'; +import { useState } from '@wordpress/element'; +import { speak } from '@wordpress/a11y'; + +/** + * Internal dependencies + */ +import isEmptyString from './is-empty-string'; + +export default function BlockRenameModal( { + blockName, + originalBlockName, + onClose, + onSave, +} ) { + const [ editedBlockName, setEditedBlockName ] = useState( blockName ); + + const nameHasChanged = editedBlockName !== blockName; + const nameIsOriginal = editedBlockName === originalBlockName; + const nameIsEmpty = isEmptyString( editedBlockName ); + + const isNameValid = nameHasChanged || nameIsOriginal; + + const autoSelectInputText = ( event ) => event.target.select(); + + const dialogDescription = useInstanceId( + BlockRenameModal, + `block-editor-rename-modal__description` + ); + + const handleSubmit = () => { + const message = + nameIsOriginal || nameIsEmpty + ? sprintf( + /* translators: %s: new name/label for the block */ + __( 'Block name reset to: "%s".' ), + editedBlockName + ) + : sprintf( + /* translators: %s: new name/label for the block */ + __( 'Block name changed to: "%s".' ), + editedBlockName + ); + + // Must be assertive to immediately announce change. + speak( message, 'assertive' ); + onSave( editedBlockName ); + + // Immediate close avoids ability to hit save multiple times. + onClose(); + }; + + return ( + +

+ { __( 'Enter a custom name for this block.' ) } +

+
{ + e.preventDefault(); + + if ( ! isNameValid ) { + return; + } + + handleSubmit(); + } } + > + + + + + + + + +
+ + ); +} diff --git a/packages/block-editor/src/components/block-rename/rename-control.js b/packages/block-editor/src/components/block-rename/rename-control.js new file mode 100644 index 00000000000000..1f646126d14a4b --- /dev/null +++ b/packages/block-editor/src/components/block-rename/rename-control.js @@ -0,0 +1,80 @@ +/** + * WordPress dependencies + */ +import { MenuItem } from '@wordpress/components'; +import { useSelect, useDispatch } from '@wordpress/data'; +import { __ } from '@wordpress/i18n'; +import { useState } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; +import { useBlockDisplayInformation } from '..'; +import isEmptyString from './is-empty-string'; +import BlockRenameModal from './modal'; + +export default function BlockRenameControl( { clientId } ) { + const [ renamingBlock, setRenamingBlock ] = useState( false ); + + const { metadata } = useSelect( + ( select ) => { + const { getBlockAttributes } = select( blockEditorStore ); + + const _metadata = getBlockAttributes( clientId )?.metadata; + return { + metadata: _metadata, + }; + }, + [ clientId ] + ); + + const { updateBlockAttributes } = useDispatch( blockEditorStore ); + + const customName = metadata?.name; + + function onChange( newName ) { + updateBlockAttributes( [ clientId ], { + metadata: { + ...( metadata && metadata ), + name: newName, + }, + } ); + } + + const blockInformation = useBlockDisplayInformation( clientId ); + + return ( + <> + { + setRenamingBlock( true ); + } } + aria-expanded={ renamingBlock } + aria-haspopup="dialog" + > + { __( 'Rename' ) } + + { renamingBlock && ( + setRenamingBlock( false ) } + onSave={ ( newName ) => { + // If the new value is the block's original name (e.g. `Group`) + // or it is an empty string then assume the intent is to reset + // the value. Therefore reset the metadata. + if ( + newName === blockInformation?.title || + isEmptyString( newName ) + ) { + newName = undefined; + } + + onChange( newName ); + } } + /> + ) } + + ); +} diff --git a/packages/block-editor/src/hooks/block-rename-ui.scss b/packages/block-editor/src/components/block-rename/style.scss similarity index 100% rename from packages/block-editor/src/hooks/block-rename-ui.scss rename to packages/block-editor/src/components/block-rename/style.scss diff --git a/packages/block-editor/src/components/block-rename/use-block-rename.js b/packages/block-editor/src/components/block-rename/use-block-rename.js new file mode 100644 index 00000000000000..1d75b6e56addbf --- /dev/null +++ b/packages/block-editor/src/components/block-rename/use-block-rename.js @@ -0,0 +1,20 @@ +/** + * WordPress dependencies + */ +import { getBlockSupport } from '@wordpress/blocks'; + +export default function useBlockRename( name ) { + const metaDataSupport = getBlockSupport( + name, + '__experimentalMetadata', + false + ); + + const supportsBlockNaming = !! ( + true === metaDataSupport || metaDataSupport?.name + ); + + return { + canRename: supportsBlockNaming, + }; +} 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 index 9063765e72a031..d7a0b001c294da 100644 --- a/packages/block-editor/src/components/block-settings-menu-controls/index.js +++ b/packages/block-editor/src/components/block-settings-menu-controls/index.js @@ -22,6 +22,8 @@ import { BlockLockMenuItem, useBlockLock } from '../block-lock'; import { store as blockEditorStore } from '../../store'; import BlockModeToggle from '../block-settings-menu/block-mode-toggle'; +import { BlockRenameControl, useBlockRename } from '../block-rename'; + const { Fill, Slot } = createSlotFill( 'BlockSettingsMenuControls' ); const BlockSettingsMenuControlsSlot = ( { @@ -44,7 +46,9 @@ const BlockSettingsMenuControlsSlot = ( { ); const { canLock } = useBlockLock( selectedClientIds[ 0 ] ); + const { canRename } = useBlockRename( selectedBlocks[ 0 ] ); const showLockButton = selectedClientIds.length === 1 && canLock; + const showRenameButton = selectedClientIds.length === 1 && canRename; // Check if current selection of blocks is Groupable or Ungroupable // and pass this props down to ConvertToGroupButton. @@ -84,6 +88,11 @@ const BlockSettingsMenuControlsSlot = ( { clientId={ selectedClientIds[ 0 ] } /> ) } + { showRenameButton && ( + + ) } { fills } { fillProps?.canMove && ! fillProps?.onlyBlock && ( testString?.trim()?.length === 0; - -function RenameModal( { blockName, originalBlockName, onClose, onSave } ) { - const [ editedBlockName, setEditedBlockName ] = useState( blockName ); - - const nameHasChanged = editedBlockName !== blockName; - const nameIsOriginal = editedBlockName === originalBlockName; - const nameIsEmpty = emptyString( editedBlockName ); - - const isNameValid = nameHasChanged || nameIsOriginal; - - const autoSelectInputText = ( event ) => event.target.select(); - - const dialogDescription = useInstanceId( - RenameModal, - `block-editor-rename-modal__description` - ); - - const handleSubmit = () => { - const message = - nameIsOriginal || nameIsEmpty - ? sprintf( - /* translators: %s: new name/label for the block */ - __( 'Block name reset to: "%s".' ), - editedBlockName - ) - : sprintf( - /* translators: %s: new name/label for the block */ - __( 'Block name changed to: "%s".' ), - editedBlockName - ); - - // Must be assertive to immediately announce change. - speak( message, 'assertive' ); - onSave( editedBlockName ); - - // Immediate close avoids ability to hit save multiple times. - onClose(); - }; - - return ( - -

- { __( 'Enter a custom name for this block.' ) } -

-
{ - e.preventDefault(); - - if ( ! isNameValid ) { - return; - } - - handleSubmit(); - } } - > - - - - - - - - -
- - ); -} - -function BlockRenameControl( props ) { - const [ renamingBlock, setRenamingBlock ] = useState( false ); - - const { clientId, customName, onChange } = props; - - const blockInformation = useBlockDisplayInformation( clientId ); - - return ( - <> - - - - - { ( { selectedClientIds } ) => { - // Only enabled for single selections. - const canRename = - selectedClientIds.length === 1 && - clientId === selectedClientIds[ 0 ]; - - // This check ensures the `BlockSettingsMenuControls` fill - // doesn't render multiple times and also that it renders for - // the block from which the menu was triggered. - if ( ! canRename ) { - return null; - } - - return ( - { - setRenamingBlock( true ); - } } - aria-expanded={ renamingBlock } - aria-haspopup="dialog" - > - { __( 'Rename' ) } - - ); - } } - - - { renamingBlock && ( - setRenamingBlock( false ) } - onSave={ ( newName ) => { - // If the new value is the block's original name (e.g. `Group`) - // or it is an empty string then assume the intent is to reset - // the value. Therefore reset the metadata. - if ( - newName === blockInformation?.title || - emptyString( newName ) - ) { - newName = undefined; - } - - onChange( newName ); - } } - /> - ) } - - ); -} - -export const withBlockRenameControls = createHigherOrderComponent( - ( BlockEdit ) => ( props ) => { - const { clientId, name, attributes, setAttributes, isSelected } = props; - - const supportsBlockNaming = hasBlockSupport( name, 'renaming', true ); - - return ( - <> - { isSelected && supportsBlockNaming && ( - <> - { - setAttributes( { - metadata: { - ...attributes?.metadata, - name: newName, - }, - } ); - } } - /> - - ) } - - - - ); - }, - 'withBlockRenameControls' -); - -addFilter( - 'editor.BlockEdit', - 'core/block-rename-ui/with-block-rename-controls', - withBlockRenameControls -); diff --git a/packages/block-editor/src/hooks/block-rename.js b/packages/block-editor/src/hooks/block-rename.js new file mode 100644 index 00000000000000..d8f414a3e9ae80 --- /dev/null +++ b/packages/block-editor/src/hooks/block-rename.js @@ -0,0 +1,52 @@ +/** + * WordPress dependencies + */ +import { createHigherOrderComponent } from '@wordpress/compose'; +import { addFilter } from '@wordpress/hooks'; +import { __ } from '@wordpress/i18n'; +import { hasBlockSupport } from '@wordpress/blocks'; +import { TextControl } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import { InspectorControls } from '../components'; + +export const withBlockRenameControl = createHigherOrderComponent( + ( BlockEdit ) => ( props ) => { + const { name, attributes, setAttributes, isSelected } = props; + + const supportsBlockNaming = hasBlockSupport( name, 'renaming', true ); + + return ( + <> + { isSelected && supportsBlockNaming && ( + + { + setAttributes( { + metadata: { + ...attributes?.metadata, + name: newName, + }, + } ); + } } + /> + + ) } + + + + ); + }, + 'withToolbarControls' +); + +addFilter( + 'editor.BlockEdit', + 'core/block-rename-ui/with-block-rename-control', + withBlockRenameControl +); diff --git a/packages/block-editor/src/hooks/index.js b/packages/block-editor/src/hooks/index.js index 730f0defe0a635..bf0406dd731dd4 100644 --- a/packages/block-editor/src/hooks/index.js +++ b/packages/block-editor/src/hooks/index.js @@ -21,8 +21,7 @@ import './content-lock-ui'; import './metadata'; import './custom-fields'; import './block-hooks'; -import './block-renaming'; -import './block-rename-ui'; +import './block-rename'; export { useCustomSides } from './dimensions'; export { useLayoutClasses, useLayoutStyles } from './layout'; diff --git a/packages/block-editor/src/style.scss b/packages/block-editor/src/style.scss index 93ab3b69a7aad3..a55756ae6f53d7 100644 --- a/packages/block-editor/src/style.scss +++ b/packages/block-editor/src/style.scss @@ -15,6 +15,7 @@ @import "./components/block-patterns-paging/style.scss"; @import "./components/block-popover/style.scss"; @import "./components/block-preview/style.scss"; +@import "./components/block-rename/style.scss"; @import "./components/block-settings-menu/style.scss"; @import "./components/block-styles/style.scss"; @import "./components/block-switcher/style.scss"; @@ -56,7 +57,6 @@ @import "./hooks/padding.scss"; @import "./hooks/position.scss"; @import "./hooks/typography.scss"; -@import "./hooks/block-rename-ui.scss"; @import "./components/block-toolbar/style.scss"; @import "./components/inserter/style.scss"; From 04ce9fa3c2c64e124d12b69b50e67712e3744791 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 14:28:17 +0000 Subject: [PATCH 02/10] Apply fix to lock down rename --- .../block-editor/src/hooks/block-rename.js | 6 +- .../editor/various/block-renaming.spec.js | 64 +++++++++++-------- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/block-editor/src/hooks/block-rename.js b/packages/block-editor/src/hooks/block-rename.js index d8f414a3e9ae80..60d13760f2baa5 100644 --- a/packages/block-editor/src/hooks/block-rename.js +++ b/packages/block-editor/src/hooks/block-rename.js @@ -16,7 +16,11 @@ export const withBlockRenameControl = createHigherOrderComponent( ( BlockEdit ) => ( props ) => { const { name, attributes, setAttributes, isSelected } = props; - const supportsBlockNaming = hasBlockSupport( name, 'renaming', true ); + const supportsBlockNaming = hasBlockSupport( + name, + '__experimentalMetadata', + false + ); return ( <> diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index f8d9548fbe8667..66fb1d2810cba8 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -144,25 +144,40 @@ test.describe( 'Block Renaming', () => { ] ); } ); - test( 'does not allow renaming of blocks that do not support renaming', async ( { - // use `core/template-part` as the block + test( 'does not allow renaming of blocks that do not support the feature', async ( { editor, page, + pageUtils, } ) => { + await pageUtils.pressKeys( 'access+o' ); + + const listView = page.getByRole( 'treegrid', { + name: 'Block navigation structure', + } ); + + // Only Group supports renaming. await editor.insertBlock( { - name: 'core/navigation', + name: 'core/paragraph', + attributes: { content: 'First Paragraph' }, } ); - // Opens the block options menu and check there is not a `Rename` option - await editor.clickBlockToolbarButton( 'Options' ); - // + // Multiselect via keyboard. + await pageUtils.pressKeys( 'primary+a' ); - const renameMenuItem = page.getByRole( 'menuitem', { - name: 'Rename', + const listViewParagraphNode = listView.getByRole( 'gridcell', { + name: 'Paragraph', + exact: true, + selected: true, } ); - // TODO: assert that the locator didn't find a DOM node at all. - await expect( renameMenuItem ).toBeHidden(); + await expect( listViewParagraphNode ).toBeVisible(); + + // Expect the Rename control not to exist at all. + await expect( + listViewParagraphNode.getByRole( 'menuitem', { + name: 'Rename', + } ) + ).toBeHidden(); } ); } ); @@ -239,23 +254,21 @@ test.describe( 'Block Renaming', () => { ] ); } ); - test( 'does not allow renaming of blocks that do not support renaming', async ( { + test( 'does now allow renaming of blocks that do not support the feature', async ( { editor, page, + pageUtils, } ) => { + // Only Group supports renaming. await editor.insertBlock( { - name: 'core/navigation', + name: 'core/paragraph', + attributes: { content: 'First Paragraph' }, } ); - await editor.openDocumentSettingsSidebar(); - - const settingsTab = page - .getByRole( 'region', { - name: 'Editor settings', - } ) - .getByRole( 'tab', { name: 'Settings' } ); + // Multiselect via keyboard. + await pageUtils.pressKeys( 'primary+a' ); - await settingsTab.click(); + await editor.openDocumentSettingsSidebar(); const advancedPanelToggle = page .getByRole( 'region', { @@ -268,11 +281,12 @@ test.describe( 'Block Renaming', () => { await advancedPanelToggle.click(); - const nameInput = page.getByRole( 'textbox', { - name: 'Block name', - } ); - - await expect( nameInput ).toBeHidden(); + // Expect the Rename control not to exist at all. + await expect( + page.getByRole( 'textbox', { + name: 'Block name', + } ) + ).toBeHidden(); } ); } ); } ); From 95c006a8f5a11a837ce1a53c065cdc7441393c6a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 14:46:59 +0000 Subject: [PATCH 03/10] Update support hook to check correct support value --- .../src/components/block-rename/use-block-rename.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/block-rename/use-block-rename.js b/packages/block-editor/src/components/block-rename/use-block-rename.js index 1d75b6e56addbf..7dc3d149fd4774 100644 --- a/packages/block-editor/src/components/block-rename/use-block-rename.js +++ b/packages/block-editor/src/components/block-rename/use-block-rename.js @@ -4,11 +4,7 @@ import { getBlockSupport } from '@wordpress/blocks'; export default function useBlockRename( name ) { - const metaDataSupport = getBlockSupport( - name, - '__experimentalMetadata', - false - ); + const metaDataSupport = getBlockSupport( name, 'renaming', true ); const supportsBlockNaming = !! ( true === metaDataSupport || metaDataSupport?.name From 79cca563a23b7d1630fee3700b577255cc7add00 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 14:47:12 +0000 Subject: [PATCH 04/10] Rename hook to match feature --- .../block-editor/src/hooks/block-rename.js | 56 ------------------- .../block-editor/src/hooks/block-renaming.js | 47 ++++++++++++++++ packages/block-editor/src/hooks/index.js | 2 +- 3 files changed, 48 insertions(+), 57 deletions(-) delete mode 100644 packages/block-editor/src/hooks/block-rename.js diff --git a/packages/block-editor/src/hooks/block-rename.js b/packages/block-editor/src/hooks/block-rename.js deleted file mode 100644 index 60d13760f2baa5..00000000000000 --- a/packages/block-editor/src/hooks/block-rename.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * WordPress dependencies - */ -import { createHigherOrderComponent } from '@wordpress/compose'; -import { addFilter } from '@wordpress/hooks'; -import { __ } from '@wordpress/i18n'; -import { hasBlockSupport } from '@wordpress/blocks'; -import { TextControl } from '@wordpress/components'; - -/** - * Internal dependencies - */ -import { InspectorControls } from '../components'; - -export const withBlockRenameControl = createHigherOrderComponent( - ( BlockEdit ) => ( props ) => { - const { name, attributes, setAttributes, isSelected } = props; - - const supportsBlockNaming = hasBlockSupport( - name, - '__experimentalMetadata', - false - ); - - return ( - <> - { isSelected && supportsBlockNaming && ( - - { - setAttributes( { - metadata: { - ...attributes?.metadata, - name: newName, - }, - } ); - } } - /> - - ) } - - - - ); - }, - 'withToolbarControls' -); - -addFilter( - 'editor.BlockEdit', - 'core/block-rename-ui/with-block-rename-control', - withBlockRenameControl -); diff --git a/packages/block-editor/src/hooks/block-renaming.js b/packages/block-editor/src/hooks/block-renaming.js index 5db06d1a652d41..48e3b801d4eb91 100644 --- a/packages/block-editor/src/hooks/block-renaming.js +++ b/packages/block-editor/src/hooks/block-renaming.js @@ -3,6 +3,15 @@ */ import { addFilter } from '@wordpress/hooks'; import { hasBlockSupport } from '@wordpress/blocks'; +import { createHigherOrderComponent } from '@wordpress/compose'; +import { __ } from '@wordpress/i18n'; +import { TextControl } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import { InspectorControls } from '../components'; +import { useBlockRename } from '../components/block-rename'; /** * Filters registered block settings, adding an `__experimentalLabel` callback if one does not already exist. @@ -38,6 +47,44 @@ export function addLabelCallback( settings ) { return settings; } +export const withBlockRenameControl = createHigherOrderComponent( + ( BlockEdit ) => ( props ) => { + const { name, attributes, setAttributes, isSelected } = props; + + const { canRename } = useBlockRename( name ); + + return ( + <> + { isSelected && canRename && ( + + { + setAttributes( { + metadata: { + ...attributes?.metadata, + name: newName, + }, + } ); + } } + /> + + ) } + + + ); + }, + 'withToolbarControls' +); + +addFilter( + 'editor.BlockEdit', + 'core/block-rename-ui/with-block-rename-control', + withBlockRenameControl +); + addFilter( 'blocks.registerBlockType', 'core/metadata/addLabelCallback', diff --git a/packages/block-editor/src/hooks/index.js b/packages/block-editor/src/hooks/index.js index bf0406dd731dd4..c088216c0645cb 100644 --- a/packages/block-editor/src/hooks/index.js +++ b/packages/block-editor/src/hooks/index.js @@ -21,7 +21,7 @@ import './content-lock-ui'; import './metadata'; import './custom-fields'; import './block-hooks'; -import './block-rename'; +import './block-renaming'; export { useCustomSides } from './dimensions'; export { useLayoutClasses, useLayoutStyles } from './layout'; From 209841fec7e4e4fa95da81dacbd587352fa8d460 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 14:48:28 +0000 Subject: [PATCH 05/10] Correct test name --- test/e2e/specs/editor/various/block-renaming.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index 66fb1d2810cba8..f9e161407d4b59 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -254,7 +254,7 @@ test.describe( 'Block Renaming', () => { ] ); } ); - test( 'does now allow renaming of blocks that do not support the feature', async ( { + test( 'does not allow renaming of blocks that do not support the feature', async ( { editor, page, pageUtils, From 233dd2f9f6dcf3c603d5602ed76b880593ff762e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 21 Nov 2023 16:04:59 +0000 Subject: [PATCH 06/10] =?UTF-8?q?Assert=20against=20a=20Block=20that=20doe?= =?UTF-8?q?sn=E2=80=99t=20support=20the=20feature?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../editor/various/block-renaming.spec.js | 54 ++++++++++++------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index f9e161407d4b59..a7b8d677913fb0 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -4,8 +4,30 @@ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'Block Renaming', () => { - test.beforeEach( async ( { admin } ) => { + test.beforeEach( async ( { admin, page } ) => { await admin.createNewPost(); + + // Registering block must be after creation of Post. + await page.evaluate( () => { + const registerBlockType = window.wp.blocks.registerBlockType; + + registerBlockType( + 'my-plugin/block-that-does-not-support-renaming', + { + title: 'No Rename Support Block', + icon: 'smiley', + supports: { + renaming: false, + }, + edit() { + return null; + }, + save() { + return null; + }, + } + ); + } ); } ); test.describe( 'Dialog renaming', () => { @@ -155,29 +177,25 @@ test.describe( 'Block Renaming', () => { name: 'Block navigation structure', } ); - // Only Group supports renaming. await editor.insertBlock( { - name: 'core/paragraph', - attributes: { content: 'First Paragraph' }, + name: 'my-plugin/block-that-does-not-support-renaming', } ); - // Multiselect via keyboard. + // Select via keyboard. await pageUtils.pressKeys( 'primary+a' ); - const listViewParagraphNode = listView.getByRole( 'gridcell', { - name: 'Paragraph', - exact: true, - selected: true, + const blockOptionsTrigger = listView.getByRole( 'button', { + name: 'Options for No Rename Support Block', } ); - await expect( listViewParagraphNode ).toBeVisible(); + await blockOptionsTrigger.click(); - // Expect the Rename control not to exist at all. - await expect( - listViewParagraphNode.getByRole( 'menuitem', { - name: 'Rename', - } ) - ).toBeHidden(); + const renameMenuItem = page.getByRole( 'menuitem', { + name: 'Rename', + } ); + + // Expect the Rename menu item not to exist at all. + await expect( renameMenuItem ).toBeHidden(); } ); } ); @@ -259,10 +277,8 @@ test.describe( 'Block Renaming', () => { page, pageUtils, } ) => { - // Only Group supports renaming. await editor.insertBlock( { - name: 'core/paragraph', - attributes: { content: 'First Paragraph' }, + name: 'my-plugin/block-that-does-not-support-renaming', } ); // Multiselect via keyboard. From f3d32b899631d4aed9a73c7621cf085a42e93447 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 22 Nov 2023 09:30:00 +0000 Subject: [PATCH 07/10] Ensure presence of options menu before Rename presence assertion --- test/e2e/specs/editor/various/block-renaming.spec.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index a7b8d677913fb0..e00c5cb88720f3 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -190,9 +190,13 @@ test.describe( 'Block Renaming', () => { await blockOptionsTrigger.click(); - const renameMenuItem = page.getByRole( 'menuitem', { - name: 'Rename', - } ); + const renameMenuItem = page + .getByRole( 'menu', { + name: 'Options for No Rename Support Block', + } ) + .getByRole( 'menuitem', { + name: 'Rename', + } ); // Expect the Rename menu item not to exist at all. await expect( renameMenuItem ).toBeHidden(); From 79ba2ccd3934c8a9e9759fa3c3b5a7a6b329833c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 22 Nov 2023 09:32:33 +0000 Subject: [PATCH 08/10] Avoid redundant creation of blocks and use Group directly --- .../editor/various/block-renaming.spec.js | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index e00c5cb88720f3..e3050fe36dec8e 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -45,18 +45,12 @@ test.describe( 'Block Renaming', () => { name: 'Block navigation structure', } ); - // Create a two blocks on the page. await editor.insertBlock( { - name: 'core/paragraph', + name: 'core/group', attributes: { content: 'First Paragraph' }, } ); - await editor.insertBlock( { - name: 'core/paragraph', - attributes: { content: 'Second Paragraph' }, - } ); - // Multiselect via keyboard. - await pageUtils.pressKeys( 'primary+a' ); + // Select via keyboard. await pageUtils.pressKeys( 'primary+a' ); // Convert to a Group block which supports renaming. @@ -209,18 +203,11 @@ test.describe( 'Block Renaming', () => { page, pageUtils, } ) => { - // Create a two blocks on the page. await editor.insertBlock( { - name: 'core/paragraph', - attributes: { content: 'First Paragraph' }, - } ); - await editor.insertBlock( { - name: 'core/paragraph', - attributes: { content: 'Second Paragraph' }, + name: 'core/group', } ); - // Multiselect via keyboard. - await pageUtils.pressKeys( 'primary+a' ); + // Select via keyboard. await pageUtils.pressKeys( 'primary+a' ); // Convert to a Group block which supports renaming. From 77f7a8a6d61db34af3360920c5182142ec09da25 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 22 Nov 2023 09:37:28 +0000 Subject: [PATCH 09/10] Add additional test coverage for options menu on unselected blocks --- .../editor/various/block-renaming.spec.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/e2e/specs/editor/various/block-renaming.spec.js b/test/e2e/specs/editor/various/block-renaming.spec.js index e3050fe36dec8e..19a2818fd3a411 100644 --- a/test/e2e/specs/editor/various/block-renaming.spec.js +++ b/test/e2e/specs/editor/various/block-renaming.spec.js @@ -195,6 +195,51 @@ test.describe( 'Block Renaming', () => { // Expect the Rename menu item not to exist at all. await expect( renameMenuItem ).toBeHidden(); } ); + + test( 'displays Rename option in related menu when block is not selected', async ( { + editor, + page, + pageUtils, + } ) => { + await pageUtils.pressKeys( 'access+o' ); + + const listView = page.getByRole( 'treegrid', { + name: 'Block navigation structure', + } ); + + await editor.insertBlock( { + name: 'core/heading', + } ); + + await editor.insertBlock( { + name: 'core/paragraph', + } ); + + // Select the Paragraph block. + await listView + .getByRole( 'link', { + name: 'Paragraph', + } ) + .click(); + + // Trigger options menu for the Heading (not the selected block). + const blockOptionsTrigger = listView.getByRole( 'button', { + name: 'Options for Heading', + } ); + + await blockOptionsTrigger.click(); + + const renameMenuItem = page + .getByRole( 'menu', { + name: 'Options for Heading', + } ) + .getByRole( 'menuitem', { + name: 'Rename', + } ); + + // Expect the Rename menu item not to exist at all. + await expect( renameMenuItem ).toBeVisible(); + } ); } ); test.describe( 'Block inspector renaming', () => { From d6f968f44769180f5c57e2d590f2cb2a6f0a40cf Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 22 Nov 2023 11:36:12 +0000 Subject: [PATCH 10/10] Update hook to check for correct support --- .../src/components/block-rename/use-block-rename.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/block-rename/use-block-rename.js b/packages/block-editor/src/components/block-rename/use-block-rename.js index 7dc3d149fd4774..a3fca66f13670e 100644 --- a/packages/block-editor/src/components/block-rename/use-block-rename.js +++ b/packages/block-editor/src/components/block-rename/use-block-rename.js @@ -4,13 +4,7 @@ import { getBlockSupport } from '@wordpress/blocks'; export default function useBlockRename( name ) { - const metaDataSupport = getBlockSupport( name, 'renaming', true ); - - const supportsBlockNaming = !! ( - true === metaDataSupport || metaDataSupport?.name - ); - return { - canRename: supportsBlockNaming, + canRename: getBlockSupport( name, 'renaming', true ), }; }