Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List View: Support insert before/after keyboard shortcuts when focus is within the list view #60651

Merged
merged 2 commits into from
Apr 12, 2024
Merged

List View: Support insert before/after keyboard shortcuts when focus is within the list view #60651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c076ff
List View: Support insert before/after keyboard shortcuts when focus …
andrewserong Apr 11, 2024
f7d4022
Update tests
andrewserong Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions packages/block-editor/src/components/list-view/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,13 @@ function ListViewBlockSelectButton(
getBlocksByClientId,
canRemoveBlocks,
} = useSelect( blockEditorStore );
const { duplicateBlocks, multiSelect, removeBlocks } =
useDispatch( blockEditorStore );
const {
duplicateBlocks,
multiSelect,
removeBlocks,
insertAfterBlock,
insertBeforeBlock,
} = useDispatch( blockEditorStore );
const isMatch = useShortcutEventMatch();
const isSticky = blockInformation?.positionType === 'sticky';
const images = useListViewImages( { clientId, isExpanded } );
Expand Down Expand Up @@ -189,6 +194,30 @@ function ListViewBlockSelectButton(
updateFocusAndSelection( updatedBlocks[ 0 ], false );
}
}
} else if ( isMatch( 'core/block-editor/insert-before', event ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we share logic here with the normal canvas? Is it worth unifying?

Copy link
Contributor Author

@andrewserong andrewserong Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for the list view needs to be slightly different to the editor canvas, as the list view does a couple of things differently:

  • We determine the blocks to update based on the focused item in addition to the block selection (so if the focused list view item is outside of the selection, we ignore the selection and use the focused list view item as the "anchor" block to add blocks either before or after)
  • After insertion, we switch focus back to the list view item so that focus remains within the list view

So, I'm not too sure how feasible it'd be to unify this further with how the editor canvas handles things. That said, it could be worth exploring, and to look into how to either neaten things or consolidate things further in a separate PR as this onKeyDownHandler callback is quite long, so if nothing else it might work to move the callback into a hook? In any case, I'll merge this PR in its current form for now, and we can look into code quality follow-ups separately 🙂

if ( event.defaultPrevented ) {
return;
}
event.preventDefault();

const { blocksToUpdate } = getBlocksToUpdate();
await insertBeforeBlock( blocksToUpdate[ 0 ] );
const newlySelectedBlocks = getSelectedBlockClientIds();

// Focus the first block of the newly inserted blocks, to keep focus within the list view.
updateFocusAndSelection( newlySelectedBlocks[ 0 ], false );
} else if ( isMatch( 'core/block-editor/insert-after', event ) ) {
if ( event.defaultPrevented ) {
return;
}
event.preventDefault();

const { blocksToUpdate } = getBlocksToUpdate();
await insertAfterBlock( blocksToUpdate.at( -1 ) );
const newlySelectedBlocks = getSelectedBlockClientIds();

// Focus the first block of the newly inserted blocks, to keep focus within the list view.
updateFocusAndSelection( newlySelectedBlocks[ 0 ], false );
} else if ( isMatch( 'core/block-editor/select-all', event ) ) {
if ( event.defaultPrevented ) {
return;
Expand Down
42 changes: 39 additions & 3 deletions test/e2e/specs/editor/various/list-view.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ test.describe( 'List View', () => {
).toBeFocused();
} );

test( 'should cut, copy, paste, select, duplicate, delete, and deselect blocks using keyboard', async ( {
test( 'should cut, copy, paste, select, duplicate, insert, delete, and deselect blocks using keyboard', async ( {
editor,
page,
pageUtils,
Expand Down Expand Up @@ -663,8 +663,44 @@ test.describe( 'List View', () => {
{ name: 'core/file', focused: true },
] );

// Move focus to the first file block, and then delete it.
await page.keyboard.press( 'ArrowUp' );
// Test insert before.
await pageUtils.pressKeys( 'primaryAlt+t' );

await expect
.poll(
listViewUtils.getBlocksWithA11yAttributes,
'Inserting a block before should move selection and focus to the inserted block.'
)
.toMatchObject( [
{ name: 'core/group' },
{ name: 'core/columns' },
{ name: 'core/file', selected: false },
{ name: 'core/paragraph', focused: true, selected: true },
{ name: 'core/file', selected: false },
] );

// Test insert after.
await pageUtils.pressKeys( 'primaryAlt+y' );

await expect
.poll(
listViewUtils.getBlocksWithA11yAttributes,
'Inserting a block before should move selection and focus to the inserted block.'
)
.toMatchObject( [
{ name: 'core/group' },
{ name: 'core/columns' },
{ name: 'core/file', selected: false },
{ name: 'core/paragraph', focused: false, selected: false },
{ name: 'core/paragraph', focused: true, selected: true },
{ name: 'core/file', selected: false },
] );

// Remove the inserted blocks.
await page.keyboard.press( 'Delete' );
await page.keyboard.press( 'Delete' );

// Delete the first File block.
await page.keyboard.press( 'Delete' );
await expect
.poll(
Expand Down
Loading