wordpress-mobile / WordPress-Android

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

Issues with Post with media created offline. #9570

Closed diegoreymendez closed 5 years ago

diegoreymendez commented 5 years ago

For a post with media created while offline, a “unable to upload this post’s media” error is displayed on the post list view. There’s no indication that the draft is saved locally or that the post itself also hasn’t been uploaded.

maxme commented 5 years ago

Current situation:

Post with media modified offline Post without media modified offline
Screenshot_1557499450 Screenshot_1557499476

My opinion: we should simply show "local changes" here (the same message with or without media). The auto-save action will try to upload media. And whatever the user decides (save a draft, publish, update), media upload will be retried anyway.

Question: when should we display unable to upload this post’s media? Only if there was a real network failure?

shiki commented 5 years ago

Subscribing @wordpress-mobile/ravenclaw

maxme commented 5 years ago

My opinion: we should simply show "local changes" here (the same message with or without media). The auto-save action will try to upload media. And whatever the user decides (save a draft, publish, update), media upload will be retried anyway.

Actually this might not be the best idea. The same situation happens when a media upload fail (network hiccup for example) and the user should also know about the post state. Maybe changing the message would be better: Unable to upload this post’s media - changes were saved locally.

When this ticket was opened:

diegoreymendez commented 5 years ago

Actually this might not be the best idea. The same situation happens when a media upload fail (network hiccup for example) and the user should also know about the post state. Maybe changing the message would be better: Unable to upload this post’s media - changes were saved locally.

Does the user care about whether it's the media or the post text that needs updating?

As a user, I think the only information I'd be interested in is to know that there are local changes that need to be uploaded.

maxme commented 5 years ago

As a user, I think the only information I'd be interested in is to know that there are local changes that need to be uploaded.

Yes, but there is a difference with a "simple" post with local changes and a post with media upload errors. We can't upload a post with local changes that have media upload errors. Media upload errors can have different causes, some of these error needs a user action (quota exceeded, wrong format, size limits, etc.).

osullivanchris commented 5 years ago

Spent a good bit of time thinking about this one and discussing related issues with @maxme and @malinajirka yesterday. My thoughts:

As I understand a scenario resulting in this state:

What we need

Issues:

  1. Current message tells user about upload error but not that it affects the post. I think this only needs a small copy tweak. The user would not expect the post state to have changed to published/saved. In the event they tried to publish/save they would have encountered an error, so we dont need to tell them the post has not published here.
  2. I don't like the way we replace typical actions with actions related to the error state. Its a bit confusing. I will write a p2 proposal on that.

Here is my proposal for now.

So showing an alert in both cases but with different users intents (resolve media issue / publish post)

Feedback welcome

IMG_20190627_141113

malinajirka commented 5 years ago

Thanks for looking into this @osullivanchris! ;)

I don't like the way we replace typical actions with actions related to the error state. Its a bit confusing. I will write a p2 proposal on that.

Hmm, interesting I was considering them as context actions (if you change the tab they may also change since the context is different). But I also see your point. 😕

Remove failed media actions

I'm wondering if the user will ever want to click on "Remove failed media" without checking the post -> checking what media have been removed. Wdyt?

osullivanchris commented 5 years ago

Thanks @malinajirka

Hmm, interesting I was considering them as context actions (if you change the tab they may also change since the context is different). But I also see your point. 😕

True perhaps I have the wrong mental model for these actions, its just a little different to what I would expect. In any case - its bigger than this task, so we can discuss whether that's something to do on paCBwp-aj-p2 and keep with what exists for now. I also found on 10106 that I ended up using a contextual approach! But in that case it was contextual to where the post lived, not contextual to a current error state

I'm wondering if the user will ever want to click on "Remove failed media" without checking the post -> checking what media have been removed. Wdyt?

Interesting. In that case would you try to push the user into the post to resolve the issue? I'm assuming its also possible to re-upload from the image in the editor.

Proposal Was worth me thinking this one through a little. But I don't think we're ready for a drastic change and its not necessary to solve this issue.

Lets keep the actions the way they are. Its consistent with other errors at least.

I think the only thing we should consider doing is change the copy to make it clearer. As we've discovered the only way the user will end up in this state is by hitting back/up. So

And once we get to that point, do we need to make a change at all?

malinajirka commented 5 years ago

Thanks @osullivanchris !

Interesting. In that case would you try to push the user into the post to resolve the issue? I'm assuming its also possible to re-upload from the image in the editor.

I think it'd make sense. When the user clicks on "Remove failed media" in the resolution dialog, we also show the post content before publishing. So the flow is

  1. User clicks on Publish
  2. Resolution dialog is shown
  3. User clicks on "Remove failed media"
  4. Failed media files are remove and the dialog is dismissed
  5. User clicks on Publish and the Post List is shown (the post is uploaded to the server)

I think the only thing we should consider doing is change the copy to make it clearer. As we've discovered the only way the user will end up in this state is by hitting back/up. So

I realized we missed two more important case.

  1. When the user adds an image and hits publish/save/update before the image upload finishes, we don't show the resolution dialog as the upload hasn't failed yet. We simply redirect the user back to the Post List and we show the error if the upload fails.

  2. When the user adds an image in offline and they hit publish/save/update, we don't show the resolution dialog -> they can see the error on the Post List screen.

maxme commented 5 years ago

Lets keep the actions the way they are. Its consistent with other errors at least.

Actions seem fine to me. The "Remove failed media" resolution dialog always felt wrong to me, I'm glad we added a "Retry" button in this resolution dialog via https://github.com/wordpress-mobile/WordPress-Android/pull/10119. I'd never blindly remove images from a post to publish it. I might do it via the editor to understand what's going on and to check my post looks good if I really have to remove something.

I think the only thing we should consider doing is change the copy to make it clearer. As we've discovered the only way the user will end up in this state is by hitting back/up. So

  • If the user hits 'publish' they get the resolution dialogue
  • If the user hits 'save' they get the resolution dialogue
  • Therefore they only end up in this state by hitting back/up. So there is not a need to communicate a lot else about the status of the post.

And once we get to that point, do we need to make a change at all?

Just to loop back to the original issue (also the last case @malinajirka described above): we show the error when they add a media while being offline.

Anyway, it seems fair to me to keep this behavior and show the error, since we already show that error on the media in the editor.

maxme commented 5 years ago

Note that this is related to the proposal from https://github.com/wordpress-mobile/WordPress-Android/issues/9555 - showing "Publishing when connection returns" status makes sense in the case of a post created offline even if there is a failed media upload.

osullivanchris commented 5 years ago

Thanks @malinajirka for highlighting the two issues we might have missed. I don't think they change anything really with our decision.

Note that this is related to the proposal from #9555 - showing "Publishing when connection returns" status makes sense in the case of a post created offline even if there is a failed media upload.

Seems like we're getting a win here too

Going to close this issue because the conversation has gotten quite convoluted and we ended up deciding against any major changes.