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

Jump to bottom

Lighter block DOM: contextual toolbar in popover #18779

Merged
merged 10 commits into from
Dec 23, 2019
Merged

Lighter block DOM: contextual toolbar in popover #18779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
50fc955
Lighter block DOM: contextual toolbar in popover
ellatrix Dec 19, 2019
c8643ae
Movers in toolbar (same style as master)
ellatrix Dec 20, 2019
ea8baaa
Smooth repositioning on move
ellatrix Dec 20, 2019
26220fa
Fix e2e test
ellatrix Dec 20, 2019
3dfb30d
Only translate for animation
ellatrix Dec 20, 2019
494faa2
Address feedback
ellatrix Dec 23, 2019
2c345a5
Specify subpixel direction to work around Chrome bug
ellatrix Dec 23, 2019
3bd6f07
Remove dead CSS
ellatrix Dec 23, 2019
9758221
Polish alignment position
ellatrix Dec 23, 2019
9830ff9
Fix toolbar position for RTL
ellatrix Dec 23, 2019
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
2 changes: 2 additions & 0 deletions packages/base-styles/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ $z-layers: (
// .edit-post-header { z-index: 30 }
".components-notice-list": 29,

// Above the block list, under the header.
".block-editor-block-list__block-popover": 29,

// Show snackbars above everything (similar to popovers)
".components-snackbar-list": 100000,
Expand Down
5 changes: 3 additions & 2 deletions packages/block-editor/src/components/block-list/block-contextual-toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ import { __ } from '@wordpress/i18n';
import NavigableToolbar from '../navigable-toolbar';
import { BlockToolbar } from '../';

function BlockContextualToolbar( { focusOnMount } ) {
function BlockContextualToolbar( { focusOnMount, moverDirection, ...props } ) {
return (
<NavigableToolbar
focusOnMount={ focusOnMount }
className="block-editor-block-contextual-toolbar"
/* translators: accessibility text for the block toolbar */
aria-label={ __( 'Block tools' ) }
{ ...props }
ellatrix marked this conversation as resolved.
Show resolved Hide resolved
>
<BlockToolbar />
<BlockToolbar moverDirection={ moverDirection } />
</NavigableToolbar>
);
}
Expand Down
88 changes: 39 additions & 49 deletions packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
isUnmodifiedDefaultBlock,
getUnregisteredTypeHandlerName,
} from '@wordpress/blocks';
import { KeyboardShortcuts, withFilters } from '@wordpress/components';
import { KeyboardShortcuts, withFilters, Popover } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import {
withDispatch,
Expand All @@ -36,15 +36,13 @@ import { compose, pure, ifCondition } from '@wordpress/compose';
* Internal dependencies
*/
import BlockEdit from '../block-edit';
import BlockMover from '../block-mover';
import BlockDropZone from '../block-drop-zone';
import BlockInvalidWarning from './block-invalid-warning';
import BlockCrashWarning from './block-crash-warning';
import BlockCrashBoundary from './block-crash-boundary';
import BlockHtml from './block-html';
import BlockBreadcrumb from './breadcrumb';
import BlockContextualToolbar from './block-contextual-toolbar';
import BlockMultiControls from './multi-controls';
import BlockInsertionPoint from './insertion-point';
import IgnoreNestedEvents from '../ignore-nested-events';
import Inserter from '../inserter';
Expand Down Expand Up @@ -421,13 +419,6 @@ function BlockListBlock( {
! hasFixedToolbar &&
isHovered &&
! isEmptyDefaultBlock;
// We render block movers and block settings to keep them tabbale even if hidden
const shouldRenderMovers =
! isNavigationMode &&
isSelected &&
! showEmptyBlockSideInserter &&
! isPartOfMultiSelection &&
! isTypingWithinBlock;
const shouldShowBreadcrumb = isNavigationMode && isSelected;
const shouldShowContextualToolbar =
! isNavigationMode &&
Expand Down Expand Up @@ -479,13 +470,6 @@ function BlockListBlock( {
};
}
const blockElementId = `block-${ clientId }`;
const blockMover = (
<BlockMover
clientIds={ clientId }
isHidden={ ! isSelected }
__experimentalOrientation={ moverDirection }
/>
);

// We wrap the BlockEdit component in a div that hides it when editing in
// HTML mode. This allows us to render all of the ancillary pieces
Expand Down Expand Up @@ -522,6 +506,9 @@ function BlockListBlock( {
// If the toolbar is being shown because of being forced
// it should focus the toolbar right after the mount.
focusOnMount={ isForcingContextualToolbar.current }
data-type={ name }
data-align={ wrapperProps ? wrapperProps[ 'data-align' ] : undefined }
moverDirection={ moverDirection }
/>
);

Expand Down Expand Up @@ -563,44 +550,48 @@ function BlockListBlock( {
clientId={ clientId }
rootClientId={ rootClientId }
/> }
<div
className={ classnames(
'block-editor-block-list__block-edit',
{ 'has-mover-inside': moverDirection === 'horizontal' },
) }
>
{ isFirstMultiSelected && (
<BlockMultiControls
rootClientId={ rootClientId }
moverDirection={ moverDirection }
/>
) }
{ shouldRenderMovers && ( moverDirection === 'vertical' ) && blockMover }
{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
ref={ breadcrumb }
/>
) }

<div className="block-editor-block-list__block-edit">
{ ( isCapturingDescendantToolbars ) && (
// A slot made available on all ancestors of the selected Block
// to allow child Blocks to render their toolbars into the DOM
// of the appropriate parent.
<ChildToolbarSlot />
) }

{ ( ! ( hasAncestorCapturingToolbars ) ) && ( shouldShowContextualToolbar || isForcingContextualToolbar.current ) && renderBlockContextualToolbar() }

{ ( hasAncestorCapturingToolbars ) && ( shouldShowContextualToolbar || isForcingContextualToolbar.current ) && (
// If the parent Block is set to consume toolbars of the child Blocks
// then render the child Block's toolbar into the Slot provided
// by the parent.
<ChildToolbar>
{ renderBlockContextualToolbar() }
</ChildToolbar>
{ ( shouldShowBreadcrumb || shouldShowContextualToolbar || isForcingContextualToolbar.current ) && (
<Popover
noArrow
animate={ false }
// Position above the anchor, pop out towards the right,
// and position in the left corner.
// To do: refactor `Popover` to make this prop clearer.
position="top right left"
Copy link
Member

Choose a reason for hiding this comment

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

"top right left" - Is that intentional? What does it mean to be both right and left?

Copy link
Member Author

Choose a reason for hiding this comment

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

The last word, I consider an experimental thing for new, currently the first word is the Y position, the second the direction in which the popover floats (from the arrow), and the last word the corner at which to put the popover. Currently, in master we only support positioning it in the middle horizontally. Ideally Popover should be refactored to make all these positioning clearer.

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 added a comment now.

// Position above the anchor, pop out towards the right,
// and position in the left corner.
// To do: refactor `Popover` to make this prop clearer.

focusOnMount={ false }
anchorRef={ blockNodeRef.current }
className="block-editor-block-list__block-popover"
__unstableSticky={ isPartOfMultiSelection ? '.wp-block.is-multi-selected' : true }
__unstableSlotName="block-toolbar"
// Allow subpixel positioning for the block movement animation.
__unstableAllowVerticalSubpixelPosition={ moverDirection !== 'horizontal' && wrapper.current }
__unstableAllowHorizontalSubpixelPosition={ moverDirection === 'horizontal' && wrapper.current }
>
{ ! hasAncestorCapturingToolbars && ( shouldShowContextualToolbar || isForcingContextualToolbar.current ) && renderBlockContextualToolbar() }
{ hasAncestorCapturingToolbars && ( shouldShowContextualToolbar || isForcingContextualToolbar.current ) && (
// If the parent Block is set to consume toolbars of the child Blocks
// then render the child Block's toolbar into the Slot provided
// by the parent.
<ChildToolbar>
{ renderBlockContextualToolbar() }
</ChildToolbar>
) }
{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
ref={ breadcrumb }
data-align={ wrapperProps ? wrapperProps[ 'data-align' ] : undefined }
/>
) }
</Popover>
) }

{
! isNavigationMode &&
! shouldShowContextualToolbar &&
Expand Down Expand Up @@ -628,7 +619,6 @@ function BlockListBlock( {
{ isValid && mode === 'html' && (
<BlockHtml clientId={ clientId } />
) }
{ shouldRenderMovers && ( moverDirection === 'horizontal' ) && blockMover }
{ ! isValid && [
<BlockInvalidWarning
key="invalid-warning"
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/block-list/breadcrumb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ import BlockTitle from '../block-title';
* @param {string} props.clientId Client ID of block.
* @return {WPComponent} The component to be rendered.
*/
const BlockBreadcrumb = forwardRef( ( { clientId }, ref ) => {
const BlockBreadcrumb = forwardRef( ( { clientId, ...props }, ref ) => {
const { setNavigationMode } = useDispatch( 'core/block-editor' );

return (
<div className="block-editor-block-list__breadcrumb">
<div className="block-editor-block-list__breadcrumb" { ...props }>
<Toolbar>
<Button ref={ ref } onClick={ () => setNavigationMode( false ) }>
<BlockTitle clientId={ clientId } />
Expand Down
39 changes: 0 additions & 39 deletions packages/block-editor/src/components/block-list/multi-controls.js
View file
Open in desktop

This file was deleted.

Loading