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

Auto-Save #221

Closed
jauyong opened this issue Feb 5, 2020 · 26 comments · Fixed by #1361
Closed

Auto-Save #221

jauyong opened this issue Feb 5, 2020 · 26 comments · Fixed by #1361
Assignees
Labels
P0 Critical, drop everything

Comments

@jauyong
Copy link

jauyong commented Feb 5, 2020

Blocked by #1247

As an author I want my progress auto saved so I don't lose my work.

Feature Brief - Epic: Document Workflow

Note: Save is in #521 and this ticket is for adding Auto-Save

Acceptance Criteria

  1. Saving & updating works similar to regular posts in WordPress, with some special casing (see below)
  2. If a story is unpublished, auto-save on 60 seconds interval (non configurable)
  3. If a story is unpublished, save by triggering the Save button
  4. If a story is published, auto-save on a given time interval, but not the story directly -- auto-save the "auto-save" version of the story as a backup.
  5. If a story is published, Save by triggering the Update button
  6. After the author navigates away(e.g. refresh) from the page without saving, a notification is shown.

Note: Save/update does not save the undo/redo stack. Undo/redo is tied to the current client state.

@jauyong jauyong added P0 Critical, drop everything Version 1.0 labels Feb 5, 2020
@jauyong
Copy link
Author

jauyong commented Mar 3, 2020

@pbakaus is the autosave interval 30 seconds? or is it configurable somewhere?

@pbakaus
Copy link
Contributor

pbakaus commented Mar 3, 2020

I don't think we should make it configurable for now, unless it's configurable in Gutenberg. /cc @samitron7

@swissspidy
Copy link
Collaborator

WordPress' default autosave interval is 60s I believe, and can be changed using the AUTOSAVE_INTERVAL constant PHP, and also be filtered by plugins

@pbakaus
Copy link
Contributor

pbakaus commented Mar 3, 2020

ok great. Let's use the constant, and therefore 60s.

@spacedmonkey
Copy link
Contributor

spacedmonkey commented Mar 4, 2020

Screenshot 2020-03-04 at 11 34 12

Gutenberg allows users to trigger a save ( as draft ). As we are not saving on keypress like google docs, we will need to do something similar. Thoughts @pbakaus

@spacedmonkey
Copy link
Contributor

Related: #85

@jauyong jauyong added the Size: M label Mar 4, 2020
@kmyram kmyram added this to the Sprint 25 milestone Mar 9, 2020
@jauyong jauyong mentioned this issue Mar 10, 2020
@jauyong jauyong changed the title Save + Auto-Save Auto-Save Apr 5, 2020
@jauyong jauyong added P1 High priority, must do soon and removed P0 Critical, drop everything labels Apr 7, 2020
@pbakaus pbakaus modified the milestones: Sprint 25, 1.0 Alpha Apr 11, 2020
@pbakaus pbakaus added P0 Critical, drop everything and removed P1 High priority, must do soon labels Apr 11, 2020
@miina miina self-assigned this Apr 13, 2020
@kmyram kmyram removed the Size: M label Apr 16, 2020
@miina
Copy link
Contributor

miina commented Apr 24, 2020

@pbakaus @samitron7 In WordPress, when there is an auto-save of a Published story that is newer than the saved post (e.g. the browser crashes or is closed without saving first, however, there's an auto-save created), this notification is displayed:
Screenshot 2020-04-24 at 10 41 01

Should we have something similar to notify the user there is a newer, auto-saved story available?

@pbakaus
Copy link
Contributor

pbakaus commented Apr 24, 2020

@miina so, WP expects you to effectively press "save" button manually from time to time to explicitely save the state? otherwise shows this? If that's like it works, I'm torn - I kinda like the simplicity of conflating auto-save and regular save. Thoughts, @samitron7 and @o-fernandez?

@o-fernandez
Copy link
Contributor

https://wordpress.org/support/article/revisions/#autosaves

Seems that this is, in essence, done to never overwrite the last thing the user saved. So WP has a revisions which are when the user explicitly saves, but there is only one autosave which is there in case the user navigates away, goes offline, etc. before they have a chance to save.

So, the principle WP follows is that:

  • a revision is created anytime a user explicitly saves. Revisions are always there. Here's an example of a revision history.
    image

  • autosaves will never create a revision, but if the user doesn't save after having made updates to their story/post, the next time they open that post they'll be prompted whether they want to review the "autosave" version and use it instead of the latest explicit save/revision.

That behavior makes sense to me if we are creating and storing revisions each time the users saves, and exposing the revision history to manage it. Otherwise, I think we can go with a simpler autosave implementation for now.

@miina
Copy link
Contributor

miina commented Apr 27, 2020

@pbakaus Yes, auto-save is a separate auto-save post kept in a database.
Note that in case of a draft, the auto-saving is directly overwriting the original post, there is no additional need to click "Save draft".

Independent of the revisions -- in case of a published post, the auto-save saves into the "backup" auto-save post, never the post itself -- otherwise, the published post would be automatically overwritten which definitely should not happen without the user explicitly prompting it. This means that in case of published posts, the user needs to click "Update" to actually update the published post.

The auto-save is still kept as a back-up though -- for displaying previews for example (already implemented), also, it would become useful if the user has worked on an unsaved (published) post for quite some time and then the browser crashes or the connection fails, etc. In this case, the notice mentioning there is a newer auto-save could be helpful to let the user know that the changes did not get fully lost meanwhile.

Thoughts?

@o-fernandez
Copy link
Contributor

In that case, @miina @pbakaus, I think it makes sense to have some logic for when the link to view and restore the autosaved version shows up. Not sure if something as simple as autosave timestamp > latest saved version when the user reopens the story would work?

@miina
Copy link
Contributor

miina commented Apr 27, 2020

@o-fernandez That should work, I'm mainly missing the UX part here though -- probably reusing the WP default notice wouldn't be the best option. WDYT?

@o-fernandez
Copy link
Contributor

Let me tag this for UX consideration @samitron7
Sam - tl;dr, see the comment above where miina shows the UI that WordPress uses to let the user know that there's an autosave version of the post that they are working on that is more recent than the saved version they opened/loaded.

This can happen because autosaved versions in WP do not overwrite the saved revision, they are just a backup/fallback in case the browser crashes, etc. But users are expected to explicitly save work before leaving the browser (they are reminded to...) So the question is, how do we show this in the UI for story editor?

@pbakaus
Copy link
Contributor

pbakaus commented Apr 27, 2020

@o-fernandez it'd be great if you could take this on and make sure the product definition/needs for both 1.0 and beyond is clear. You might use my prev spec on the topic as foundation. Does that work for you?

@o-fernandez
Copy link
Contributor

I'm fine with that. Yes.
Our agreement from today, IIRC, is that we'll go with the approach of:

  • Revisions are not supported for MVP/1.0, we will consider them in the future.
  • Autosave will just overwrite the latest for any story in progress/drafts.
  • For published files, autosave will be disabled.

In the future we will do the UX work to support revisions properly and, once we do that, we can also consider the proper 2-file approach to autosave in which user can "restore/recover" from autosaved file in case of a crash, but otherwise autosave won't impact the revisions/saved drafts.

@miina let me know if this is what you also understand the approach we'll take to be. Happy to iterate on this if there is a better alternative.

@dvoytenko
Copy link
Contributor

@o-fernandez @miina I think before making a final decision on this we should take a look where the two-file approach is now. It seems to me that it must be pretty functional since we are using it for preview. And if so, it might be easier for us to use it right away then doing auto-save disabling/etc. E.g. we might be at a place where we can relatively easily enable auto-save into the second (draft) file and offer a user a chance to re-publish it.

@o-fernandez
Copy link
Contributor

@dvoytenko IIRC the main constrain at the moment is UX bandwidth, figuring out the right way to show users the second file and restoring from it, which is why I think we discussed going down the proposed path in my last comment.

If it would be much easier to do the 2-file approach, let's discuss with @samitron7 and @pbakaus and properly do overall prioritization and think through the timeline.

@dvoytenko
Copy link
Contributor

@o-fernandez It might be a significant UX effort, but I'm not yet convinced that it will be. And depending where things are technology wise, you may be able to make the necessary UX calls yourself and we can refine them with UX later. For instance, we might end up in the following situation:

Currently we do:

  1. For a draft document we show buttons: "Save draft", "Preview", and "Publish".
  2. For a published document we show buttons: "Switch to draft", "Preview", and "Update".
  3. There's also a version where we have "Schedule" instead of "Publish".

It's possible that we may just need to change this like this:

  1. For a draft document: no changes
  2. For a published document with no changes: "View" and "Unpublish".
  3. For a published document with changes: "Save new draft", "Preview new draft", "Republish", "Unpublish".

This is too many buttons in the last one. We could instead remove "Unpublish" and stick it somewhere on the side panel or into a "...". I'm not sure how commonly people would want it.

We can definitely punt on this, but for posterity, I'd definitely want to understand:

  • From PM point of view: is this a good flow?
  • From technical point of view: how far are we actually from being able to do this?

@miina miina mentioned this issue Apr 28, 2020
2 tasks
@miina
Copy link
Contributor

miina commented Apr 28, 2020

@dvoytenko Here is the current PR for auto-saving which is addressing this issue: #1361

With this PR.
Current state for Draft:

  • auto-saves in the determined time interval (based on the AUTOSAVE_INTERVAL value).
  • Overwrites the original post (as in WordPress)
  • Disables the update button if there's nothing to save

Current state for Published post:

  • Creates an auto-save (or updates it!) when generating a Preview
  • Never overwrites the original post, keeps a separate special auto-save post (as in WordPress)
  • Has the logic for updating the auto-save as in case of a Draft, however, that's now commented out in the PR due to the potential changes post-sync yesterday

Things that need doing for enabling restoring an auto-saved post in case a browser crash:

  • Detecting if the auto-save is newer than the last saved published story
  • UX in case that is so
  • Restoring the editor state from the auto-saved post when the user decides to do so.

If we decide to not implement auto-saving for a published post right now, the PR mentioned above should already be ready (other than adding some tests). We could easily add auto-saving for a published post separately once we have UX.

Note: all this is independent of revisions, these should be investigated and looked into in a separate issue.

@dvoytenko
Copy link
Contributor

@o-fernandez @miina IIUC, this sounds like covers all our needs for the flow I described above. The only thing that's missing here is one more action to updatePublishedStoryFromTheDraftFile. @miina could you describe how hard would it be to construct such an action?

@o-fernandez
Copy link
Contributor

For a published post, I think the key options that we need to give the user are:

  • Switch to draft
  • Preview
  • Update

The one thing I don't know how to add there is to have a way to "revert to published version" button. In essence, once you publish and then you make edits, the edits you make are autosaved to the 2nd file but not published. You need the option to either Publish them or Revert to the published version.

Does this make sense? May be worth to chat live about this and iron it out, if we want to go down this route. Otherwise, I think just not autosaving once someone has published, only option being to republish before they navigate away, it's an OK approach for now.

@dvoytenko
Copy link
Contributor

@o-fernandez It makes sense. But if we count, auto-save as an action, we have the following actions total for a published post:

  • Save/auto-save as a draft
  • Switch to draft (or "Unpublish")
  • View/Preview
  • Update (republish the latest changes from a draft to a published version)
  • Revert latest draft back to the publish version (or delete draft)

Some of these actions do not need to be displayed maybe. Some only need to be displayed when new changes have been saved. Some could be hidden inside a "...". Etc.

I think we just need:

  1. Confirm a set of actions to be certain.
  2. Confirm how much effort is to do all of these actions technically from the current state.
  3. See if we can arrange some "clear" UX that may need to be improved, but ok to start with.

@miina
Copy link
Contributor

miina commented Apr 29, 2020

The only thing that's missing here is one more action to updatePublishedStoryFromTheDraftFile. @miina could you describe how hard would it be to construct such an action?

We could make an API request to pull in the data from the latest auto-save, prepare it, and then use restore from History to restore that state. The action would become unavailable if the user saves meanwhile (or makes new changes perhaps). Doesn't seem very complicated.

@o-fernandez o-fernandez added the Status: Needs More Info Follow-up required in order to be actionable. label Apr 29, 2020
@dvoytenko dvoytenko assigned o-fernandez and unassigned miina Apr 29, 2020
@miina
Copy link
Contributor

miina commented Apr 29, 2020

Perhaps it would make sense to create a new issue for all the actions described above, it looks like the original issue (auto-saving for a draft) is clear for now and will be closed with #1361. Thoughts?

@dvoytenko
Copy link
Contributor

@miina I agree. @o-fernandez lets open the followup issue on the two-file approach.

@o-fernandez o-fernandez removed the Status: Needs More Info Follow-up required in order to be actionable. label Apr 29, 2020
@o-fernandez
Copy link
Contributor

Ok, I agree. Let's then keep this ticket to what it is (per the description) and we can have another ticket if things change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Critical, drop everything
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants