-
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
Add initial API to register patterns from themes and plugins #21074
Conversation
ca8fd6f
to
a199ca5
Compare
Size Change: +545 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
@@ -0,0 +1,35 @@ | |||
# Patterns | |||
|
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.
Let's add a disclaimer that this are is still under active development.
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.
Thinking of adding __experimental
prefix as well.
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, this allows translating these. Had two minor suggestions, looks good otherwise. Don't have an opinion of the overall code nor tested it.
docs/designers-developers/developers/block-api/block-patterns.md
Outdated
Show resolved
Hide resolved
docs/designers-developers/developers/block-api/block-patterns.md
Outdated
Show resolved
Hide resolved
Co-Authored-By: Alex Kirk <[email protected]>
docs/designers-developers/developers/block-api/block-patterns.md
Outdated
Show resolved
Hide resolved
Co-Authored-By: Alex Kirk <[email protected]>
/* | ||
* Register default patterns if not registered in Core already. | ||
*/ | ||
if ( ! WP_Patterns_Registry::get_instance()->is_registered( 'text-two-columns' ) ) { | ||
register_pattern( 'core/text-two-columns', gutenberg_load_block_pattern( 'text-two-columns' ) ); | ||
register_pattern( 'core/two-buttons', gutenberg_load_block_pattern( 'two-buttons' ) ); | ||
register_pattern( 'core/cover-abc', gutenberg_load_block_pattern( 'cover-abc' ) ); | ||
register_pattern( 'core/two-images', gutenberg_load_block_pattern( 'two-images' ) ); | ||
} |
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 is problematic for the plugin build.
See:
function register_pattern( $pattern_name, $pattern_properties ) { | ||
return WP_Patterns_Registry::get_instance()->register( $pattern_name, $pattern_properties ); | ||
} |
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 better to have named this as specific to blocks: register_block_pattern
?
Same goes for:
unregister_pattern
WP_Patterns_Registry
When considering the WordPress function API broadly, it's not obvious by these names they have anything to do with blocks.
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 it was decided to name these "Patterns" and not "Block Patterns". cc @mtias
Few suggestions:
More on #3. You are using the html code from the command: |
I am concerned about the names of the PHP functions being suggested. Without a meaningful descriptor, they go against the function naming guidelines:
This is because, in WordPress, a "pattern" could be many things or it could be nothing. https://make.wordpress.org/core/2020/03/26/whats-new-in-gutenberg-25-march/#comment-38306
In short:
|
I can't seem to find where this was discussed right now, but it was and I believe we settled on "Patterns" over "Block Patterns" and I believe the function names should follow the actual naming of the concepts in the UI otherwise it gets confusing, feel free to open an issue to rediscuss this more largely though. |
I think calling the function |
The API looks like this: