Fix: Store post meta changes are overridden when custom fields meta box is enabled #64505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SantosGuillamot
added
[Type] Bug
|
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
|
This was referenced Aug 14, 2024
|
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. |
SantosGuillamot
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
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. |
51 tasks
|
I will investigate if is possible to update it without the need to reload the site. Otherwise is not really user friendly. Another possibility could be hiding the panel if the bindings preference is enabled? Or adding a similar warning like we have on the custom fields switch. 
Edit: It seems that if you click on update and then on save, it gets updated on the block without a page reload. Screen.Recording.2024-08-22.at.12.11.06.mov |
|
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? |
gziolo
added
the
[Feature] 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. |
youknowriad
deleted the
fix/custom-fields-not-updated-with-metaboxes
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.metachanges 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.