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

Zoom Out: Remove zoom-out toolbar #66039

Merged
merged 9 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
62 changes: 0 additions & 62 deletions packages/block-editor/src/components/block-popover/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import clsx from 'clsx';
*/
import { useMergeRefs } from '@wordpress/compose';
import { Popover } from '@wordpress/components';
import { useSelect } from '@wordpress/data';
import {
forwardRef,
useMemo,
Expand All @@ -22,8 +21,6 @@ import {
import { useBlockElement } from '../block-list/use-block-props/use-block-refs';
import usePopoverScroll from './use-popover-scroll';
import { rectUnion, getVisibleElementBounds } from '../../utils/dom';
import { store as blockEditorStore } from '../../store';
import { unlock } from '../../lock-unlock';

const MAX_POPOVER_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER;

Expand Down Expand Up @@ -77,38 +74,12 @@ function BlockPopover(
};
}, [ selectedElement ] );

const { isZoomOut, parentSectionBlock, isSectionSelected } = useSelect(
( select ) => {
const {
isZoomOut: isZoomOutSelector,
getSectionRootClientId,
getParentSectionBlock,
getBlockOrder,
} = unlock( select( blockEditorStore ) );

return {
isZoomOut: isZoomOutSelector(),
parentSectionBlock:
getParentSectionBlock( clientId ) ?? clientId,
isSectionSelected: getBlockOrder(
getSectionRootClientId()
).includes( clientId ),
};
},
[ clientId ]
);

// This element is used to position the zoom out view vertical toolbar
// correctly, relative to the selected section.
const parentSectionElement = useBlockElement( parentSectionBlock );

const popoverAnchor = useMemo( () => {
if (
// popoverDimensionsRecomputeCounter is by definition always equal or greater
// than 0. This check is only there to satisfy the correctness of the
// exhaustive-deps rule for the `useMemo` hook.
popoverDimensionsRecomputeCounter < 0 ||
( isZoomOut && ! parentSectionElement ) ||
! selectedElement ||
( bottomClientId && ! lastSelectedElement )
) {
Expand All @@ -117,35 +88,6 @@ function BlockPopover(

return {
getBoundingClientRect() {
// The zoom out view has a vertical block toolbar that should always
// be on the edge of the canvas, aligned to the top of the currently
// selected section. This condition changes the anchor of the toolbar
// to the section instead of the block to handle blocks that are
// not full width and nested blocks to keep section height.
if ( isZoomOut && isSectionSelected ) {
// Compute the height based on the parent section of the
// selected block, because the selected block may be
// shorter than the section.
const canvasElementRect = getVisibleElementBounds(
__unstableContentRef.current
);
const parentSectionElementRect =
getVisibleElementBounds( parentSectionElement );
const anchorHeight =
parentSectionElementRect.bottom -
parentSectionElementRect.top;

// Always use the width of the section root element to make sure
// the toolbar is always on the edge of the canvas.
const anchorWidth = canvasElementRect.width;
return new window.DOMRectReadOnly(
canvasElementRect.left,
parentSectionElementRect.top,
anchorWidth,
anchorHeight
);
}

return lastSelectedElement
? rectUnion(
getVisibleElementBounds( selectedElement ),
Expand All @@ -157,13 +99,9 @@ function BlockPopover(
};
}, [
popoverDimensionsRecomputeCounter,
isZoomOut,
parentSectionElement,
selectedElement,
bottomClientId,
lastSelectedElement,
isSectionSelected,
__unstableContentRef,
] );

if ( ! selectedElement || ( bottomClientId && ! lastSelectedElement ) ) {
Expand Down
17 changes: 15 additions & 2 deletions packages/block-editor/src/components/block-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
isReusableBlock,
isTemplatePart,
} from '@wordpress/blocks';
import { ToolbarGroup } from '@wordpress/components';
import { ToolbarGroup, ToolbarButton } from '@wordpress/components';

/**
* Internal dependencies
Expand All @@ -35,6 +35,7 @@ import { store as blockEditorStore } from '../../store';
import __unstableBlockNameContext from './block-name-context';
import NavigableToolbar from '../navigable-toolbar';
import { useHasBlockToolbar } from './use-has-block-toolbar';
import Shuffle from './shuffle';
import { unlock } from '../../lock-unlock';

/**
Expand Down Expand Up @@ -67,6 +68,7 @@ export function PrivateBlockToolbar( {
isUsingBindings,
hasParentPattern,
hasContentOnlyLocking,
showShuffleButton,
} = useSelect( ( select ) => {
const {
getBlockName,
Expand All @@ -79,6 +81,7 @@ export function PrivateBlockToolbar( {
getBlockParentsByBlockName,
getTemplateLock,
getParentSectionBlock,
isZoomOutMode,
} = unlock( select( blockEditorStore ) );
const selectedBlockClientIds = getSelectedBlockClientIds();
const selectedBlockClientId = selectedBlockClientIds[ 0 ];
Expand Down Expand Up @@ -119,6 +122,7 @@ export function PrivateBlockToolbar( {
shouldShowVisualToolbar: isValid && isVisual,
toolbarKey: `${ selectedBlockClientId }${ parentClientId }`,
showParentSelector:
! isZoomOutMode() &&
parentBlockType &&
getBlockEditingMode( parentClientId ) !== 'disabled' &&
hasBlockSupport(
Expand All @@ -130,6 +134,7 @@ export function PrivateBlockToolbar( {
isUsingBindings: _isUsingBindings,
hasParentPattern: _hasParentPattern,
hasContentOnlyLocking: _hasTemplateLock,
showShuffleButton: isZoomOutMode(),
};
}, [] );

Expand Down Expand Up @@ -179,7 +184,7 @@ export function PrivateBlockToolbar( {
key={ toolbarKey }
>
<div ref={ toolbarWrapperRef } className={ innerClasses }>
{ ! isMultiToolbar && isLargeViewport && (
{ showParentSelector && ! isMultiToolbar && isLargeViewport && (
<BlockParentSelector />
) }
{ ( shouldShowVisualToolbar || isMultiToolbar ) &&
Expand All @@ -205,6 +210,14 @@ export function PrivateBlockToolbar( {
{ ! hasContentOnlyLocking &&
shouldShowVisualToolbar &&
isMultiToolbar && <BlockGroupToolbar /> }
{ showShuffleButton && (
<ToolbarGroup>
<Shuffle
clientId={ blockClientIds[ 0 ] }
as={ ToolbarButton }
/>
</ToolbarGroup>
) }
{ shouldShowVisualToolbar && (
<>
<BlockControls.Slot
Expand Down
15 changes: 2 additions & 13 deletions packages/block-editor/src/components/block-tools/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
default as InsertionPoint,
} from './insertion-point';
import BlockToolbarPopover from './block-toolbar-popover';
import ZoomOutPopover from './zoom-out-popover';
import { store as blockEditorStore } from '../../store';
import usePopoverScroll from '../block-popover/use-popover-scroll';
import ZoomOutModeInserters from './zoom-out-mode-inserters';
Expand Down Expand Up @@ -75,11 +74,8 @@ export default function BlockTools( {
isGroupable,
} = useSelect( blockEditorStore );
const { getGroupingBlockName } = useSelect( blocksStore );
const {
showEmptyBlockSideInserter,
showBlockToolbarPopover,
showZoomOutToolbar,
} = useShowBlockTools();
const { showEmptyBlockSideInserter, showBlockToolbarPopover } =
useShowBlockTools();

const {
duplicateBlocks,
Expand Down Expand Up @@ -211,13 +207,6 @@ export default function BlockTools( {
/>
) }

{ showZoomOutToolbar && (
<ZoomOutPopover
__unstableContentRef={ __unstableContentRef }
clientId={ clientId }
/>
) }

{ /* Used for the inline rich text toolbar. Until this toolbar is combined into BlockToolbar, someone implementing their own BlockToolbar will also need to use this to see the image caption toolbar. */ }
{ ! isZoomOutMode && ! hasFixedToolbar && (
<Popover.Slot
Expand Down
20 changes: 0 additions & 20 deletions packages/block-editor/src/components/block-tools/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -187,23 +187,3 @@
top: 50%;
left: 50%;
}

.zoom-out-toolbar {

.block-editor-block-mover-button.block-editor-block-mover-button:focus-visible::before,
.zoom-out-toolbar-button:focus::before,
.block-editor-block-toolbar-shuffle:focus::before,
.block-selection-button_drag-handle:focus::before {
box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color);
}

.block-editor-block-mover {
background: none;
border: none;
}

// Make the spacing consistent between controls.
.zoom-out-toolbar-button {
height: $button-size-next-default-40px;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ export function useShowBlockTools() {
getSettings,
__unstableGetEditorMode,
isTyping,
getBlockOrder,
getSectionRootClientId,
} = unlock( select( blockEditorStore ) );

const clientId =
Expand All @@ -44,17 +42,7 @@ export function useShowBlockTools() {
! isTyping() &&
editorMode === 'edit' &&
isEmptyDefaultBlock;
const isZoomOut = editorMode === 'zoom-out';
const isSectionSelected = getBlockOrder(
getSectionRootClientId()
).includes( clientId );
const _showZoomOutToolbar =
clientId &&
isZoomOut &&
! _showEmptyBlockSideInserter &&
isSectionSelected;
const _showBlockToolbarPopover =
! _showZoomOutToolbar &&
! getSettings().hasFixedToolbar &&
! _showEmptyBlockSideInserter &&
hasSelectedBlock &&
Expand All @@ -63,7 +51,6 @@ export function useShowBlockTools() {
return {
showEmptyBlockSideInserter: _showEmptyBlockSideInserter,
showBlockToolbarPopover: _showBlockToolbarPopover,
showZoomOutToolbar: _showZoomOutToolbar,
};
}, [] );
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,24 @@ function ZoomOutModeInserters() {
const [ isReady, setIsReady ] = useState( false );
const {
hasSelection,
insertionPoint,
blockOrder,
blockInsertionPointVisible,
setInserterIsOpened,
sectionRootClientId,
selectedBlockClientId,
} = useSelect( ( select ) => {
const {
getSettings,
getInsertionPoint,
getBlockOrder,
getSelectionStart,
getSelectedBlockClientId,
getSectionRootClientId,
isBlockInsertionPointVisible,
} = unlock( select( blockEditorStore ) );

const root = getSectionRootClientId();

return {
hasSelection: !! getSelectionStart().clientId,
insertionPoint: getInsertionPoint(),
blockOrder: getBlockOrder( root ),
blockInsertionPointVisible: isBlockInsertionPointVisible(),
sectionRootClientId: root,
setInserterIsOpened:
getSettings().__experimentalSetIsInserterOpened,
Expand All @@ -64,41 +58,32 @@ function ZoomOutModeInserters() {
return null;
}

return [ undefined, ...blockOrder ].map( ( clientId, index ) => {
const shouldRenderInsertionPoint =
blockInsertionPointVisible && insertionPoint?.index === index;
const previousClientId = selectedBlockClientId;
const index = blockOrder.findIndex(
( clientId ) => selectedBlockClientId === clientId
);
const nextClientId = blockOrder[ index + 1 ];

const previousClientId = clientId;
const nextClientId = blockOrder[ index ];

const isSelected =
selectedBlockClientId === previousClientId ||
selectedBlockClientId === nextClientId;

return (
<BlockPopoverInbetween
key={ index }
previousClientId={ previousClientId }
nextClientId={ nextClientId }
>
{ ! shouldRenderInsertionPoint && isSelected && (
<ZoomOutModeInserterButton
onClick={ () => {
setInserterIsOpened( {
rootClientId: sectionRootClientId,
insertionIndex: index,
tab: 'patterns',
category: 'all',
} );
showInsertionPoint( sectionRootClientId, index, {
operation: 'insert',
} );
} }
/>
) }
</BlockPopoverInbetween>
);
} );
return (
<BlockPopoverInbetween
previousClientId={ previousClientId }
nextClientId={ nextClientId }
>
<ZoomOutModeInserterButton
Comment on lines +67 to +72
Copy link
Contributor

Choose a reason for hiding this comment

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

We lost the previous inserter. This only shows the inserter after the selected block. There should be one before and after the selected block.

Copy link
Contributor Author

@youknowriad youknowriad Oct 17, 2024

Choose a reason for hiding this comment

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

This was a request by @richtabor

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it interfered with the block toolbar.

Copy link
Contributor

Choose a reason for hiding this comment

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

It will still interfere with the toolbar when you scroll down though, right? Is there a solution we can have without removing it entirely? The inserters don't render until they're present on the screen, so removing the top inserter highlights an issue where there is no focusable Add pattern button available for the keyboard on patterns that extend past the bottom of the viewport.

Copy link
Contributor

Choose a reason for hiding this comment

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

The inserters don't render until they're present on the screen, so removing the top inserter highlights an issue where there is no focusable Add pattern button available for the keyboard on patterns that extend past the bottom of the viewport.

@jeryj Would you be able to raise an Issue for that one? Any suggestions on a fix would be good. I assume it's "add back the inserters" but that will require input from Design so feel free to ping the design group here.

Copy link
Contributor

Choose a reason for hiding this comment

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

onClick={ () => {
setInserterIsOpened( {
rootClientId: sectionRootClientId,
insertionIndex: index,
tab: 'patterns',
category: 'all',
} );
showInsertionPoint( sectionRootClientId, index, {
operation: 'insert',
} );
} }
/>
</BlockPopoverInbetween>
);
}

export default ZoomOutModeInserters;
Loading
Loading