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

FSE: Create more robust autosaving including UI #29577

Open
annezazu opened this issue Mar 5, 2021 · 17 comments
Open

FSE: Create more robust autosaving including UI #29577

annezazu opened this issue Mar 5, 2021 · 17 comments
Labels
[Feature] History History, undo, redo, revisions, autosave. [Feature] Saving Related to saving functionality [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. [Type] Discussion For issues that are high-level and not yet ready to implement. [Type] Enhancement A suggestion for improvement.

Comments

@annezazu
Copy link
Contributor

annezazu commented Mar 5, 2021

What problem does this address?

As part of the second FSE outreach call for testing, someone reported a crashing experiencing yet found that there weren't any autosaves in place. This begs the question of what's currently expected for autosaving especially since FSE includes so many entities. It's unclear to me what's expected here but this is something that users have grown accustomed to so it should be considered.

What is your proposed solution?

This is tough to say due to the multi-entity saving process we currently have! Perhaps something the autosave could reflect that with a multi-entity selection option for what to restore.

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. Needs Technical Feedback Needs testing from a developer perspective. [Feature] History History, undo, redo, revisions, autosave. [Feature] Full Site Editing [Type] Discussion For issues that are high-level and not yet ready to implement. Needs Design Needs design efforts. labels Mar 5, 2021
@jameskoster
Copy link
Contributor

jameskoster commented Mar 5, 2021

I feel like there are several parts to this:

  1. Autosaving should work as normal. That is to say, if I edit an entity in the Site Editor, an autosaved draft should be created in the background.
  2. The UI could potentially reflect autosaves per entity, and it may be possible to discard those changes before entering the save flow. Or maybe even save changes independently.
  3. If I exit without saving changes, when I next return to the Site Editor I should be notified that autosaves exist per entity, with options to restore all, or specific entities.
  4. If I exit without saving then edit an entity with autosaves in isolation (like a Template Part), I should see the standard autosave notice we see on posts / pages.

For that second point, since we now denote the name of global blocks like Template Parts and Reusable Blocks in the Toolbar, and may soon list which Template Parts the current template consumes (#29147), we might include something there. Dots are common patterns to indicate unsaved changes in other apps:

Screenshot 2021-03-05 at 09 43 29

We should be careful with the dot though, since we have in the past used this to indicate when a template has been customised from its stock theme-supplied state.

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 5, 2021

We also need to remember that Reusable blocks now have a similar save structure as template parts. At the moment it is not possible to discard a save. This can be tricky as undoing might not bring back the original reusable block. Forcing the user to either recreate the contents of the reusable block or to just accept whatever changes will be saved writing over the original Reusable block. I will go ahead and create an issue for it if one does not exist. I believe this issue might work: #29269

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 15, 2021

Looking at what we have today and making some adjustments.
Going for a minimum change.

1-Changing the Site Title.
2-Clicking Apply.
3-Unchecking the Header. (Discard changes)
4-Clicking Save.
5-Saved. (Still in the Full Site Editing screen. One is not pushed back into the page content as is currently the behavior in FSE. As it is up to the user to choose when to go back to Page content editing.)

Save-FSE-disregard-changes-Header.mp4

Figma prototype:
https://www.figma.com/proto/De3yU46oeTResqpxkRPojb/FSE?node-id=30%3A133&scaling=scale-down-width

@hedgefield
Copy link

I agree with @jameskoster, an autosave should save a draft for any and all changes. Whether someone wants to discard some of those changes before publishing the draft is up to them. Showing the changes in the pre-publish sidebar like that also makes for a cool changes summary!

Minor thing: for the label in Paal's mockup I would suggest "changes will be lost" or "discarding changes". Now it looks like an action you still have to take, like the "remove image" red link that appears when selecting a featured image.

@jameskoster
Copy link
Contributor

Yeah I think it is fair to say that if a user actively un-checks an entity in this flow, they have acknowledged that it will not be saved. We probably don't need to display anything extra.

This is probably a bridge too far at this point, but it will be interesting to see the canvas update based on which entities are checked / unchecked in the save sidebar.

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 18, 2021

Here is another version. This can be used in Full Site Editing and in Reusable blocks.

FSE-Changes-will-be-lost

It would in general for various issues be good to look at what would be a good first version. Then iterate from there.
Pushing forward the very first version while still holding a larger vision in mind.

@jameskoster
Copy link
Contributor

I still don't think you need the "Changes will be lost" text. It's basically communicating the same thing as the un-checked checkbox? :)

@paaljoachim
Copy link
Contributor

paaljoachim commented Mar 18, 2021

I initially also thought that one could just uncheck for items one does not want to save. But to make it real clear it is nice to also give a message confirmation. Just in case the users mind is wandering a bit too far into something else...:)

@javierarce
Copy link
Contributor

I also feel we don't need that text. It's a bit redundant and makes the UI quite noisy (imagine if a user had 5 unchecked changes).

What if instead, we make the text above a little scarier?

Select the changes you want to save
Some changes may affect other areas of your site.
Unchecked changes will be lost.

@paaljoachim
Copy link
Contributor

Yes! That is even better. Thanks @javierarce Javier!

FSE-Changes-will-be-lost-next-version

@javierarce
Copy link
Contributor

javierarce commented Mar 19, 2021

I really liked @jameskoster solution to indicate which areas have been modified and allow to save and discard the changes. Here's a little iteration on his idea.

In this version, the dots are just indicators without any interactivity and the actions are accessible through an ellipsis icon.

Screen.Recording.2021-03-19.at.16.57.16.mov

Here's a Figma file with this prototype.

@jameskoster
Copy link
Contributor

Looking good @javierarce! Using an ellipsis for the actions makes much more sense :)

I can see why the Sidebar bubbled up to the top of the list when you reverted the Header changes, but I wonder if we need to do that? Is the benefit of displaying unsaved areas at the top of the list worth the trade-off in the lost semantics (IE list placement matching position in the document tree)?

@javierarce
Copy link
Contributor

Maybe I'm overdesigning this, but if themes end up having several custom parts, I think that placing the modified areas at the top will make the list easier to read and work with. If the general use case is to have three or four areas, I think keeping the list ordered as you proposed probably makes more sense.

@javierarce
Copy link
Contributor

I've just seen this issue and I was wondering if it would make sense to give the option to revert the template state in the popover.

Template areas without changes would have this one option, while modified templates would allow to Save, Discard, and Revert.

@jameskoster
Copy link
Contributor

Yup, I think that makes sense. But it should also be possible to revert a template part while editing it in isolation.

@jameskoster
Copy link
Contributor

I can't make up my mind on whether the areas should jump around when changes are discarded 🙃 Hopefully it's something we can try when a PR has been assembled.

I think the design in #29577 (comment) is ready for development. But this is blocked by #29147.

I suspect the autosaving behaviours described here will need to be separate.

@jameskoster jameskoster added Needs Dev Ready for, and needs developer efforts and removed Needs Design Needs design efforts. labels Mar 26, 2021
@paaljoachim
Copy link
Contributor

paaljoachim commented Apr 2, 2021

Clicking a dot to accomplish an action is not an obvious action. This is hidden UX. As one can not see that an action is hidden behind a dot. See my comment here: #29871 (comment)

It would be nice to go with an obvious in our face save or discard save method. Atleast as a start. Then one can gradually iterate.

Such as I show in this comment further above. #29577 (comment)

If the user unchecks both header and footer then the button can change to say "Discard save"

Discard-Save-button

Bottom line is having an easy to see method on discarding a save. Discarding a save the above header and footer would revert back to the last changes that were saved or go back to default state. @critterverse Channing and @jameskoster James.

@annezazu annezazu added the [Feature] Saving Related to saving functionality label Dec 8, 2022
@annezazu annezazu added [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") and removed [Feature] Full Site Editing labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] History History, undo, redo, revisions, autosave. [Feature] Saving Related to saving functionality [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. [Type] Discussion For issues that are high-level and not yet ready to implement. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants