-
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
Update: Move pattern actions to the editor package. #60785
Update: Move pattern actions to the editor package. #60785
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
78ba07b
to
10ee051
Compare
Size Change: +353 B (+0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
10ee051
to
08b7d22
Compare
e16eefc
to
b6183fe
Compare
await removeTemplates( items ); | ||
} else { | ||
for ( const template of items ) { | ||
if ( template.type === TEMPLATE_POST_TYPE ) { |
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.
Why do we need all the post type checks. Or in other words, why reverting changes is different between template, template parts. And I don't see where we're handling 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.
Why do we need all the post type checks. Or in other words, why reverting changes is different between template, template parts. And I don't see where we're handling patterns?
There was is revert for patterns that was already the case before, one can not change the original theme patterns only duplicate and create a new version. Regarding the difference between reverting a template and reverting a template part. When we revert a template part we simply delete the user post storing from the database, when we revert a template we make an API request to the server to retrieve the original blocks in that template and store those original blocks as the user content. I don't know why we do things differently in both cases, we are doing it like that for a long time.
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.
Regarding the difference between reverting a template and reverting a template part. When we revert a template part we simply delete the user post storing from the database, when we revert a template we make an API request to the server to retrieve the original blocks in that template and store those original blocks as the user content. I don't know why we do things differently in both cases, we are doing it like that for a long time.
I think we should remove the post in both cases to be honest.
There was is revert for patterns that was already the case before, one can not change the original theme patterns only duplicate and create a new version.
Why do we allow the revert action for patterns then (you added || isPattern
)
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.
Why do we allow the revert action for patterns then (you added || isPattern)
I thought it was possible to see patterns with template parts but that does not seems to be the case so I removed is || isPattern
.
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.
I think we should remove the post in both cases to be honest.
I agree but it seems a considerable effort was put in the current revert template. I think the endpoint to request theme file may have been included because of that so I guess there are some reasons for why the template is not simply deleted. One thing may be revisions which would be lost but there may be other reasons. We could have a separate PR unifying this but it seems it should not be done here to increase its visibility.
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.
@jorgefilipecosta Let's track this in an issue or something (may be good to try to ping the folks that introduced this in the first place for more info/context and why it's not the same)
Flaky tests detected in 716c8bd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9035846878
|
I noticed that we have a "move to trash" and a "delete" action for patterns. Both appear to do something similar for patterns (or at least it looks similar in the UI), we need to clarify these things later. |
This reverts commit 66ade70.
I noticed that some required actions were missing or unnecessary actions were displayed in patterns and template parts. Is this problem related to this PR? |
Similar to #60395, and #60092.
The actions related to patterns will be moved to the editor package in this PR, which will enable their use in the edit post package. This includes the ability to export a pattern as JSON, rename, delete, and perform other functions. However, we have not yet moved the "duplicateTemplatePartAction" action as it requires the movement of certain components to the editor package as well. These components will be moved in a separate pull request.
The resetTemplatePart action and resetTemplateAction were joined together as the template action already mostly supported template parts
The delete template and template pattern should be joined together in a follow-up PR.
Testing Instructions
Open the patterns in dataviews, try all of its actions and verify they work as expected.