-
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
DataViews: Register the deletePost action like any third-party action. #62913
Conversation
The other similar PR #62647 got blocked because we need access to the store and we need isElligible to be reactive (which is hard to solve). So I'm trying another action that doesn't have these requirements in this PR. But the current PR is also blocked for two reasons:
|
Size Change: +188 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
The block editor uses this package in the browser. URL is available globally, there is no need to import it.
b917103
to
103d676
Compare
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. |
This should be ready for review. |
@@ -91,6 +93,7 @@ export function initializeEditor( | |||
enableFSEBlocks: settings.__unstableEnableFullSiteEditingBlocks, | |||
} ); | |||
} | |||
registerDefaultActions(); |
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 couldn't find a better place to call this. I kind of maps what we do for blocks.
has_theme_file: boolean; | ||
} | ||
|
||
export type Post = TemplateOrTemplatePart | BasePost; |
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 like a lot that we could use the "actions" refactoring to slowly augment our "entities" types. In the long run, this will help a lot when build UI for entities.
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.
Everything worked well on the tests I did 👍
@@ -0,0 +1,11 @@ | |||
diff --git a/node_modules/postcss-urlrebase/index.d.ts b/node_modules/postcss-urlrebase/index.d.ts |
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 is this change required?
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.
Something about fixing the typescript types. But @sirreal would know more precisely.
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.
A browser package (editor, I believe) uses postcss in the browser.
This dependency expected to work in a node environment (most common for postcss stuff) and use the URL type imported from node. This is not available in the environment. However, it's fine to use the URL global, which is what this change does. Otherwise, the package would error when it tried to import a non-existent (node.js) 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.
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.
Add package patch note here: #63015
TEMPLATE_ORIGINS.custom | ||
) && | ||
! template.has_theme_file && | ||
! template?.has_theme_file |
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 is the same check now.. #63021
WordPress#62913) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Related #61084
Similar to #62647
What?
In #62052 an API to register and unregister dataviews actions has been implemented. But in order to allow third-party developers to be able to unregister these actions, we need to be using the same actions in Core to register the core actions.
The current PR explore the possibility to use the API to register one action: "delete post".
Testing Instructions
1- Open the patterns dataviews and try removing a user created pattern.
2- You should be able to see the "delete" action in the actions dropdown menu
3- you can try to use the action.