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

Render Selected Block Tools in Header when using Top Toolbar #55787

Merged
merged 20 commits into from
Nov 10, 2023
Merged

Render Selected Block Tools in Header when using Top Toolbar #55787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2c813ae
Add selected block tools slot to edit post header
jeryj Nov 1, 2023
45e1888
Add selected block tools slot to site editor header
jeryj Nov 1, 2023
8b0cd57
Add selected tools slot to widget header
jeryj Nov 1, 2023
0d580ba
Add contextual toolbar as fill when top toolbar option is on
jeryj Nov 1, 2023
c31e186
Remove absolute positioned block tool hacks
jeryj Sep 19, 2023
d634599
Move inline rich popover into header if top toolbar slot is available
jeryj Nov 1, 2023
7c96b45
isBlockToolsCollapsed state and styles for edit site header
jeryj Nov 2, 2023
1b3186e
Top Toolbar styles
jeryj Nov 3, 2023
b7348ae
Only render top toolbar in header slot if large viewport
jeryj Nov 3, 2023
ea00096
Cleanup CSS of selected block toolbar
jeryj Nov 7, 2023
1c1cd73
Remove unnecessary flex-shrink properties
jeryj Nov 8, 2023
62a4591
Test that block toolbar is scrollable
jeryj Nov 8, 2023
27432cc
Move tab order of block tools test
jeryj Nov 8, 2023
dd90378
Remove header slot in favor of rendering BlockContextualToolbar directly
jeryj Nov 9, 2023
238f4e5
Check for isZoomOutMode for site editor header
jeryj Nov 9, 2023
5e57aee
Fix edit post edit template header layout
jeryj Nov 9, 2023
8c9dd02
Target closest toolbar instead of classname
jeryj Nov 10, 2023
dd7cade
Use BlockContextualToolbar privately
jeryj Nov 10, 2023
558ff4a
Add top toolbar to customize widgets editor
jeryj Nov 10, 2023
4941d98
Consistent naming of isLargeViewport
jeryj Nov 10, 2023
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
8 changes: 8 additions & 0 deletions packages/block-editor/src/components/block-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@
}
}

.block-editor-block-contextual-toolbar.is-fixed {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it hasn't been introduce in this PR but the isFixed prop in the contextual toolbar is weird. It's "contextual" toolbar why have an isFixed prop :). Anyway, I'm curious when this was introduced and the reasoning for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know the reason why it was added, but my interpretation of contextual meant "tools filled contextually with relevant block actions." As in, the block tools were all contextual relevant to the block selected. I did not interpret "contextual" as "adjacent to." I think changing the component name to have Popover in it would be clearer, like <BlockToolbarPopover />.

Copy link
Contributor

Choose a reason for hiding this comment

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

We already have things like BlockPopover and SelectedBlockPopover which is probably very similar to what you propose. It seems we do need to bring some clarity to the components and more importantly have a clear purpose for each component that stays clear as we iterate and refactor.

position: sticky;
top: 0;
z-index: z-index(".block-editor-block-popover");
display: block;
width: 100%;
}
Comment on lines +59 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Styles for small screen fixed toolbar. This was previously spread over the edit-post, edit-site, and edit-widgets packages, but looks like a core style that is applicable for the standalone editor design.


// on desktop browsers the fixed toolbar has tweaked borders
@include break-medium() {
.block-editor-block-contextual-toolbar.is-fixed {
Expand Down
223 changes: 41 additions & 182 deletions packages/block-editor/src/components/block-tools/block-contextual-toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,8 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import {
forwardRef,
useLayoutEffect,
useEffect,
useRef,
useState,
} from '@wordpress/element';
import { hasBlockSupport, store as blocksStore } from '@wordpress/blocks';
import { useSelect } from '@wordpress/data';
import {
ToolbarItem,
ToolbarButton,
ToolbarGroup,
} from '@wordpress/components';
import { next, previous } from '@wordpress/icons';
import { useViewportMatch } from '@wordpress/compose';

/**
* Internal dependencies
Expand All @@ -32,141 +18,46 @@ import BlockToolbar from '../block-toolbar';
import { store as blockEditorStore } from '../../store';
import { useHasAnyBlockControls } from '../block-controls/use-has-block-controls';

function UnforwardedBlockContextualToolbar(
{ focusOnMount, isFixed, ...props },
ref
) {
// When the toolbar is fixed it can be collapsed
const [ isCollapsed, setIsCollapsed ] = useState( false );
const toolbarButtonRef = useRef();

const isLargeViewport = useViewportMatch( 'medium' );
const {
blockType,
blockEditingMode,
hasParents,
showParentSelector,
selectedBlockClientId,
} = useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientIds,
getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockType } = select( blocksStore );
const selectedBlockClientIds = getSelectedBlockClientIds();
const _selectedBlockClientId = selectedBlockClientIds[ 0 ];
const parents = getBlockParents( _selectedBlockClientId );
const firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( firstParentClientId );
const parentBlockType = getBlockType( parentBlockName );

return {
selectedBlockClientId: _selectedBlockClientId,
blockType:
_selectedBlockClientId &&
getBlockType( getBlockName( _selectedBlockClientId ) ),
blockEditingMode: getBlockEditingMode( _selectedBlockClientId ),
hasParents: parents.length,
showParentSelector:
parentBlockType &&
getBlockEditingMode( firstParentClientId ) === 'default' &&
hasBlockSupport(
parentBlockType,
'__experimentalParentSelector',
true
) &&
selectedBlockClientIds.length <= 1 &&
getBlockEditingMode( _selectedBlockClientId ) === 'default',
};
}, [] );

useEffect( () => {
setIsCollapsed( false );
}, [ selectedBlockClientId ] );

const isLargerThanTabletViewport = useViewportMatch( 'large', '>=' );
const isFullscreen =
document.body.classList.contains( 'is-fullscreen-mode' );

/**
* The following code is a workaround to fix the width of the toolbar
* it should be removed when the toolbar will be rendered inline
* FIXME: remove this layout effect when the toolbar is no longer
* absolutely positioned
*/
useLayoutEffect( () => {
// don't do anything if not fixed toolbar
if ( ! isFixed ) {
return;
}

const blockToolbar = document.querySelector(
'.block-editor-block-contextual-toolbar'
);

if ( ! blockToolbar ) {
return;
}

if ( ! blockType ) {
blockToolbar.style.width = 'initial';
return;
}

if ( ! isLargerThanTabletViewport ) {
// set the width of the toolbar to auto
blockToolbar.style = {};
return;
}

if ( isCollapsed ) {
// set the width of the toolbar to auto
blockToolbar.style.width = 'auto';
return;
}

// get the width of the pinned items in the post editor or widget editor
const pinnedItems = document.querySelector(
'.edit-post-header__settings, .edit-widgets-header__actions'
);
// get the width of the left header in the site editor
const leftHeader = document.querySelector(
'.edit-site-header-edit-mode__end'
);

const computedToolbarStyle = window.getComputedStyle( blockToolbar );
const computedPinnedItemsStyle = pinnedItems
? window.getComputedStyle( pinnedItems )
: false;
const computedLeftHeaderStyle = leftHeader
? window.getComputedStyle( leftHeader )
: false;

const marginLeft = parseFloat( computedToolbarStyle.marginLeft );
const pinnedItemsWidth = computedPinnedItemsStyle
? parseFloat( computedPinnedItemsStyle.width )
: 0;
const leftHeaderWidth = computedLeftHeaderStyle
? parseFloat( computedLeftHeaderStyle.width )
: 0;

// set the new witdth of the toolbar
blockToolbar.style.width = `calc(100% - ${
leftHeaderWidth +
pinnedItemsWidth +
marginLeft +
( pinnedItems || leftHeader ? 2 : 0 ) + // Prevents button focus border from being cut off
( isFullscreen ? 0 : 160 ) // the width of the admin sidebar expanded
}px)`;
}, [
isFixed,
isLargerThanTabletViewport,
isCollapsed,
isFullscreen,
blockType,
] );
export default function BlockContextualToolbar( {
focusOnMount,
isFixed,
...props
} ) {
const { blockType, blockEditingMode, hasParents, showParentSelector } =
useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientIds,
getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockType } = select( blocksStore );
const selectedBlockClientIds = getSelectedBlockClientIds();
const _selectedBlockClientId = selectedBlockClientIds[ 0 ];
const parents = getBlockParents( _selectedBlockClientId );
const firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( firstParentClientId );
const parentBlockType = getBlockType( parentBlockName );

return {
selectedBlockClientId: _selectedBlockClientId,
blockType:
_selectedBlockClientId &&
getBlockType( getBlockName( _selectedBlockClientId ) ),
blockEditingMode: getBlockEditingMode( _selectedBlockClientId ),
hasParents: parents.length,
showParentSelector:
parentBlockType &&
getBlockEditingMode( firstParentClientId ) === 'default' &&
hasBlockSupport(
parentBlockType,
'__experimentalParentSelector',
true
) &&
selectedBlockClientIds.length <= 1 &&
getBlockEditingMode( _selectedBlockClientId ) === 'default',
};
}, [] );

const isToolbarEnabled =
blockType &&
Expand All @@ -183,12 +74,10 @@ function UnforwardedBlockContextualToolbar(
const classes = classnames( 'block-editor-block-contextual-toolbar', {
'has-parent': hasParents && showParentSelector,
'is-fixed': isFixed,
'is-collapsed': isCollapsed,
} );

return (
<NavigableToolbar
ref={ ref }
focusOnMount={ focusOnMount }
focusEditorOnEscape
className={ classes }
Expand All @@ -197,37 +86,7 @@ function UnforwardedBlockContextualToolbar(
variant={ isFixed ? 'unstyled' : undefined }
{ ...props }
>
{ ! isCollapsed && <BlockToolbar hideDragHandle={ isFixed } /> }
{ isFixed && isLargeViewport && blockType && (
<ToolbarGroup
className={
isCollapsed
? 'block-editor-block-toolbar__group-expand-fixed-toolbar'
: 'block-editor-block-toolbar__group-collapse-fixed-toolbar'
}
>
<ToolbarItem
as={ ToolbarButton }
ref={ toolbarButtonRef }
icon={ isCollapsed ? next : previous }
onClick={ () => {
setIsCollapsed( ( collapsed ) => ! collapsed );
toolbarButtonRef.current.focus();
} }
label={
isCollapsed
? __( 'Show block tools' )
: __( 'Hide block tools' )
}
/>
</ToolbarGroup>
) }
<BlockToolbar hideDragHandle={ isFixed } />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Collapsed button is now a part of the Site Editor header, as that was the only place it was used. Also, the block toolbar does not need to know about the Collapsed state.

The rest of the red in this file is related to removing the absolute positioning hacks.

Copy link
Contributor

Choose a reason for hiding this comment

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

The component here is named BlockContextualToolbar. It was never meant to render the "top toolbar". The original idea has been:

  • If hasFixedToolbar is false (default), the BlockContextualToolbar is rendered, otherwise it's not and you are (consumer of block-editor package, in our case edit-* packages) responsible of rendering the block toolbar by using the <BlockToolbar /> component directly.

I know this has changed over time but this PR might be the opportunity to restore that simple behavior.

</NavigableToolbar>
);
}

export const BlockContextualToolbar = forwardRef(
UnforwardedBlockContextualToolbar
);

export default BlockContextualToolbar;
30 changes: 18 additions & 12 deletions packages/block-editor/src/components/block-tools/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ export default function BlockTools( {
moveBlocksDown,
} = useDispatch( blockEditorStore );

const selectedBlockToolsRef = useRef( null );

function onKeyDown( event ) {
if ( event.defaultPrevented ) return;

Expand Down Expand Up @@ -132,7 +130,7 @@ export default function BlockTools( {
insertBeforeBlock( clientIds[ 0 ] );
}
} else if ( isMatch( 'core/block-editor/unselect', event ) ) {
if ( selectedBlockToolsRef?.current?.contains( event.target ) ) {
if ( event.target.closest( '[role=toolbar]' ) ) {
// This shouldn't be necessary, but we have a combination of a few things all combining to create a situation where:
// - Because the block toolbar uses createPortal to populate the block toolbar fills, we can't rely on the React event bubbling to hit the onKeyDown listener for the block toolbar
// - Since we can't use the React tree, we use the DOM tree which _should_ handle the event bubbling correctly from a `createPortal` element.
Expand Down Expand Up @@ -164,6 +162,12 @@ export default function BlockTools( {
const blockToolbarRef = usePopoverScroll( __unstableContentRef );
const blockToolbarAfterRef = usePopoverScroll( __unstableContentRef );

// Conditions for fixed toolbar
// 1. Not zoom out mode
// 2. It's a large viewport. If it's a smaller viewport, let the floating toolbar handle it as it already has styles attached to make it render that way.
// 3. Fixed toolbar is enabled
const isTopToolbar = ! isZoomOutMode && hasFixedToolbar && isLargeViewport;

return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<div { ...props } onKeyDown={ onKeyDown }>
Expand All @@ -173,13 +177,11 @@ export default function BlockTools( {
__unstableContentRef={ __unstableContentRef }
/>
) }
{ ! isZoomOutMode &&
( hasFixedToolbar || ! isLargeViewport ) && (
<BlockContextualToolbar
ref={ selectedBlockToolsRef }
isFixed
/>
) }
{ /* If there is no slot available, such as in the standalone block editor, render within the editor */ }

{ ! isLargeViewport && ( // Small viewports always get a fixed toolbar
<BlockContextualToolbar isFixed />
) }

{ showEmptyBlockSideInserter && (
<EmptyBlockInserter
Expand All @@ -191,14 +193,18 @@ export default function BlockTools( {
needed for navigation and zoom-out mode. */ }
{ ! showEmptyBlockSideInserter && hasSelectedBlock && (
<SelectedBlockTools
ref={ selectedBlockToolsRef }
__unstableContentRef={ __unstableContentRef }
clientId={ clientId }
/>
) }

{ /* Used for the inline rich text toolbar. */ }
<Popover.Slot name="block-toolbar" ref={ blockToolbarRef } />
{ ! isTopToolbar && (
<Popover.Slot
name="block-toolbar"
ref={ blockToolbarRef }
/>
) }
{ children }
{ /* Used for inline rich text popovers. */ }
<Popover.Slot
Expand Down
16 changes: 6 additions & 10 deletions packages/block-editor/src/components/block-tools/selected-block-tools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { forwardRef, useRef, useEffect } from '@wordpress/element';
import { useRef, useEffect } from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';
import { useShortcut } from '@wordpress/keyboard-shortcuts';

Expand All @@ -21,10 +21,11 @@ import useBlockToolbarPopoverProps from './use-block-toolbar-popover-props';
import useSelectedBlockToolProps from './use-selected-block-tool-props';
import { useShouldContextualToolbarShow } from '../../utils/use-should-contextual-toolbar-show';

function UnforwardedSelectedBlockTools(
{ clientId, showEmptyBlockSideInserter, __unstableContentRef },
ref
) {
export default function SelectedBlockTools( {
clientId,
showEmptyBlockSideInserter,
__unstableContentRef,
} ) {
const {
capturingClientId,
isInsertionPointVisible,
Expand Down Expand Up @@ -101,7 +102,6 @@ function UnforwardedSelectedBlockTools(
>
{ shouldShowContextualToolbar && (
<BlockContextualToolbar
ref={ ref }
// If the toolbar is being shown because of being forced
// it should focus the toolbar right after the mount.
focusOnMount={ isToolbarForced.current }
Expand All @@ -128,7 +128,3 @@ function UnforwardedSelectedBlockTools(

return null;
}

export const SelectedBlockTools = forwardRef( UnforwardedSelectedBlockTools );

export default SelectedBlockTools;
Loading
Loading