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

Expose Template part block variations to the Inserter #30032

Merged
merged 2 commits into from
Mar 22, 2021

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Mar 19, 2021

Description

Related: #30020

This PR exposes the Template part block variation to the Inserter. In order to do that I have created an area block attribute in Template Part which is taken into account in isActive function to find a matching block variation.

This new attribute is removed after the actual save of the Template part and is only used for the Placeholder/insertion part for various reasons like passing this value to the Template part entity, proper display information in BlockCard etc...

It is removed because the Template part block renders its own entity internally. Any attributes on the outer block itself are just block attributes, and thus saved as part of the post it is a child of (template, page, etc). The area term allows us to save that value with the template part post type.

In block variations isActive function first we check for area block attribute and if is not there it means that either we have selected the base Template part block or that it has been created so we try to find a matching block variation from the entity (post type).

Testing instructions

  1. You have to enable a block-based theme
  2. Go to a page/post etc
  3. Add a Header or Footer block from any Inserter (main, slash, etc..)
  4. Observe that the proper information are show in all places (BlockCard, icons..)
  5. Two scenarios here:
    1. Click New template part and observe in Advanced settings the area. Also that the proper information are shown.
    2. Click Choose existing and see that the area in Advanced settings comes from the existing template part.

In both cases before you click anything, you can see in the code editor that the area block attribute is set and after either action from the above, the area block attribute has been removed.

Screenshots

tplpvariations

Removal of the attribute

removedAttribute

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Mar 19, 2021

Size Change: +27 B (0%)

Total Size: 1.41 MB

Filename Size Change
build/block-library/index.js 148 kB +27 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.77 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.63 kB 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-editor/index.js 127 kB 0 B
build/block-editor/style-rtl.css 12.4 kB 0 B
build/block-editor/style.css 12.4 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 479 B 0 B
build/block-library/blocks/button/style.css 479 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 364 B 0 B
build/block-library/blocks/buttons/style.css 363 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 421 B 0 B
build/block-library/blocks/columns/style.css 421 B 0 B
build/block-library/blocks/cover/editor-rtl.css 605 B 0 B
build/block-library/blocks/cover/editor.css 605 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.24 kB 0 B
build/block-library/blocks/cover/style.css 1.24 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 199 B 0 B
build/block-library/blocks/file/editor.css 198 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.46 kB 0 B
build/block-library/blocks/freeform/editor.css 2.46 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.11 kB 0 B
build/block-library/blocks/gallery/style.css 1.1 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 159 B 0 B
build/block-library/blocks/latest-comments/editor.css 158 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 269 B 0 B
build/block-library/blocks/latest-comments/style.css 269 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/list/editor-rtl.css 65 B 0 B
build/block-library/blocks/list/editor.css 65 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 626 B 0 B
build/block-library/blocks/navigation-link/editor.css 627 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 685 B 0 B
build/block-library/blocks/navigation-link/style.css 682 B 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.11 kB 0 B
build/block-library/blocks/navigation/editor.css 1.11 kB 0 B
build/block-library/blocks/navigation/style-rtl.css 204 B 0 B
build/block-library/blocks/navigation/style.css 205 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 170 B 0 B
build/block-library/blocks/page-list/editor.css 170 B 0 B
build/block-library/blocks/page-list/style-rtl.css 537 B 0 B
build/block-library/blocks/page-list/style.css 536 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 63 B 0 B
build/block-library/blocks/preformatted/style.css 63 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/query/editor-rtl.css 795 B 0 B
build/block-library/blocks/query/editor.css 794 B 0 B
build/block-library/blocks/quote/editor-rtl.css 61 B 0 B
build/block-library/blocks/quote/editor.css 61 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 165 B 0 B
build/block-library/blocks/search/editor.css 165 B 0 B
build/block-library/blocks/search/style-rtl.css 342 B 0 B
build/block-library/blocks/search/style.css 344 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 236 B 0 B
build/block-library/blocks/separator/style.css 236 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 201 B 0 B
build/block-library/blocks/site-logo/editor.css 201 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 115 B 0 B
build/block-library/blocks/site-logo/style.css 115 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 776 B 0 B
build/block-library/blocks/social-links/editor.css 776 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 317 B 0 B
build/block-library/blocks/spacer/editor.css 317 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 552 B 0 B
build/block-library/blocks/template-part/editor.css 551 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/editor-rtl.css 50 B 0 B
build/block-library/blocks/verse/editor.css 50 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 504 B 0 B
build/block-library/blocks/video/editor.css 503 B 0 B
build/block-library/blocks/video/style-rtl.css 187 B 0 B
build/block-library/blocks/video/style.css 187 B 0 B
build/block-library/common-rtl.css 1.1 kB 0 B
build/block-library/common.css 1.1 kB 0 B
build/block-library/editor-rtl.css 9.47 kB 0 B
build/block-library/editor.css 9.47 kB 0 B
build/block-library/reset-rtl.css 374 B 0 B
build/block-library/reset.css 376 B 0 B
build/block-library/style-rtl.css 8.88 kB 0 B
build/block-library/style.css 8.89 kB 0 B
build/block-library/theme-rtl.css 700 B 0 B
build/block-library/theme.css 701 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.3 kB 0 B
build/components/index.js 284 kB 0 B
build/components/style-rtl.css 16.2 kB 0 B
build/components/style.css 16.2 kB 0 B
build/compose/index.js 11.2 kB 0 B
build/core-data/index.js 16.7 kB 0 B
build/customize-widgets/index.js 5.99 kB 0 B
build/customize-widgets/style-rtl.css 378 B 0 B
build/customize-widgets/style.css 379 B 0 B
build/data-controls/index.js 831 B 0 B
build/data/index.js 8.87 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 787 B 0 B
build/dom-ready/index.js 577 B 0 B
build/dom/index.js 4.97 kB 0 B
build/edit-navigation/index.js 11.9 kB 0 B
build/edit-navigation/style-rtl.css 1.41 kB 0 B
build/edit-navigation/style.css 1.4 kB 0 B
build/edit-post/index.js 307 kB 0 B
build/edit-post/style-rtl.css 7.05 kB 0 B
build/edit-post/style.css 7.04 kB 0 B
build/edit-site/index.js 27.2 kB 0 B
build/edit-site/style-rtl.css 4.51 kB 0 B
build/edit-site/style.css 4.5 kB 0 B
build/edit-widgets/index.js 20.2 kB 0 B
build/edit-widgets/style-rtl.css 3.14 kB 0 B
build/edit-widgets/style.css 3.14 kB 0 B
build/editor/index.js 41.9 kB 0 B
build/editor/style-rtl.css 3.9 kB 0 B
build/editor/style.css 3.9 kB 0 B
build/element/index.js 4.61 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.75 kB 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 4.01 kB 0 B
build/is-shallow-equal/index.js 699 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/index.js 3.15 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.85 kB 0 B
build/nux/index.js 3.41 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.89 kB 0 B
build/primitives/index.js 1.42 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/react-i18n/index.js 1.46 kB 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/reusable-blocks/index.js 3.78 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/rich-text/index.js 13.3 kB 0 B
build/server-side-render/index.js 2.58 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 3.02 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

*/
variations.forEach( ( variation ) => {
if ( variation.isActive ) return;
variation.isActive = ( blockAttributes, variationAttributes ) => {
Copy link
Member

Choose a reason for hiding this comment

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

I know this predates the PR, but the isActive nomenclature is quite confusing to me. What does it represent?

Copy link
Contributor Author

@ntsekouras ntsekouras Mar 22, 2021

Choose a reason for hiding this comment

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

From the docs:

A function that accepts a block’s attributes and the variation’s attributes and determines if a variation is active. This function doesn’t try to find a match dynamically based on all block’s attributes, as in many cases some attributes are irrelevant. An example would be for embed block where we only care about providerNameSlug attribute’s value.

This is useful for cases you want to use information from the block variation, after a block’s creation. In this case though we have a combination with information from variation and entity attributes (before and after creation).

const { area, theme, slug } = blockAttributes;
// We first check the `area` block attribute which is set during insertion.
// This property is removed on the creation of a template part.
if ( area ) return area === variationAttributes.area;
Copy link
Member

Choose a reason for hiding this comment

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

I like this flow, it should make a lot of things more flexible for full template patterns

slug: 'template-part',
content: serialize( innerBlocks ),
};
// If we have `area` set from block attributes, means an exposed
Copy link
Member

Choose a reason for hiding this comment

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

The comments are very helpful, thanks for adding them

Copy link
Member

@gziolo gziolo Mar 22, 2021

Choose a reason for hiding this comment

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

This is how the area is sourced:

$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;
}

It feels like we should make it work similar to the meta in the post content that is stored in attributes and magically saved together with the block to the right place. @mcsf, what do you think?

@mtias
Copy link
Member

mtias commented Mar 19, 2021

This is looking good to me

@gziolo gziolo requested a review from mcsf March 22, 2021 06:41
// block variation was inserted. So add this prop to the template
// part entity on creation. Afterwards remove `area` value from
// block attributes.
if ( [ 'header', 'footer' ].includes( area ) ) record.area = area;
Copy link
Member

Choose a reason for hiding this comment

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

There are more values available and they are filterable on the server:

$default_area_values = array(
WP_TEMPLATE_PART_AREA_HEADER,
WP_TEMPLATE_PART_AREA_FOOTER,
WP_TEMPLATE_PART_AREA_SIDEBAR,
WP_TEMPLATE_PART_AREA_UNCATEGORIZED,
);
/**
* Filters the list of allowed template part area values.
*
* @param array $default_area_values An array of supported area values.
*/
return apply_filters( 'default_wp_template_part_areas', $default_area_values );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but we are exposing only the above two in the Inserter (header, footer). When we decide to add the Sidebar we will include this as well. Regarding the uncategorized, is the default if not provided so we don't need anything explicit provided there for the base Template part block.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but we are exposing only the above two in the Inserter (header, footer). When we decide to add the Sidebar we will include this as well.

How do you plan to ensure it happens when someone changes the list of block variations? I don't think those two should know of each other. You also need to keep in mind that block variations can be filtered.

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 got what you mean now, thanks for explaining. I removed the check since it is handled on the server and it defaults to uncategorized if the provided value is not allowed.

slug: 'template-part',
content: serialize( innerBlocks ),
};
// If we have `area` set from block attributes, means an exposed
Copy link
Member

@gziolo gziolo Mar 22, 2021

Choose a reason for hiding this comment

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

This is how the area is sourced:

$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;
}

It feels like we should make it work similar to the meta in the post content that is stored in attributes and magically saved together with the block to the right place. @mcsf, what do you think?

@gziolo gziolo added the [Type] Bug An existing feature does not function as intended label Mar 22, 2021
@ntsekouras ntsekouras force-pushed the try/expose-template-part-variations branch from 0d6d2f4 to 168fd03 Compare March 22, 2021 08:24
@gziolo
Copy link
Member

gziolo commented Mar 22, 2021

Nice work. I think it's the way to go with the current setup of the Template Part block and how the block variations work.

It is removed because the Template part block renders its own entity internally. Any attributes on the outer block itself are just block attributes, and thus saved as part of the post it is a child of (template, page, etc). The area term allows us to save that value with the template part post type.

I think this is the part that is a bit confusing because the same information can be sourced from two places depending on the flow. It raises the question of whether we could treat block related entities similar to how blocks can work with a post's meta:
https://developer.wordpress.org/block-editor/reference-guides/block-api/block-attributes/#meta

It might be impossible in practice to abstract it, but I'm curious what people think. /cc @mcsf, @youknowriad and @ellatrix.

@ntsekouras ntsekouras force-pushed the try/expose-template-part-variations branch from fb15117 to 07fcc58 Compare March 22, 2021 09:38
@carolinan
Copy link
Contributor

carolinan commented Mar 22, 2021

I followed the testing instructions above and the PR worked well for me with TT1 Blocks.

@carolinan
Copy link
Contributor

If I type /footer and then select an existing header template part (listed in templateParts in theme.json),
the area is general, is that expected?

@ntsekouras
Copy link
Contributor Author

ntsekouras commented Mar 22, 2021

Thanks for testing Carolina :)

If I type /footer and then select an existing header template part (listed in templateParts in theme.json),
the area is general, is that expected?

Yes. If we end up wanting to use an existing Template part it should use its own attributes/props. So in your case I guess the area of the template part you selected was not defined or set to the corresponding value for general, correct?

Two scenarios here:
Click New template part and observe in Advanced settings the area. Also that the proper information are shown.
Click Choose existing and see that the area in Advanced settings comes from the existing template part.

@carolinan
Copy link
Contributor

carolinan commented Mar 22, 2021

Yes. If we end up wanting to use an existing Template part it should use its own attributes/props. So in your case I guess the area of the template part you selected was not defined or set to the corresponding value for general, correct?

They are defined. My test themes templateParts are updated to match this PR:
#29828 (comment)

	"templateParts": [
		{
			"name": "header",
			"area": "header"
		},
		{
			"name": "footer",
			"area": "footer"
		}
	],

While TT1 Blocks (where, according to above video, it works) uses the old format
https://github.com/WordPress/theme-experiments/blob/master/tt1-blocks/experimental-theme.json#L2

Could that be it?

@gziolo
Copy link
Member

gziolo commented Mar 22, 2021

While TT1 Blocks uses the old format
https://github.com/WordPress/theme-experiments/blob/master/tt1-blocks/experimental-theme.json#L2

Could that be it?

It might be related, @ntsekouras would know better. I opened PR to updated the format in the TT1 Blocks theme last week: WordPress/theme-experiments#232.

@ntsekouras
Copy link
Contributor Author

Could that be it?

Good catch 👍

It seems so, yes :) - I'll change how this is handled with the new format.

@ntsekouras
Copy link
Contributor Author

ntsekouras commented Mar 22, 2021

@carolinan can you give it one more try? I updated the code to check for the new format.

edit: No need to test yet, as a PR was merged 4 hours ago that changed the implementation for getting the template parts, so my code won't work well :). I have to rebase and reimplement

@ntsekouras ntsekouras force-pushed the try/expose-template-part-variations branch from e49900a to 83fce5a Compare March 22, 2021 15:18
@ntsekouras
Copy link
Contributor Author

It's working with the new format with the rebase :) @gziolo had made the proper transformations.

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

The code looks good to me, using the block attribute only in creation as a bridge to having that term saved with the entity makes sense.

Tested as well and it works great!

@ntsekouras ntsekouras merged commit 1c2c0ae into trunk Mar 22, 2021
@ntsekouras ntsekouras deleted the try/expose-template-part-variations branch March 22, 2021 15:57
@github-actions github-actions bot added this to the Gutenberg 10.3 milestone Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants