Post change conflict resolution dialog

[mzorz]

This is a follow up to PR #8940

Comes from https://github.com/wordpress-mobile/WordPress-Android/issues/5984#issuecomment-453706747

After implementing #8940 as a fix to https://github.com/wordpress-mobile/WordPress-Android/issues/8932, posts with local changes are not updated with the remote, and there is no indication if the remote had changes as well.

Quoting @megsfulton here for completeness:

One approach we could take that would be minimally disruptive but still communicate that there's an issue to be resolved:

• When the post list is fetched, display a line on the post card indicating that there's a conflict. Similar to how "Local Changes" appears but with some sort of warning icon.
• When the user opens the post ask them how they would like to resolve the conflict. Until they open the post to take an action both conflicted states would continue to exist.

What this PR does is:

• [x] show a new status that tells the user if a Post has a conflict (needs design review, it only shows Conflicted for now).
• [x] when the user taps on a Post with a Conflicted status, it shows a dialog asking whether they would like to load and the Post from the remote, or edit the local copy. They can cancel and not take any action by dismissing the dialog (either tap back arrow key or tap on the screen outside the dialog area)

Corresponding FluxC PR https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/1077

Note: this needs copy review, the text and approach here does not intend to be final but to show what the functionality would feel like. Also, the new status's color and icon should probably be different than now? cc @megsfulton .

To test: CASE A: (show the new status on the Posts list)

1. Create a draft while I am online in the app. That'll create the draft in remote.
2. Go offline
3. Make changes to the post and go back. This will save the changes as local draft
4. Go to web or any other client and make changes to the post.
5. Publish the post.
6. on the app, you should be on the posts list. turn airplane mode off
7. pull to refresh.
8. you should see now the post has a "Conflicted" state.

Dialog shown here:

CASE B: (simple conflict resolution dialog - edit local)

1. follow steps 1 to 8 above.
2. now tap on the post
3. a new dialog should be shown, offering 2 options: EDIT LOCAL and LOAD REMOTE
4. if you tap EDIT LOCAL, the local version is loaded. Beware once you tap on it, you've made a choice, but the contents are not lost (these are added to the revision history).
5. if you tap LOAD REMOTE, an attempt to load the latest version from remote is made. You'll need to be online for this (this prerequisite is checked for, and otherwise a toast is shown). You'll see how it gets updated.

CASE C: (simple conflict resolution dialog - load remote)

1. follow steps 1 to 8 in case A above.
2. now tap on the post
3. a new dialog should be shown, offering 2 options: EDIT LOCAL and LOAD REMOTE
4. if you tap LOAD REMOTE, an attempt to load the latest version from remote is made. You'll need to be online for this (this prerequisite is checked for, and otherwise a toast is shown). You'll see how it gets updated.

Update release notes:

• [x] If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

[elibud]

Can we review the copy? As a user "Load from remote" doesn't make a lot of sense to me. Maybe loop in editorial?

[mzorz]

Can we review the copy? As a user "Load from remote" doesn't make a lot of sense to me. Maybe loop in editorial?

Yes @elibud, this is what I wrote in the PR description, got probably lost 😄 :

Note: this needs copy review, the text and approach here does not intend to be final but to show what the functionality would feel like. Also, the new status's color and icon should probably be different than now? cc @megsfulton .

I'd first wait for the functionality and UX to be agreed upon, then bring Editorial in if needed 👍

[iamthomasbishop]

The terminology seems a little confusing here. A couple suggestions, and we should get copy review as well:

• Conflicted label on post card: This is a little odd, what about making this the warning/red color, with a warning icon and changing the label to Conflict or Version Conflict?

• Dialog: I'd propose the following – Title: Version Conflict, Body: The remote version of this Post is different from the version saved on your device. Which version would you like to edit?, Actions: Back Edit Local, Edit Remote. I'm not sure if "local" and "remote" will make sense to everyone, but the messaging seems clearer overall, and the addition of an escape hatch seems appropriate.

// cc @megsfulton for any additions or rebuttals :)

[elibud]

Sorry about that, I should read carefully before commenting @mzorz 🤦‍♀️

[megsfulton]

For the status label I like what @iamthomasbishop suggested. Version conflict in red with the notice outline icon

For the dialog copy, I think we need to provide more information so the user can make an informed choice on which version to keep. Also we need to make it clear that only one version will continue to exist so the choice they're making which version to "keep."

Resolve version sync conflict This post has two versions that are in conflict. Select the version you would like to keep.

Local Synced on Jan. 14, 2019 at 4:53 PM

Cloud Synced on Jan. 15, 2019 at 10:33 AM

Keep Local Keep Cloud

Then depending on which version they keep, can we have the editor open with the version they chose?

[iamthomasbishop]

I love the recommendations on dialog copy, @megsfulton! I'm not sure how I feel about "cloud" but it's definitely more approachable than "remote".

[mzorz]

Thanks for the input @megsfulton ! This is how the status on the Post card looks so far:

Regarding the dialog, can we change the wording from: Synced on ... to Saved on... ? I think it better reflects what's going on when you see the Local and Cloud titles, as there hasn't been any sync yet (we're trying to figure out which copy to sync here, right?).

Also using the basic Dialog I cannot use bold in within the message - we can totally do that with a custom Dialog but implies a bit more of effort, so I wanted to run it through you first and see if making a custom Dialog is worth that effort:

How does that look to you? Also note I'm re-using an old function we had that uses the @ character for showing the time instead of using the word at, which is hard to translate to some languages when connecting two different values like we're doing in this case.

[mzorz]

I love the recommendations on dialog copy, @megsfulton! I'm not sure how I feel about "cloud" but it's definitely more approachable than "remote".

For self-hosted sites I think "cloud" could sound strange? Maybe "web" and "local" sounds more familiar to end users? just thinking aloud

[mzorz]

Then depending on which version they keep, can we have the editor open with the version they chose?

Re: this, if they choose to open the remote we still have to go fetch that version (we only know it has deferred but we don't have the actual copy to be able to edit it right away), so doing these also means we'd have to maybe show some progress, wait for it, and once we have it open in the editor, which I'm not really sure about for cases where connection may not be super good.

Could we leave the functionality as follows for now until we're in a better place to tackle, as a mini project maybe?

That'd be:

• tap keep local: save to remote (we can also open the editor as we're doing now)
• tap keep remote: fetch remote and update (as we're doing now). Leaving the user to tap on it again once the list item gets updated (as shown in the gif above).

Looking forward to hearing your thoughts @megsfulton @iamthomasbishop @oguzkocer

[megsfulton]

For self-hosted sites I think "cloud" could sound strange? Maybe "web" and "local" sounds more familiar to end users? just thinking aloud

Agree. I like the suggestion of using "web" instead.

https://user-images.githubusercontent.com/6597771/51284401-2ec10280-19ca-11e9-87d9-9ddfa8d9a4a0.png

The icon in this looks a little strange like it didn't resize right. Can we try using the filled version of the notice icon and see what happens?

And this is small but can we make the label Version conflict with a lowercase c.

Regarding the dialog, can we change the wording from: Synced on ... to Saved on... ? I

👍

Also using the basic Dialog I cannot use bold in within the message - we can totally do that with a custom Dialog but implies a bit more of effort, so I wanted to run it through you first and see if making a custom Dialog is worth that effort:

Totally fine re: bolding the text. Can we add a line break and get rid of the : between the version label and the time stamp? So it would read like:

Local Saved on...

Web Saved on...

Could we leave the functionality as follows for now until we're in a better place to tackle, as a mini project maybe?

My only thought on this is that maybe we make the behavior the same between the two actions? So that both actions do what's described above for "tap to keep remote."

What do you think @iamthomasbishop ?

[mzorz]

Thank you @megsfulton ! I think I addressed all of your comments:

• using web instead of cloud
• icon not resized right, I did a small adjustment in 8b3371837d4109add6b297fb3840c6c89199f1eb as I saw it was different from other gridicons in the app (please let me know how it looks like now.. I feel I'm blind! 😅 ) - if this doesn't work then we can try with the filled version as suggested 👍
• using Version conflict with lowercase c.
• changed wording from Synced to Saved
• added line break after Local and Web subtitles

My only thought on this is that maybe we make the behavior the same between the two actions? So that both actions do what's described above for "tap to keep remote."

Totally agreed on this. I think given the premise is "keep this or that", then both actions should save (either fetch the remote and save locally if they tap on Keep web or push the local copy to the remote if they tap on Keep local). Next time they tap, they open the resolved Post. Sounds good?

[mzorz]

My only thought on this is that maybe we make the behavior the same between the two actions? So that both actions do what's described above for "tap to keep remote."

Totally agreed on this. I think given the premise is "keep this or that", then both actions should save (either fetch the remote and save locally if they tap on Keep web or push the local copy to the remote if they tap on Keep local). Next time they tap, they open the resolved Post. Sounds good?

I've been thinking about it and I think it's best to open the Editor 😕 . There are some situations that the Editor handles well, for example we could be having broken media items in the Post (that is, media items that did not finish uploading, or have been cancelled). When such a thing happens, the Editor takes care of all the logic to avoid uploading broken Posts (that is, images for which the URL are local device paths). If we were to allow the user to "keep local copy" without opening the editor, we would need to replicate the logic to clean it up / tell the user about it (which has its own complexity).

At this point I think the easiest is to:

• a) open the Editor if the user wants to keep the local copy so the current flows cover it all, and
• b) if the user chooses to keep the remote copy, show a toast when the fetch starts to tell the user it should be ready after a few seconds, and a snackbar once it's been loaded so the user can press there to open the Post (or on the list item as currently).

Wdyt?

[mzorz]

b) if the user chooses to keep the remote copy, show a toast when the fetch starts to tell the user it should be ready after a few seconds, and a snackbar once it's been loaded so the user can press there to open the Post (or on the list item as currently).

I've put this together in 0516716804 to show how it looks. I think it closes the loop nicely 😄 , but let me know what you think @megsfulton :

[iamthomasbishop]

This is coming along really nicely, and I'll let Megs chime in with her feedback. I like the general flow, but I noticed a couple things:

• The snackbar that shows when a version is loaded – perhaps we could find clearer messaging. "Local (or Web) version applied." or similar?
• From there, I would expect the snackbar to show an Undo action instead of a recommendation to open, seems that'd be more natural. Once the conflict is cleared, the user is opening the post as usual

Other than that, I think we're on the right path. Thanks for the hard work thus far on this issue!

[megsfulton]

Agree with what you've proposed on the interaction behavior @mzorz.

From there, I would expect the snackbar to show an Undo action

+1

I started looking at the text I originally proposed so the snackbar message could make sense, then I got to thinking that we should be more upfront with telling the user that they're taking a destructive action. I don't think making this text change impacts the flow or interactions that we've discussed.

Could we update the dialog text to be:

Resolve sync conflict
This post has two versions that are in conflict. Select the version you would like to discard.

Local
Saved on...

Web
Saved on...

Discard Local / Discard Web 

For the snackbar: Local/Web version discarded UNDO

For the loading message: Updating Post

[megsfulton]

icon not resized right, I did a small adjustment in 8b33718 as I saw it was different from other gridicons in the app (please let me know how it looks like now.. I feel I'm blind! 😅 ) - if this doesn't work then we can try with the filled version as suggested

Can we try the filled version? I'm still seeing that the ! isn't centered in the circle. I'm not 100% certain changing the icon will fix that but it may make it less noticeable.

[mzorz]

• changed icon in 45470afaab

  

• changed copy in favor of https://github.com/wordpress-mobile/WordPress-Android/pull/8989#issuecomment-455316263 and also changed behavior to make conflict resolution discard local or web version instead of keeping one or the other, and also added snackbar UNDO behavior (73232fb387)

One thing though. When you choose to discard the web version, I need to send the local version up to the server and once that happens we also were already showing a Snackbar as part of saving a draft (essentially that's what's happening). I cannot override that other snackbar 😕 . So I tried just leaving that one. This effectively means there's no Web version discarded UNDO snackbar when you choose to discard the web version and keep the local one. That said however, I think this shouldn't be a problem as the user can still edit the Revisions History as usual and take it up from there.

This is the flow for Discard Web:

And this is the flow for Discard local with UNDO:

Wdyt @megsfulton @iamthomasbishop ?

[megsfulton]

Thanks for helping work through all the small changes on this @mzorz. It's looking really good!

It's a bummer there's no easy way to override that snackbar.

The only other solution I could think of to keep the behaviors consistent would be to artificially delay the post upload in order to display the undo snackbar. Something like Discarding web post UNDO and begin the upload a few seconds after that's displayed.

As much as I like always giving users an easy and quick way to undo mistakes on mobile, that approach might be kind of weird? So I'm also okay with keeping it the way it is handled in the above gifs.

[mzorz]

The only other solution I could think of to keep the behaviors consistent would be to artificially delay the post upload in order to display the undo snackbar. Something like Discarding web post UNDO and begin the upload a few seconds after that's displayed.

This seems to work nicely @megsfulton , here's how it looks now:

Let me know if it's good now, so I can ask for another pass in code review 👍 😄

[megsfulton]

That looks good!

One thing I'm noticing in the gifs is that discarding the web version shows the snackbar with the local message Local version discarded. Can that be updated to be Web version discarded instead?

After that, I think it's good to go!

[mzorz]

One thing I'm noticing in the gifs is that discarding the web version shows the snackbar with the local message Local version discarded.

Oh totally it was bad in the gifs and I fixed it before commenting, but then was struggling with another completely unrelated issue to make it run and capture the gifs again for the comment, and then I forgot to mention that when I finally made the comment :) Nice catch BTW 😅

Thanks for all your help in the review, @megsfulton !

@oguzkocer this should be ready for a second pass on the code. Thanks again!

[mzorz]

There you go, just for completeness:

[mzorz]

hi @oguzkocer thanks for the thorough review and the tips here and there! I think I've addressed all your comments now - after going ahead with each one I had to bring the nullifying statements back (the ones we got rid of / changed in 94948513da44049a59d03ae1bae53d06bf37e756, 9a38333dcd2edcc175351470cfc676ed5e8aaf73, and b3bcec01c91394f0aa39ac5255ca565f83bfdd3d) as they were before, did it in fd320bc3a0.

They were there for a purpose: coordinating the snackbars (in this sense this check here is of importance.), otherwise this was the output:

As opposed to what can be seen in https://github.com/wordpress-mobile/WordPress-Android/pull/8989#issuecomment-455626723

I understand it may not be as clear by looking into the code, I can introduce a new variable to make it explicit if not. Wdyt?

Other than that, this should be ready to go - let me know your thoughts 🙇

[oguzkocer]

@mzorz I took the liberty to merge origin/develop into this branch to resolve the conflict which resulted in an automatic merge. I'll test things once more and merge this in 🤞

[mzorz]

Thank you @oguzkocer! :+1: