-
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
Clean up bundled pattern titles & categories #30998
Conversation
lib/block-patterns.php
Outdated
// Register categories used for block patterns. | ||
register_block_pattern_category( 'query', array( 'label' => __( 'Query', 'gutenberg' ) ) ); | ||
register_block_pattern_category( 'text', array( 'label' => __( 'Text', 'gutenberg' ) ) ); | ||
register_block_pattern_category( 'buttons', array( 'label' => __( 'Buttons', 'gutenberg' ) ) ); |
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 was registering these inside the init filter because if I didn't I was getting an error while the admin was loading, but I don't see that any more
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 sure registering these categories is needed, except for query
that doesn't exists in core
. Also not introduced in this PR, but the registration of the other categories might not be needed.. Will look now..
Size Change: +19 B (0%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
5e78de3
to
a50313d
Compare
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 LGTM, I just rebased since I think that's all it's needed for the tests to go green.
lib/block-patterns.php
Outdated
@@ -227,11 +230,9 @@ function() { | |||
unregister_block_pattern( 'core/' . $core_block_pattern ); | |||
} | |||
|
|||
register_block_pattern_category( 'buttons', array( 'label' => _x( 'Buttons', 'Block pattern category', 'default' ) ) ); | |||
register_block_pattern_category( 'columns', array( 'label' => _x( 'Columns', 'Block pattern category', 'default' ) ) ); |
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 seems that the registration of categories happens in core
:
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/block-patterns.php#L42
and we do not unregister the categories, only the core patterns.
I think it's okay to remove these registrations and the above, except query
that is not in core
.
Was any specific reason for these lines @MaggieCabrera ?
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.
yeah, initially they were named differently and then they ended having the same names as the ones on core, so I'd say we can remove the duplicates, yes
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.
Cool, I've removed them in 3c3cea8 and it seems to work fine. Mind giving the PR one last look?
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 good. Thanks!
Some minor followup from #30763, #29973, and #30469.
This PR: