-
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
Skip 'Site Updated' message if only post meta was saved #62284
Conversation
I'm not sure of the best way to address this bug. Is the code here on the right track? Modifying Also, I'm unsure if if just checking for I'll keep looking at it, but any thoughts appreciated! |
Size Change: +26 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
return await { | ||
values: dispatch.saveEntityRecord( kind, name, record, options ), | ||
metaChange: !! edits?.meta, | ||
}; |
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 don't think we should touch core-data for this change. I think core-data shouldn't have flags like that that are specific to given entities.
@@ -203,7 +203,7 @@ export const saveDirtyEntities = | |||
registry | |||
.dispatch( noticesStore ) | |||
.createErrorNotice( __( 'Saving failed.' ) ); | |||
} else { | |||
} else if ( ! values.every( ( value ) => value.metaChange ) ) { |
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.
metaChange is not the only thing that should prevent the "site updated" notice. I think we should think about this notice a bit more holistically.
Should we show "site updated" when we change just a single entity (like a page or post or a template) or something?
Maybe we should try to be smarter here. If the "pendingRecords" is a single entity we just show ${ entityTitle } saved successfully
and if there are multiple records we should "Site updated" or something.
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 decided to try a simpler approach in #63223, related comment here, let me know what you think 🙏
I also don't think this is a bug or something that is specific to block bindings or 6.6, so it's probably better addressed outside the 6.6 rush. |
An alternative approach might include modifying dirtyEntityRecords and doing a more substantial overhaul inside of saveDirtyEntities, though it seems to me it would be a fairly substantial change. |
@youknowriad Great, thanks for the feedback!
@SantosGuillamot What do you think about this? |
I agree that we should think more holistically and not think only about post meta changes. Actually, I see it related to what I shared here about triggering the "Save panel" not only for post meta changes but potentially for other things like post description, excerpt... (especially when they have their own block binding source) If we need to work on all of that to solve that issue, I agree that moving it outside 6.6 is probably the best. |
@youknowriad @SantosGuillamot I created an issue to further discuss how we could approach this: #62383 |
Since it seems there is a new pull request for the same purpose, should we close this one and keep the discussion there? |
Closed in favor of #63223 |
What?
This PR aims to avoid rendering the "Site Updated" message if only post metadata in an entity has changed, namely in relation to editing meta via block bindings.
Why?
Addresses #62236
We don't want to erroneously show the "Site Updated" message.
How?
Right now, it adds a flag to
saveEditedEntityRecord()
if edits include post meta, and prevents the "Site Updated" message from showing if this is the only kind of edit that occurred.Testing Instructions
1. Register post meta by adding this snippet to your theme's functions.php
2. In the post editor, add a paragraph block bound to the custom field using the Code Editor
Testing Instructions for Keyboard
Screenshots or screencast