-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Use modal for pattern duplication flow as workaround for changing sync status #54764
Conversation
Size Change: +940 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
a04bad1
to
a362c82
Compare
@SaxonF if you have a moment to share your wisdom here, it would be greatly appreciated 🙏 What are your thoughts on this proof of concept that reuses the pattern creation modals for duplication? It facilitates the generally accepted workaround for not being able to change a pattern's sync status, i.e. duplicating/copying the pattern and setting a new sync status. This would come at the cost of a little extra friction if the user simply wants an exact duplicate. As it stands now on trunk, if you duplicate a pattern, it all happens immediately and you are taken to the new pattern's view page without the ability to change the sync status. |
Nice. I like it @aaronrobertshaw ! I think the added friction is fine. |
a362c82
to
d191427
Compare
sprintf( | ||
// translators: %s: The new pattern's title e.g. 'Call to action (copy)'. | ||
__( '"%s" duplicated.' ), | ||
pattern.title |
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 shows the infamous [object object]
in the snackbar as shown in the video too.
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, I completely missed that but just ran into it while adding the duplicate pattern command. I'll fix it up tomorrow (f3c2b9e)
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 👍. Just some API design nitpicks.
packages/edit-site/src/components/create-template-part-modal/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/create-template-part-modal/index.js
Outdated
Show resolved
Hide resolved
Thanks for the suggested improvements @kevin940726 👍 I've pushed a commit addressing these. If you have the time to give this another quick sanity check, I'll then get it merged to unblock some additional pattern commands work. |
Flaky tests detected in 9609356. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6490049005
|
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.
Small misunderstanding. I'll give it a 👍 anyway.
packages/edit-site/src/components/create-template-part-modal/index.js
Outdated
Show resolved
Hide resolved
This issue has been addressed via the refactor in #55292. |
Related:
What?
Why?
See #53320
TL;DR
How?
Testing Instructions
Screenshots or screencast
Screen.Recording.2023-09-24.at.5.46.04.pm.mp4