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 block: Add category panel #29159

Merged
merged 9 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions packages/block-library/src/template-part/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
"type": "string"
},
"tagName": {
"type": "string",
"default": "div"
"type": "string"
}
},
"supports": {
Expand Down
89 changes: 89 additions & 0 deletions packages/block-library/src/template-part/edit/advanced-controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/**
* WordPress dependencies
*/
import { useEntityProp } from '@wordpress/core-data';
import { SelectControl, TextControl } from '@wordpress/components';
import { sprintf, __ } from '@wordpress/i18n';
import { InspectorAdvancedControls } from '@wordpress/block-editor';

/**
* Internal dependencies
*/
import { getTagBasedOnArea } from './get-tag-based-on-area';

const AREA_OPTIONS = [
{ label: __( 'Header' ), value: 'header' },
{ label: __( 'Footer' ), value: 'footer' },
{
label: __( 'General' ),
value: 'uncategorized',
},
];

export function TemplatePartAdvancedControls( {
tagName,
setAttributes,
isEntityAvailable,
templatePartId,
} ) {
const [ area, setArea ] = useEntityProp(
'postType',
'wp_template_part',
'area',
templatePartId
);

const [ title, setTitle ] = useEntityProp(
'postType',
'wp_template_part',
'title',
templatePartId
);

return (
<InspectorAdvancedControls>
{ isEntityAvailable && (
<>
<TextControl
label={ __( 'Title' ) }
value={ title }
onChange={ ( value ) => {
setTitle( value );
} }
onFocus={ ( event ) => event.target.select() }
/>

<SelectControl
label={ __( 'Area' ) }
labelPosition="top"
options={ AREA_OPTIONS }
value={ area }
onChange={ setArea }
/>
</>
) }
<SelectControl
label={ __( 'HTML element' ) }
options={ [
{
label: sprintf(
/* translators: %s: HTML tag based on area. */
__( 'Default based on area (%s)' ),
`<${ getTagBasedOnArea( area ) }>`
),
value: '',
},
{ label: '<header>', value: 'header' },
{ label: '<main>', value: 'main' },
{ label: '<section>', value: 'section' },
{ label: '<article>', value: 'article' },
{ label: '<aside>', value: 'aside' },
{ label: '<footer>', value: 'footer' },
{ label: '<div>', value: 'div' },
] }
value={ tagName || '' }
onChange={ ( value ) => setAttributes( { tagName: value } ) }
/>
</InspectorAdvancedControls>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const AREA_TAGS = {
footer: 'footer',
header: 'header',
unactegorized: 'div',
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo 😬

Suggested change
unactegorized: 'div',
uncategorized: 'div',

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe we should consider renaming to something else, now that we're using "area" rather than "category.

Maybe unassigned? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Some template parts need not be assigned to a specific area, but "uncategorized" or "unassigned" might make them feel miss-placed or unorganised. That doesn't feel like the appropriate tone.

Perhaps something like "General" or "Universal"?

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo Mar 17, 2021

Choose a reason for hiding this comment

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

"uncategorized" or "unassigned" seems appropriate in the case that an item exists and this value has never been assigned or distinguished. Or that no other existing value settings are appropriate.

"General" and "Universal" could definitely be 'areas', but they do not fit the same purpose as uncategorized/unassigned. We cannot assume something is "Universal" just because it doesn't fit into the existing categories, and when an item is not universal and does not fit into existing categories something like unassigned/uncategorized serves a purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

#29937 to fix the typos.

Perhaps something like "General" or "Universal"?

I just realized/remembered on the user facing side, 'uncategorized' is being displayed as "General".

};

export function getTagBasedOnArea( area ) {
return AREA_TAGS[ area ] || AREA_TAGS.unactegorized;
}
53 changes: 18 additions & 35 deletions packages/block-library/src/template-part/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@
import { useSelect } from '@wordpress/data';
import {
BlockControls,
InspectorAdvancedControls,
InspectorControls,
useBlockProps,
Warning,
store as blockEditorStore,
} from '@wordpress/block-editor';
import {
SelectControl,
Dropdown,
PanelBody,
ToolbarGroup,
ToolbarButton,
Spinner,
Expand All @@ -25,13 +21,14 @@ import { store as coreStore } from '@wordpress/core-data';
/**
* Internal dependencies
*/
import TemplatePartNamePanel from './name-panel';
import TemplatePartInnerBlocks from './inner-blocks';
import TemplatePartPlaceholder from './placeholder';
import TemplatePartSelection from './selection';
import { TemplatePartAdvancedControls } from './advanced-controls';
import { getTagBasedOnArea } from './get-tag-based-on-area';

export default function TemplatePartEdit( {
attributes: { slug, theme, tagName: TagName = 'div' },
attributes: { slug, theme, tagName },
setAttributes,
clientId,
} ) {
Expand All @@ -40,9 +37,9 @@ export default function TemplatePartEdit( {
// Set the postId block attribute if it did not exist,
// but wait until the inner blocks have loaded to allow
// new edits to trigger this.
const { isResolved, innerBlocks, isMissing } = useSelect(
const { isResolved, innerBlocks, isMissing, area } = useSelect(
( select ) => {
const { getEntityRecord, hasFinishedResolution } = select(
const { getEditedEntityRecord, hasFinishedResolution } = select(
coreStore
);
const { getBlocks } = select( blockEditorStore );
Expand All @@ -53,16 +50,20 @@ export default function TemplatePartEdit( {
templatePartId,
];
const entityRecord = templatePartId
? getEntityRecord( ...getEntityArgs )
? getEditedEntityRecord( ...getEntityArgs )
: null;
const hasResolvedEntity = templatePartId
? hasFinishedResolution( 'getEntityRecord', getEntityArgs )
? hasFinishedResolution(
'getEditedEntityRecord',
getEntityArgs
)
: false;

return {
innerBlocks: getBlocks( clientId ),
isResolved: hasResolvedEntity,
isMissing: hasResolvedEntity && ! entityRecord,
area: entityRecord?.area,
};
},
[ templatePartId, clientId ]
Expand All @@ -71,6 +72,7 @@ export default function TemplatePartEdit( {
const blockProps = useBlockProps();
const isPlaceholder = ! slug;
const isEntityAvailable = ! isPlaceholder && ! isMissing;
const TagName = tagName || getTagBasedOnArea( area );

// 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.
Expand All @@ -91,31 +93,12 @@ export default function TemplatePartEdit( {

return (
<>
<InspectorControls>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice call moving all this to another file. Its definitely a bit more organized now.

<PanelBody>
{ isEntityAvailable && (
<TemplatePartNamePanel postId={ templatePartId } />
) }
</PanelBody>
</InspectorControls>
<InspectorAdvancedControls>
<SelectControl
label={ __( 'HTML element' ) }
options={ [
{ label: __( 'Default (<div>)' ), value: 'div' },
{ label: '<header>', value: 'header' },
{ label: '<main>', value: 'main' },
{ label: '<section>', value: 'section' },
{ label: '<article>', value: 'article' },
{ label: '<aside>', value: 'aside' },
{ label: '<footer>', value: 'footer' },
] }
value={ TagName }
onChange={ ( value ) =>
setAttributes( { tagName: value } )
}
/>
</InspectorAdvancedControls>
<TemplatePartAdvancedControls
tagName={ tagName }
setAttributes={ setAttributes }
isEntityAvailable={ isEntityAvailable }
templatePartId={ templatePartId }
/>
<TagName { ...blockProps }>
{ isPlaceholder && (
<TemplatePartPlaceholder
Expand Down
26 changes: 0 additions & 26 deletions packages/block-library/src/template-part/edit/name-panel.js

This file was deleted.

21 changes: 18 additions & 3 deletions packages/block-library/src/template-part/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
function render_block_core_template_part( $attributes ) {
$content = null;
$area = WP_TEMPLATE_PART_AREA_UNCATEGORIZED;

if ( ! empty( $attributes['postId'] ) && get_post_status( $attributes['postId'] ) ) {
// If we have a post ID and the post exists, which means this template part
Expand All @@ -40,7 +41,11 @@ function render_block_core_template_part( $attributes ) {
if ( $template_part_post ) {
// A published post might already exist if this template part was customized elsewhere
// or if it's part of a customized template.
$content = $template_part_post->post_content;
$content = $template_part_post->post_content;
$area_terms = get_the_terms( $template_part_post, 'wp_template_part_area' );
if ( ! is_wp_error( $area_terms ) && false !== $area_terms ) {
$area = $area_terms[0]->name;
}
} else {
// Else, if the template part was provided by the active theme,
// render the corresponding file content.
Expand All @@ -66,8 +71,18 @@ function render_block_core_template_part( $attributes ) {
} else {
$content = wp_make_content_images_responsive( $content );
}
$content = do_shortcode( $content );
$html_tag = esc_attr( $attributes['tagName'] );
$content = do_shortcode( $content );

if ( empty( $attributes['tagName'] ) ) {
$area_tags = array(
WP_TEMPLATE_PART_AREA_HEADER => 'header',
WP_TEMPLATE_PART_AREA_FOOTER => 'footer',
WP_TEMPLATE_PART_AREA_UNCATEGORIZED => 'div',
);
$html_tag = null !== $area && isset( $area_tags[ $area ] ) ? $area_tags[ $area ] : $area_tags[ WP_TEMPLATE_PART_AREA_UNCATEGORIZED ];
} else {
$html_tag = esc_attr( $attributes['tagName'] );
}
$wrapper_attributes = get_block_wrapper_attributes();

return "<$html_tag $wrapper_attributes>" . str_replace( ']]>', ']]&gt;', $content ) . "</$html_tag>";
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/select-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ function SelectControl(
</DownArrowWrapper>
}
labelPosition={ labelPosition }
{ ...props }
>
<Select
{ ...props }
Expand Down
3 changes: 1 addition & 2 deletions packages/e2e-tests/fixtures/blocks/core__template-part.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
"isValid": true,
"attributes": {
"slug": "header",
"theme": "twentytwenty",
"tagName": "div"
"theme": "twentytwenty"
},
"innerBlocks": [],
"originalContent": ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ const createTemplatePart = async (
);
await openDocumentSettingsSidebar();

const nameInputSelector =
'.block-editor-block-inspector .components-text-control__input';
const nameInput = await page.waitForSelector( nameInputSelector );
const advancedPanelXPath = `//div[contains(@class,"interface-interface-skeleton__sidebar")]//button[@class="components-button components-panel__body-toggle"][contains(text(),"Advanced")]`;
const advancedPanel = await page.waitForXPath( advancedPanelXPath );
await advancedPanel.click();
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if current e2e utils ensureSidebarOpened, findSidebarPanelToggleButtonWithTitle, findSidebarPanelWithTitle, may be useful here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would be great, but just checked them and they are only working in edit-post context 😞

Copy link
Contributor

Choose a reason for hiding this comment

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

are only working in edit-post context

Isn't that the context this function is being run in? It looks like it is used in test setup in before all, after createNewPost and before siteEditor.visit(). Similarly it uses openDocumentSettingsSidebar a few lines above this which is also reliant on edit-post* selectors.


const nameInputXPath = `${ advancedPanelXPath }/ancestor::div[contains(@class, "components-panel__body")]//div[contains(@class,"components-base-control__field")]//label[contains(text(), "Title")]/following-sibling::input`;
const nameInput = await page.waitForXPath( nameInputXPath );
await nameInput.click();

// Select all of the text in the title field.
Expand Down