Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Patterns: Add rename/delete options for pattern categories in site editor #55035
Changes from 11 commits
8119ad4
b9fc306
153b607
5186f87
cefb9ba
8c4ceea
c1d4917
4941541
2614ad4
62412cc
036963d
460ff0d
0404250
5343203
ecc9f4a
4d45614
f8cfd0b
0fb34a9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.