Skip to content

Block switcher: don't get all parsed patterns when unopened #57020

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

Closed
ellatrix wants to merge 1 commit into from
Closed

Block switcher: don't get all parsed patterns when unopened #57020

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
228 changes: 140 additions & 88 deletions packages/block-editor/src/components/block-switcher/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,20 @@ import BlockStylesMenu from './block-styles-menu';
import PatternTransformationsMenu from './pattern-transformations-menu';
import useBlockDisplayTitle from '../block-title/use-block-display-title';

export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
function DropDownPanel( {
clientIds,
blocks,
onClose,
blockVariationTransformations,
hasPossibleBlockTransformations,
hasPossibleBlockVariationTransformations,
} ) {
const { replaceBlocks, multiSelect, updateBlockAttributes } =
useDispatch( blockEditorStore );
const blockInformation = useBlockDisplayInformation( blocks[ 0 ].clientId );
const {
possibleBlockTransformations,
canRemove,
hasBlockStyles,
icon,
patterns,
} = useSelect(
( select ) => {
Expand All @@ -47,57 +52,30 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
__experimentalGetPatternTransformItems,
canRemoveBlocks,
} = select( blockEditorStore );
const { getBlockStyles, getBlockType } = select( blocksStore );
const { getBlockStyles } = select( blocksStore );
const rootClientId = getBlockRootClientId(
Array.isArray( clientIds ) ? clientIds[ 0 ] : clientIds
);
const [ { name: firstBlockName } ] = blocks;
const _isSingleBlockSelected = blocks.length === 1;
const styles =
_isSingleBlockSelected && getBlockStyles( firstBlockName );
let _icon;
if ( _isSingleBlockSelected ) {
_icon = blockInformation?.icon; // Take into account active block variations.
} else {
const isSelectionOfSameType =
new Set( blocks.map( ( { name } ) => name ) ).size === 1;
// When selection consists of blocks of multiple types, display an
// appropriate icon to communicate the non-uniformity.
_icon = isSelectionOfSameType
? getBlockType( firstBlockName )?.icon
: copy;
}
return {
possibleBlockTransformations: getBlockTransformItems(
blocks,
rootClientId
),
canRemove: canRemoveBlocks( clientIds, rootClientId ),
hasBlockStyles: !! styles?.length,
icon: _icon,
patterns: __experimentalGetPatternTransformItems(
blocks,
rootClientId
),
};
},
[ clientIds, blocks, blockInformation?.icon ]
[ clientIds, blocks ]
);

const blockVariationTransformations = useBlockVariationTransforms( {
clientIds,
blocks,
} );

const blockTitle = useBlockDisplayTitle( {
clientId: Array.isArray( clientIds ) ? clientIds[ 0 ] : clientIds,
maximumLength: 35,
} );

const isSingleBlock = blocks.length === 1;
const isReusable = isSingleBlock && isReusableBlock( blocks[ 0 ] );
const isTemplate = isSingleBlock && isTemplatePart( blocks[ 0 ] );

function selectForMultipleBlocks( insertedBlocks ) {
if ( insertedBlocks.length > 1 ) {
multiSelect(
Expand Down Expand Up @@ -128,6 +106,119 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
selectForMultipleBlocks( transformedBlocks );
}

const hasPatternTransformation = !! patterns?.length && canRemove;
const hasBlockOrBlockVariationTransforms =
hasPossibleBlockTransformations ||
hasPossibleBlockVariationTransformations;
const showDropDown =
hasBlockStyles ||
hasBlockOrBlockVariationTransforms ||
hasPatternTransformation;

if ( ! showDropDown ) {
return null;
}

return (
<div className="block-editor-block-switcher__container">
{ hasPatternTransformation && (
<PatternTransformationsMenu
blocks={ blocks }
patterns={ patterns }
onSelect={ ( transformedBlocks ) => {
onPatternTransform( transformedBlocks );
onClose();
} }
/>
) }
{ hasBlockOrBlockVariationTransforms && (
<BlockTransformationsMenu
className="block-editor-block-switcher__transforms__menugroup"
possibleBlockTransformations={
possibleBlockTransformations
}
possibleBlockVariationTransformations={
blockVariationTransformations
}
blocks={ blocks }
onSelect={ ( name ) => {
onBlockTransform( name );
onClose();
} }
onSelectVariation={ ( name ) => {
onBlockVariationTransform( name );
onClose();
} }
/>
) }
{ hasBlockStyles && (
<BlockStylesMenu
hoveredBlock={ blocks[ 0 ] }
onSwitch={ onClose }
/>
) }
</div>
);
}

export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
const blockInformation = useBlockDisplayInformation( blocks[ 0 ].clientId );
const { possibleBlockTransformations, canRemove, hasBlockStyles, icon } =
useSelect(
( select ) => {
const {
getBlockRootClientId,
getBlockTransformItems,
canRemoveBlocks,
} = select( blockEditorStore );
const { getBlockStyles, getBlockType } = select( blocksStore );
const rootClientId = getBlockRootClientId(
Array.isArray( clientIds ) ? clientIds[ 0 ] : clientIds
);
const [ { name: firstBlockName } ] = blocks;
const _isSingleBlockSelected = blocks.length === 1;
const styles =
_isSingleBlockSelected && getBlockStyles( firstBlockName );
let _icon;
if ( _isSingleBlockSelected ) {
_icon = blockInformation?.icon; // Take into account active block variations.
} else {
const isSelectionOfSameType =
new Set( blocks.map( ( { name } ) => name ) ).size ===
1;
// When selection consists of blocks of multiple types, display an
// appropriate icon to communicate the non-uniformity.
_icon = isSelectionOfSameType
? getBlockType( firstBlockName )?.icon
: copy;
}
return {
possibleBlockTransformations: getBlockTransformItems(
blocks,
rootClientId
),
canRemove: canRemoveBlocks( clientIds, rootClientId ),
hasBlockStyles: !! styles?.length,
icon: _icon,
};
},
[ clientIds, blocks, blockInformation?.icon ]
);

const blockVariationTransformations = useBlockVariationTransforms( {
clientIds,
blocks,
} );

const blockTitle = useBlockDisplayTitle( {
clientId: Array.isArray( clientIds ) ? clientIds[ 0 ] : clientIds,
maximumLength: 35,
} );

const isSingleBlock = blocks.length === 1;
const isReusable = isSingleBlock && isReusableBlock( blocks[ 0 ] );
const isTemplate = isSingleBlock && isTemplatePart( blocks[ 0 ] );

/**
* The `isTemplate` check is a stopgap solution here.
* Ideally, the Transforms API should handle this
Expand All @@ -137,7 +228,7 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
!! possibleBlockTransformations.length && canRemove && ! isTemplate;
const hasPossibleBlockVariationTransformations =
!! blockVariationTransformations?.length;
const hasPatternTransformation = !! patterns?.length && canRemove;

if (
! hasBlockStyles &&
! hasPossibleBlockTransformations &&
Copy link
Contributor

@ntsekouras ntsekouras Dec 14, 2023

Choose a reason for hiding this comment

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

Unfortunately we can't do this refactoring because we could end up with en empty Dropdown. This highlights an existing bug though because we should add the && ! hasPatternTransformation check here. If we have no styles or block transformations, but we have patterns, now we do not render the Dropdown.

Copy link
Member Author

@ellatrix ellatrix Dec 14, 2023

Choose a reason for hiding this comment

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

But it exists in trunk :D

Copy link
Member Author

@ellatrix ellatrix Dec 14, 2023

Choose a reason for hiding this comment

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

Ok, I guess we need lighter has* selectors?

Copy link
Contributor

@ntsekouras ntsekouras Dec 14, 2023

Choose a reason for hiding this comment

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

Yeah probably some has selectors. I actually introduced a private one yesterday - not for this case. We would need something that filters first by blockType and then parses, but what concerns me a bit is that with lazy loading the patterns parsing will have to be refactored anyway. So maybe wait a bit for these? 🤔

Copy link
Contributor

@ntsekouras ntsekouras Dec 14, 2023

Choose a reason for hiding this comment

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

But it exists in trunk :D

I opened a PR for the bug in trunk

Expand Down Expand Up @@ -180,13 +271,6 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
blocks.length
);

const hasBlockOrBlockVariationTransforms =
hasPossibleBlockTransformations ||
hasPossibleBlockVariationTransformations;
const showDropDown =
hasBlockStyles ||
hasBlockOrBlockVariationTransforms ||
hasPatternTransformation;
return (
<ToolbarGroup>
<ToolbarItem>
Expand Down Expand Up @@ -218,54 +302,22 @@ export const BlockSwitcherDropdownMenu = ( { clientIds, blocks } ) => {
} }
menuProps={ { orientation: 'both' } }
>
{ ( { onClose } ) =>
showDropDown && (
<div className="block-editor-block-switcher__container">
{ hasPatternTransformation && (
<PatternTransformationsMenu
blocks={ blocks }
patterns={ patterns }
onSelect={ (
transformedBlocks
) => {
onPatternTransform(
transformedBlocks
);
onClose();
} }
/>
) }
{ hasBlockOrBlockVariationTransforms && (
<BlockTransformationsMenu
className="block-editor-block-switcher__transforms__menugroup"
possibleBlockTransformations={
possibleBlockTransformations
}
possibleBlockVariationTransformations={
blockVariationTransformations
}
blocks={ blocks }
onSelect={ ( name ) => {
onBlockTransform( name );
onClose();
} }
onSelectVariation={ ( name ) => {
onBlockVariationTransform(
name
);
onClose();
} }
/>
) }
{ hasBlockStyles && (
<BlockStylesMenu
hoveredBlock={ blocks[ 0 ] }
onSwitch={ onClose }
/>
) }
</div>
)
}
{ ( { onClose } ) => (
<DropDownPanel
clientIds={ clientIds }
blocks={ blocks }
onClose={ onClose }
blockVariationTransformations={
blockVariationTransformations
}
hasPossibleBlockTransformations={
hasPossibleBlockTransformations
}
hasPossibleBlockVariationTransformations={
hasPossibleBlockVariationTransformations
}
/>
) }
</DropdownMenu>
) }
</ToolbarItem>
Expand Down