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

Remove 'Discard Local Changes' from the overflow menu #9561

Closed
diegoreymendez opened this issue Apr 9, 2019 · 33 comments · Fixed by #10127
Closed

Remove 'Discard Local Changes' from the overflow menu #9561

diegoreymendez opened this issue Apr 9, 2019 · 33 comments · Fixed by #10127

Comments

@diegoreymendez
Copy link

diegoreymendez commented Apr 9, 2019

Make a change to a post -> Click on the overflow menu -> Discard local changes -> blocking dialog “A connection is required to …..”

@maxme
Copy link
Contributor

maxme commented Apr 30, 2019

Not sure what's the best solution here. Currently we can't discard local changes without fetching the latest version from the server.

Solutions:

  1. We could keep and save in the DB a second version of a post when the user starts to edit it. When discarding local changes we could just revert to that one. Problem: it could be different from the version on the server. IMO, This solution also will make the EditPostActivity even more complex for an uncommon problem.
  2. We could keep the feature as it, eventually replace the dialog by a Snackbar.

@diegoreymendez thoughts?

@diegoreymendez
Copy link
Author

diegoreymendez commented Apr 30, 2019

When I think about discarding changes (within the editor window) I'm definitely thinking about the current editing session - not about the entire local post. The reason for this thinking is that I may not be completely aware of all the local changes I made since the post was last saved to the servers.

This makes the discussion a bit trickier though because the next logical question is "How do you discard all local changes?" (not just the changes in the current editing session).

I've been thinking about this for a while. The problem we have is that this depends a bit on the synching strategy that we use for posts. To clarify what I mean:

  1. If posts are NOT automatically saved when you go back online: we need to have a mechanism to discard ALL local changes and go back to the version of the post that's in the servers. This is kind of where we are right now, although I don't think we have such a mechanism.

  2. If posts are automatically saved when you go online: if you wanted to go back to an older post version (to discard changes like you would in option 1) then you can simply pick a different revision instead.

@diegoreymendez
Copy link
Author

Also, to make it more complicated… what happens if the app crashes when you’re editing a post?

Currently iOS saves those changes locally. The post will have local changes but it wouldn’t be right to save those automagically (or would it)?

@maxme
Copy link
Contributor

maxme commented May 10, 2019

2. If posts are automatically saved when you go online: if you wanted to go back to an older post version (to discard changes like you would in option 1) then you can simply pick a different revision instead.

We tend to this solution (save automatically when online or when back online). Picking a different revision is probably the best solution, simply because using the Revision History, users can actually look at differences and know what they're discarding / picking.

The problem remains in offline mode, the Revision History requires a network connection (and I don't like the idea of fetching and caching everything from the post list).

@shiki shiki added the [Status] Needs Discussion The issue needs to be discussed by the team before it can be resolved. label May 15, 2019
@shiki
Copy link
Member

shiki commented May 15, 2019

Marked as Needs Discussion since there was some confusion on how this should be implemented. Or should this even be implemented?

@yaelirub yaelirub added the Design Needed A design solution is needed. label May 15, 2019
@diegoreymendez
Copy link
Author

I think this one is rather important. We want to figure out:

  • Whether we want the user to be able to discard changes, and whether discarding changes:
    • Discards the last editing session, or
    • Discards all changes since the last sync.

On top of this, we can have local changes as a result of the App crashing. We need to figure out if we need to be able to distinguish between:

  • Posts with local changes that were saved / published.
  • Posts with local changes due to an App crash.

I think this needs to come out of a UX discussion even before we discuss the technical aspects of it.

@maxme
Copy link
Contributor

maxme commented May 16, 2019

My opinion about the "Discard Local Changes" feature:

  • We don't need this feature at all (never seen this in any other editor, feels unfamiliar). As a user, I'm not sure if it's going to discard the current editing session or pull the version from the server.
  • We need a decent undo/redo to edit recent changes in the editor (it's OK to only have undo/redo for the current editing session).
  • We need to let the user know about the revision history if they need to restore an "old" version.

@diegoreymendez
Copy link
Author

Assuming we agreed on not allowing the user to discard changes (I think this may work)...

Let's evaluate for a second the tools the user would still have to roll back their changes:

  1. Undo / redo
  2. Pick a different revision of the post.

The first one is pretty straightforward and would only work for the current editing session.

The second one is a bit trickier because while it works well with posts that have been saved while online, we currently don't store revisions for posts saved while offline.

As a user, I think it would be awesome if we could make local revisions and remote revisions work similarly - whenever I tap "Save" it should be meaningful (it's tricky I know) whether I'm online or offline. It's a checkpoint that I'd want to be stored.

I think I'd be fine with closing this issue, if we can agree to at least discuss the possibility of having local revisions.

@diegoreymendez
Copy link
Author

Also, if we agree on removing “Discard changes” entirely, we need to define what happens when you make edits to a post and close it.

@malinajirka
Copy link
Contributor

We don't need this feature at all (never seen this in any other editor, feels unfamiliar). As a user, I'm not sure if it's going to discard the current editing session or pull the version from the server.

I agree. I also think answering the question "what are local changes" could be quite difficult when we release remote-auto-save which works in the background and it might have already overridden the remote-auto-save revision.

We need a decent undo/redo to edit recent changes in the editor (it's OK to only have undo/redo for the current editing session).

👍 We need to improve the current behavior of the Undo/Redo.

As a user, I think it would be awesome if we could make local revisions and remote revisions work similarly - whenever I tap "Save" it should be meaningful (it's tricky I know) whether I'm online or offline. It's a checkpoint that I'd want to be stored.

Makes sense to me. I agree that when you manually tap on Save you expect the post will be safely stored and if it creates a revision while online it should also create a revision while offline as the user doesn't need to be even aware they are not online. It can get a bit tricky as published posts don't have "save" button but "update" button, but I don't want to go too deep in the "local revisions" discussion on this ticket.

@diegoreymendez
Copy link
Author

diegoreymendez commented May 22, 2019

Subscribing @wordpress-mobile/ravenclaw

Even if we agree on all that's discussed above, I'm still wondering what should happen when we make edits to the post and go back to the post list without saving or publishing those changes. I think at the very least, we'd need to reach a conclusion on what should happen in that scenario.

Another way to put it is: If we don't offer the possibility to discard the changes, then how is pressing back different from pressing "Save".

Also: if the user creates a new post, writes something and taps back... since we won't be asking to discard changes should that post become a Draft automatically?

@diegoreymendez
Copy link
Author

diegoreymendez commented May 22, 2019

My personal take on my own questions above...

If we don't offer the possibility to discard the changes, then how is pressing back different from pressing "Save".

We should probably remove the option to just save changes. Saving would not longer be a separate option. This should go through design.

If the user creates a new post, writes something and taps back... since we won't be asking to discard changes should that post become a Draft automatically?

Since all changes would be automatically saved, the post would become a draft. The trickiest scenario is probably what happens to a published post in an XMLRPC site with changes when you tap back, but we can discuss this in a separate issue.

@maxme
Copy link
Contributor

maxme commented May 22, 2019

if the user creates a new post, writes something and taps back... since we won't be asking to discard changes should that post become a Draft automatically?

This is what happen in the Android app currently. We have 3 options for the back button and the back action:

  • discard changes (I'd hate that, too easy to lose your changes).
  • show a dialog and ask the user to either discard or save.
  • save changes and upload them (current Android app behavior).

@diegoreymendez
Copy link
Author

@maxme - Thanks for the clarification, I keep forgetting this is specifically for Android. We can close this issue.

@diegoreymendez
Copy link
Author

As a result of this discussion I opened an issue in iOS to match this behavior.

@malinajirka
Copy link
Contributor

This is what happen in the Android app currently. We have 3 options for the back button and the back action:

  • discard changes (I'd hate that, too easy to lose your changes).
  • show a dialog and ask the user to either discard or save.
  • save changes and upload them (current Android app behavior).

I think we might want to replicate Calypso's behavior
-> show a dialog and tell the user they might lose their changes -> trigger remote-auto-save if they decide to leave. The remote-auto-save will try to save only title,content and excerpt. All changes can be lost if the remote-auto-save request fails.

@maxme
Copy link
Contributor

maxme commented May 23, 2019

I'm a bit confused about this ticket :). This was initially about the "Discard local changes" button in the overflow menu.

I think we agreed on the fact that with a good Undo/Redo and by introducing local revisions we could remove that button. If this is the case and we decide to follow that path, we should open a new "action" ticket to implement that and remove the "Discard local changes" button.

Then the discussion slipped on "discard changes" while leaving the editor which is another topic and was solved differently in iOS and Android.

@malinajirka
Copy link
Contributor

Completely agree, sorry my bad.

@osullivanchris
Copy link

This thread is super dense. Thanks @malinajirka @maxme @diegoreymendez for the excellent thought put into this.

I spent a long time thinking about the complexities but I actually think a copy issue is at the heart of the problem. First I'm gonna use an example to frame my suggestion:

  • On Monday I write a post and publish it, successfully uploading to server
  • On Tuesday I'm offline and make some edits
  • On Wednesday I'm offline and make more edits

Effectively, these are different objects. This diagram hopefully makes sense. Although there are 3 potential 'objects' (v1, v2, v3) if saved after each session, the user only ever sees the latest object. I am not saying we should show all three items in the UI. But if you picture them as 3 cards its a useful thought experiment.
IMG_1090

The 'discard local changes' button was probably designed with the Tuesday session in mind. The use case is, I want to forget about the changes I've made locally and use the server draft/published post.

Where it gets complicated is if we go to Wednesday's editing session. When we say 'discard local changes' it is very unclear whether it will discard the session changes or all local changes. I believe that 'Discard local changes' button was not designed with this use case in mind.

Proposal: change the copy of 'discard local changes' to mean 'revert to server version'. We can come up with better language for that. But I think it retains the value of the original action, without the confusion of nested offline versions.

Implications

  1. If the user does want to return from the Wednesday offline version to the Tuesday offline version it is tricky. There are two ways we can do that though
    (a) have some kind of undo action within any editing session, which would allow a user to edit back through all changes within the session. The result of (undo) x (number of changes) would return the user to the Tuesday version of the post.
    (b) Versioning. I agree with @diegoreymendez making offline and online revisions work similarly would be valuable. But I think we could make that a separate issue, and for now make local drafts a bit more simple in that there can only ever be one local draft.

  2. Autosaving and back issues
    Please see my sketch on related issue 11753
    Rather than an explicit 'save' action, I think what we are going towards is autosave as an expectation. Therefore, I would not have an explicit 'save' action. I would have an automatic save. Then inform the user what has happened with a toast, and the option to undo from the toast message if there was a mistake. The only exception I would make is moving from an unpublished state to a published state, where we have decided to show a confirmation dialogue to ensure no mistake with a big action.

Summarising my proposal

  1. Keep the 'Discard local changes' action but change the name to something that means 'kill all the local changes and give me the server version'
  2. Make online and offline revisions work more similar. It would be valuable for a user to be able to view drafts offline as this is an extension of writing offline which we wish to enable
  3. Rather than an explicit save action, move towards autosave with undo options
  4. Introduce 'undo' within the writing flow as well

However I believe 2-4 are separate larger issues. Point 1 would be my resolution for this current issue.

Aware this is very complex so let me know if I've missed something or if you disagree

@maxme
Copy link
Contributor

maxme commented Jun 19, 2019

  • On Monday I write a post and publish it, successfully uploading to server
  • On Tuesday I'm offline and make some edits
  • On Wednesday I'm offline and make more edits

It can be ever more complicated than that if you use another online device (or Calypso) to edit the published version of this post after Tuesday. Since the app is offline, it won't be able to get that change.

  1. Keep the 'Discard local changes' action but change the name to something that means 'kill all the local changes and give me the server version'

Just wanted to point something you maybe missed: in offline mode, we can't fetch the server version, so we're back at the original problem with the blocking dialog A connection is required to discard local changes. There is no guarantee the post wasn't modified from somewhere else so we can't just revert from a cached server version.

Point 1 would be my resolution for this current issue.

I'd go with that solution, change the name of that button and keep the blocking dialog for now in offline mode.

Having offline and online revisions seems like the best solution to me and it's also a good solution for other tickets.

@osullivanchris
Copy link

It can be ever more complicated than that if you use another online device (or Calypso) to edit the published version of this post after Tuesday. Since the app is offline, it won't be able to get that change.

and

Just wanted to point something you maybe missed: in offline mode, we can't fetch the server version, so we're back at the original problem with the blocking dialog A connection is required to discard local changes. There is no guarantee the post wasn't modified from somewhere else so we can't just revert from a cached server version.

Good points thank you. The solution I proposed is less elegant now that you've mentioned those issues. The blocking dialogue is not a great experience.

Knowing the info you have shared, when I say 'server version' I guess its more like, 'forget about changes I've made specifically on this device'. Would it be possible to discard what has been done locally, show whatever the last version retrieved from the server was? And of course fetch the latest update when returning online.

@maxme
Copy link
Contributor

maxme commented Jun 19, 2019

Would it be possible to discard what has been done locally, show whatever the last version retrieved from the server was?

Yes it's possible, this is the last point in this comment. It requires some big changes on the technical side to keep that server copy somewhere and manage it correctly.

And of course fetch the latest update when returning online.

You mean overwrite the local version (restored from an old server version) from the current server version? That seems a bit weird to me.

@osullivanchris
Copy link

Yes it's possible, this is the last point in this comment. It requires some big changes on the technical side to keep that server copy somewhere and manage it correctly.

Seems like this is a lot of work compared with the value we're getting

You mean overwrite the local version (restored from an old server version) from the current server version? That seems a bit weird to me.

No, sorry. I meant that when the user discards local and returns to their latest server version, that server version would then update when they reconnect.

Discussing further with @maxme and will revert with proposal

@osullivanchris
Copy link

Per paCBwp-9N-p2 @maxme and I are considering removing 'discard local changes' as an option due to the inconsistencies and complexities that it creates. Details to follow.

@osullivanchris
Copy link

We are going to remove 'Discard Local Changes' from the Android overflow menu.

  • Inconsistencies with iOS and web
  • Low usage
  • All solutions add more complexity than value added
  • Users accidentally tapping the button and losing work they've invested in per #9971

We will separately look to improve revision history to allow users to achieve this task if necessary.

@osullivanchris osullivanchris changed the title Can't Discard Local Changes while offline. Remove 'Discard Local Changes' from the overflow menu Jun 24, 2019
@osullivanchris osullivanchris removed the Design Needed A design solution is needed. label Jun 24, 2019
@yaelirub yaelirub removed the [Status] Needs Discussion The issue needs to be discussed by the team before it can be resolved. label Jun 24, 2019
@yaelirub
Copy link

@maxme, can we create small issues to implement this?
closing this issue

@maxme
Copy link
Contributor

maxme commented Jun 25, 2019

@maxme
Copy link
Contributor

maxme commented Jun 25, 2019

Note: I re-opened this issue since "Discard Local Changes" wasn't removed yet.

@maxme maxme reopened this Jun 25, 2019
@maxme maxme added 3 and removed 13 labels Jun 26, 2019
maxme added a commit that referenced this issue Jul 5, 2019
…card-local-changes

Fix #9561: Remove "Discard Local Changes" in the Editor and in PostPreviewActivity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants