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

Template editing: improve revert notices #50302

Merged
merged 1 commit into from
May 15, 2023
Merged
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
12 changes: 9 additions & 3 deletions packages/edit-site/src/components/list/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ import RenameMenuItem from './rename-menu-item';
export default function Actions( { template } ) {
const { removeTemplate, revertTemplate } = useDispatch( editSiteStore );
const { saveEditedEntityRecord } = useDispatch( coreStore );
const { createSuccessNotice, createErrorNotice } =
const { createSuccessNotice, createErrorNotice, removeNotice } =
useDispatch( noticesStore );

const isRemovable = isTemplateRemovable( template );
const isRevertable = isTemplateRevertable( template );

Expand All @@ -30,16 +29,23 @@ export default function Actions( { template } ) {
}

async function revertAndSaveTemplate() {
const noticeId = 'edit-site-template-reverted';
removeNotice( noticeId );
try {
await revertTemplate( template, { allowUndo: false } );
await saveEditedEntityRecord(
'postType',
template.type,
template.id
);
const notice =
template.type === 'wp_template'
? __( 'Template reverted.' )
: __( 'Template part reverted.' );

createSuccessNotice( __( 'Entity reverted.' ), {
createSuccessNotice( notice, {
type: 'snackbar',
id: noticeId,
Copy link
Member

@Mamaduka Mamaduka May 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the static ID, removing it above is unnecessary.

I've used a similar method in #45348.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, my thinking in this case was that because the unique id prevents the stacking of snackbars, if you revert multiple templates one after the other the user doesn't get a clear indication of the success of each revert, ie. they revert one and the snackbar appears and stays, they revert another and the same snackbar is still just sitting there. But with the remove added a new snackbar appears for each revert to give confirmation, but still only one at a time.

Do you think this makes sense?

} );
} catch ( error ) {
const errorMessage =
Expand Down
7 changes: 3 additions & 4 deletions packages/edit-site/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@ export function setIsSaveViewOpened( isOpen ) {
export const revertTemplate =
( template, { allowUndo = true } = {} ) =>
async ( { registry } ) => {
const noticeId = 'edit-site-template-reverted';
registry.dispatch( noticesStore ).removeNotice( noticeId );
if ( ! isTemplateRevertable( template ) ) {
registry
.dispatch( noticesStore )
Expand Down Expand Up @@ -466,17 +468,14 @@ export const revertTemplate =
.dispatch( noticesStore )
.createSuccessNotice( __( 'Template reverted.' ), {
type: 'snackbar',
id: noticeId,
actions: [
{
label: __( 'Undo' ),
onClick: undoRevert,
},
],
} );
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may be needed for some other usage of revert template, but I couldn't see where.

registry
.dispatch( noticesStore )
.createSuccessNotice( __( 'Template reverted.' ) );
}
} catch ( error ) {
const errorMessage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export default function EntitiesSavedStates( { close, onSave = identity } ) {
const { __unstableMarkLastChangeAsPersistent } =
useDispatch( blockEditorStore );

const { createSuccessNotice, createErrorNotice } =
const { createSuccessNotice, createErrorNotice, removeNotice } =
useDispatch( noticesStore );

// To group entities by type.
Expand Down Expand Up @@ -148,6 +148,8 @@ export default function EntitiesSavedStates( { close, onSave = identity } ) {
};

const saveCheckedEntitiesAndActivate = () => {
const saveNoticeId = 'site-editor-save-success';
removeNotice( saveNoticeId );
const entitiesToSave = dirtyEntityRecords.filter(
( { kind, name, key, property } ) => {
return ! unselectedEntities.some(
Expand Down Expand Up @@ -208,6 +210,7 @@ export default function EntitiesSavedStates( { close, onSave = identity } ) {
} else {
createSuccessNotice( __( 'Site updated.' ), {
type: 'snackbar',
id: saveNoticeId,
} );
}
} )
Expand Down