-
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
Editor: Create own sub-registry in default EditorProvider use #15989
Conversation
import { storeConfig } from '../../store'; | ||
import applyMiddlewares from '../../store/middlewares'; | ||
|
||
const withRegistryProvider = createHigherOrderComponent( |
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 feel like I already wrote this code before. Do you think there's value in exposing a generic version of this in the data module? (I'm on the fence personally)
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 feel like I already wrote this code before. Do you think there's value in exposing a generic version of this in the data module? (I'm on the fence personally)
It is identical to the one in block-editor
, yes. I'm not sure either about making it generic. I'm not a huge fan of the fact we have to do a null
return render, and I don't know that if that was possible to avoid, we'd be able to change it while maintaining the implementation as being a higher-order component. I was inclined to leave it for future consideration.
@@ -91,6 +91,7 @@ class Editor extends Component { | |||
settings={ editorSettings } | |||
post={ post } | |||
initialEdits={ initialEdits } | |||
useSubRegistry={ false } |
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 feel this prop should be passed down from editor/provider
to block-editor/provider
as well right (unless we access the block-editor
store from the editor
components (which we probably do on a second thought but maybe we shouldn't) ?
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 feel this prop should be passed down from
editor/provider
toblock-editor/provider
as well right (unless we access theblock-editor
store from theeditor
components (which we probably do on a second thought but maybe we shouldn't) ?
Right now we just explicitly pass false
:
useSubRegistry={ false } |
Which I think is how we want it to be:
- If you wanted your own block editor with its own subregistry, you'd render BlockEditorProvider yourself
- If you wanted your own post editor with its own subregistry, you'd render EditorProvider which would create a subregistry. The EditorProvider would pass
useSubRegistry={ false }
to its BlockEditorProvider, but that block editor still operates in the same subregistry as created for the EditorProvider
In the end, we get a flatter hierarchy of registries. For the main post editor, we still only have a single registry.
For what becomes of the embedded editor in #14715, there is a subregistry which is used both by the EditorProvider and its BlockEditorProvider.
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.
Makes sense, thanks for the explanation.
Extracted from #14715
Related: #14678, #14367
This pull request seeks to introduce automated sub-registry creation for the
EditorProvider
component. It is identical in purpose and in implementation to theBlockEditorProvider
enhancements merged in #14678.This will bring consistency in editor creation, where #14715 proposes to reimplement reusable blocks using its own rendering of
EditorProvider
to leverage both the blocks-editing behaviors of@wordpress/block-editor
, and the post lifecycle management of@wordpress/editor
.Implementation Notes:
This introduces API changes to
@wordpress/editor
and@wordpress/block-editor
in the form of exposing their respective base data store configurations. For additional context, this was explored and discussed previously in #14367 (comment) .Testing Instructions:
There should be no regressions in standard use of the editor.
You may place a breakpoint at any code within
@wordpress/editor
which has access to the registry to confirm there is no effective difference in reference betweenmaster
and this branch, since sub-registries should only be created when not at the top-level.