-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix template part variations registration subscription. #31772
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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( () => { | ||||||||||||||||
const definedVariations = select( | ||||||||||||||||
editorStore | ||||||||||||||||
).__experimentalGetDefaultTemplatePartAreas(); | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 gutenberg/packages/block-library/src/navigation-link/index.php Lines 325 to 331 in ff4c3c4
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see you're already created #31761 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. 😁 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||||||
); | ||||||||||||||||
} ); | ||||||||||||||||
} |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nm, that seems to be a WP version issue that we should be able to work around with fallbacks.