-
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 rename/delete options for pattern categories in site editor #55035
Conversation
Size Change: +3.86 kB (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
I noticed a potential bug. When you delete the category, the patterns in it end up in Uncategorized, but it seems a refresh is needed for them to show up there. |
Thanks for the heads up. We might need to invalidate another resolution. I'll look into it. |
Flaky tests detected in 4d45614. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6503474980
|
packages/edit-site/src/components/page-patterns/rename-category-menu-item.js
Outdated
Show resolved
Hide resolved
packages/patterns/src/components/rename-pattern-category-modal.js
Outdated
Show resolved
Hide resolved
@SaxonF do you have any early thoughts on whether the current inclusion of a dropdown in the pattern category header is okay for an initial means of allowing renaming and deleting categories in the site editor? |
@aaronrobertshaw this aligns with some of the recent page layout explorations so I think its a fine start |
🎉 Nice simple approach - thanks for picking this up! |
|
||
if ( ! category?.id ) { | ||
return null; | ||
} |
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 isn't quite as complex as the rename dialog, but I wonder if we still want to pull the modal and the logic into the patterns package?
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.
There are a few semi-related things underway at the moment that might get consolidated into, or update, the patterns package. For example, rename modal/command for patterns themselves, alternate duplication flows etc.
I went in circles a bit before deciding to simply get a few proof of concepts up so it was a little clearer what could or should be extracted to the patterns package.
There's also the fact that some code has already been refactored lately to combine components for templates, template parts, and patterns. That blurs the lines somewhat and makes consistently separating all patterns-related stuff out more difficult.
To begin with, adding new modal components to the patterns package seemed like the easiest line to draw. The ConfirmDialog
fell on the other side of that line.
I'd like to see how the different uses for the modals, e.g. menu items vs commands etc, shape up before we go bringing everything in. I'm certainly not opposed to that once we know more about our requirements.
I also wonder if some of that refactoring should happen when we consolidate different types of patterns e.g. template parts 🤔
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.
That sounds like a good approach, thanks for outlining your thinking around it.
Fixes both the sidebar navigation screen and pattern page header.
2c4be1b
to
036963d
Compare
What happens to the patterns in the deleted category? If they're fine, that's good - but we should indicate that they're not deleted in the rename modal help text. |
Only the pattern category is deleted. If they are no longer assigned to any "active" categories they'd appear as uncategorized. Otherwise, they'll show under their other assigned categories.
I take it you're referring to the delete confirm dialog and the "Are you sure you want to delete X" text? Having a quick look through the codebase I didn't spot any other delete prompts that added further explanation. Do you have some preferred wording then for this case @richtabor? |
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.
Mostly looks good to me! Just some UI issues that would be nice to address before merge.
packages/edit-site/src/components/page-patterns/delete-category-menu-item.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/page-patterns/delete-category-menu-item.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/page-patterns/rename-category-menu-item.js
Outdated
Show resolved
Hide resolved
packages/patterns/src/components/rename-pattern-category-modal.js
Outdated
Show resolved
Hide resolved
packages/patterns/src/components/rename-pattern-category-modal.js
Outdated
Show resolved
Hide resolved
packages/patterns/src/components/rename-pattern-category-modal.js
Outdated
Show resolved
Hide resolved
Appreciate the review @kevin940726 🙇 I've made all the suggested fixes and clean-up tweaks. Let me know if there is anything I missed. |
I think it'd be good to instill confidence that this action does not delete the patterns in the category. How about this below, where we include the pattern (confirming the pattern for the user) and mentioning that patterns will not be deleted as well: Are you sure you want to delete the category "{name}"? The patterns will not be deleted. |
I've updated the delete prompt. @richtabor do you think we need to restrict the max width of the confirm dialog or split "The patterns will not be deleted" into a second paragraph or anything? |
Would a |
I guess there's still the possibility of long category names stretching out the modal width if we just use a The create pattern modal applies a class name and restricts the width to |
Let's also change to "OK" to "Delete". |
What's your preference from the options below?
The width limit used above is the |
Width limit works best. Option 1. |
✅ Done. Thanks for the guidance here, Rich. Any other refinements you'd like to see on this one? |
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! Thanks!
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.
LGTM thanks!
Fixes: #54699
What?
Adds ability to rename or delete pattern categories from the Site Editor's pattern management page.
Why?
There's currently no UI in the site editor for users to be able to rename or delete pattern categories. This means to correct typos in category names or clean up their categories list, a user needs to know about and navigate manually to
wp-admin/edit-tags.php?taxonomy=wp_pattern_category
How?
Testing Instructions
Screenshots or screencast
Screen.Recording.2023-10-04.at.7.23.45.pm.mp4