-
Notifications
You must be signed in to change notification settings - Fork 88
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 initial scaffold block command #96
Conversation
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.
👍 Looks like a great start! I've left some inline comments.
src/Scaffold_Command.php
Outdated
* | ||
* [--theme] | ||
* : Create files in the active theme directory. Specify a theme | ||
* with `--theme=<theme>` to have the file placed in that theme. |
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.
Argument descriptions are wordwrapped automatically, so no need to wordwrap here.
src/Scaffold_Command.php
Outdated
$data = $this->extract_args( $assoc_args, $defaults ); | ||
|
||
if ( ! in_array( $data['category'], $registered_categories ) ) { | ||
WP_CLI::error( "Invalid block category specified. Block categories need to match one of the registered values." ); |
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.
This can be handled by adding YAML doc to the argument definition:
[--category=<category>]
: The category name to help users browse and discover your block.
---
options:
- common
- etc.
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.
Nice feature. I will use YAML instead 👍
src/Scaffold_Command.php
Outdated
* @param array $assoc_args | ||
* @return string|null | ||
*/ | ||
private function maybe_extract_dashicon( $assoc_args ) { |
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.
I'm not a huge fan of functions that start with maybe_
because it implies their behavior is ambiguous. Can we come up with a better name for it?
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.
Welcome to the club. I followed the existing code: maybe_create_themes_dir
and maybe_create_plugins_dir
. I will figure out something else. I want to avoid code duplication here.
src/Scaffold_Command.php
Outdated
/** | ||
* Generate the machine name for function declarations. | ||
* | ||
* @param string $slug Slug name to . |
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.
Looks like this description is incomplete
src/Scaffold_Command.php
Outdated
* @return string | ||
*/ | ||
private function generate_machine_name( $slug ) { | ||
return preg_replace( '/-/', '_', $slug ); |
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.
Is there a reason not to use str_replace()
, which is more performant?
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.
I used refactor method feature in my IDE and didn't look at the code. You are totally right, I will update.
templates/block-js.mustache
Outdated
var el = wp.element.createElement; | ||
var __ = wp.i18n.__; | ||
|
||
// Visit https://wordpress.org/gutenberg/handbook/block-api/ to learn about Block API |
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.
Should this be linked to in the wp scaffold block
command description?
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.
We should add it there, too.
* : The dashicon to make it easier to identify your block. | ||
* | ||
* [--category=<category>] | ||
* : The category name to help users browse and discover your block. |
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.
Could/should this category impact scaffolded code?
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.
Yes, it is consumed here: https://github.com/wp-cli/scaffold-command/pull/96/files#diff-5384a491183730a8d9db2944d7fcb8d6R13.
@@ -201,6 +200,106 @@ private function _scaffold( $slug, $assoc_args, $defaults, $subdir, $templates ) | |||
} | |||
} | |||
|
|||
/** | |||
* Generate PHP, JS and CSS code for registering a custom block. | |||
* |
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.
Would this benefit from a longer explanation of what a block is, etc.?
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.
Added.
templates/block-php.mustache
Outdated
'wp-blocks', | ||
'wp-i18n', | ||
'wp-element', | ||
), time() ); |
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's probably a bad idea to use time()
as the script version in production. If you want this to automatically update when the file is changed, you could use filemtime()
to get the modified time for the file and append this instead.
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.
I copied it from one of the examples. Agreed on filemtime
:)
templates/block-js.mustache
Outdated
return el( | ||
'p', | ||
{ className: props.className }, | ||
__( 'Replace with your content!', '{{textdomain}}' ) |
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.
Would it be helpful to have a more substantive example?
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.
I think this example is good, maybe we should comment it differently. @mtias @youknowriad, do you have some thoughts on that?
@danielbachhuber for your feedback, I will update PR tomorrow. |
src/Scaffold_Command.php
Outdated
* | ||
* [--theme] | ||
* : Create files in the active theme directory. Specify a theme | ||
* with `--theme=<theme>` to have the file placed in that theme. |
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.
I'd advocate for not including a theme
parameter here. Blocks are content and probably shouldn't stop being supported when you switch themes.
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.
I wasn't sure if include theme option here, but I decided to do it based on the Block API documentation which includes the following:
Blocks are the fundamental element of the Gutenberg editor. They are the primary way in which plugins and themes can register their own functionality and extend the capabilities of the editor.
@mtias, what is your opinion about it?
@danielbachhuber I addressed your comments, added documentation, and functional tests. It's ready for another look. |
Success: Created block 'Movie block'. | ||
|
||
Generate PHP code for registering a custom taxonomy. | ||
|
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.
Good.
|
||
// Visit https://wordpress.org/gutenberg/handbook/block-api/ to learn about Block API | ||
wp.blocks.registerBlockType( '{{namespace}}/{{slug}}', { | ||
title: __( '{{title_ucfirst}}', '{{textdomain}}' ), |
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.
@gziolo This does not seem to be the correct way of defining the textdomain. https://github.com/WordPress/gutenberg/tree/master/i18n
I don't think there is any proper way of defining the text domain WordPress/gutenberg#4147
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.
This was fixed after this PR got merged in c74032f
Add initial scaffold block command
Related issue: #88.
This PR implements a scaffolding command dedicated to Gutenberg blocks. It's a first iteration which is focused on a very basic static block which can't be edited in Gutenberg. In the follow-up PR we can cover more advanced examples like a dynamic block or server-side rendered block using Post meta.
To use the generated code you need to require php file inside your plugin or theme, depending which option you pick.
You can learn more about writing blocks in this article and in the official documentation.
Example usage
Commands should be executed from the WordPress folder or with
--path
option.wp scaffold block my-block --theme
wp scaffold block my-another-block --plugin=my-plugin-name --category=widget
wp scaffold block my-yet-another-block --theme --dashicon=editor-paragraph
Example output
Command:
wp scaffold block movie --title="movie block" --theme
wordpress/wp-content/themes/twentyseventeen/blocks/movie.php
:wordpress/wp-content/themes/twentyseventeen/blocks/movie/block.js
:wordpress/wp-content/themes/twentyseventeen/blocks/movie/editor.css
:Commands that produce errors
wp scaffold block MyBlock
wp scaffold block my-block
wp scaffold block my-block --category=unknown
wp scaffold block movie --title=Movie --theme=simple-life
wp scaffold block movie --title=Movie --plugin=simple-life
TODO
Block examples: