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

Add a function to register a block style variation #7997

Merged
merged 4 commits into from
Jul 26, 2018

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Jul 17, 2018

This PR adds a helper function to easily add a new block style variation as this is a common extensibility pattern.

Testing instructions

  • add wp.blocks. registerBlockStyle( 'core/paragraph', { name: 'big', label: 'Big Style' })
  • The call should be executed before the initialization of the editor (registerCoreBlocks)
  • Notice the new style variation appearing in the paragraph block's switcher.

Related #7551

@youknowriad youknowriad added the [Feature] Extensibility The ability to extend blocks or the editing experience label Jul 17, 2018
@youknowriad youknowriad self-assigned this Jul 17, 2018
@youknowriad youknowriad requested review from mtias and a team July 17, 2018 10:16
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Code looks good! But I'm getting wp.blocks.registerBlockStyleVariation is not a function when I try to test this. I think we need to export the new function from blocks/api/index.js.

@@ -330,3 +330,25 @@ export const getChildBlockNames = ( blockName ) => {
export const hasChildBlocks = ( blockName ) => {
return select( 'core/blocks' ).hasChildBlocks( blockName );
};

/**
* Registers a new block style variation for the given block
Copy link
Member

Choose a reason for hiding this comment

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

Missing a period (.) at the end of the sentence here.

* Registers a new block style variation for the given block
*
* @param {string} blockName Block type name.
* @param {Object} styleVariation Block style variation.
Copy link
Member

Choose a reason for hiding this comment

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

Should we document what properties this object has?

@param {Object} styleVariation Object containing `name` which is the class name applied to the block and `label` which identifies the variation to the user.

Copy link
Member

Choose a reason for hiding this comment

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

Yes we should 😅

* @param {string} blockName Block type name.
* @param {Object} styleVariation Block style variation.
*/
export const registerBlockStyleVariation = ( blockName, styleVariation ) => {
Copy link
Member

Choose a reason for hiding this comment

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

registerBlockStyle seems simpler to me

/**
* Registers a new block style variation for the given block
*
* @param {string} blockName Block type name.
Copy link
Member

Choose a reason for hiding this comment

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

Block type name might actually be slightly confusing comment here because it implies blocks can have types and that’s what this is? That’s how I read it. It would be better to just say “name of block”. Maybe even provide an example like “Name of block (example: “latest-posts”).”

Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

Works for me. 👍

* @param {Object} styleVariation Object containing `name` which is the class name applied to the block and `label` which identifies the variation to the user.
*/
export const registerBlockStyle = ( blockName, styleVariation ) => {
addFilter( 'blocks.registerBlockType', blockName + '/' + styleVariation.name, ( settings, name ) => {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: We could use template strings here.

@youknowriad youknowriad dismissed noisysocks’s stale review July 18, 2018 11:58

Good catch, it's fixed.

@Soean
Copy link
Member

Soean commented Jul 26, 2018

We should add this to the docs, see #8145

@youknowriad youknowriad merged commit d5f2d79 into master Jul 26, 2018
@youknowriad youknowriad deleted the add/block-styles-variation branch July 26, 2018 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants