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

Try adding 'Template Parts' as a tab on the inserter. #22612

Closed
wants to merge 14 commits into from
53 changes: 39 additions & 14 deletions packages/block-editor/src/components/inserter/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import InserterSearchForm from './search-form';
import InserterPreviewPanel from './preview-panel';
import InserterBlockList from './block-list';
import BlockPatterns from './block-patterns';
import TemplateParts from './template-parts';

const stopKeyPropagation = ( event ) => event.stopPropagation();

Expand All @@ -41,6 +42,7 @@ function InserterMenu( {
getBlockIndex,
getBlockSelectionEnd,
getBlockOrder,
hasTemplateParts,
} = useSelect(
( select ) => {
const {
Expand All @@ -57,6 +59,8 @@ function InserterMenu( {
}
}
return {
hasTemplateParts: getSettings()
.__experimentalEnableFullSiteEditing,
hasPatterns: !! getSettings().__experimentalBlockPatterns
?.length,
destinationRootClientId: destRootClientId,
Expand Down Expand Up @@ -172,6 +176,36 @@ function InserterMenu( {
</div>
);

const templatePartsTab = (
<div className="block-editor-inserter__scrollable">
<TemplateParts
onInsert={ onInsertBlocks }
filterValue={ filterValue }
/>
</div>
);

const tabsToUse = [
{
name: 'blocks',
/* translators: Blocks tab title in the block inserter. */
title: __( 'Blocks' ),
},
];
if ( showPatterns ) {
tabsToUse.push( {
name: 'patterns',
/* translators: Patterns tab title in the block inserter. */
title: __( 'Patterns' ),
} );
}
if ( hasTemplateParts ) {
tabsToUse.push( {
name: 'template parts',
/* translators: Template Parts tab title in the block inserter. */
title: __( 'Template Parts' ),
} );
}
// Disable reason (no-autofocus): The inserter menu is a modal display, not one which
// is always visible, and one which already incurs this behavior of autoFocus via
// Popover's focusOnMount.
Expand All @@ -186,31 +220,22 @@ function InserterMenu( {
>
<div className="block-editor-inserter__main-area">
<InserterSearchForm onChange={ setFilterValue } />
{ showPatterns && (
{ tabsToUse.length > 1 && (
<TabPanel
className="block-editor-inserter__tabs"
tabs={ [
{
name: 'blocks',
/* translators: Blocks tab title in the block inserter. */
title: __( 'Blocks' ),
},
{
name: 'patterns',
/* translators: Patterns tab title in the block inserter. */
title: __( 'Patterns' ),
},
] }
tabs={ tabsToUse }
>
{ ( tab ) => {
if ( tab.name === 'blocks' ) {
return blocksTab;
} else if ( tab.name === 'template parts' ) {
return templatePartsTab;
}
return patternsTab;
} }
</TabPanel>
) }
{ ! showPatterns && blocksTab }
{ tabsToUse.length === 1 && blocksTab }
</div>
{ showInserterHelpPanel && hoveredItem && (
<div className="block-editor-inserter__preview-container">
Expand Down
6 changes: 4 additions & 2 deletions packages/block-editor/src/components/inserter/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ $block-inserter-tabs-height: 44px;
flex-shrink: 0;
}

.block-editor-inserter__patterns-item {
.block-editor-inserter__patterns-item,
.block-editor-inserter__template-part-item {
border-radius: $radius-block-ui;
cursor: pointer;
margin-top: $grid-unit-20;
Expand All @@ -271,7 +272,8 @@ $block-inserter-tabs-height: 44px;
}
}

.block-editor-inserter__patterns-item-title {
.block-editor-inserter__patterns-item-title,
.block-editor-inserter__template-part-item-title {
padding: $grid-unit-05;
font-size: 12px;
text-align: center;
Expand Down
197 changes: 197 additions & 0 deletions packages/block-editor/src/components/inserter/template-parts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
/**
* WordPress dependencies
*/
import { useSelect, useDispatch } from '@wordpress/data';
import { parse, createBlock } from '@wordpress/blocks';
import { useMemo, useCallback } from '@wordpress/element';
import { ENTER, SPACE } from '@wordpress/keycodes';
import { __, sprintf } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import BlockPreview from '../block-preview';
import InserterPanel from './panel';
import useAsyncList from './use-async-list';

/**
* External dependencies
*/
import { groupBy } from 'lodash';

function TemplatePartPlaceholder() {
return (
<div className="block-editor-inserter__template-part-item is-placeholder" />
);
}

function TemplatePartItem( { templatePart, onInsert } ) {
const { id, slug, theme } = templatePart;
// The 'raw' property is not defined for a brief period in the save cycle.
// The fallback prevents an error in the parse function while saving.
const content = templatePart.content.raw || '';
const blocks = useMemo( () => parse( content ), [ content ] );
const { createSuccessNotice } = useDispatch( 'core/notices' );

const onClick = useCallback( () => {
const templatePartBlock = createBlock( 'core/template-part', {
postId: id,
slug,
theme,
} );
onInsert( templatePartBlock );
createSuccessNotice(
sprintf(
/* translators: %s: template part title. */
__( 'Template Part "%s" inserted.' ),
slug
),
{
type: 'snackbar',
}
);
}, [ id, slug, theme ] );

return (
<div
className="block-editor-inserter__template-part-item"
role="button"
onClick={ onClick }
onKeyDown={ ( event ) => {
if ( ENTER === event.keyCode || SPACE === event.keyCode ) {
onClick();
}
} }
tabIndex={ 0 }
aria-label={ templatePart.slug }
>
<BlockPreview blocks={ blocks } />
<div className="block-editor-inserter__template-part-item-title">
{ templatePart.slug }
</div>
</div>
);
}

function TemplatePartsByTheme( { templateParts, onInsert } ) {
const templatePartsByTheme = useMemo( () => {
return Object.values( groupBy( templateParts, 'meta.theme' ) );
}, [ templateParts ] );
const currentShownTPs = useAsyncList( templateParts );

return (
<>
{ templatePartsByTheme.length &&
templatePartsByTheme.map( ( templatePartList ) => (
<InserterPanel
key={ templatePartList[ 0 ].meta.theme }
title={ templatePartList[ 0 ].meta.theme }
>
{ templatePartList.map( ( templatePart ) => {
return currentShownTPs.includes( templatePart ) ? (
<TemplatePartItem
key={ templatePart.id }
templatePart={ templatePart }
onInsert={ onInsert }
/>
) : (
<TemplatePartPlaceholder
key={ templatePart.id }
/>
);
} ) }
</InserterPanel>
) ) }
</>
);
}

function TemplatePartSearchResults( { templateParts, onInsert, filterValue } ) {
const filteredTPs = useMemo( () => {
// Filter based on value.
const lowerFilterValue = filterValue.toLowerCase();
const searchResults = templateParts.filter(
( { slug, meta: { theme } } ) =>
slug.toLowerCase().includes( lowerFilterValue ) ||
theme.toLowerCase().includes( lowerFilterValue )
);
// Order based on value location.
searchResults.sort( ( a, b ) => {
// First prioritize index found in slug.
const indexInSlugA = a.slug
.toLowerCase()
.indexOf( lowerFilterValue );
const indexInSlugB = b.slug
.toLowerCase()
.indexOf( lowerFilterValue );
if ( indexInSlugA !== -1 && indexInSlugB !== -1 ) {
return indexInSlugA - indexInSlugB;
} else if ( indexInSlugA !== -1 ) {
return -1;
} else if ( indexInSlugB !== -1 ) {
return 1;
}
// Second prioritize index found in theme.
return (
a.meta.theme.toLowerCase().indexOf( lowerFilterValue ) -
b.meta.theme.toLowerCase().indexOf( lowerFilterValue )
);
} );
return searchResults;
}, [ filterValue, templateParts ] );

const currentShownTPs = useAsyncList( filteredTPs );

return (
<>
{ filteredTPs.map( ( templatePart ) => (
<InserterPanel
key={ templatePart.id }
title={ templatePart.meta.theme }
>
{ currentShownTPs.includes( templatePart ) ? (
<TemplatePartItem
key={ templatePart.id }
templatePart={ templatePart }
onInsert={ onInsert }
/>
) : (
<TemplatePartPlaceholder key={ templatePart.id } />
) }
</InserterPanel>
) ) }
</>
);
}

export default function TemplateParts( { onInsert, filterValue } ) {
const templateParts = useSelect( ( select ) => {
return select( 'core' ).getEntityRecords(
'postType',
'wp_template_part',
{
status: [ 'publish', 'auto-draft' ],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add

Suggested change
status: [ 'publish', 'auto-draft' ],
resolved: true,
status: [ 'publish', 'auto-draft' ],
theme,

to only show template parts for the current theme? (This would mean we'd need to get theme via the getCurrentThemeSelector.)

Copy link
Contributor Author

@Addison-Stavlo Addison-Stavlo Jun 5, 2020

Choose a reason for hiding this comment

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

I don't think so. The theme is arbitrary, and user created template parts will be saved under a theme tag that does not match the current theme. Similarly, if we have the template part saved, the theme it originated from shouldn't be a restriction on if we can use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. Just to make sure though, could that be a problem when exporting a theme?

Copy link
Contributor

@ockham ockham Jun 8, 2020

Choose a reason for hiding this comment

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

To put on my devil's advocate's hat, I'm not even 100% convinced that we should allow inserting template parts from a different theme TBH 🤔 E.g. I'd expect a theme's header template part to kind of reflect its overall look and feel, so using a different theme's template part might give a rather inconsistent look and feel.

Furthermore, if we make available the template parts from all installed themes, it could lead to an overwhelming number of template parts; and it could be confusing to find the current theme's ones (e.g. to pick the right header). (Clearly, the latter is mitigated by grouping template parts by theme, as this PR does.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Template parts from non-active themes will only show if you have customized them.

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'd expect a theme's header template part to kind of reflect its overall look and feel, so using a different theme's template part might give a rather inconsistent look and feel.

It definitely could, but it allows users to choose from other template parts they have created or customized. If they want to use them over the theme's supplied header template part, they can choose to do so. If the user still feels it doesn't fully match the style, they can be further customized.

Maybe in the future we may need to take steps to make the template part supplied by the current theme to be more prominent (default to the top of the list or something). But since "Template parts from non-active themes will only show if you have customized them.", this may not be necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Template parts from non-active themes will only show if you have customized them.

That could be confusing enough tho, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could be confusing enough tho, no?

Good point, it could be confusing between base template parts supplied by the theme vs. ones you have customized from that theme? In which case maybe we could end up presenting it as 'Customized from 'theme-name''. as opposed to just 'theme-name'

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should show the non-customized ones if they have a customized counterpart.

}
);
}, [] );

if ( ! templateParts || ! templateParts.length ) {
return null;
}

if ( filterValue ) {
return (
<TemplatePartSearchResults
templateParts={ templateParts }
onInsert={ onInsert }
filterValue={ filterValue }
/>
);
}

return (
<TemplatePartsByTheme
templateParts={ templateParts }
onInsert={ onInsert }
/>
);
}