From 4b9fc094123e5dd426bb12bf5f64e729714c7544 Mon Sep 17 00:00:00 2001 From: Adam Zielinski Date: Fri, 19 Jun 2020 18:55:24 +0200 Subject: [PATCH] [Navigation screen] Separate block navigator focus from the editor focus (#22996) * Separate the navigator selection from the editor area selection * add __experimental prefix and use list of selected blocks instead of a single id * Don't attempt to call __experimentalSelectBlock when it's not provided * Drop onClick and only pass selectBlock to BlockNavigationBlock * Remove selection CSS * Stop rendering movers for selected block * Add "Go to block" navigator menu action * Fix unit test * Restore the previous way of focusing items in the navigator * Lint * Ensure clicking go to block always works * Add a README file --- .../developers/data/data-core-block-editor.md | 1 + .../src/components/block-actions/index.js | 10 ++-- .../src/components/block-navigation/block.js | 46 +++++++++++++++---- .../src/components/block-navigation/branch.js | 2 +- .../components/block-navigation/style.scss | 8 ---- .../components/block-settings-menu/README.md | 13 ++++++ .../block-settings-dropdown.js | 45 ++++++++++++++++-- packages/block-editor/src/store/actions.js | 41 +++++++++++------ .../block-editor/src/store/test/actions.js | 5 ++ packages/components/src/tree-grid/cell.js | 18 +++++--- packages/components/src/tree-grid/item.js | 18 ++++++-- .../src/tree-grid/roving-tab-index-item.js | 16 +++---- .../menu-editor/navigation-structure-area.js | 14 +++--- 13 files changed, 173 insertions(+), 64 deletions(-) create mode 100644 packages/block-editor/src/components/block-settings-menu/README.md diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md index 04bbcae93c77cc..8e102d13e5a623 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -946,6 +946,7 @@ Generator that triggers an action used to duplicate a list of blocks. _Parameters_ - _clientIds_ `Array`: +- _updateSelection_ `boolean`: # **enterFormattedText** diff --git a/packages/block-editor/src/components/block-actions/index.js b/packages/block-editor/src/components/block-actions/index.js index 249dd6e2e8a821..872e199004a997 100644 --- a/packages/block-editor/src/components/block-actions/index.js +++ b/packages/block-editor/src/components/block-actions/index.js @@ -14,7 +14,11 @@ import { hasBlockSupport, switchToBlockType } from '@wordpress/blocks'; */ import { useNotifyCopy } from '../copy-handler'; -export default function BlockActions( { clientIds, children } ) { +export default function BlockActions( { + clientIds, + children, + __experimentalUpdateSelection: updateSelection, +} ) { const { canInsertBlockType, getBlockRootClientId, @@ -59,10 +63,10 @@ export default function BlockActions( { clientIds, children } ) { rootClientId, blocks, onDuplicate() { - return duplicateBlocks( clientIds ); + return duplicateBlocks( clientIds, updateSelection ); }, onRemove() { - removeBlocks( clientIds ); + return removeBlocks( clientIds, updateSelection ); }, onInsertBefore() { insertBeforeBlock( first( castArray( clientIds ) ) ); diff --git a/packages/block-editor/src/components/block-navigation/block.js b/packages/block-editor/src/components/block-navigation/block.js index 8008d010cef403..82ac444e73e14b 100644 --- a/packages/block-editor/src/components/block-navigation/block.js +++ b/packages/block-editor/src/components/block-navigation/block.js @@ -9,10 +9,13 @@ import classnames from 'classnames'; import { __experimentalTreeGridCell as TreeGridCell, __experimentalTreeGridItem as TreeGridItem, + MenuGroup, + MenuItem, } from '@wordpress/components'; - +import { __ } from '@wordpress/i18n'; import { moreVertical } from '@wordpress/icons'; -import { useState } from '@wordpress/element'; +import { useState, useRef, useEffect } from '@wordpress/element'; +import { useDispatch } from '@wordpress/data'; /** * Internal dependencies @@ -29,8 +32,8 @@ import { useBlockNavigationContext } from './context'; export default function BlockNavigationBlock( { block, - onClick, isSelected, + onClick, position, level, rowCount, @@ -38,26 +41,35 @@ export default function BlockNavigationBlock( { terminatedLevels, path, } ) { + const cellRef = useRef( null ); const [ isHovered, setIsHovered ] = useState( false ); const [ isFocused, setIsFocused ] = useState( false ); + const { selectBlock: selectEditorBlock } = useDispatch( + 'core/block-editor' + ); const { clientId } = block; // Subtract 1 from rowCount, as it includes the block appender. const siblingCount = rowCount - 1; const hasSiblings = siblingCount > 1; const hasRenderedMovers = showBlockMovers && hasSiblings; - const hasVisibleMovers = isHovered || isSelected || isFocused; + const hasVisibleMovers = isHovered || isFocused; const moverCellClassName = classnames( 'block-editor-block-navigation-block__mover-cell', { 'is-visible': hasVisibleMovers } ); const { - __experimentalFeatures: withBlockNavigationBlockSettings, + __experimentalFeatures: withExperimentalFeatures, } = useBlockNavigationContext(); const blockNavigationBlockSettingsClassName = classnames( 'block-editor-block-navigation-block__menu-cell', { 'is-visible': hasVisibleMovers } ); + useEffect( () => { + if ( withExperimentalFeatures && isSelected ) { + cellRef.current.focus(); + } + }, [ withExperimentalFeatures, isSelected ] ); return ( { ( { ref, tabIndex, onFocus } ) => (
@@ -86,7 +99,7 @@ export default function BlockNavigationBlock( { /> onClick( block.clientId ) } isSelected={ isSelected } position={ position } siblingCount={ siblingCount } @@ -130,7 +143,7 @@ export default function BlockNavigationBlock( { ) } - { withBlockNavigationBlockSettings && ( + { withExperimentalFeatures && ( @@ -144,7 +157,24 @@ export default function BlockNavigationBlock( { onFocus, } } disableOpenOnArrowDown - /> + __experimentalSelectBlock={ onClick } + > + { ( { onClose } ) => ( + + { + // If clientId is already selected, it won't be focused (see block-wrapper.js) + // This removes the selection first to ensure the focus will always switch. + await selectEditorBlock( null ); + await selectEditorBlock( clientId ); + onClose(); + } } + > + { __( 'Go to block' ) } + + + ) } + ) } ) } diff --git a/packages/block-editor/src/components/block-navigation/branch.js b/packages/block-editor/src/components/block-navigation/branch.js index e76c5dd9b8d6af..86d999e83fadb1 100644 --- a/packages/block-editor/src/components/block-navigation/branch.js +++ b/packages/block-editor/src/components/block-navigation/branch.js @@ -59,7 +59,7 @@ export default function BlockNavigationBranch( props ) { selectBlock( clientId ) } + onClick={ selectBlock } isSelected={ selectedBlockClientId === clientId } level={ level } position={ position } diff --git a/packages/block-editor/src/components/block-navigation/style.scss b/packages/block-editor/src/components/block-navigation/style.scss index 0fd3a2ea983268..8131e5936ba770 100644 --- a/packages/block-editor/src/components/block-navigation/style.scss +++ b/packages/block-editor/src/components/block-navigation/style.scss @@ -56,14 +56,6 @@ $tree-item-height: 36px; margin-right: 6px; } - &.is-selected .block-editor-block-icon svg, - &.is-selected:focus .block-editor-block-icon svg { - color: $white; - background: $dark-gray-primary; - box-shadow: 0 0 0 $border-width $dark-gray-primary; - border-radius: $border-width; - } - .block-editor-block-navigation-block__menu-cell, .block-editor-block-navigation-block__mover-cell, .block-editor-block-navigation-block__contents-cell { diff --git a/packages/block-editor/src/components/block-settings-menu/README.md b/packages/block-editor/src/components/block-settings-menu/README.md new file mode 100644 index 00000000000000..98b0a33a9ae939 --- /dev/null +++ b/packages/block-editor/src/components/block-settings-menu/README.md @@ -0,0 +1,13 @@ +BlockSettingsMenu +============ + +This is a menu that allows the user to act on the block (duplicate, remove, etc). + +# BlockSettingsDropdown + +This is the dropdown itself, it covers the bulk of the logic of this component. + +## Props + +`__experimentalSelectBlock` - A callback. If passed, interacting with dropdown options (such as duplicate) will not update the +editor selection. Instead, every time a selection change should happen the callback will be called with a proper clientId. 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 a4e52afcbe3f90..ea5ddb9aad1129 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 @@ -1,7 +1,7 @@ /** * External dependencies */ -import { castArray, flow } from 'lodash'; +import { castArray, flow, noop } from 'lodash'; /** * WordPress dependencies @@ -15,6 +15,8 @@ import { } from '@wordpress/components'; import { useSelect } from '@wordpress/data'; import { moreHorizontal } from '@wordpress/icons'; + +import { Children, cloneElement, useCallback } from '@wordpress/element'; import { serialize } from '@wordpress/blocks'; /** @@ -33,7 +35,12 @@ const POPOVER_PROPS = { isAlternate: true, }; -export function BlockSettingsDropdown( { clientIds, ...props } ) { +export function BlockSettingsDropdown( { + clientIds, + __experimentalSelectBlock, + children, + ...props +} ) { const blockClientIds = castArray( clientIds ); const count = blockClientIds.length; const firstBlockClientId = blockClientIds[ 0 ]; @@ -56,8 +63,23 @@ export function BlockSettingsDropdown( { clientIds, ...props } ) { }; }, [] ); + const updateSelection = useCallback( + __experimentalSelectBlock + ? async ( clientIdsPromise ) => { + const ids = await clientIdsPromise; + if ( ids && ids[ 0 ] ) { + __experimentalSelectBlock( ids[ 0 ] ); + } + } + : noop, + [ __experimentalSelectBlock ] + ); + return ( - + { ( { canDuplicate, canInsertDefaultBlock, @@ -103,7 +125,11 @@ export function BlockSettingsDropdown( { clientIds, ...props } ) { { canDuplicate && ( { __( 'Duplicate' ) } @@ -142,10 +168,19 @@ export function BlockSettingsDropdown( { clientIds, ...props } ) { fillProps={ { onClose } } clientIds={ clientIds } /> + { typeof children === 'function' + ? children( { onClose } ) + : Children.map( ( child ) => + cloneElement( child, { onClose } ) + ) } { ! isLocked && ( { _n( diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 89ce0d29b1615d..3b07e1e11021e3 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -2,19 +2,17 @@ * External dependencies */ import { castArray, first, get, includes, last, some } from 'lodash'; - /** * WordPress dependencies */ import { - getDefaultBlockName, + cloneBlock, createBlock, + getDefaultBlockName, hasBlockSupport, - cloneBlock, } from '@wordpress/blocks'; import { speak } from '@wordpress/a11y'; import { __ } from '@wordpress/i18n'; - /** * Internal dependencies */ @@ -32,7 +30,7 @@ function* ensureDefaultBlock() { // To avoid a focus loss when removing the last block, assure there is // always a default block if the last of the blocks have been removed. if ( count === 0 ) { - yield insertDefaultBlock(); + return yield insertDefaultBlock(); } } @@ -165,6 +163,7 @@ export function* selectPreviousBlock( clientId ) { if ( previousBlockClientId ) { yield selectBlock( previousBlockClientId, -1 ); + return [ previousBlockClientId ]; } } @@ -183,6 +182,7 @@ export function* selectNextBlock( clientId ) { if ( nextBlockClientId ) { yield selectBlock( nextBlockClientId ); + return [ nextBlockClientId ]; } } @@ -596,8 +596,15 @@ export function* removeBlocks( clientIds, selectPrevious = true ) { return; } + let previousBlockId; if ( selectPrevious ) { - yield selectPreviousBlock( clientIds[ 0 ] ); + previousBlockId = yield selectPreviousBlock( clientIds[ 0 ] ); + } else { + previousBlockId = yield select( + 'core/block-editor', + 'getPreviousBlockClientId', + clientIds[ 0 ] + ); } yield { @@ -607,7 +614,8 @@ export function* removeBlocks( clientIds, selectPrevious = true ) { // To avoid a focus loss when removing the last block, assure there is // always a default block if the last of the blocks have been removed. - yield* ensureDefaultBlock(); + const defaultBlockId = yield* ensureDefaultBlock(); + return [ previousBlockId || defaultBlockId ]; } /** @@ -888,8 +896,9 @@ export function* setNavigationMode( isNavigationMode = true ) { * Generator that triggers an action used to duplicate a list of blocks. * * @param {string[]} clientIds + * @param {boolean} updateSelection */ -export function* duplicateBlocks( clientIds ) { +export function* duplicateBlocks( clientIds, updateSelection = true ) { if ( ! clientIds && ! clientIds.length ) { return; } @@ -908,7 +917,7 @@ export function* duplicateBlocks( clientIds ) { return; } const blockNames = blocks.map( ( block ) => block.name ); - // Return early if blocks don't support multipe usage. + // Return early if blocks don't support multiple usage. if ( some( blockNames, @@ -925,13 +934,19 @@ export function* duplicateBlocks( clientIds ) { rootClientId ); const clonedBlocks = blocks.map( ( block ) => cloneBlock( block ) ); - yield insertBlocks( clonedBlocks, lastSelectedIndex + 1, rootClientId ); - if ( clonedBlocks.length > 1 ) { + yield insertBlocks( + clonedBlocks, + lastSelectedIndex + 1, + rootClientId, + updateSelection + ); + if ( clonedBlocks.length > 1 && updateSelection ) { yield multiSelect( first( clonedBlocks ).clientId, last( clonedBlocks ).clientId ); } + return clonedBlocks.map( ( block ) => block.clientId ); } /** @@ -963,7 +978,7 @@ export function* insertBeforeBlock( clientId ) { clientId, rootClientId ); - yield insertDefaultBlock( {}, rootClientId, firstSelectedIndex ); + return yield insertDefaultBlock( {}, rootClientId, firstSelectedIndex ); } /** @@ -995,7 +1010,7 @@ export function* insertAfterBlock( clientId ) { clientId, rootClientId ); - yield insertDefaultBlock( {}, rootClientId, firstSelectedIndex + 1 ); + return yield insertDefaultBlock( {}, rootClientId, firstSelectedIndex + 1 ); } /** diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index d0eb2098adc52f..4976fca7687153 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -799,6 +799,11 @@ describe( 'actions', () => { expect( actions ).toEqual( [ select( 'core/block-editor', 'getBlockRootClientId', clientId ), select( 'core/block-editor', 'getTemplateLock', undefined ), + select( + 'core/block-editor', + 'getPreviousBlockClientId', + 'myclientid' + ), { type: 'REMOVE_BLOCKS', clientIds: [ clientId ], diff --git a/packages/components/src/tree-grid/cell.js b/packages/components/src/tree-grid/cell.js index f0b881143dba65..dd4e2654d64e5e 100644 --- a/packages/components/src/tree-grid/cell.js +++ b/packages/components/src/tree-grid/cell.js @@ -1,20 +1,24 @@ +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + /** * Internal dependencies */ import TreeGridItem from './item'; -export default function TreeGridCell( { - children, - withoutGridItem = false, - ...props -} ) { +export default forwardRef( function TreeGridCell( + { children, withoutGridItem = false, ...props }, + ref +) { return ( { withoutGridItem ? ( children ) : ( - { children } + { children } ) } ); -} +} ); diff --git a/packages/components/src/tree-grid/item.js b/packages/components/src/tree-grid/item.js index dd2eda288add30..1d79b5640731b3 100644 --- a/packages/components/src/tree-grid/item.js +++ b/packages/components/src/tree-grid/item.js @@ -1,8 +1,20 @@ +/** + * WordPress dependencies + */ +import { forwardRef } from '@wordpress/element'; + /** * Internal dependencies */ import RovingTabIndexItem from './roving-tab-index-item'; -export default function TreeGridItem( { children, ...props } ) { - return { children }; -} +export default forwardRef( function TreeGridItem( + { children, ...props }, + ref +) { + return ( + + { children } + + ); +} ); diff --git a/packages/components/src/tree-grid/roving-tab-index-item.js b/packages/components/src/tree-grid/roving-tab-index-item.js index b876aa5be565bd..ef9a28cf87e292 100644 --- a/packages/components/src/tree-grid/roving-tab-index-item.js +++ b/packages/components/src/tree-grid/roving-tab-index-item.js @@ -1,19 +1,19 @@ /** * WordPress dependencies */ -import { useRef } from '@wordpress/element'; +import { useRef, forwardRef } from '@wordpress/element'; /** * Internal dependencies */ import { useRovingTabIndexContext } from './roving-tab-index-context'; -export default function RovingTabIndexItem( { - children, - as: Component, - ...props -} ) { - const ref = useRef(); +export default forwardRef( function RovingTabIndexItem( + { children, as: Component, ...props }, + forwardedRef +) { + const localRef = useRef(); + const ref = forwardedRef || localRef; const { lastFocusedElement, setLastFocusedElement, @@ -32,4 +32,4 @@ export default function RovingTabIndexItem( { } return { children }; -} +} ); diff --git a/packages/edit-navigation/src/components/menu-editor/navigation-structure-area.js b/packages/edit-navigation/src/components/menu-editor/navigation-structure-area.js index d1a831d71fd189..f8e66704a85258 100644 --- a/packages/edit-navigation/src/components/menu-editor/navigation-structure-area.js +++ b/packages/edit-navigation/src/components/menu-editor/navigation-structure-area.js @@ -10,23 +10,21 @@ import { Panel, PanelBody, } from '@wordpress/components'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; export default function NavigationStructureArea( { blocks, initialOpen } ) { + const [ selectedBlockId, setSelectedBlockId ] = useState( null ); const isSmallScreen = useViewportMatch( 'medium', '<' ); - const selectedBlockClientIds = useSelect( - ( select ) => select( 'core/block-editor' ).getSelectedBlockClientIds(), - [] - ); - const { selectBlock } = useDispatch( 'core/block-editor' ); const showNavigationStructure = !! blocks.length; const content = showNavigationStructure && ( <__experimentalBlockNavigationTree blocks={ blocks } - selectedBlockClientId={ selectedBlockClientIds[ 0 ] } - selectBlock={ selectBlock } + selectedBlockClientId={ selectedBlockId } + selectBlock={ ( id ) => { + setSelectedBlockId( id ); + } } __experimentalFeatures showNestedBlocks showAppender