-
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 user categories to site editor sidebar navigation screen #53837
Patterns: Add user categories to site editor sidebar navigation screen #53837
Conversation
Size Change: +90 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
6c97291
to
e6a30e7
Compare
Flaky tests detected in dc66a2f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5971249296
|
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.
Looking good so far @glendaviesnz ✨
It's testing pretty well for me. I've left some questions and comments inline.
Other than those, my main thoughts were around the display of the categories in the sidebar screen.
- I'd expect the
Uncategorized
pattern to display at the bottom of the categories. - Might just be me, but now user patterns have merged, the separation of
My patterns
from the rest of the categories felt a bit off. I'm no designer though 😅
packages/edit-site/src/components/page-patterns/use-patterns.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/page-patterns/use-patterns.js
Outdated
Show resolved
Hide resolved
Looking really good @glendaviesnz !
I think we can now change "My patterns" to "All patterns" and move to the top of the categories list (no gap in-between). It should include all patterns regardless of source. We can work on a follow up issue to filter patterns by author/source. We should also update the copy action so it just copies into the same category. Instead of "copy", maybe we can think of a term that's similar to "remix"? |
This originally started out as "Duplicate" and later updated to "Copy to My patterns" as the duplicate couldn't be assigned to the same theme pattern category. Now the duplicated pattern can reside in the same category, can we just revert back to this option being "Duplicate", like non-theme patterns? We can also make it such that after duplicating and navigating back to the Patterns page, it goes to the original category not "My patterns", which should remove some forked logic and simplify things. |
1ff8812
to
20cfeb8
Compare
@SaxonF, @aaronrobertshaw I have replaced It would be good to add some filters at the top of the pattern list page to just display own synced or unsynced patterns, so have left some of that code in place, but I agree with you @SaxonF that we should handle that in a follow-up issue/PR. |
fb06c49
to
dc66a2f
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.
Thanks for all the effort on pattern categories @glendaviesnz 🙇
I took the liberty to push some fixes for a few typos and update the PR title to something more accurate. Hope you don't mind.
The pattern categories in the sidebar are looking good! All the counts appeared to be correct for me.
I hit a few issues around duplicating patterns though. The first time I attempted to duplicate a theme pattern I got an error via the snackbar but not thinking clearly, I refreshed the page before checking the logs for errors. After that refresh, I couldn't replicate this issue.
This was the snackbar notice in case it helps:
After successfully duplicating a theme pattern a couple of times, I attempted to duplicate one of those copies. This resulted in an error both in the snackbar and the console showing a 400 response from the wp_pattern_category
endpoint.
I'm out of time today but happy to help did deeper on this Monday.
Thanks for testing @jasmussen - this is an issue with the default tag input component as it doesn't register the tag unless you hit enter or add a ',' - I already have an open task to fix this somehow on the tracking issue. |
Thanks for the clarification! This is exciting work. |
@aaronrobertshaw I think I have fixed the issues with duplicating 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.
Appreciate the fixes so far @glendaviesnz 👍
Testing this PR again, I've found a few more issues.
- When duplicating a pattern with the "All patterns" category selected, you get a duplicate "All patterns" displayed in the sidebar (in one test this required a refresh to present itself). Duplicating a pattern in this way also doesn't put the new pattern in the same categories as the original.
- Duplicating some theme patterns also didn't seem to be getting the correct category assigned to them. The duplicate would show in the "All patterns" category but not the original theme pattern's category, at least until the page is refreshed.
It's probably easiest to replicate these issues after deleting all custom patterns, and the pattern category taxonomy terms.
Screen.Recording.2023-08-28.at.5.33.00.pm.mp4
packages/edit-site/src/components/page-patterns/duplicate-menu-item.js
Outdated
Show resolved
Hide resolved
Seems to be a caching issue somewhere, will try and work out why sometimes the category lists are not being refreshed when new patterns are added to them |
@aaronrobertshaw this change should fix the issue with theme patterns not being added to categories without a page refresh. |
…es rather than the category of the current list
d6b1c5d
to
5b368a3
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.
Nice work polishing up this PR @glendaviesnz 👍
It's now testing as advertised for me. The previous issues I encountered have all been resolved.
I think this one is ready to merge into the parent PR when the time is right.
This was merged with some failing tests, but is going into a feature branch which needs rebasing to get the failing tests resolved, so failing tests will be sorted on that branch prior to merging to trunk. |
#53837) Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
What?
Lists user patterns by category in the site editor patterns UI.
Depends on User Patterns Category Select PR.
Why?
With the addition the ability for users to categorise their patterns we also need a way for them to be displayed by category in the site editor patterns page.
How?
Adds the user added categories to the category listing in left sidebar of site editor patterns page.
This PR was part of a group of PRs that were merged back into #53835 before being merged into trunk.
A full list of the PRs involved in this work can be found here.
Testing Instructions
Footer
and make sure it displays along with the theme Footer patterns (be aware that although the TT3 theme footer patterns are listed underFooters
the category slug is actuallyfooter
so the user category name needs to match this).Screenshots or screencast
pattern-categories-site-editor.mp4