Skip to content
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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,10 @@ export const saveEditedEntityRecord =
recordId
);
const record = { [ entityIdKey ]: recordId, ...edits };
return await dispatch.saveEntityRecord( kind, name, record, options );
return await {
values: dispatch.saveEntityRecord( kind, name, record, options ),
metaChange: !! edits?.meta,
};
Copy link
Contributor

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.

};

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/store/private-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export const saveDirtyEntities =
registry
.dispatch( noticesStore )
.createErrorNotice( __( 'Saving failed.' ) );
} else {
} else if ( ! values.every( ( value ) => value.metaChange ) ) {
Copy link
Contributor

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.

Copy link
Contributor Author

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 🙏

registry
.dispatch( noticesStore )
.createSuccessNotice( __( 'Site updated.' ), {
Expand Down
Loading