Move the block manager to block editor package#39672
Closed
talldan wants to merge 11 commits into
Closed
Conversation
talldan
added
[Type] Enhancement
|
Size Change: +84 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Mar 23, 2022
Contributor
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
I guess the reason for moving the BlockManager and relying on the preferences package is to be able to use the BlockManager in multiple editors.
The more I think about edit-post and edit-site (at least these two), the more I see these as the same editor and that at some point we'll need to merge.
That said maybe we could still have a reusable BlockManager component in block editor but without the dependency to preferences:
<BlockManager value={ allowedBlocks } onChange={ onChange } />
| "@wordpress/keyboard-shortcuts": "file:../keyboard-shortcuts", | ||
| "@wordpress/keycodes": "file:../keycodes", | ||
| "@wordpress/notices": "file:../notices", | ||
| "@wordpress/preferences": "file:../preferences", |
Contributor
There was a problem hiding this comment.
I'm not really sure that I like that personally, I feel the block-editor package shouldn't be depending on the "preferences" package. The reason is that the "preferences" package is something that is generic now but I see it as more tied to WP especially if we implement later API based preferences (persist in the user profile)
I guess we can implement this WP relationship in a non-intrusive way, using adapters or something but maybe we should cross that bridge at that moment?
Contributor
Author
There was a problem hiding this comment.
Yeah, this is something I was unsure about.
There's also #39632 where I've migrated some of the block-editor's persisted data to the preferences package, that would also add the dependency.
Contributor
There was a problem hiding this comment.
In #39632 it seems it's even harder to remove that dependency. So maybe we can just accept it as a dependency but make sure that any "WP" dependency should be added in an optional "adapter" layer
Member
There was a problem hiding this comment.
You can also look at the Block Directory integration:
https://github.com/WordPress/gutenberg/tree/trunk/packages/block-directory
At least outside of the WordPress core, the block manager UI isn't so important as you can have full control over the blocks used.
It's shaped as an optional plugin that integrates with the block editor. Although, I'm not sure how that would work with the preferences store, to be honest.
Contributor
Author
There was a problem hiding this comment.
The block directory renders into slots IIRC, so it seems a bit easier to make optional.
As Riad mentions, #39632 (block insert usage, which is used to power the quick inserter) is a more difficult thing to make optional. That does seem more important to a standalone editor, though perhaps a fallback to in-memory persistence could be an option.
The only way I can see to keep it working would be to extract the whole feature from the block-editor package. There would need to be a way to listen for block insertion events so that insertUsage could be updated. The block manager could also potentially live in a different package and that would solve the dependency issue for this one.
But there's no hurry to merge this or #39632, so it could be worth exploring how a flexible persistence layer in preferences might be integrated first.
Member
There was a problem hiding this comment.
Yes, the block directory integration was easier to make optional. It's worth also pursuing the same approach because it makes the editor easier to extend assuming you would find a decent way to orchestrate the preferences store. It would benefit also plugin authors. I'm happy to pair with you to discuss the challenges and possible ideas.
Contributor
Author
There was a problem hiding this comment.
There's an experiment here to try making the persistence aspect of preferences optional/configurable - #39795.
This was referenced Mar 24, 2022
talldan
force-pushed
the
move/block-manager-to-block-editor-package
branch
from
ba28c26 to
e60bcd3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #41004
What?
Moves the block manager component to the block-editor package.
Why?
It will allow the block manager to be more easily implement in other editors (like the site editor).
Now that
hiddenBlockTypesare stored in the preferences store (since #39132), this change is easier to make.How?
It's worth discussing the changes that require most attention from reviewers first. To make it easier for other editors to implement the block manager, I've made some changes to the block editor settings.
Previously, the block manager worked by modifying the
allowedBlockssetting (removing any hidden block types). It also set anotherdefaultAllowedBlockseditor setting, which represented the allowed blocks before any modification.I've changed this. It seems much simpler to use
hiddenBlockTypesas an editor setting rather thandefaultAllowedBlockTypesand makecanInsertBlockTypeUnmemoizedcheck thehiddenBlockTypeslist, so that's the change I've made.I'm open to other option, including changing it back. An alternative could also be for the block editor package to pull in
hiddenBlockTypesstraight from the preferences store instead of using a setting, but it'd have to somehow know whichscopeto use.The rest of the changes are pretty straightforward:
withSelecttouseSelectscopeprop to indicate which preferences it uses.useHiddenBlockTypeshook to interact with the preferences store (which could be worth exporting and then the post editor's selectors and actions could be deprecated).Testing Instructions
There shouldn't be any change in functionality in the post editor. To test:
Expected: The blocks that were hidden are not visible in the inserter.