-
Notifications
You must be signed in to change notification settings - Fork 813
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
Dotcom Patterns: Hide categories that start with an underscore #36763
Dotcom Patterns: Hide categories that start with an underscore #36763
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
} | ||
} | ||
|
||
// Unregister existing categories so that we can insert them in the desired order (alphabetically). |
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 don't control the order of categories. That's controlled by the editor. See Automattic/wp-calypso#77130 (comment).
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 new territory for me, so forgive me if I ask silly questions, but why doesn't this affect the order of categories in the Assembler?
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.
Because in Assembler we force the order here
foreach ( $pattern_categories as $slug => &$category_properties ) { | ||
// Rename category labels. | ||
if ( 'posts' === $slug ) { | ||
$category_properties['label'] = __( |
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 don't need this anymore because we overwrite the existing categories with Dotcom categories.
); | ||
|
||
// Move the Featured category to be the first category. | ||
if ( isset( $pattern_categories['featured'] ) ) { |
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 featured category will be moved to the top in WordPress/gutenberg#60210. For now, we cannot control that ordering.
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 to me!
6ec0390
to
ef7b698
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.
Apologies that it took me a minute to get through this review, @miksansegundo. Jetpack testing is new to me.
- I was able to test these changes on a simple site. I didn't manage to do it on Atomic, but the changes are so small that I don't think I need to go through the hoops required to do that testing.
- I verified that the Assembler looks the same, as well as the pattern inserter flyout panel in the block editor.
- The API endpoint now returns patterns in a different order than before. I take it this is expected, @miksansegundo? We are essentially saying that all API consumers will need to handle ordering themselves, right?
As I noted in https://github.com/Automattic/dotcom-forge/issues/6073, we'll need to update the /ptk/categories
endpoint separately to implement the corresponding changes for the pattern library.
That's right. Patterns and categories are going to be returned in the order they are registered, and API consumers have the responsibility to change that, as the editor does it here. |
* Hide Dotcom categories that start with underscore * changelog * Fix version
Fixes https://github.com/Automattic/dotcom-forge/issues/6073
Proposed changes:
Note
We are hiding the category, not the patterns in it.
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions: