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-select: don't focus first selected block #19762

Merged
merged 4 commits into from
Jan 21, 2020
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
15 changes: 4 additions & 11 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,8 @@ function BlockListBlock( {
const onBlockError = () => setErrorState( true );

const blockType = getBlockType( name );
const blockLabel = isFirstMultiSelected ?
__( 'Multiple selected blocks' ) :
// translators: %s: Type of block (i.e. Text, Image etc)
sprintf( __( 'Block: %s' ), blockType.title );
// translators: %s: Type of block (i.e. Text, Image etc)
const blockLabel = sprintf( __( 'Block: %s' ), blockType.title );

// Handing the focus of the block on creation and update

Expand Down Expand Up @@ -148,18 +146,13 @@ function BlockListBlock( {
const isMounting = useRef( true );

useEffect( () => {
if ( ! isMultiSelecting && ! isNavigationMode ) {
if ( isSelected ) {
focusTabbable( ! isMounting.current );
} else if ( isFirstMultiSelected ) {
wrapper.current.focus();
}
if ( ! isMultiSelecting && ! isNavigationMode && isSelected ) {
focusTabbable( ! isMounting.current );
}

isMounting.current = false;
}, [
isSelected,
isFirstMultiSelected,
isMultiSelecting,
isNavigationMode,
] );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const FocusCapture = forwardRef( ( {
isReverse,
containerRef,
noCapture,
hasMultiSelection,
}, ref ) => {
const isNavigationMode = useSelect( ( select ) =>
select( 'core/block-editor' ).isNavigationMode()
Expand All @@ -52,6 +53,11 @@ const FocusCapture = forwardRef( ( {
// selected, enable Navigation mode and select the first or last block
// depending on the direction.
if ( ! selectedClientId ) {
if ( hasMultiSelection ) {
containerRef.current.focus();
return;
}

setNavigationMode( true );

const tabbables = focus.tabbable.find( containerRef.current );
Expand Down
39 changes: 30 additions & 9 deletions packages/block-editor/src/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { overEvery, find, findLast, reverse, first, last } from 'lodash';
/**
* WordPress dependencies
*/
import { useRef } from '@wordpress/element';
import { useRef, useEffect } from '@wordpress/element';
import {
computeCaretRect,
focus,
Expand All @@ -19,6 +19,7 @@ import {
} from '@wordpress/dom';
import { UP, DOWN, LEFT, RIGHT, TAB, isKeyboardEvent, ESCAPE } from '@wordpress/keycodes';
import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
Expand Down Expand Up @@ -164,6 +165,7 @@ function selector( select ) {
isNavigationMode,
isSelectionEnabled,
getBlockSelectionStart,
isMultiSelecting,
} = select( 'core/block-editor' );

const selectedBlockClientId = getSelectedBlockClientId();
Expand All @@ -182,6 +184,7 @@ function selector( select ) {
isNavigationMode: isNavigationMode(),
isSelectionEnabled: isSelectionEnabled(),
blockSelectionStart: getBlockSelectionStart(),
isMultiSelecting: isMultiSelecting(),
};
}

Expand Down Expand Up @@ -217,6 +220,7 @@ export default function WritingFlow( { children } ) {
isNavigationMode,
isSelectionEnabled,
blockSelectionStart,
isMultiSelecting,
} = useSelect( selector, [] );
const {
multiSelect,
Expand Down Expand Up @@ -348,18 +352,16 @@ export default function WritingFlow( { children } ) {
return;
}

const clientId = selectedBlockClientId || selectedFirstClientId;

// In Edit mode, Tab should focus the first tabbable element after the
// content, which is normally the sidebar (with block controls) and
// Shift+Tab should focus the first tabbable element before the content,
// which is normally the block toolbar.
// Arrow keys can be used, and Tab and arrow keys can be used in
// Navigation mode (press Esc), to navigate through blocks.
if ( clientId ) {
const wrapper = getBlockDOMNode( clientId );

if ( selectedBlockClientId ) {
if ( isTab ) {
const wrapper = getBlockDOMNode( selectedBlockClientId );

if ( isShift ) {
if ( target === wrapper ) {
// Disable focus capturing on the focus capture element, so
Expand All @@ -383,6 +385,17 @@ export default function WritingFlow( { children } ) {
} else if ( isEscape ) {
setNavigationMode( true );
}
} else if ( hasMultiSelection && isTab && target === container.current ) {
// See comment above.
noCapture.current = true;

if ( isShift ) {
focusCaptureBeforeRef.current.focus();
} else {
focusCaptureAfterRef.current.focus();
}

return;
}

// When presing any key other than up or down, the initial vertical
Expand Down Expand Up @@ -486,7 +499,11 @@ export default function WritingFlow( { children } ) {
}
}

const selectedClientId = selectedBlockClientId || selectedFirstClientId;
useEffect( () => {
if ( hasMultiSelection && ! isMultiSelecting ) {
container.current.focus();
}
}, [ hasMultiSelection, isMultiSelecting ] );

// Disable reason: Wrapper itself is non-interactive, but must capture
// bubbling events from children to determine focus transition intents.
Expand All @@ -495,22 +512,26 @@ export default function WritingFlow( { children } ) {
<div className="block-editor-writing-flow">
<FocusCapture
ref={ focusCaptureBeforeRef }
selectedClientId={ selectedClientId }
selectedClientId={ selectedBlockClientId }
containerRef={ container }
noCapture={ noCapture }
hasMultiSelection={ hasMultiSelection }
/>
<div
ref={ container }
onKeyDown={ onKeyDown }
onMouseDown={ onMouseDown }
tabIndex={ hasMultiSelection ? '0' : undefined }
aria-label={ hasMultiSelection ? __( 'Multiple selected blocks' ) : undefined }
>
{ children }
</div>
<FocusCapture
ref={ focusCaptureAfterRef }
selectedClientId={ selectedClientId }
selectedClientId={ selectedBlockClientId }
containerRef={ container }
noCapture={ noCapture }
hasMultiSelection={ hasMultiSelection }
isReverse
/>
<div
Expand Down
18 changes: 10 additions & 8 deletions packages/edit-post/src/components/visual-editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ function VisualEditor() {
<VisualEditorGlobalKeyboardShortcuts />
<MultiSelectScrollIntoView />
<Typewriter>
<WritingFlow>
<ObserveTyping>
<CopyHandler>
<PostTitle />
<BlockList />
</CopyHandler>
</ObserveTyping>
</WritingFlow>
<CopyHandler>
<WritingFlow>
<ObserveTyping>
<CopyHandler>
<PostTitle />
<BlockList />
</CopyHandler>
</ObserveTyping>
</WritingFlow>
</CopyHandler>
</Typewriter>
<__experimentalBlockSettingsMenuFirstItem>
{ ( { onClose } ) => <BlockInspectorButton onClick={ onClose } /> }
Expand Down
5 changes: 3 additions & 2 deletions packages/edit-post/src/components/visual-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
}

.edit-post-visual-editor > .block-editor__typewriter,
.edit-post-visual-editor > .block-editor__typewriter > .block-editor-writing-flow,
.edit-post-visual-editor > .block-editor__typewriter > .block-editor-writing-flow > .block-editor-writing-flow__click-redirect {
.edit-post-visual-editor > .block-editor__typewriter > div,
.edit-post-visual-editor > .block-editor__typewriter > div > .block-editor-writing-flow,
.edit-post-visual-editor > .block-editor__typewriter > div > .block-editor-writing-flow > .block-editor-writing-flow__click-redirect {
height: 100%;
}

Expand Down