wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

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

Closed diegoreymendez closed 4 years ago

diegoreymendez commented 5 years ago

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

maxme commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

diegoreymendez commented 5 years ago

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

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:

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

maxme commented 5 years ago

My opinion about the "Discard Local Changes" feature:

diegoreymendez commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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:

diegoreymendez commented 4 years ago

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

diegoreymendez commented 4 years ago

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

malinajirka commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Completely agree, sorry my bad.

maxme commented 4 years ago

Re-opening this since it was originally about the "discard changes" button in the overflow menu.

shiki commented 4 years ago

@charliescheer mentioned in p4a5px-2oP-p2 that a user reported accidentally tapping on this button. We should also add a confirmation before discarding the changes.

maxme commented 4 years ago

@iamthomasbishop - pinging you here, since we had a discussion about the "discard local changes" button in the overflow menu

yaelirub commented 4 years ago

Labeled "13" as we need design input

maxme commented 4 years ago

@osullivanchris we need some design input on that one.

I proposed to remove this option from the editor since we have a revision history and undo/redo buttons.

But if we decide to keep it, we should implement #9971 and define what this button does exactly, there are different options:

osullivanchris commented 4 years ago

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:

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 commented 4 years ago
  • 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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

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

yaelirub commented 4 years ago

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

maxme commented 4 years ago

@yaelirub I opened https://github.com/wordpress-mobile/WordPress-Android/issues/10102 and https://github.com/wordpress-mobile/WordPress-iOS/issues/11994

maxme commented 4 years ago

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