Skip to content

Commit

Permalink
Prevent focus loss after removing a block from List View (#39031)
Browse files Browse the repository at this point in the history
* Fix: Prevent focus loss after removing a block from List View

See: #39026

* Keep consistancy with shortcut

* Add e2e tests for block selection after removing.

* Typo fix

* Next block should be selected if very first block gets removed

* Added e2e test for checking default block selection

* Code comment & readability improved
  • Loading branch information
delowardev authored Mar 15, 2022
1 parent 136b0a6 commit 05b8a7d
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,26 @@ export function BlockSettingsDropdown( {
const blockClientIds = castArray( clientIds );
const count = blockClientIds.length;
const firstBlockClientId = blockClientIds[ 0 ];
const { onlyBlock } = useSelect(
const {
onlyBlock,
previousBlockClientId,
nextBlockClientId,
selectedBlockClientIds,
} = useSelect(
( select ) => {
const { getBlockCount } = select( blockEditorStore );
const {
getBlockCount,
getPreviousBlockClientId,
getNextBlockClientId,
getSelectedBlockClientIds,
} = select( blockEditorStore );
return {
onlyBlock: 1 === getBlockCount(),
previousBlockClientId: getPreviousBlockClientId(
firstBlockClientId
),
nextBlockClientId: getNextBlockClientId( firstBlockClientId ),
selectedBlockClientIds: getSelectedBlockClientIds(),
};
},
[ firstBlockClientId ]
Expand All @@ -72,7 +87,7 @@ export function BlockSettingsDropdown( {
};
}, [] );

const updateSelection = useCallback(
const updateSelectionAfterDuplicate = useCallback(
__experimentalSelectBlock
? async ( clientIdsPromise ) => {
const ids = await clientIdsPromise;
Expand All @@ -86,6 +101,33 @@ export function BlockSettingsDropdown( {

const blockTitle = useBlockDisplayTitle( firstBlockClientId, 25 );

const updateSelectionAfterRemove = useCallback(
__experimentalSelectBlock
? () => {
const blockToSelect =
previousBlockClientId || nextBlockClientId;

if (
blockToSelect &&
// From the block options dropdown, it's possible to remove a block that is not selected,
// in this case, it's not necessary to update the selection since the selected block wasn't removed.
selectedBlockClientIds.includes( firstBlockClientId ) &&
// Don't update selection when next/prev block also is in the selection ( and gets removed ),
// In case someone selects all blocks and removes them at once.
! selectedBlockClientIds.includes( blockToSelect )
) {
__experimentalSelectBlock( blockToSelect );
}
}
: noop,
[
__experimentalSelectBlock,
previousBlockClientId,
nextBlockClientId,
selectedBlockClientIds,
]
);

const label = sprintf(
/* translators: %s: block name */
__( 'Remove %s' ),
Expand Down Expand Up @@ -139,7 +181,7 @@ export function BlockSettingsDropdown( {
onClick={ flow(
onClose,
onDuplicate,
updateSelection
updateSelectionAfterDuplicate
) }
shortcut={ shortcuts.duplicate }
>
Expand Down Expand Up @@ -197,7 +239,7 @@ export function BlockSettingsDropdown( {
onClick={ flow(
onClose,
onRemove,
updateSelection
updateSelectionAfterRemove
) }
shortcut={ shortcuts.remove }
>
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/list-view/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function ListViewBlock( {
[ clientId, selectBlock ]
);

const selectDuplicatedBlock = useCallback(
const updateSelection = useCallback(
( newClientId ) => {
selectBlock( undefined, newClientId );
},
Expand Down Expand Up @@ -257,7 +257,7 @@ function ListViewBlock( {
onFocus,
} }
disableOpenOnArrowDown
__experimentalSelectBlock={ selectDuplicatedBlock }
__experimentalSelectBlock={ updateSelection }
/>
) }
</TreeGridCell>
Expand Down
125 changes: 125 additions & 0 deletions packages/e2e-tests/specs/editor/various/list-view.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,131 @@ describe( 'List view', () => {
expect( console ).not.toHaveErrored();
} );

// Check for regression of https://github.com/WordPress/gutenberg/issues/39026
it( 'should select previous block after removing selected one', async () => {
// Insert some blocks of different types.
await insertBlock( 'Image' );
await insertBlock( 'Heading' );
await insertBlock( 'Paragraph' );

// Open list view.
await openListView();

// The last inserted paragraph block should be selected in List View.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
);

// Paragraph options button.
const paragraphOptionsButton = await page.waitForSelector(
'tr.block-editor-list-view-leaf:last-child button[aria-label="Options for Paragraph block"]'
);

await paragraphOptionsButton.click();

const paragraphRemoveButton = await page.waitForXPath(
'//button[contains(., "Remove Paragraph")]'
);

// Remove paragraph.
await paragraphRemoveButton.click();

// Heading block should be selected as previous block.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
);
} );

// Check for regression of https://github.com/WordPress/gutenberg/issues/39026
it( 'should select next block after removing the very first block', async () => {
// Insert some blocks of different types.
await insertBlock( 'Image' );
await insertBlock( 'Heading' );
await insertBlock( 'Paragraph' );

// Open list view.
await openListView();

// The last inserted paragraph block should be selected in List View.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
);

// Go to the image block in list view.
await pressKeyTimes( 'ArrowUp', 2 );
await pressKeyTimes( 'Enter', 1 );

// Image block should have selected.
await page.waitForXPath( '//a[contains(., "Image(selected block)")]' );

// Image options dropdown.
const imageOptionsButton = await page.waitForSelector(
'tr.block-editor-list-view-leaf:first-child button[aria-label="Options for Image block"]'
);

await imageOptionsButton.click();

const imageRemoveButton = await page.waitForXPath(
'//button[contains(., "Remove Image")]'
);

// Remove Image block.
await imageRemoveButton.click();

// Heading block should be selected as next block.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
);
} );

/**
* 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.
*/
it( 'should select default paragraph block after removing all blocks', async () => {
// Insert some blocks of different types.
await insertBlock( 'Image' );
await insertBlock( 'Heading' );

// Open list view.
await openListView();

// The last inserted heading block should be selected in List View.
const headingBlock = await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
);

await headingBlock.click();

// Select all two blocks.
await pressKeyWithModifier( 'shift', 'ArrowUp' );

// Both Image and Heading blocks should have selected.
await page.waitForXPath(
'//a[contains(., "Heading(selected block)")]'
);
await page.waitForXPath( '//a[contains(., "Image(selected block)")]' );

const imageOptionsButton = await page.waitForSelector(
'tr.block-editor-list-view-leaf:first-child button[aria-label="Options for Image block"]'
);

// Blocks options dropdown.
await imageOptionsButton.click();

const blocksRemoveButton = await page.waitForXPath(
'//button[contains(., "Remove blocks")]'
);

// Remove all blocks.
await blocksRemoveButton.click();

// Newly created default paragraph block should be selected.
await page.waitForXPath(
'//a[contains(., "Paragraph(selected block)")]'
);
} );

it( 'should expand nested list items', async () => {
// Insert some blocks of different types.
await insertBlock( 'Cover' );
Expand Down

0 comments on commit 05b8a7d

Please sign in to comment.