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 fcf8b13d26be7c..a4087ed84cee3c 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 @@ -103,7 +103,8 @@ export function BlockSettingsDropdown( { }, [ firstBlockClientId ] ); - const { getBlockOrder } = useSelect( blockEditorStore ); + const { getBlockOrder, getSelectedBlockClientIds } = + useSelect( blockEditorStore ); const shortcuts = useSelect( ( select ) => { const { getShortcutRepresentation } = select( keyboardShortcutsStore ); @@ -124,13 +125,14 @@ export function BlockSettingsDropdown( { const { selectBlock, toggleBlockHighlight } = useDispatch( blockEditorStore ); + const hasSelectedBlocks = selectedBlockClientIds.length > 0; const updateSelectionAfterDuplicate = useCallback( async ( clientIdsPromise ) => { if ( __experimentalSelectBlock ) { const ids = await clientIdsPromise; if ( ids && ids[ 0 ] ) { - __experimentalSelectBlock( ids[ 0 ] ); + __experimentalSelectBlock( ids[ 0 ], false ); } } }, @@ -139,20 +141,26 @@ export function BlockSettingsDropdown( { const updateSelectionAfterRemove = useCallback( () => { if ( __experimentalSelectBlock ) { - let blockToSelect = previousBlockClientId || firstParentClientId; + let blockToFocus = previousBlockClientId || firstParentClientId; - // Select the first block if there's no previous block nor parent block. - if ( ! blockToSelect ) { - blockToSelect = getBlockOrder()[ 0 ]; + // Focus the first block if there's no previous block nor parent block. + if ( ! blockToFocus ) { + blockToFocus = getBlockOrder()[ 0 ]; } - __experimentalSelectBlock( blockToSelect ); + // Only update the selection if the original selection is removed. + const shouldUpdateSelection = + hasSelectedBlocks && getSelectedBlockClientIds().length === 0; + + __experimentalSelectBlock( blockToFocus, shouldUpdateSelection ); } }, [ __experimentalSelectBlock, previousBlockClientId, firstParentClientId, getBlockOrder, + hasSelectedBlocks, + getSelectedBlockClientIds, ] ); const removeBlockLabel = @@ -209,12 +217,17 @@ export function BlockSettingsDropdown( { if ( event.defaultPrevented ) return; if ( - isMatch( 'core/block-editor/remove', event ) + isMatch( 'core/block-editor/remove', event ) && + canRemove ) { event.preventDefault(); updateSelectionAfterRemove( onRemove() ); } else if ( - isMatch( 'core/block-editor/duplicate', event ) + isMatch( + 'core/block-editor/duplicate', + event + ) && + canDuplicate ) { event.preventDefault(); updateSelectionAfterDuplicate( onDuplicate() ); @@ -222,7 +235,8 @@ export function BlockSettingsDropdown( { isMatch( 'core/block-editor/insert-after', event - ) + ) && + canInsertDefaultBlock ) { event.preventDefault(); onInsertAfter(); @@ -230,7 +244,8 @@ export function BlockSettingsDropdown( { isMatch( 'core/block-editor/insert-before', event - ) + ) && + canInsertDefaultBlock ) { event.preventDefault(); onInsertBefore(); diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 6af78758149241..ca5e414ae65769 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -41,7 +41,7 @@ function ListViewBlockSelectButton( isExpanded, ariaLabel, ariaDescribedBy, - updateSelection, + updateFocusAndSelection, }, ref ) { @@ -56,6 +56,7 @@ function ListViewBlockSelectButton( getPreviousBlockClientId, getBlockRootClientId, getBlockOrder, + canRemoveBlocks, } = useSelect( blockEditorStore ); const { removeBlocks } = useDispatch( blockEditorStore ); const isMatch = useShortcutEventMatch(); @@ -86,26 +87,37 @@ function ListViewBlockSelectButton( const firstBlockClientId = isDeletingSelectedBlocks ? selectedBlockClientIds[ 0 ] : clientId; + const firstBlockRootClientId = + getBlockRootClientId( firstBlockClientId ); + + const blocksToDelete = isDeletingSelectedBlocks + ? selectedBlockClientIds + : [ clientId ]; + + // Don't update the selection if the blocks cannot be deleted. + if ( ! canRemoveBlocks( blocksToDelete, firstBlockRootClientId ) ) { + return; + } - let blockToSelect = + let blockToFocus = getPreviousBlockClientId( firstBlockClientId ) ?? // If the previous block is not found (when the first block is deleted), // fallback to focus the parent block. - getBlockRootClientId( firstBlockClientId ); + firstBlockRootClientId; + + removeBlocks( blocksToDelete, false ); - removeBlocks( - isDeletingSelectedBlocks - ? selectedBlockClientIds - : [ clientId ], - false - ); + // Update the selection if the original selection has been removed. + const shouldUpdateSelection = + selectedBlockClientIds.length > 0 && + getSelectedBlockClientIds().length === 0; // If there's no previous block nor parent block, focus the first block. - if ( ! blockToSelect ) { - blockToSelect = getBlockOrder()[ 0 ]; + if ( ! blockToFocus ) { + blockToFocus = getBlockOrder()[ 0 ]; } - updateSelection( blockToSelect ); + updateFocusAndSelection( blockToFocus, shouldUpdateSelection ); } } diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index a80f5092479a8d..20a385537f9b8e 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -167,17 +167,15 @@ function ListViewBlock( { [ clientId, selectBlock ] ); - const updateSelection = useCallback( - ( newClientId ) => { - const selectedBlockClientIds = getSelectedBlockClientIds(); - // Select the block to be focused if there isn't any block selected. - if ( ! selectedBlockClientIds.length ) { - selectBlock( undefined, newClientId, null, null ); + const updateFocusAndSelection = useCallback( + ( focusClientId, shouldSelectBlock ) => { + if ( shouldSelectBlock ) { + selectBlock( undefined, focusClientId, null, null ); } const getFocusElement = () => { const row = treeGridElementRef.current?.querySelector( - `[role=row][data-block="${ newClientId }"]` + `[role=row][data-block="${ focusClientId }"]` ); if ( ! row ) return null; // Focus the first focusable in the row, which is the ListViewBlockSelectButton. @@ -297,7 +295,7 @@ function ListViewBlock( { selectedClientIds={ selectedClientIds } ariaLabel={ blockAriaLabel } ariaDescribedBy={ descriptionId } - updateSelection={ updateSelection } + updateFocusAndSelection={ updateFocusAndSelection } />
) } diff --git a/test/e2e/specs/editor/various/list-view.spec.js b/test/e2e/specs/editor/various/list-view.spec.js index 01f930ac2866c5..971d571128bce7 100644 --- a/test/e2e/specs/editor/various/list-view.spec.js +++ b/test/e2e/specs/editor/various/list-view.spec.js @@ -4,6 +4,12 @@ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'List View', () => { + test.use( { + listViewUtils: async ( { page, pageUtils, editor }, use ) => { + await use( new ListViewUtils( { page, pageUtils, editor } ) ); + }, + } ); + test.beforeEach( async ( { admin } ) => { await admin.createNewPost(); } ); @@ -115,146 +121,6 @@ test.describe( 'List View', () => { await expect( listView.getByRole( 'row' ) ).toHaveCount( 2 ); } ); - // Check for regression of https://github.com/WordPress/gutenberg/issues/39026. - test( 'selects the previous block after removing the selected one', async ( { - editor, - page, - pageUtils, - } ) => { - // Insert a couple of blocks of different types. - await editor.insertBlock( { name: 'core/image' } ); - await editor.insertBlock( { name: 'core/heading' } ); - await editor.insertBlock( { name: 'core/paragraph' } ); - - // Open List View. - await pageUtils.pressKeys( 'access+o' ); - const listView = page.getByRole( 'treegrid', { - name: 'Block navigation structure', - } ); - - // The last inserted block should be selected. - await expect( - listView.getByRole( 'gridcell', { - name: 'Paragraph', - exact: true, - selected: true, - } ) - ).toBeVisible(); - - // Remove the Paragraph block via its options menu in List View. - await listView - .getByRole( 'button', { name: 'Options for Paragraph' } ) - .click(); - await page.getByRole( 'menuitem', { name: /Delete/i } ).click(); - - // Heading block should be selected as previous block. - await expect( - editor.canvas.getByRole( 'document', { - name: 'Block: Heading', - } ) - ).toBeFocused(); - } ); - - // Check for regression of https://github.com/WordPress/gutenberg/issues/39026. - test( 'selects the next block after removing the very first block', async ( { - editor, - page, - pageUtils, - } ) => { - // Insert a couple of blocks of different types. - await editor.insertBlock( { name: 'core/image' } ); - await editor.insertBlock( { name: 'core/heading' } ); - await editor.insertBlock( { name: 'core/paragraph' } ); - - // Open List View. - await pageUtils.pressKeys( 'access+o' ); - const listView = page.getByRole( 'treegrid', { - name: 'Block navigation structure', - } ); - - // The last inserted block should be selected. - await expect( - listView.getByRole( 'gridcell', { - name: 'Paragraph', - exact: true, - selected: true, - } ) - ).toBeVisible(); - - // Select the image block in List View. - await pageUtils.pressKeys( 'ArrowUp', { times: 2 } ); - await expect( - listView.getByRole( 'link', { - name: 'Image', - } ) - ).toBeFocused(); - await page.keyboard.press( 'Enter' ); - - // Remove the Image block via its options menu in List View. - await listView - .getByRole( 'button', { name: 'Options for Image' } ) - .click(); - await page.getByRole( 'menuitem', { name: /Delete/i } ).click(); - - // Heading block should be selected as previous block. - await expect( - editor.canvas.getByRole( 'document', { - name: 'Block: Heading', - } ) - ).toBeFocused(); - } ); - - /** - * When all the blocks gets removed from the editor, it inserts a default - * paragraph block; make sure that paragraph block gets selected after - * removing blocks from ListView. - */ - test( 'selects the default paragraph block after removing all blocks', async ( { - editor, - page, - pageUtils, - } ) => { - // Insert a couple of blocks of different types. - await editor.insertBlock( { name: 'core/image' } ); - await editor.insertBlock( { name: 'core/heading' } ); - - // Open List View. - await pageUtils.pressKeys( 'access+o' ); - const listView = page.getByRole( 'treegrid', { - name: 'Block navigation structure', - } ); - - // The last inserted block should be selected. - await expect( - listView.getByRole( 'gridcell', { - name: 'Heading', - exact: true, - selected: true, - } ) - ).toBeVisible(); - - // Select the Image block as well. - await pageUtils.pressKeys( 'shift+ArrowUp' ); - await expect( - listView.getByRole( 'gridcell', { - name: 'Image', - exact: true, - selected: true, - } ) - ).toBeVisible(); - - // Remove both blocks. - await listView - .getByRole( 'button', { name: 'Options for Image' } ) - .click(); - await page.getByRole( 'menuitem', { name: /Delete blocks/i } ).click(); - - // Newly created paragraph block should be selected. - await expect( - editor.canvas.getByRole( 'document', { name: /Empty block/i } ) - ).toBeFocused(); - } ); - test( 'expands nested list items', async ( { editor, page, @@ -562,6 +428,7 @@ test.describe( 'List View', () => { editor, page, pageUtils, + listViewUtils, } ) => { // Insert some blocks of different types. await editor.insertBlock( { @@ -587,46 +454,11 @@ test.describe( 'List View', () => { await editor.insertBlock( { name: 'core/file' } ); // Open List View. - await pageUtils.pressKeys( 'access+o' ); - const listView = page.getByRole( 'treegrid', { - name: 'Block navigation structure', - } ); - - async function getBlocksWithA11yAttributes() { - const selectedRows = await listView - .getByRole( 'row' ) - .filter( { - has: page.getByRole( 'gridcell', { selected: true } ), - } ) - .all(); - const selectedClientIds = await Promise.all( - selectedRows.map( ( row ) => row.getAttribute( 'data-block' ) ) - ); - const focusedClientId = await listView - .getByRole( 'row' ) - .filter( { has: page.locator( ':focus' ) } ) - .last() - .getAttribute( 'data-block' ); - // Don't use the util to get the unmodified default block when it's empty. - const blocks = await page.evaluate( () => - window.wp.data.select( 'core/block-editor' ).getBlocks() - ); - function recursivelyApplyAttributes( _blocks ) { - return _blocks.map( ( block ) => ( { - name: block.name, - selected: selectedClientIds.includes( block.clientId ), - focused: block.clientId === focusedClientId, - innerBlocks: recursivelyApplyAttributes( - block.innerBlocks - ), - } ) ); - } - return recursivelyApplyAttributes( blocks ); - } + const listView = await listViewUtils.openListView(); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'The last inserted block should be selected and focused.' ) .toMatchObject( [ @@ -638,8 +470,8 @@ test.describe( 'List View', () => { await page.keyboard.press( 'Delete' ); await expect .poll( - getBlocksWithA11yAttributes, - 'Deleting a block should move focus to the previous block' + listViewUtils.getBlocksWithA11yAttributes, + 'Deleting a block should move focus and selection to the previous block' ) .toMatchObject( [ { name: 'core/group' }, @@ -652,7 +484,7 @@ test.describe( 'List View', () => { await page.keyboard.press( 'ArrowDown' ); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'Move focus but do not select the second column' ) .toMatchObject( [ @@ -670,18 +502,18 @@ test.describe( 'List View', () => { await page.keyboard.press( 'Delete' ); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'Deleting a inner block moves focus to the previous inner block' ) .toMatchObject( [ { name: 'core/group' }, { name: 'core/columns', - selected: false, + selected: true, innerBlocks: [ { name: 'core/column', - selected: true, + selected: false, focused: true, }, ], @@ -700,7 +532,7 @@ test.describe( 'List View', () => { await page.keyboard.press( 'Backspace' ); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'Deleting multiple blocks moves focus to the parent block' ) .toMatchObject( [ @@ -725,7 +557,7 @@ test.describe( 'List View', () => { await page.keyboard.press( 'Backspace' ); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'Deleting the first block moves focus to the second block' ) .toMatchObject( [ @@ -740,7 +572,7 @@ test.describe( 'List View', () => { await pageUtils.pressKeys( 'access+z' ); await expect .poll( - getBlocksWithA11yAttributes, + listViewUtils.getBlocksWithA11yAttributes, 'Deleting the only block left will create a default block and focus/select it' ) .toMatchObject( [ @@ -750,5 +582,259 @@ test.describe( 'List View', () => { focused: true, }, ] ); + + await editor.insertBlock( { name: 'core/heading' } ); + await page.evaluate( () => + window.wp.data.dispatch( 'core/block-editor' ).clearSelectedBlock() + ); + await listView + .getByRole( 'gridcell', { name: 'Paragraph' } ) + .getByRole( 'link' ) + .focus(); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Block selection is cleared and focus is on the paragraph block' + ) + .toMatchObject( [ + { name: 'core/paragraph', selected: false, focused: true }, + { name: 'core/heading', selected: false }, + ] ); + + await pageUtils.pressKeys( 'access+z' ); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Deleting blocks without existing selection will not select blocks' + ) + .toMatchObject( [ + { name: 'core/heading', selected: false, focused: true }, + ] ); + + // Insert a block that is locked and cannot be removed. + await editor.insertBlock( { + name: 'core/file', + attributes: { lock: { move: false, remove: true } }, + } ); + // Click on the Heading block to select it. + await listView + .getByRole( 'gridcell', { name: 'Heading', exact: true } ) + .click(); + await listView + .getByRole( 'gridcell', { name: 'File' } ) + .getByRole( 'link' ) + .focus(); + for ( const keys of [ 'Delete', 'Backspace', 'access+z' ] ) { + await pageUtils.pressKeys( keys ); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Trying to delete locked blocks should not do anything' + ) + .toMatchObject( [ + { name: 'core/heading', selected: true, focused: false }, + { name: 'core/file', selected: false, focused: true }, + ] ); + } + } ); + + test( 'block settings dropdown menu', async ( { + editor, + page, + pageUtils, + listViewUtils, + } ) => { + // Insert some blocks of different types. + await editor.insertBlock( { name: 'core/heading' } ); + await editor.insertBlock( { name: 'core/file' } ); + + // Open List View. + const listView = await listViewUtils.openListView(); + + await listView + .getByRole( 'button', { name: 'Options for Heading' } ) + .click(); + + await page + .getByRole( 'menu', { name: 'Options for Heading' } ) + .getByRole( 'menuitem', { name: 'Duplicate' } ) + .click(); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Should duplicate a block and move focus' + ) + .toMatchObject( [ + { name: 'core/heading', selected: false }, + { name: 'core/heading', selected: false, focused: true }, + { name: 'core/file', selected: true }, + ] ); + + await page.keyboard.press( 'Shift+ArrowUp' ); + await listView + .getByRole( 'button', { name: 'Options for Heading' } ) + .first() + .click(); + await page + .getByRole( 'menu', { name: 'Options for Heading' } ) + .getByRole( 'menuitem', { name: 'Delete blocks' } ) + .click(); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Should delete multiple selected blocks using the dropdown menu' + ) + .toMatchObject( [ + { name: 'core/file', selected: true, focused: true }, + ] ); + + await page.keyboard.press( 'ArrowRight' ); + const optionsForFileToggle = listView + .getByRole( 'row' ) + .filter( { + has: page.getByRole( 'gridcell', { name: 'File' } ), + } ) + .getByRole( 'button', { name: 'Options for File' } ); + const optionsForFileMenu = page.getByRole( 'menu', { + name: 'Options for File', + } ); + await expect( + optionsForFileToggle, + 'Pressing arrow right should move focus to the menu dropdown toggle button' + ).toBeFocused(); + + await page.keyboard.press( 'Enter' ); + await expect( + optionsForFileMenu, + 'Pressing Enter should open the menu dropdown' + ).toBeVisible(); + + await page.keyboard.press( 'Escape' ); + await expect( + optionsForFileMenu, + 'Pressing Escape should close the menu dropdown' + ).toBeHidden(); + await expect( + optionsForFileToggle, + 'Should move focus back to the toggle button' + ).toBeFocused(); + + await page.keyboard.press( 'Space' ); + await expect( + optionsForFileMenu, + 'Pressing Space should also open the menu dropdown' + ).toBeVisible(); + + await pageUtils.pressKeys( 'primaryAlt+t' ); // Keyboard shortcut for Insert before. + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Pressing keyboard shortcut should also work when the menu is opened and focused' + ) + .toMatchObject( [ + { name: 'core/paragraph', selected: true, focused: false }, + { name: 'core/file', selected: false, focused: false }, + ] ); + await expect( + optionsForFileMenu, + 'The menu should be closed after pressing keyboard shortcut' + ).toBeHidden(); + + await optionsForFileToggle.click(); + await pageUtils.pressKeys( 'access+z' ); // Keyboard shortcut for Delete. + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Deleting blocks should move focus and selection' + ) + .toMatchObject( [ + { name: 'core/paragraph', selected: true, focused: true }, + ] ); + + // Insert a block that is locked and cannot be removed. + await editor.insertBlock( { + name: 'core/file', + attributes: { lock: { move: false, remove: true } }, + } ); + await optionsForFileToggle.click(); + await expect( + optionsForFileMenu.getByRole( 'menuitem', { name: 'Delete' } ), + 'The delete menu item should be hidden for locked blocks' + ).toBeHidden(); + await pageUtils.pressKeys( 'access+z' ); + await expect + .poll( + listViewUtils.getBlocksWithA11yAttributes, + 'Pressing keyboard shortcut should not delete locked blocks either' + ) + .toMatchObject( [ + { name: 'core/paragraph' }, + { name: 'core/file', selected: true }, + ] ); + await expect( + optionsForFileMenu, + 'The dropdown menu should also be visible' + ).toBeVisible(); } ); } ); + +/** @typedef {import('@playwright/test').Locator} Locator */ +class ListViewUtils { + #page; + #pageUtils; + #editor; + + constructor( { page, pageUtils, editor } ) { + this.#page = page; + this.#pageUtils = pageUtils; + this.#editor = editor; + + /** @type {Locator} */ + this.listView = page.getByRole( 'treegrid', { + name: 'Block navigation structure', + } ); + } + + /** + * @return {Promise} The list view locator. + */ + openListView = async () => { + await this.#pageUtils.pressKeys( 'access+o' ); + return this.listView; + }; + + getBlocksWithA11yAttributes = async () => { + const selectedRows = await this.listView + .getByRole( 'row' ) + .filter( { + has: this.#page.getByRole( 'gridcell', { selected: true } ), + } ) + .all(); + const selectedClientIds = await Promise.all( + selectedRows.map( ( row ) => row.getAttribute( 'data-block' ) ) + ); + const focusedRows = await this.listView + .getByRole( 'row' ) + .filter( { has: this.#page.locator( ':focus' ) } ) + .all(); + const focusedClientId = + focusedRows.length > 0 + ? await focusedRows[ focusedRows.length - 1 ].getAttribute( + 'data-block' + ) + : null; + // Don't use the util to get the unmodified default block when it's empty. + const blocks = await this.#page.evaluate( () => + window.wp.data.select( 'core/block-editor' ).getBlocks() + ); + function recursivelyApplyAttributes( _blocks ) { + return _blocks.map( ( block ) => ( { + name: block.name, + selected: selectedClientIds.includes( block.clientId ), + focused: block.clientId === focusedClientId, + innerBlocks: recursivelyApplyAttributes( block.innerBlocks ), + } ) ); + } + return recursivelyApplyAttributes( blocks ); + }; +}