-
Notifications
You must be signed in to change notification settings - Fork 4.6k
ContentOnlyControls: Refactor ad hoc fields to use setValue instead of updateBlockAttributes #73680
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
ContentOnlyControls: Refactor ad hoc fields to use setValue instead of updateBlockAttributes #73680
Conversation
…f updateBlockAttributes
|
Size Change: -75 B (0%) Total Size: 2.58 MB
ℹ️ View Unchanged
|
aaronrobertshaw
left a comment
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.
Nice work @andrewserong 👍
This is testing as advertised for me.
✅ Replacing or clearing images works
✅ Adding links to images and buttons works
✅ Rich text updates function correctly
✅ Functionality is consistent between this branch and trunk
Screen.Recording.2025-12-02.at.3.09.03.pm.mp4
LGTM 🚢
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Nice, thanks for the quick review @aaronrobertshaw! 🙇 |
What?
Part of #73261
Follow up to:
One step of refactoring ad hoc ContentOnlyControls to better align with the Fields / DataForms API: don't pass in
updateBlockAttributesto the Edit component's config, and instead use thesetValuemethod to provide mapping, and defer to the DataForm to make theupdateBlockAttributescall.Why?
As raised in #73261, the current implementation doesn't conform to the Fields API and adds some not-quite-random-but-inappropriate values to the
configobject for the Edit component. Ideally, we want to get these ad hoc ContentOnlyControls (media, link, richtext) closer to a state where they could become canonical parts of the DataForm / controls APIs — or at the very least, they should be as close as possible to that structure.Note: this PR just looks at
updateBlockAttributes. There are other things to be refactored in separate PRs, e.g.clientIdwhich is a bit tangled up with how rich text currently works. My hope is to look into other refactors in follow-ups and keep these changes fairly small.How?
onChangecallback passed toDataFormas this is already handled insetValueonChangeto handle theupdateBlockAttributescallsupdateBlockAttributesfunction from being passed in to the Edit configbackgroundTesting Instructions
trunkScreenshots or screencast
The only visual change is that the Cover block's background control will be shown by default now, instead of there being an empty Cover block area: