-
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 editing of pattern categories to site editor #54640
Patterns: Add editing of pattern categories to site editor #54640
Conversation
); | ||
}; | ||
|
||
export default function PatternCategories( { post } ) { |
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 involved a bit of copying from the editor package because of the different way we are handling editing posts in the site editor, it would be nice to reduce the duplication at some point but I think this would be better handled post 6.4 - I think this small amount of copy paste is ok in the meantime.
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.
Sounds good, perhaps we should create an issue to follow up on this so it doesn't slip through the cracks.
Size Change: +961 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
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 will be good to get this one into 6.4 as the re-categorization of patterns was pretty awkward having to leave the site editor to edit the pattern via the post type editor.
The PR tests well. The only thing that caught me out was not tokenizing the incomplete tokens in the category field as we do in the pattern creation modal.
I'm not sure if we should be consistent with the pattern creation modal or the normal post-type category field. For the site editor, I'd probably lean towards the modal but it can be a follow-up.
The fit in the sidebar was a little clunky but I found that was due to the revisions rendering an empty row. I pushed a commit that cleans up that one. So given that, it might be better to get a fresh set of eyes to give a final review on this PR.
Before | After |
---|---|
@@ -64,6 +70,7 @@ export default function TemplatePanel() { | |||
> | |||
<LastRevision /> | |||
</PanelRow> | |||
{ postType === 'wp_block' && <PatternCategories post={ record } /> } |
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 we be using the recently consolidated constants here and in other places instead of 'wp_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.
If desired, perhaps it could be included in a code quality follow-up as suggested for refactoring the PatternCategories
component.
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.
Have switched to the constant.
); | ||
}; | ||
|
||
export default function PatternCategories( { post } ) { |
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.
Sounds good, perhaps we should create an issue to follow up on this so it doesn't slip through the cracks.
I have switched it to tokenize on blur to match the modal. |
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.
Code-wise LGTM 👍
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.
For future reference, this file is largely copied/based on https://github.com/WordPress/gutenberg/blob/08b6f1c0a887eeebaf203bbf7a3f96cedd3b618c/packages/editor/src/components/post-taxonomies/flat-term-selector.js.
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 adding the link. The copy pasta was flagged earlier with the possibility of a dedicated follow-up issue discussed.
What?
Adds the ability to edit categories to the site editor pattern editor.
Why?
Currently there is no way to edit the categories of a pattern in the site editor.
How?
Adds a category editing row to the right panel of the pattern editor page.
Testing Instructions
Screenshots or screencast
site-edit-patternb-cats.mp4