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

Template Part: Swap block action places #42221

Merged
merged 5 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ $z-layers: (
".reusable-blocks-menu-items__convert-modal": 1000001,
".edit-site-create-template-part-modal": 1000001,
".block-editor-block-lock-modal": 1000001,
".block-editor-template-part__selection-modal": 1000001,
Copy link
Member Author

Choose a reason for hiding this comment

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

We need the modal above the dropdown.


// Note: The ConfirmDialog component's z-index is being set to 1000001 in packages/components/src/confirm-dialog/styles.ts
// because it uses emotion and not sass. We need it to render on top its parent popover.
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export {
export { default as BlockSettingsMenu } from './block-settings-menu';
export { default as BlockSettingsMenuControls } from './block-settings-menu-controls';
export { default as BlockTitle } from './block-title';
export { default as __experimentalUseBlockDisplayTitle } from './block-title/use-block-display-title';
Mamaduka marked this conversation as resolved.
Show resolved Hide resolved
export { default as BlockToolbar } from './block-toolbar';
export { default as BlockTools } from './block-tools';
export {
Expand Down
56 changes: 32 additions & 24 deletions packages/block-library/src/template-part/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,15 @@ import { isEmpty } from 'lodash';
*/
import { useSelect } from '@wordpress/data';
import {
BlockControls,
BlockSettingsMenuControls,
useBlockProps,
__experimentalUseNoRecursiveRenders as useNoRecursiveRenders,
Warning,
store as blockEditorStore,
__experimentalUseBlockDisplayTitle as useBlockDisplayTitle,
__experimentalUseNoRecursiveRenders as useNoRecursiveRenders,
__experimentalUseBlockOverlayActive as useBlockOverlayActive,
} from '@wordpress/block-editor';
import {
ToolbarGroup,
ToolbarButton,
Spinner,
Modal,
} from '@wordpress/components';
import { Spinner, Modal, MenuItem } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { store as coreStore } from '@wordpress/core-data';
import { useState } from '@wordpress/element';
Expand All @@ -43,6 +39,7 @@ export default function TemplatePartEdit( {
attributes,
setAttributes,
clientId,
isSelected,
} ) {
const { slug, theme, tagName, layout = {} } = attributes;
const templatePartId = createTemplatePartId( theme, slug );
Expand Down Expand Up @@ -85,6 +82,7 @@ export default function TemplatePartEdit( {
},
[ templatePartId, clientId ]
);
const blockTitle = useBlockDisplayTitle( { clientId, maximumLength: 25 } );
const { templateParts } = useAlternativeTemplateParts(
area,
templatePartId
Expand All @@ -105,6 +103,14 @@ export default function TemplatePartEdit( {
const isEntityAvailable = ! isPlaceholder && ! isMissing && isResolved;
const TagName = tagName || areaObject.tagName;

// The `isSelected` check ensures the `BlockSettingsMenuControls` fill
// doesn't render multiple times. The block controls has similar internal check.
Comment on lines +105 to +106
Copy link
Member Author

Choose a reason for hiding this comment

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

const canReplace =
isSelected &&
isEntityAvailable &&
hasReplacements &&
( area === 'header' || area === 'footer' );

// We don't want to render a missing state if we have any inner blocks.
// A new template part is automatically created if we have any inner blocks but no entity.
if (
Expand Down Expand Up @@ -158,21 +164,23 @@ export default function TemplatePartEdit( {
/>
</TagName>
) }
{ isEntityAvailable &&
hasReplacements &&
( area === 'header' || area === 'footer' ) && (
<BlockControls>
<ToolbarGroup className="wp-block-template-part__block-control-group">
<ToolbarButton
onClick={ () =>
setIsTemplatePartSelectionOpen( true )
}
>
{ __( 'Replace' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
) }
{ canReplace && (
<BlockSettingsMenuControls>
{ () => (
<MenuItem
onClick={ () => {
setIsTemplatePartSelectionOpen( true );
} }
>
{ sprintf(
/* translators: %s: block name */
__( 'Replace %s' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is helpful 👍

blockTitle
) }
</MenuItem>
) }
</BlockSettingsMenuControls>
) }
{ isEntityAvailable && (
<TemplatePartInnerBlocks
tagName={ TagName }
Expand All @@ -189,7 +197,7 @@ export default function TemplatePartEdit( {
) }
{ isTemplatePartSelectionOpen && (
<Modal
className="block-editor-template-part__selection-modal"
overlayClassName="block-editor-template-part__selection-modal"
title={ sprintf(
// Translators: %s as template part area title ("Header", "Footer", etc.).
__( 'Choose a %s' ),
Expand Down
22 changes: 13 additions & 9 deletions packages/block-library/src/template-part/editor.scss
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
.block-editor-template-part__selection-modal {
z-index: z-index(".block-editor-template-part__selection-modal");

// To keep modal dimensions consistent as subsections are navigated, width
// and height are used instead of max-(width/height).
@include break-small() {
width: calc(100% - #{ $grid-unit-20 * 2 });
height: calc(100% - #{ $header-height * 2 });
}
@include break-medium() {
width: $break-medium - $grid-unit-20 * 2;
}
@include break-large() {
height: 70%;
.components-modal__frame {
ntsekouras marked this conversation as resolved.
Show resolved Hide resolved
@include break-small() {
width: calc(100% - #{ $grid-unit-20 * 2 });
height: calc(100% - #{ $header-height * 2 });
}
@include break-medium() {
width: $break-medium - $grid-unit-20 * 2;
}
@include break-large() {
height: 70%;
}
}
}
2 changes: 0 additions & 2 deletions packages/edit-site/src/components/block-editor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import NavigateToLink from '../navigate-to-link';
import { SidebarInspectorFill } from '../sidebar';
import { store as editSiteStore } from '../../store';
import BlockInspectorButton from './block-inspector-button';
import EditTemplatePartMenuButton from '../edit-template-part-menu-button';
import BackButton from './back-button';
import ResizableEditor from './resizable-editor';

Expand Down Expand Up @@ -155,7 +154,6 @@ export default function BlockEditor( { setIsInserterOpen } ) {
onChange={ onChange }
useSubRegistry={ false }
>
<EditTemplatePartMenuButton />
<TemplatePartConverter />
<__experimentalLinkControl.ViewerFill>
{ useCallback(
Expand Down

This file was deleted.

1 change: 1 addition & 0 deletions packages/edit-site/src/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
* Internal dependencies
*/
import './components';
import './template-part-edit';
82 changes: 82 additions & 0 deletions packages/edit-site/src/hooks/template-part-edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { BlockControls } from '@wordpress/block-editor';
import { store as coreStore } from '@wordpress/core-data';
import { ToolbarButton } from '@wordpress/components';
import { addFilter } from '@wordpress/hooks';
import { createHigherOrderComponent } from '@wordpress/compose';

/**
* Internal dependencies
*/
import { useLocation } from '../components/routes';
import { useLink } from '../components/routes/link';

function EditTemplatePartMenuItem( { attributes } ) {
const { theme, slug } = attributes;
const { params } = useLocation();
const templatePart = useSelect(
( select ) => {
return select( coreStore ).getEntityRecord(
'postType',
'wp_template_part',
// Ideally this should be an official public API.
`${ theme }//${ slug }`
);
},
[ theme, slug ]
);

const linkProps = useLink(
{
postId: templatePart?.id,
postType: templatePart?.type,
},
{
fromTemplateId: params.postId,
}
);

if ( ! templatePart ) {
return null;
}

return (
<BlockControls group="other">
<ToolbarButton
{ ...linkProps }
onClick={ ( event ) => {
linkProps.onClick( event );
} }
>
{ __( 'Edit' ) }
</ToolbarButton>
</BlockControls>
);
}

export const withEditBlockControls = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const { attributes, name } = props;
const isDisplayed = name === 'core/template-part' && attributes.slug;

return (
<>
<BlockEdit { ...props } />
{ isDisplayed && (
<EditTemplatePartMenuItem attributes={ attributes } />
) }
</>
);
},
'withEditBlockControls'
);

addFilter(
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting choice using the filter here. I don't mind it personally, and like you mention, there's a parallel with the move-to-widget-area. If it weren't for the useLink and useLocation imports, I'd say it might be good to see if we can somehow get it into the block itself in block-library, but maybe not worth the effort of untangling all that since the filter appears to work well 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

If it weren't for the useLink and useLocation imports...

This is the exact reason I decided to use the filter, plus the "Edit" action is currently Site Editor only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds fair, particularly given that template parts is very site editor centric — so sounds like a reasonable argument for the site editor to have special handling for this block!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review, @andrewserong 🙇

'editor.BlockEdit',
'core/edit-site/template-part-edit-button',
withEditBlockControls
);