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

Edit Site: Add a "Revert to default" option for templates and template parts. #23421

Closed
kjellr opened this issue Jun 24, 2020 · 39 comments · Fixed by #28141
Closed

Edit Site: Add a "Revert to default" option for templates and template parts. #23421

kjellr opened this issue Jun 24, 2020 · 39 comments · Fixed by #28141
Assignees
Labels
Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.

Comments

@kjellr
Copy link
Contributor

kjellr commented Jun 24, 2020

In the full-site editor, there should be an option to revert your template or template part back to the default that the theme provides.

Currently, this is only possible by visiting the “Templates” page (under Appearance) and removing the relevant custom templates.

@noahshrader noahshrader self-assigned this Jul 27, 2020
@noahshrader noahshrader added the [Status] In Progress Tracking issues with work in progress label Jul 28, 2020
@ockham
Copy link
Contributor

ockham commented Aug 20, 2020

The revert isn't too hard to implement, but we'll need designs for this.

cc/ @dubielzyk 🙂

@ockham ockham added the Needs Design Needs design efforts. label Aug 20, 2020
@dubielzyk
Copy link

Would love to collaborate on this since I'm looking into doing this on Template parts and also on Templates via Document settings (#20877) 😊

@dubielzyk
Copy link

I don't have enough knowledge around templates to suggest a good solution, but for template parts I can imagine a simple solution is adding it to the overflow menu:

image

My assumption is that this is a lower priority use case to justify a front and center UI positioning. This feels right to me, but would love thoughts

@jeyip jeyip self-assigned this Sep 3, 2020
@jeyip
Copy link
Contributor

jeyip commented Sep 3, 2020

there should be an option to revert your template or template part back to the default that the theme provides.

Any thoughts on the UX for this or Jon's designs in general @MichaelArestad @shaunandrews?

@jeyip jeyip removed their assignment Sep 3, 2020
@noahtallen
Copy link
Member

This design has a similar interaction but called "discard changes": #19259 (comment)

cc @jameskoster

We need to clarify exactly what "discard changes" means. This would only be an option available to template parts customized to be different from the theme. This would not apply to template parts you've created for yourself.

For it to work, we would have to persist the theme-provided slug in case you change the name of the template part.

Then, what does revert to default do? Does it delete the template part CPT from the database which you've been using, thereby letting the HTML file resolve directly? (And is that an undoable action?) Does it overwrite your CPT with the contents of the theme HTML file?

@Addison-Stavlo
Copy link
Contributor

Does it delete the template part CPT from the database which you've been using, thereby letting the HTML file resolve directly? (And is that an undoable action?) Does it overwrite your CPT with the contents of the theme HTML file?

Good questions. It could just find the auto-draft for that theme-provided slug and theme name and start referencing that instead. But it wouldn't delete anything which means if the user customized and reverted from the same template part multiple times they would have multiple copies. Maybe switching its reference to the theme supplied auto-draft while deleting the customized one at the same time makes sense?

Regarding reverting a Template itself, it seems like we would just have to delete it? 🤔 I could be thinking about it wrong, but if it has the same slug then a published Template CPT would always take priority over a theme supplied auto draft.

@dubielzyk
Copy link

If this isn't an undoable action, we'll probably want a confirmation modal that ask and informs the user that this isn't an undoable change.

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented Sep 4, 2020

If this isn't an undoable action, we'll probably want a confirmation modal

Yeah, that makes sense. As far as reverting the Template Part is concerned, Im thinking we should go with not deleting anything for now and we can revisit this if it does become an issue.

So technically speaking for the Template Part revert flow:

  • When we auto-draft a template part from a theme, we save another copy of the slug as original_slug in the meta field (where theme already exists).
  • If that original_slug value exists for a template part entity in the 'published' state, AND a corresponding 'auto-draft' exists, we will show a 'revert' option.
  • Upon selecting 'revert', we update the template part block's slug attribute to correspond with the original and clear the ID field so it goes back to picking up the theme supplied auto-draft.

There would still be some odd behaviors in the above example though. Consider a user has customized/saved the Template as well and switches themes. If they revert their template part back to theme default once, it will pick up that old themes auto-draft Template Part as expected. If they customize it a second time, there will be no option to revert as no new auto-draft will be created since that theme is no longer active. 🤔

If they wanted to 'revert' to the new theme's template part in that case, the revert option couldn't really be stable as it would have to expect all themes to name all template parts by the same slugs. So in order to change to the new themes header for example, they would have to go through the 'choose another template part' flow and find it there... or revert/delete the customized Template itself to pick up the new theme's template.

@jameskoster
Copy link
Contributor

This is a great convo!

This design has a similar interaction but called "discard changes"

It should be noted that the referenced design is not prescriptive :D

Based on the discussion here, I think the "discard changes" concept is slightly different to the "revert to default" action proposed in this issue.

With the "discard changes" action I was concentrating on any local/transient edits to existing template parts in the flow of editing a document (or a template). It is not an action that would be permanently visible - it would only appear when there are unsaved changes of a template part.

As an example, if I am editing my page.html template and during that flow decide to remove the site logo block in my header.html template part, clicking "discard changes" in the context of header.html would re-insert the site logo. Any other edits I'd made to the broader template would however be preserved. I believe discarding changes is an action that would also be made available during multi-entity saving, so it doesn't feel like a super-high priority.


Reverting to default feels like an action that would only be available whenever a given template part (or template) that was supplied by a theme has been edited by the user. I say "edited" but as I understand it, literal editing of theme templates or template parts is not intended functionality. Instead, users will have the opportunity to duplicate those templates to create custom versions which then can be fully edited. We're still missing a design for this flow.

Assuming the above is correct, "editing" a themes singular.html template would spawn a new custom singular.html template that effectively replaces the theme version. In this case, reverting to default would delete the custom singular.html template, thus reinstating the closest equivalent template in the active theme. This is a far-reaching action, and I'm not sure it should be prominent in the post editing UI. A confirmation dialog seems like a good idea regardless, as the only way to undo would be to remove the template from the trash in the template management view.

@noahtallen
Copy link
Member

it would only appear when there are unsaved changes of a template part.

Ah, so it's basically a "reset to previous save" kind of interaction. A little bit more than the normal undo, and less than "revert to theme."

Instead, users will have the opportunity to duplicate those templates to create custom versions which then can be fully edited

Yes, you're mostly right about that. Currently, as soon as the user modifies the template part in the site editor, a "copy" of the template part is published to the database. From that point forward, references to that template part use the user-modified database copy rather than the theme HTML file. So the "duplication" thing is totally transparent currently, and this flow is implemented and working.

Another way of putting it is that when you make a change in the site editor to a template, part you are not modifying the theme HTML block files.

editing" a themes singular.html template would spawn a new custom singular.html template that effectively replaces the theme version. In this case, reverting to default would delete the custom singular.html template, thus reinstating the closest equivalent template in the active theme

Yep! To clarify, rather than that second singular.html template being stored as an HTML file, the user-modified copy exists in the database like any other post. So deleting the user-modified template means deleting (or I guess trashing) that post from the database.

@noahtallen
Copy link
Member

Consider a user has customized/saved the Template as well and switches themes.

What if we only allowed the "revert to theme" action if the same theme is active?

@jameskoster
Copy link
Contributor

Yeah this gets complicated when we consider theme switches :D

If I've customised a template and subsequently switched themes, I am not sure that I would expect the "Revert to default" action to reinstate a template from my old theme. My interpretation is that this action is more akin to saying "Get rid of my customisations and use my current active theme template instead". Would be great to get @kjellr's insight on that though.

Somewhat related, I opened #25071 to discuss in greater detail how customised templates are handled on theme switch.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 9, 2020

My interpretation is that this action is more akin to saying "Get rid of my customisations and use my current active theme template instead". Would be great to get @kjellr's insight on that though.

Yeah, that's what I'd expect too. Perhaps a better way to frame this would be "Reset to the theme's default layout".

@Addison-Stavlo
Copy link
Contributor

Yeah, that's what I'd expect too. Perhaps a better way to frame this would be "Reset to the theme's default layout".

For Templates, I 100% agree. For template parts on their own, if they are still using a customized version of a template from a previous theme the "reset to theme's default" for only the template part makes no sense since that new themes template parts have no context with respect to that customized template that is still in use.

What if we only allowed the "revert to theme" action if the same theme is active?

Re: template parts - I don't think we need to make that a hard condition. If the auto-draft of the same original slug/theme exists they should be able to revert to it. If the same theme isn't active, we just cannot guarantee that the auto-draft will exist so reverting in this case may only be available once if at all.

@jameskoster
Copy link
Contributor

A potentially wild idea that would remove the need for the "revert" functionality:

Could we just expose both the theme template and the custom template together in any template management interface(s)?

Then if you want to "revert", you simply delete your custom template. The language is simpler, and more representative of what is actually happening.

@noahtallen
Copy link
Member

I don't think that quite covers all of the cases, because another step of that process would be pointing any template part blocks to reference the theme template part instead of the custom template part. 🤔

@Addison-Stavlo
Copy link
Contributor

Could we just expose both the theme template and the custom template together in any template management interface(s)?

This is true, and it is something that is currently happening with the template part preview window that pops up from selecting a template part from the placeholder, or more recently from that dropdown in the rename toolbar. That is, the theme supplied auto-draft will be in that list. Although, it may have the same name if the customized version was not renamed, but the difference should be apparent in the preview of the template part at least. It just takes a little bit more active work than clicking a "revert" button, but may really be adequate as-is.

I don't think that quite covers all of the cases, because another step of that process would be pointing any template part blocks to reference the theme template part instead of the custom template part.

Oh, thats something I haven't considered at all with this 'revert' flow. So when we revert to default on one template part block, all template part blocks (on different templates/pages/etc.) should revert as well? Im not sure if this should or shouldn't be the case, but does seems like a complex case.

@jameskoster
Copy link
Contributor

Oh that's interesting. So if I've customised a theme template part I can still actually use both the theme version and my own custom version in different templates? Does that apply to top-level templates as well? IE if a theme supplies a "full width" template and I customise it, would both still appear as options in the template selection dropdown?

So when we revert to default on one template part block, all template part blocks (on different templates/pages/etc.) should revert as well? Im not sure if this should or shouldn't be the case, but does seems like a complex case.

If these reversions are not global I think the UX gets super weird. If they are local to the current document / template, then reverting a template part wouldn't actually be any different to simply selecting the default equivalent in the selection dropdown (assuming the above is correct). I think these should feel like separate actions, otherwise why have a separate UI? It seems to me that this action should be global, and due to that perhaps not accessible from the editor, rather from a dedicated template management interface (#20477) instead.

FWIW I think this is another argument to phrase this as deletion rather than reversion. Deleting a template part feels inherently global, to me at least. Reverting is more ambiguous.

To make this work we'd need a strong visual design that clearly communicates the relationship between the theme template and the customised version.

@jasmussen
Copy link
Contributor

jasmussen commented Sep 14, 2020

This is really interesting! I'm guessing this scenario is also "undo current changes" rather than reverting way back to the theme files?

Per what Jay (and Matias as quoted) suggest, there will likely be cases where you can't revert back to a template file because you switched themes and "single" doesn't exist anymore, but perhaps "singular" does. Could you revert to that?

Alternately, we do have a diffing tool for comparing blocks that were changed externally, perhaps that would work? Smarts in this dialog would definitely be nice.

However to be perfectly clear, what brought me here was sheer excitement at how close this is to usable, and because I personally (and selfishly) need this revert feature as I'm developing a theme. I suspect other theme developers such as Kjell need it too. And for that reason, I'd be tempted to suggest we go with the tiniest possible solution as step one, and upgrade that as we learn more from using that.

If that tiny v1 solution is a modal dialog that says "This template has been customized. [Revert to closest template match] [Cancel]” I'd be one happy camper. At least for the near future 😅

Also, this older ticket has some thoughts on how the dot (as used for responsive changes) can provide a path to listing and clearing them:
#19909

@noahtallen
Copy link
Member

If that tiny v1 solution is a modal dialog that says "This template has been customized. [Revert to closest template match] [Cancel]” I'd be one happy camper. At least for the near future

nice! So basically just use the closest match via the template hierarchy.

What about template parts, though, which don't have any hierarchy?

@jasmussen
Copy link
Contributor

I'd like a sanity check from @kjellr whether he feels the same. But for this dev purpose exactly, I'd be fine with simply "delete custom template part", especially if paired with a simple "are you sure" confirm. Presumably in my case header.html from template-parts will be used if referred to, when the customed version of it gets deleted.

@jameskoster
Copy link
Contributor

The tricky thing with reverting after switching to a theme where an equivalent template doesn't exist, is that you aren't reverting anything – you're deleting the (now) custom template. To "revert" is to go back to what you had before. That isn't what would happen with this action and that could be really confusing.

As a subtle (but I think important) change, how about instead of the blue dot denoting "customized", it instead denotes "custom template"? I believe this could help establish the following heuristic:

  • Theme templates cannot be edited or deleted, but can be used as a base from which to create new custom templates
  • Custom templates can be both edited or deleted, and even reverted when the conditions are right.

This helps in the scenarios touched upon in this discussion:

  1. User customises a theme template — that new template is marked as custom and the site owner can revert their changes to reinstate the default theme template.
  2. The same user switches to a theme that has a different "single" template. Their custom template persists the switch, but can be deleted rather than reverted.
  3. The same user switches theme again, this time to a theme that doesn't include a "single" template at all. Once again their custom template persists, and can be deleted.

Thinking more broadly about templates, this principle has more widespread application. For example a template that a user generates from scratch would also be marked as a custom template.

I appreciate this may be out of scope for this issue in particular. So to bring this back around...

If that tiny v1 solution is a modal dialog that says "This template has been customized. [Revert to closest template match] [Cancel]” I'd be one happy camper.

Should we make the language a little more explicit based on the scenarios outlined above?

  1. If the theme hasn't changed: "This template has been customized. [Revert my customizations]".
  2. If the theme has changed and equivalent template exists: "This is a custom template. [Delete template]".
  3. If the theme has changed and equivalent template doesn't exist: "This is a custom template. [Delete template]".

I feel this creates a clearer distinction around reverting templates versus deleting them.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 15, 2020

If the template you originally edited is no longer available, I agree that we should reframe this as a "delete" instead of a "revert". "Revert" makes it sounds like it's changing to something you've seen before, but that might not be the case.

@jameskoster's recommendation about Custom Templates makes sense to me:

If the theme hasn't changed: "This template has been customized. [Revert my customizations]".
If the theme has changed and equivalent template exists: "This is a custom template. [Delete template]".
If the theme has changed and equivalent template doesn't exist: "This is a custom template. [Delete template]".

That said, in all of these "delete" cases, users will likely wonder "What will happen to this page if the custom template is deleted?". So it's worth thinking more about how to communicate that to them.


Also, this is a bit of a different discussion, but reading this...

As a subtle (but I think important) change, how about instead of the blue dot denoting "customized", it instead denotes "custom template"? I believe this could help establish the following heuristic:

... made me think that we'll need a clearer way of letting the user know that it's a custom template — the dot alone seems likely to be misunderstood.

@jameskoster
Copy link
Contributor

Yeah I think there should be a confirmation dialog when deleting a template. Possibly when reverting too :)

It is not always going to be possible to succinctly explain what will happen to any documents that are using the template that is being deleted though. A simple solution might be to inform the user of how many documents will be affected. That would at least give a rough indication of how significant the deletion will be.

... made me think that we'll need a clearer way of letting the user know that it's a custom template — the dot alone seems likely to be misunderstood.

I agree. Labelling templates as "theme template" or "custom template" is one option. Another might be to use an "Author" label, e.g. "Author: Theme name", "Author: username". I'm sure there are other (better) options to consider too.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 15, 2020

Yeah I think there should be a confirmation dialog when deleting a template. Possibly when reverting too :)

It is not always going to be possible to succinctly explain what will happen to any documents that are using the template that is being deleted though. A simple solution might be to inform the user of how many documents will be affected. That would at least give a rough indication of how significant the deletion will be.

Yeah, a message like this would go a long way:

Are you sure? These four pages will now use your theme's default template: 

• [List of pages]

[ Ok ][ Cancel ]

@noahtallen
Copy link
Member

Are you sure? These four pages will now use your theme's default template:

The more we get into this, the trickier it becomes to implement 😅 😅 (It's tricky because WordPress' template resolution algorithm relies exclusively on the context of a request to the front end of the site, so there's not really anything explicitly mapping templates into the hierarchy, or pages for that matter. It's all implicit at runtime from what I can tell.)

But it's cool to think about what could be possible!

@jasmussen
Copy link
Contributor

Even just a basic dumb delete for starters could solve the need today and help inform where we go. The site editor is still marked as experimental, seems okay to go basic before we go deep.

@mtias
Copy link
Member

mtias commented Sep 25, 2020

+1 on making this simple — deleting a custom template that is being loaded instead of the theme one is what would achieve a revert, and we should make this action simpler. We can warn users that they'd lose their customizations.

@jameskoster
Copy link
Contributor

In the latest designs for the template settings there is an actions menu of sorts. I think it would make sense to include this revert action there like so:

revert

This same UI component can potentially be reused on the Template Mosaic view:

mosaic

@jasmussen
Copy link
Contributor

What does "Move to trash" do?

@jameskoster
Copy link
Contributor

Ignore that, haha.

Revert to default would be the action available to Theme templates that have been customised by the user.

Move to trash would be the action available for templates that are entirely user-generated.

They wouldn't be visible concurrently like this. Apologies for that oversight y'all.

@jameskoster
Copy link
Contributor

Small update here based on the latest designs for other template actions (spinning up a variation).

revert

No major changes really. The blue dot is still used to signify a customised theme template, and reverting still takes places in the template dropdown in the top bar.

It would be great to get thoughts on the most effective language for use in the menu item name / description.

@jameskoster
Copy link
Contributor

I think this is ready for a try PR at least :) We can omit the blue dot for now.

@skorasaurus
Copy link
Member

+1; I was about to make a ticket for this.

I installed the Q theme and started experimenting a little bit with templates and accidentally broke things and wanted to revert my changes and just restart from scratch.

As a developer, the experience wasn't intuitive or clear.

What I ended up doing was:

Going to Appearances > Templates and trashing them as mentioned in the initial comment, and then repeating the process for template-parts;
changed site's theme to a non-FSE theme; then switched back to Q.

@carolinan
Copy link
Contributor

Related discussion: #22469 (comment)

@Copons Copons removed the [Status] In Progress Tracking issues with work in progress label Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet