-
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
useBlockProps: use ref callback #28917
Conversation
Size Change: +82 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
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.
Thanks for looking at this so fast Ella! 🚀
packages/block-editor/src/components/block-list/use-block-props/use-event-handlers.js
Show resolved
Hide resolved
packages/block-editor/src/components/block-list/use-block-props/index.js
Outdated
Show resolved
Hide resolved
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 have tested and it works as expected - thanks Ella! 💯
4174544
to
fe83590
Compare
I'd wager that it worked properly. I didn't verify with bisect but I came to suspect this PR after I noticed in the Button block that |
Description
Fixes: #28880
Alternative to #28658.
I added a
useEffect
-like cleanup pattern touseMergeRefs
, which allows something to be cleaned up more easily than with the current way ref callbacks work. Ref callbacks are normally called withnull
when a new one is called, but unless you set the node as a property of the ref callback, you don't have access to it. It's probably possible to do, but it makes the code much harder to understand than theuseEffect
pattern that everyone already knows. So if a ref callback returns a cleanup function, we'll call the cleanup function instead of calling the callback a second time with null. It's still possible to have the old behaviour by simply not returning a cleanup function. I think this pattern will be very useful for many components switching from an effect hook to a ref callback.How has this been tested?
Screenshots
Types of changes
Checklist: