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.

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

Multi-selection: fix select all in Safari #42340

Merged
merged 3 commits into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
// When selecting multiple blocks, we provide an additional selection indicator.
&.is-navigate-mode .block-editor-block-list__block.is-selected,
&.is-navigate-mode .block-editor-block-list__block.is-hovered,
.block-editor-block-list__block.is-multi-selected:not([contenteditable]),
.block-editor-block-list__block.is-multi-selected:not(.is-partially-selected),
.block-editor-block-list__block.is-highlighted,
.block-editor-block-list__block.is-highlighted ~ .is-multi-selected {
&::after {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export function useBlockClassNames( clientId ) {
getSettings,
hasSelectedInnerBlock,
isTyping,
__unstableIsFullySelected,
} = select( blockEditorStore );
const { outlineMode } = getSettings();
const isDragging = isBlockBeingDragged( clientId );
Expand All @@ -44,10 +45,13 @@ export function useBlockClassNames( clientId ) {
clientId,
checkDeep
);
const isMultiSelected = isBlockMultiSelected( clientId );
return classnames( {
'is-selected': isSelected,
'is-highlighted': isBlockHighlighted( clientId ),
'is-multi-selected': isBlockMultiSelected( clientId ),
'is-multi-selected': isMultiSelected,
'is-partially-selected':
isMultiSelected && ! __unstableIsFullySelected(),
'is-reusable': isReusableBlock( getBlockType( name ) ),
'is-dragging': isDragging,
'has-child-selected': isAncestorOfSelectedBlock,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
isRTL,
} from '@wordpress/dom';
import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes';
import { useSelect } from '@wordpress/data';
import { useDispatch, useSelect } from '@wordpress/data';
import { useRefEffect } from '@wordpress/compose';

/**
Expand Down Expand Up @@ -131,12 +131,15 @@ export function getClosestTabbable(
export default function useArrowNav() {
const {
getSelectedBlockClientId,
getMultiSelectedBlocksStartClientId,
getMultiSelectedBlocksEndClientId,
getPreviousBlockClientId,
getNextBlockClientId,
getSettings,
hasMultiSelection,
__unstableIsFullySelected,
} = useSelect( blockEditorStore );
const { selectBlock } = useDispatch( blockEditorStore );
return useRefEffect( ( node ) => {
// Here a DOMRect is stored while moving the caret vertically so
// vertical position of the start position can be restored. This is to
Expand Down Expand Up @@ -186,7 +189,35 @@ export default function useArrowNav() {
const { ownerDocument } = node;
const { defaultView } = ownerDocument;

// If there is a multi-selection, the arrow keys should collapse the
// selection to the start or end of the selection.
if ( hasMultiSelection() ) {
// Only handle if we have a full selection (not a native partial
// selection).
if ( ! __unstableIsFullySelected() ) {
return;
}

if ( event.defaultPrevented ) {
return;
}

if ( ! isNav ) {
return;
}

if ( isShift ) {
return;
}

event.preventDefault();

if ( isReverse ) {
selectBlock( getMultiSelectedBlocksStartClientId() );
} else {
selectBlock( getMultiSelectedBlocksEndClientId(), -1 );
}

return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { first, last } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -13,7 +8,6 @@ import { useSelect } from '@wordpress/data';
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';
import { __unstableUseBlockRef as useBlockRef } from '../block-list/use-block-props/use-block-refs';

function selector( select ) {
const {
Expand Down Expand Up @@ -44,10 +38,6 @@ export default function useMultiSelection() {
selectedBlockClientId,
isFullSelection,
} = useSelect( selector, [] );
const selectedRef = useBlockRef( selectedBlockClientId );
// These must be in the right DOM order.
const startRef = useBlockRef( first( multiSelectedBlockClientIds ) );
const endRef = useBlockRef( last( multiSelectedBlockClientIds ) );

/**
* When the component updates, and there is multi selection, we need to
Expand All @@ -66,26 +56,6 @@ export default function useMultiSelection() {
}

if ( ! hasMultiSelection || isMultiSelecting ) {
if ( ! selectedBlockClientId || isMultiSelecting ) {
return;
}

const selection = defaultView.getSelection();

if ( selection.rangeCount && ! selection.isCollapsed ) {
const blockNode = selectedRef.current;
const { startContainer, endContainer } =
selection.getRangeAt( 0 );

if (
!! blockNode &&
( ! blockNode.contains( startContainer ) ||
! blockNode.contains( endContainer ) )
) {
selection.removeAllRanges();
}
}

return;
}

Expand All @@ -105,25 +75,8 @@ export default function useMultiSelection() {
// able to select across instances immediately.
node.contentEditable = true;

// For some browsers, like Safari, it is important that focus happens
// BEFORE selection.
defaultView.getSelection().removeAllRanges();
node.focus();

// The block refs might not be immediately available
// when dragging blocks into another block.
if ( ! startRef.current || ! endRef.current ) {
return;
}

const selection = defaultView.getSelection();
const range = ownerDocument.createRange();

// These must be in the right DOM order.
range.setStartBefore( startRef.current );
range.setEndAfter( endRef.current );

selection.removeAllRanges();
selection.addRange( range );
},
[
hasMultiSelection,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,11 @@ export default function useSelectionObserver() {

function onSelectionChange( event ) {
const selection = defaultView.getSelection();
// If no selection is found, end multi selection and disable the
// contentEditable wrapper.

if ( ! selection.rangeCount ) {
setContentEditableWrapper( node, false );
return;
}

// If selection is collapsed and we haven't used `shift+click`,
// end multi selection and disable the contentEditable wrapper.
// We have to check about `shift+click` case because elements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ describe( 'Multi-block selection', () => {
await pressKeyWithModifier( 'primary', 'a' );
await pressKeyWithModifier( 'primary', 'a' );

await testNativeSelection();
expect( await getSelectedFlatIndices() ).toEqual( [ 1, 2, 3 ] );

// TODO: It would be great to do this test by spying on `wp.a11y.speak`,
Expand Down Expand Up @@ -420,7 +419,6 @@ describe( 'Multi-block selection', () => {
await page.mouse.move( coord2.x, coord2.y, { steps: 10 } );
await page.mouse.up();

await testNativeSelection();
expect( await getSelectedFlatIndices() ).toEqual( [ 1, 2 ] );
} );

Expand Down