-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add 'Clear customizations' button to template list page #36802
Conversation
<MenuItem | ||
info={ __( 'Restore template to theme default' ) } | ||
onClick={ () => { | ||
revertTemplate( template ); |
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.
The main outstanding issue seems to be that revertTemplate
doesn't actually save the edited entity records to complete the reversion. The listing page doesn't update because of that.
It's really designed for use in the editor where there's a separate save button. It also treats the revert as something that can be undone. That may be more difficult to achieve here if we have to save the changes.
The options here:
- Don't use this action, create an entirely different action or make the revert code local to this component
- Customize
revertTemplate
so that it accepts additional params to make it work on this screen - Something else that I haven't thought of
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.
We could, not that it's preferred, add the entity saving workflow to this screen too. I think it would look weird though.
Could we add an optional option force: true
to indicate that it should save immediately after edited? If that doesn't work we can create a separate action for it.
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 implemented something similar to force: true
but I didn't like how it means that passing an argument completely changes the behaviour of the action.
Ideally I'd split revertTemplate
up into two functions—one that saves, one that doesn't—and have them both call into a private action which does the entity editing bits. This is really difficult to do unless we change edit-site
to use thunks, though.
So instead I went with e9e7ede which has the button immediately save the entity after reverting. I added a allowUndo
option so that the success notice, when we implement notices, doesn't allow the user to undo something that's been already saved.
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 think we should also inform users that this is an irreversible action.
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 think we should also inform users that this is an irreversible action.
I think let's follow up on that. I see "Ability to undo an action" on the list (#36597). We have the same problem with the Remove action.
Size Change: -33 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
I guess @talldan and I ended up pairing on this so just need a ✅ from @jameskoster and should be good to merge. |
I think there are some failing tests because the new I can patch that up in a bit. |
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.
Approving because it works, but we'll certainly need a Snackbar confirmation. I suppose we can tackle that in #36808.
I didn't get round to this, but should be a simple fix. |
@talldan, I will look into and fix the failing e2e test. |
Thanks everyone. |
* Add 'Clear customizations' button * Save entity after reverting it * Correctly 'allowUndo' default Co-authored-by: Robert Anderson <robert@noisysocks.com> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Description
Closes #36611.
Adds a Clear customizations button to the dropdown in the Site Editor list view.
Kapture.2021-11-24.at.16.59.05.mp4
How has this been tested?
Screenshots
Types of changes
Enhancement
Checklist:
*.native.js
files for terms that need renaming or removal).