-
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
Move the block manager to block editor package #39672
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
a11fa9d
Move block manager verbatim to the block-editor package
talldan f4f10ed
Implement a `useHiddenBlockTypes` hook
talldan 55505f8
Switch to useSelect from withSelect
talldan c28488f
Update classnames
talldan e50ef19
Fix circular dependency
talldan bdcdd25
Set a scope on the post editor block manager and pass it through corr…
talldan a2d574e
Expose defaultAllowedBlockTypes in block editor settings
talldan 807902f
Make block manager more easily implemented in other editors
talldan 031603e
Fix missing deps
talldan 86e3468
Update snapshot
talldan e60bcd3
Update e2e test selector
talldan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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'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?
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.
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.
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.
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
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.
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.
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.
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.
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, 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.
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's an experiment here to try making the persistence aspect of preferences optional/configurable - #39795.