-
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
Patterns: Add new pattern categories #46144
Conversation
Thanks for this. Categories look good to me. I think this mainly needs some technical input. |
@jasmussen any thoughts about this or the rest suggested categories from #44501 ? |
Right, I think we need wider input on that, based on conversation in WordPress/pattern-directory#190 CC: @WordPress/gutenberg-design |
Given the work in #45814 I don't think we need the 404 and search results categories. It's not really helpful to see those categories permanently when their usefulness is entirely dependent on which template you're editing. Similar story with things like 'Pagination', 'Comments', 'Post Content', and 'Products' (which definitely shouldn't be included here btw – it's something a plugin like WooCommerce would add). It doesn't really make sense to see the 'Comments' category when you're editing something like an Archive. It may be ok in the short term, but it feels like we need a way to associate these categories with either |
lib/compat/wordpress-6.2/class-gutenberg-rest-block-patterns-controller-6-2.php
Show resolved
Hide resolved
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.
Thanks for working on this! It looks good, I don't have much to add.
lib/compat/wordpress-6.2/class-gutenberg-rest-block-patterns-controller-6-2.php
Outdated
Show resolved
Hide resolved
383fb05
to
45b5fbc
Compare
* | ||
* @var string | ||
*/ | ||
const REQUEST_ROUTE = '/wp/v2/block-patterns/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.
The changes in setup and tear down were copied from core.
} | ||
|
||
/** | ||
* Abstract methods that we must implement. | ||
*/ | ||
public function test_register_routes() { |
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.
These tests are already in core and we made no changes there.
@jasmussen Should there still be a "featured" category? I thought the featured patterns would be shown in the pattern explorer - unless this is temporary, and the featured category would be removed when the explorer is merged? (my goal with this question is to avoid having two kinds of "featured" at once) |
For the block editor, this feedback suggests that yes, we still need a featured category. However it isn't necessary in the same way as it used to be, where it was needed because it was the "default" category that was opened. Now patterns are in a flyout, and you can choose a category yourself. So it seems we can do without? CC: @WordPress/gutenberg-design |
That's a good point. The Featured category isn't so compelling because it has no contextual meaning. It seems more likely that you'd be drawn to the other categories. This brings me back to the idea of 'featured' being a boolean that can be true for any pattern. Then we can order by featured (as a default) to increase the prominence of those patterns, both in the pattern directory and the pattern browser in-product. |
I think the discussion and decisions about Similar discussions are made in this issue as well, example:
Personally I think this PR in order to land, needs:
|
I think Joen's suggestion for mapping here is good, with the exception of the Columns and Text categories. Those just don't map nicely to the new categories – the patterns therein ideally need recategorisation. If you look at them on the directory (text, columns) it is fairly obvious that the majority could fit nicely into the new categories (hopefully we can sort by script 🤞).
I noticed you removed all the site building categories. I think Post Content, Pagination, Comments, Comment Pagination, and Archive Headings can remain with the caveat that they only appear when you're editing a compatible template.
Appreciate that may be something to handle in a follow-up. |
Agreed. Let's roll with this and revisit featured in a separate discussion/PR in the near term. For mapping, I want to +1 @jasmussen and @jameskoster read :) Let me know what else might be needed to move forward. Thank you for the great discussion, all! |
I am wondering about the Call to action. If these are just buttons can we just call these buttons? |
45b5fbc
to
b805867
Compare
I added the rest categories from: WordPress/pattern-directory#190 (comment)
Yeah, this can be handled in a follow up and is more tied to @jorgefilipecosta 's work to show patterns based on a template, which also involves an API addition there. |
b805867
to
d3337b8
Compare
I assume it would also be part of that work to hide the headers / footers categories in the post editor? I still find it so strange to see those there 🙈 Otherwise the changes here seem ok, though I am noticing that my editor crashes when navigating around the Patterns tab: patterns.mp4 |
d3337b8
to
2cbcc74
Compare
Size Change: +1.5 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Thanks Nik, looking good. Final request so far as I can see is to update the categories for a couple of the core patterns:
Probably fine to do that in a follow-up if you prefer. |
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.
Thanks so much for taking on this work. Looks good, minus my notes and design approval.
phpunit/class-gutenberg-rest-block-patterns-controller-test.php
Outdated
Show resolved
Hide resolved
lib/compat/wordpress-6.2/class-gutenberg-rest-block-patterns-controller-6-2.php
Show resolved
Hide resolved
lib/compat/wordpress-6.2/class-gutenberg-rest-block-patterns-controller-6-2.php
Show resolved
Hide resolved
@jameskoster these can be done separately. I'll have to check if they are loaded from Pattern Directory or existing in core and act accordingly. Do you think anything else is needed to land and iterate on the rest of this work, in coordination with Pattern Directory? |
It looks good to me, but it's late on a Friday, so would likely benefit from some fresh eyes next week. |
Jay is AFK this week, but I'm happy to help provide a sanity check. Nik do you recall what the outstanding questions were? In principle I'm always happy to merge and iterate, and I don't think what we've had has worked that well, so I'm generally open to trying things, especially if we can tweak as we go. |
Just the current decisions about keeping the |
Oh that's fine, sure! We can always revisit that one later. |
Adds a new non-public `WP_REST_Block_Patterns_Controller::migrate_pattern_categories()` method to automatically migrate existing content's pattern categories to the new ones introduced in [55098]. Old to New `'buttons'` to `'call-to-action'` `'columns'` to `'text'` `'query'` to `'posts'` Reference: * Part of [WordPress/gutenberg#46144 Gutenberg PR 46144] Follow-up to [55098], [53152]. Props ntsekouras, annezazu, jameskoster, joen, hellofromTonya, mcsf, paaljoachim, ryelle. Fixes #57532. Built from https://develop.svn.wordpress.org/trunk@55125 git-svn-id: http://core.svn.wordpress.org/trunk@54658 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Adds a new non-public `WP_REST_Block_Patterns_Controller::migrate_pattern_categories()` method to automatically migrate existing content's pattern categories to the new ones introduced in [55098]. Old to New `'buttons'` to `'call-to-action'` `'columns'` to `'text'` `'query'` to `'posts'` Reference: * Part of [WordPress/gutenberg#46144 Gutenberg PR 46144] Follow-up to [55098], [53152]. Props ntsekouras, annezazu, jameskoster, joen, hellofromTonya, mcsf, paaljoachim, ryelle. Fixes #57532. Built from https://develop.svn.wordpress.org/trunk@55125 git-svn-id: https://core.svn.wordpress.org/trunk@54658 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Adds a new non-public `WP_REST_Block_Patterns_Controller::migrate_pattern_categories()` method to automatically migrate existing content's pattern categories to the new ones introduced in [55098]. Old to New `'buttons'` to `'call-to-action'` `'columns'` to `'text'` `'query'` to `'posts'` Reference: * Part of [WordPress/gutenberg#46144 Gutenberg PR 46144] Follow-up to [55098], [53152]. Props ntsekouras, annezazu, jameskoster, joen, hellofromTonya, mcsf, paaljoachim, ryelle. Fixes #57532. Built from https://develop.svn.wordpress.org/trunk@55125 git-svn-id: http://core.svn.wordpress.org/trunk@54658 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
Part of code extracted from: #45548
Part of: #44501
This PR adds some new pattern categories and adds a migration step for patterns that use old categories. For now I've mapped only
'buttons' => 'call-to-action' and 'query' => 'posts'
. Also the php code will be updated, as I wanted to move things forward with this..Notes
Testing Instructions
Patterns
tabbuttons
are shown incall to action
.Screenshots or screencast