-
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
Site Editor: Create more robust autosaving including UI #29577
Comments
I feel like there are several parts to this:
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: 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. |
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 |
Looking at what we have today and making some adjustments. 1-Changing the Site Title. Save-FSE-disregard-changes-Header.mp4Figma prototype: |
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. |
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. |
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? :) |
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...:) |
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?
|
Yes! That is even better. Thanks @javierarce Javier! |
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.movHere's a Figma file with this prototype. |
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)? |
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. |
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. |
Yup, I think that makes sense. But it should also be possible to revert a template part while editing it in isolation. |
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. |
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" 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. |
Quick note that as work evolves with real time collaboration #52593, we'll need to revisit this if we are going to rely on auto-saving. This begs the question of if we need a "multi-entity" autosave endpoint rather than triggering separate autosaves for each entity. |
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.
The text was updated successfully, but these errors were encountered: