-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix: Store post meta changes are overridden when custom fields meta box is enabled #64505
Fix: Store post meta changes are overridden when custom fields meta box is enabled #64505
Conversation
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. |
@youknowriad @gziolo It seems this part of the code was introduced in this pull request with a similar issue. It'd be great to get your feedback on whether this is the proper place to solve the problem or if we should look for alternatives. |
Size Change: +90 B (+0.01%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
Flaky tests detected in 64bf487. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10389129537
|
The problem is that "meta" can be edited both in the "custom fields" UI (as a metabox) and in the editor (in bound blocks). So this PR creates the opposite problem, if you try to edit the field in the "custom fields UI", it's not saved properly. We need somehow to merge the field values. |
What happens if users change one meta value inside the block (using core data) and another using meta boxes? Are both values saved correctly? |
Changes work independently but when you use both places at the same time they are overridden. It seems tricky to merge the values if meta box doesn't use the store. I'll try to find another solution. |
Oh okay, I've just realized that it is expected to save the value of the "custom fields UI" when clicking "Save" and not just when clicking "Update". |
I think there might be a way to do so if we can identify the custom fields changed in the store, which I believe it is possible comparing the original vs the edit. If we have that information, we can modify the for ( const [ key, value ] of formData.entries() ) {
if ( value in modifiedFields ) {
formData.set(
key.replace( '[key]', '[value]' ),
post.meta[ value ]
);
}
} It seems a bit hardcoded to how meta fields work, but I am not sure if we can avoid that. Another possibility I guess it could be passing the This is a video of how it works with that code:
Demo.editing.custom.fields.UI.mp4Let me know if this path could make sense or I should explore other options. |
I made this commit to illustrate what I meant. It seems to work, but I'm happy to iterate on it or explore the server approach. |
Let's keep in mind that Meta Boxes / Custom Fields are rendered on the server: All the operations trigger AJAX requests that update HTML partials, for example, updating post meta value: In effect, keeping everything in sync is rather a huge effort, and I doubt it's worth investing more time into it, given the end goal for Block Bindings is to completely replace what Custom Fields offers today. Users also need to explicitly opt in into Custom Fields through Preferences: I left a longer #64217 (comment) in the parent issue, but I proposed that we disable editing of attributes connected to Post Meta in the post editor when Custom Fields UI is enabled. |
I'm working on a prototype of that right now. |
Now that the alternate fix has landed: Should we close this one for now and reopen it later if we arrive at the conclusion that we need deeper integration between bindings and meta boxes? |
Let's close as mentioned here #64505 (comment) Later we can check if more integration if needed, or maybe in a future if that meta box can be deprecated. |
What?
Fix #64217 and #23078
Right now, any changes to post meta made in the store are overridden when the custom fields meta box is enabled.
In this pull request, I'm exploring how to solve that.
Why?
It doesn't save changes that user expect to be saved.
How?
I must say that it is not clear to me how this can be solved. So far, it seems including
post.meta
changes in the additional params of the meta box requests solves the issue. But I am not sure about the implications.With the initial code, the following works:
Here you have some demo showing that:
Before
Editing.custom.field.demo.not.working.mp4
After
Editing.value.demo.working.mp4
Meta box update
Editing.from.metabox.demo.mp4
Testing Instructions
Follow step-by-step reproduction instructions of the mentioned issues and check that the issues are solved.