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.

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

Jump to bottom

Fix template part variations registration subscription. #31772

Closed
wants to merge 3 commits into from
Closed

Fix template part variations registration subscription. #31772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b907d55
fix import to not be ignored by build
Addison-Stavlo May 12, 2021
c2966a8
comment subscription
Addison-Stavlo May 12, 2021
c563f58
rename for verbosity
Addison-Stavlo May 13, 2021
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
4 changes: 3 additions & 1 deletion packages/block-library/src/template-part/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import { layout } from '@wordpress/icons';
*/
import metadata from './block.json';
import edit from './edit';
import './variations';
import setupVariations from './variations';

const { name } = metadata;
export { metadata, name };

setupVariations();

export const settings = {
icon: layout,
__experimentalLabel: ( { slug, theme } ) => {
Expand Down
95 changes: 49 additions & 46 deletions packages/block-library/src/template-part/variations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,55 +6,58 @@ import { store as editorStore } from '@wordpress/editor';
import { store as blocksStore } from '@wordpress/blocks';
import { dispatch, select, subscribe } from '@wordpress/data';

const unsubscribe = subscribe( () => {
const definedVariations = select(
editorStore
).__experimentalGetDefaultTemplatePartAreas();
export default function setupVatiations() {
// Subscribe to wait for the variation definitions to initialize in the editor store.
const unsubscribe = subscribe( () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The global subscribe/select/dispatch is something we avoid using as much as we can. For what reason are we subscribing here? I assume we're waiting for something but what is this thing we're waiting for here?

(We should probably document this somewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its waiting for the template part area definitions from the php side to initialize in the editor store.

Im looking at refactoring to avoid subscribe and register these variations from the server side here -#31761

Copy link
Contributor Author

@Addison-Stavlo Addison-Stavlo May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like that fix is hit or miss for some folks, still have to investigate its inconsistencies.

For now, I would think this subscribe should be alright. Technically its only present until the store initializes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like that fix is hit or miss for some folks, still have to investigate its inconsistencies.

Nm, that seems to be a WP version issue that we should be able to work around with fallbacks.

const definedVariations = select(
editorStore
).__experimentalGetDefaultTemplatePartAreas();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it'd be useful in this case, but just wanted to note it's possible to register variations via register_block_type_from_metadata eg

gutenberg/packages/block-library/src/navigation-link/index.php

Lines 325 to 331 in ff4c3c4

register_block_type_from_metadata(
__DIR__ . '/navigation-link',
array(
'render_callback' => 'render_block_core_navigation_link',
'variations' => array_merge( $built_ins, $variations ),
)
);

There is still some JS needed for icon setting/isActive logic, since there's a bit more API work to be done to help serialize that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see you're already created #31761 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you! I saw your PR registering them that way and it works great here. 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oddly that approach doesn't seem to work with everyones testing. For some reason a couple folks seem to have that filter run before the actual php side registers the variations (?)- #31761 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

See example in #29095

Server variation support landed in WP 5.8, so there's still a need for fallback variations with WP 5.7 and below, until Gutenberg requires at least 5.8.

gutenberg/gutenberg.php

Line 6 in 815b767

* Requires at least: 5.6

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 makes sense, thanks!


if ( ! definedVariations?.length ) {
return;
}
unsubscribe();
if ( ! definedVariations?.length ) {
return;
}
unsubscribe();

const variations = definedVariations
.filter( ( { area } ) => 'uncategorized' !== area )
.map( ( { area, label, description, icon } ) => {
return {
name: area,
title: label,
description,
icon,
attributes: { area },
scope: [ 'inserter' ],
const variations = definedVariations
.filter( ( { area } ) => 'uncategorized' !== area )
.map( ( { area, label, description, icon } ) => {
return {
name: area,
title: label,
description,
icon,
attributes: { area },
scope: [ 'inserter' ],
};
} );

/**
* Add `isActive` function to all `Template Part` variations, if not defined.
* `isActive` function is used to find a variation match from a created
* Block by providing its attributes.
*/
variations.forEach( ( variation ) => {
if ( variation.isActive ) return;
variation.isActive = ( blockAttributes, variationAttributes ) => {
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;
// Find a matching variation from the created template part
// by checking the entity's `area` property.
if ( ! slug ) return false;
const entity = select( coreDataStore ).getEntityRecord(
'postType',
'wp_template_part',
`${ theme }//${ slug }`
);
return entity?.area === variationAttributes.area;
};
} );

/**
* Add `isActive` function to all `Template Part` variations, if not defined.
* `isActive` function is used to find a variation match from a created
* Block by providing its attributes.
*/
variations.forEach( ( variation ) => {
if ( variation.isActive ) return;
variation.isActive = ( blockAttributes, variationAttributes ) => {
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;
// Find a matching variation from the created template part
// by checking the entity's `area` property.
if ( ! slug ) return false;
const entity = select( coreDataStore ).getEntityRecord(
'postType',
'wp_template_part',
`${ theme }//${ slug }`
);
return entity?.area === variationAttributes.area;
};
dispatch( blocksStore ).addBlockVariations(
'core/template-part',
variations
);
} );

dispatch( blocksStore ).addBlockVariations(
'core/template-part',
variations
);
} );
}