-
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
Extensibility: support the PluginDocumentSettingPanel slot in the site editor #59985
Extensibility: support the PluginDocumentSettingPanel slot in the site editor #59985
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. |
Size Change: +35 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
I'll try to do extensive testing later today. I think I've dozens of similar snippets that use Side note: @ryanwelcher and I chatted during WCAsia about posibility of the new API for registering the document settings panels. A basic API mockup: /**
* Registers a document settings panel for Featured Image.
*/
registerDocumentSetting( 'core/featured-image', {
// Label displayed in Preferences modal.
label: 'Featured Image',
// The position in the document settings sidebar. Similar to core's admin menu pages.
position: 5,
// Optional. The scope for the document settings. We can also use `site` and `post`.
scope: [ 'edit-site', 'edit-post' ],
// A React component to render the settings.
edit: () => {}
} ) |
@Mamaduka Interesting though, I've been thinking about this as well and had similar ideas. I think there's a potential to go even further. Instead of having to register "document settings", we would register "fields":
That said, my thinking at the moment is that we're still a bit early there and while we have "fields" as internal API for now, I think we'll probably make a lot of changes (some breaking) as we advance in dataviews, forms and the tasks related to the admin redesign. |
@@ -161,7 +162,7 @@ export function reinitializeEditor() { | |||
} | |||
|
|||
export { default as PluginBlockSettingsMenuItem } from './components/block-settings-menu/plugin-block-settings-menu-item'; | |||
export { default as PluginDocumentSettingPanel } from './components/sidebar/plugin-document-setting-panel'; | |||
export { PluginDocumentSettingPanel }; |
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.
Should we wrap this in a deprecated call to let extenders know about the change so they can change their implementations?
I don't think we would need a removal timeframe for now. But from what I can tell here it doesn't make sense to use the version exported through there going forward.
It's definitely important that it is here though to not break anything :)
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.
Yes, I wondering about it. Should we deprecate now or give it some time (maybe move more slots and check feedback)
@youknowriad, the "fields" API looks more flexible, especially when we consider new concepts being introduced for admin redesign. Do you think there's room for both APIs? I'm guessing the "Document" and "Block" sidebars will remain even after the redesign. |
yes, I think the current APIs will remain but I'm not sure that we'd need an additional |
It makes perfect sense to move all plugin components to That said, do we have some examples of plugins using the |
One additional question that comes up for me is how we best handle some of the API inconsistencies between the two editors. Something I often run into is how I can access the current post id / the post type being edited. The way that is done is, as far as I'm aware, different. So this change will have a potential to cause the panels to be broken in the site editor. |
That's a possibility primarily if the fills rely on |
Adding the plugins to the site editor requires explicitly loading the JS in the site editor. So in other words, there's no breakage to be expected here.
I think we've made huge improvements here lately, both editors rely on |
If a script is enqueued using |
Would be really cool to get an up to date docs page or blog post about all this :) I see the question of how to access data in the same way across the site and post editor pop up all the 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.
FWIW I've given this a test in its current state.
- Pasting the example snippet into each editor results in the expected panel display as advertied
- Testing with a plugin using a single
enqueue_block_editor_assets
action adds the panel to both editors
So as George and Fabian flagged, it does seem to open the door for potential breakage depending on what existing plugins rely on e.g. edit-post
.
Based on my test, the chances of breakage are pretty low, especially if people migrate away from deprecated selectors when using WP 6.5. Unfortunately, that rarely happens; in reality, devs don't have time, or plugins have to support multiple WP versions. Here's my example plugin that triggers an error in the site editor. Luckily, the error is contained by the plugin boundary. Repo: https://github.com/Mamaduka/test-plugin-document-setting-panel Screenshot |
Oh this is cool :P |
So it feels to me like we should move forward, we're still very early in release cycle, and assess feedback received before next WP beta. |
Agree with that. Lets get it in now so we have time to test & get feedback :) |
…e editor (WordPress#59985) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
There are two issues related to this change, as it relates to the post and page editors.
These points relate to use of WordPress 6.5.3 without the Gutenberg plugin. |
@markhowellsmead What issue are you seeing in 6.5.3? I just tested and Also in the Gutenberg plugin the code still works just with a deprecation notice. So this should not break anything gutenberg/packages/edit-post/src/deprecated.js Lines 39 to 45 in 8d7c8fd
|
Apologies, the JS error was prompted by custom code in our project. |
@fabiankaegy, would this blog post be similar to the following suggested idea for a Developer Blog post: WordPress/developer-blog-content#53? If not, would you mind opening a new Topic Idea with the specific content you'd like to be covered in a Developer Blog post? I'd be up for writing such a post 🙂 |
Related #52632
What?
This PR is going to set a precent about how we merge extensibility APIs between post and site editors, so we need to review and test properly.
The idea is that all the slots that exit in either post or site editors can be shared through the
editor
package. That way, a plugin registering a fill can work both in the post editor and site editor (assuming the plugin loads its JS in both pages).How?
@wordpress/edit-post
to.@wordpress/editor
Testing Instructions
You should be able to paste something like the following in both post and site editors and have the panel show up in the sidebar of pages.