Block Library: Add EntityProvider, controlled InnerBlocks and basic template blocks.
#17135
Conversation
epiqueras
added
[Feature] Blocks
epiqueras
requested review from
ajitbohra,
ellatrix,
gziolo,
jorgefilipecosta,
nerrad,
Soean,
talldan,
TimothyBJacobs and
youknowriad
as code owners
| acc[ entity.name ] = { kind: entity.kind, context: createContext() }; | ||
| return acc; | ||
| }, {} ), | ||
| post: { kind: 'postType', context: createContext() }, |
There was a problem hiding this comment.
If I remember properly, there's no way to ensure the name is unique across kinds. so we might be better using a two level objects [kind][name] or just an array of entities.
Also, there might be a better way to automatically create contexts for these dynamic entities (like post).
There was a problem hiding this comment.
be better using a two level objects [kind][name]
Yes
Also, there might be a better way to automatically create contexts for these dynamic entities (like post).
We could add this to the store and load them asynchronously, but it seems unnecessarily complex.
There was a problem hiding this comment.
| ); | ||
|
|
||
| const { editEntityRecord } = useDispatch( 'core' ); | ||
| const setValue = useCallback( |
There was a problem hiding this comment.
I noticed that you use useCallback a lot across your hooks usage. I personally feel this memoization has more costs than regular functions. I guess it depends on the usage but shouldn't we be adding these only when we measure performance degradation?
There was a problem hiding this comment.
This is a library hook, so the cost will be determined by the user, unknown to us.
Users don't expect the function returned to change (like with useState), and they might pass it directly to pure components and break their optimization.
The cost of useCallback is minimal even if paid everywhere the hook is used. A single pure component de-optimization probably offsets it.
useCallbackCost * nTimesUsed < potentiallyLargePureComponentDeOptimizationCost * probabilityThisHappens
There was a problem hiding this comment.
I agree that API-wise, these seems like a good approach. I'm not sure I understand the changes in InnerBlocks entirely yet and as I said, I'm still convinced that we might be better served with separate block lists (without intersections) for separate entities. For example here if I serialize the blocks for a given entity, I'm pretty certain the serialization will include the blocks from other entities in the produces HTML (post-content block).
epiqueras
force-pushed
the
update/the-editor-store-to-use-core-data-entities
branch
from
4d5de1b
to
a020241
Compare
It's just allowing the parent to sync with it through value and on change props.
No, that's explicitly controlled by the post content block not having a save component. |
|
Closed in favor of #17153. |
youknowriad
deleted the
add/entity-provider-controlled-inner-blocks-and-basic-template-blocks
branch
cc @youknowriad
Description
This PR puts together the best ideas in #17020 and #17118 to implement Full-Site editing blocks.
Here is the prior art of this refined approach: #17118 (comment).
The Site Title Block was intentionally left out to reduce the scope of this PR, but the Post Content Block was kept so that we have a block to use the new "semi-controlled"
InnerBlockswith.How has this been tested?
The blocks were tested manually to work as expected.
Types of Changes
New Feature: Add new template blocks: Post, Post Title, and Post Content.
Checklist: