Light block: introduce useBlockWrapperProps #23034
Conversation
ellatrix
requested review from
ajitbohra,
Soean,
talldan and
youknowriad
as code owners
|
Size Change: +563 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
[Feature] Blocks
|
Wow, this looks really neat! I definitely prefer this to the previous approaches. One question: should we call it |
|
I don't mind either way... I slightly prefer something shorter, but at the end of the day it doesn't matter much. |
packages/block-editor/src/components/block-list/block-wrapper.js
Outdated
Show resolved
Hide resolved
| }; | ||
| } | ||
|
|
||
| const BlockComponent = forwardRef( |
There was a problem hiding this comment.
This is cool, when do you think we should remove this? should we add a deprecated call with a version?
There was a problem hiding this comment.
Since it's experimental, maybe we can just remove it. Depending on what we do, we might have to leave it since some block components are still classes. Maybe the ref idea could replace it though.
There was a problem hiding this comment.
We could yes, I saw some third-party usage already. So let's be nice to them and deprecate for a release or two.
|
@youknowriad That's an excellent idea! I'll explore it quick. |
|
@youknowriad Oh, actually, I don't think it would be that easy because we still have to deal with the other props that need to be passed down to the block. :/ |
One possibility would be to apply the style and classname using DOM apis and not React props but not sure how good is that |
|
@youknowriad I was thinking about that, and it would work fine if there were a limited amount of props controlled by us, but we have a filter on the block edit component where plugins are able to add additional props, so I think it's better to pass them straight to the react element. |
|
Starting to wonder whether the hook is the best API. I mean it is but with third-party random props we can already have issues as blocks using the hook won't "merge" all props (imagine blocks having attached event handlers). I'd love if we can officially restrict that hook and make it more predictable, it's harming us in a lot of ways. |
Event handlers are ok. We don't attach any event handlers with react.
What do you mean? |
|
Let me elaborate :)
|
ellatrix
changed the title
|
@youknowriad Yes, I'll add a deprecation. |
ellatrix
force-pushed
the
try/use-block
branch
from
2ff7bab to
bd8be8c
Compare
|
Notably, the following blocks still use the
|
|
@ZebulanStanphill Yes, remaining ones should be done in #25515. |
...and make it backwards-compatible. This label was updated in WordPress/gutenberg#23034.
...and make it backwards-compatible. This label was updated in WordPress/gutenberg#23034. Co-authored-by: Bart <bartlomiej.kalisz@gmail.com>
Description
This PR introduces
useBlockWrapperProps, a hook used to lightly mark any element as a block.To use, just pass the returned props to the element:
If the component defines a ref for the element, it must be passed through
useBlockWrapperProps. If no ref is defined,useBlockWrapperPropswill create one and pass it to the element.You can pass any props to
useBlockWrapperPropsand they will be returned, but you don't have to. In case ofstyleandclassNameprops,useBlockWrapperPropswill automatically merge them. If you don't pass them throughuseBlockWrapperProps, you have to merge them yourself.Both are equally ok:
A hook means that it can work for any component. No need to have a
<Block as />component andblock()HoC so that it works in different scenarios. Just one API. We don't need access to the element/component at all, we only need to define some props and have access to the ref.I think it's really nice that this leaves the original element structure intact, since it doesn't need to be wrapped in another component.
I could imagine a similar API for
InnerBlocks, which would avoid having things like the uglypassedPropsetc.How has this been tested?
Screenshots
Types of changes
Checklist: