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

Make parent block selector visible and offset the toolbar. #28598

Merged
merged 8 commits into from
Feb 2, 2021
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
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
Expand All @@ -12,29 +17,38 @@ import NavigableToolbar from '../navigable-toolbar';
import { BlockToolbar } from '../';

function BlockContextualToolbar( { focusOnMount, ...props } ) {
const { blockType } = useSelect( ( select ) => {
const { getBlockName, getSelectedBlockClientIds } = select(
'core/block-editor'
);
const { blockType, hasParents } = useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientIds,
} = select( 'core/block-editor' );
const { getBlockType } = select( blocksStore );
const selectedBlockClientIds = getSelectedBlockClientIds();
const selectedBlockClientId = selectedBlockClientIds[ 0 ];
return {
blockType:
selectedBlockClientId &&
getBlockType( getBlockName( selectedBlockClientId ) ),
hasParents: getBlockParents( selectedBlockClientId ).length,
};
}, [] );
if ( blockType ) {
if ( ! hasBlockSupport( blockType, '__experimentalToolbar', true ) ) {
return null;
}
}

// Shifts the toolbar to make room for the parent block selector.
const classes = classnames( 'block-editor-block-contextual-toolbar', {
'has-parent': hasParents,
} );

return (
<div className="block-editor-block-contextual-toolbar-wrapper">
<NavigableToolbar
focusOnMount={ focusOnMount }
className="block-editor-block-contextual-toolbar"
className={ classes }
/* translators: accessibility text for the block toolbar */
aria-label={ __( 'Block tools' ) }
{ ...props }
Expand Down
76 changes: 50 additions & 26 deletions packages/block-editor/src/components/block-parent-selector/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import { getBlockType, store as blocksStore } from '@wordpress/blocks';
import { ToolbarButton } from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { __, sprintf } from '@wordpress/i18n';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import BlockIcon from '../block-icon';
import { useShowMoversGestures } from '../block-toolbar/utils';

/**
* Block parent selector component, displaying the hierarchy of the
Expand All @@ -18,32 +20,52 @@ import BlockIcon from '../block-icon';
* @return {WPComponent} Parent block selector.
*/
export default function BlockParentSelector() {
const { selectBlock } = useDispatch( 'core/block-editor' );
const { parentBlockType, firstParentClientId, shouldHide } = useSelect(
( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientId,
} = select( 'core/block-editor' );
const { hasBlockSupport } = select( blocksStore );
const selectedBlockClientId = getSelectedBlockClientId();
const parents = getBlockParents( selectedBlockClientId );
const _firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( _firstParentClientId );
const _parentBlockType = getBlockType( parentBlockName );
return {
parentBlockType: _parentBlockType,
firstParentClientId: _firstParentClientId,
shouldHide: ! hasBlockSupport(
_parentBlockType,
'__experimentalParentSelector',
true
),
};
},
[]
const { selectBlock, toggleBlockHighlight } = useDispatch(
'core/block-editor'
);
const {
parentBlockType,
firstParentClientId,
shouldHide,
hasReducedUI,
} = useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientId,
getSettings,
} = select( 'core/block-editor' );
const { hasBlockSupport } = select( blocksStore );
const selectedBlockClientId = getSelectedBlockClientId();
const parents = getBlockParents( selectedBlockClientId );
const _firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( _firstParentClientId );
const _parentBlockType = getBlockType( parentBlockName );
const settings = getSettings();
return {
parentBlockType: _parentBlockType,
firstParentClientId: _firstParentClientId,
shouldHide: ! hasBlockSupport(
_parentBlockType,
'__experimentalParentSelector',
true
),
hasReducedUI: settings.hasReducedUI,
};
}, [] );

// Allows highlighting the parent block outline when focusing or hovering
// the parent block selector within the child.
const nodeRef = useRef();
const { gestures: showMoversGestures } = useShowMoversGestures( {
ref: nodeRef,
onChange( isFocused ) {
if ( isFocused && hasReducedUI ) {
return;
}
toggleBlockHighlight( firstParentClientId, isFocused );
},
} );

if ( shouldHide ) {
return null;
Expand All @@ -54,13 +76,15 @@ export default function BlockParentSelector() {
<div
className="block-editor-block-parent-selector"
key={ firstParentClientId }
ref={ nodeRef }
{ ...showMoversGestures }
>
<ToolbarButton
className="block-editor-block-parent-selector__button"
onClick={ () => selectBlock( firstParentClientId ) }
label={ sprintf(
/* translators: %s: Name of the block's parent. */
__( 'Select parent (%s)' ),
__( 'Select %s' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

In #26135 this change was also proposed, but I don't think it's a good idea for situations where a block might be nested in same block type - e.g. a group within a group. That seems very confusing, I think it's probably quite confusing already with the icon being the same as the block icon.

Are there other options? I proposed just making it 'Select parent'. Alternatively could the text wrap in the text labels mode?

cc @tellthemachines.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the same block nested within itself is enough of a special case that we might just as well treat that one separately. "Select parent" is rather technical and cumbersome for most cases (for example, selecting the Gallery that contains an Image, or the Quote that has a paragraph, etc). I think all of these cases benefit from a more straightforward phrasing.

parentBlockType.title
) }
showTooltip
Expand Down
13 changes: 7 additions & 6 deletions packages/block-editor/src/components/block-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ export default function BlockToolbar( { hideDragHandle } ) {
};
}, [] );

// Handles highlighting the current block outline on hover or focus of the
// block type toolbar area.
const { toggleBlockHighlight } = useDispatch( 'core/block-editor' );
const nodeRef = useRef();

const { showMovers, gestures: showMoversGestures } = useShowMoversGestures(
{
ref: nodeRef,
Expand All @@ -79,6 +80,8 @@ export default function BlockToolbar( { hideDragHandle } ) {
}
);

// Account for the cases where the block toolbar is rendered within the
// header area and not contextually to the block.
const displayHeaderToolbar =
useViewportMatch( 'medium', '<' ) || hasFixedToolbar;

Expand All @@ -104,12 +107,10 @@ export default function BlockToolbar( { hideDragHandle } ) {

return (
<div className={ classes }>
{ ! isMultiToolbar && ! displayHeaderToolbar && (
<BlockParentSelector clientIds={ blockClientIds } />
) }
<div ref={ nodeRef } { ...showMoversGestures }>
{ ! isMultiToolbar && (
<div className="block-editor-block-toolbar__block-parent-selector-wrapper">
<BlockParentSelector clientIds={ blockClientIds } />
</div>
) }
{ ( shouldShowVisualToolbar || isMultiToolbar ) && (
<ToolbarGroup className="block-editor-block-toolbar__block-controls">
<BlockSwitcher clientIds={ blockClientIds } />
Expand Down
26 changes: 10 additions & 16 deletions packages/block-editor/src/components/block-toolbar/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@
}
}

.block-editor-block-contextual-toolbar.has-parent {
margin-left: calc(#{$grid-unit-60} + #{$grid-unit-10});
}

.block-editor-block-parent-selector {
position: absolute;
top: -$border-width;
left: calc(-#{$grid-unit-60} - #{$grid-unit-10} - #{$border-width});
}

// Block controls.
.block-editor-block-toolbar__block-controls {
// The !important modifier should be removed when https://github.com/WordPress/gutenberg/issues/24898 refactors the spacing grid.
Expand Down Expand Up @@ -91,22 +101,6 @@
}
}

.block-editor-block-toolbar__block-parent-selector-wrapper {
position: absolute;
top: -1px;
left: -1px;
opacity: 0;
transition: all 60ms linear;
z-index: -1; // This makes it slide out from underneath the toolbar.

@include reduce-motion("transition");

.is-showing-movers & {
opacity: 1;
transform: translateY(-($block-toolbar-height + $grid-unit-15));
}
}

.show-icon-labels {
.block-editor-block-toolbar {
.components-button.has-icon {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ async function testGroupKeyboardNavigation(
await page.keyboard.press( 'Tab' );
await expectLabelToHaveFocus( currentBlockLabel );
await pressKeyWithModifier( 'shift', 'Tab' );
await expectLabelToHaveFocus( 'Select parent (Group)' );
await expectLabelToHaveFocus( 'Select Group' );
await page.keyboard.press( 'ArrowRight' );
await expectLabelToHaveFocus( currentBlockTitle );
}
Expand Down