samuelclay / NewsBlur

NewsBlur is a personal news reader that brings people together to talk about the world. A new sound of an old instrument.
http://www.newsblur.com
MIT License
6.87k stars 999 forks source link

Android: Replace confirmation dialog with undobar when marking folder as read #584

Open manderson23 opened 10 years ago

manderson23 commented 10 years ago

Inspired by https://getsatisfaction.com/newsblur/topics/any-plans-for-undo-action-button-for-incorrectly-mark-all-as-read-on-a-feed-or-folder I thought it might be nice to replace the dialog with an undobar (like GMail/Pocket see https://plus.google.com/+RomanNurik/posts/RA9WEEGWYp6).

It wouldn't interrupt the flow of using the app as much and seems to be an Android UI pattern that is becoming more popular.

Any thoughts?

dosiecki commented 10 years ago

Does the API have an undo function, or would we need to just defer the call in-app for some amount of time? The new ReadingAction framework in the last patch would make the latter not too terrible to do.

samuelclay commented 10 years ago

Just defer the call client-side for 10-15 sec.

manderson23 commented 10 years ago

I haven't investigated this much yet so don't know the detail of the implementation. Wanted to sound you out first.

The only potential problem I could think of was delaying the call would the cause the feed to still appear unread when returning to the main feed list. The list would then refresh again when the action was applied? Or could it be implemented so that the feed would disappear when returning to the main feed list but reappear as unread if undo was pressed?

dosiecki commented 10 years ago

As of last week, the actual API call is decoupled from the local UI updates and is deferred until the user is online. We could just as easily add a forced delay of 10s on top of that. The UI to perform the undo would just need to have the ID of the ReadingAction passed to it so that if the undo is performed, the DB row is cleared.

The big part of the feature would be implementing the local DB code to perform mark-read actions in reverse and increment feed counts. The new framework makes this easier, but not trivial.

manderson23 commented 10 years ago

OK. I'll maybe investigate in more detail to get a better idea of what is involved.

dosiecki commented 10 years ago

As a side note, this will be much easier to implement when we finish moving all logic for reading actions into the new ReadingAction class. The goal is for each possible user activity to abstract out:

It would be sensible enough to add another abstraction for

All that said, I have seen some apps that use the undo abstraction simply commit the pending local action when the current Activity is exited. (this way the UI is never updated in the first place if the action is undone and if the Activity is exited, there is no way for the undo to happen anyhow.) Thus making it sensible to just defer the local portion until the activity ends and the remote portion until the timeout.

manderson23 commented 9 years ago

Seems that in 5.0/material design Snackbars (http://www.google.com/design/spec/components/snackbars-toasts.html) are now an alternative to undobars. I've noticed that the latest GMail app uses a Snackbar instead.

samuelclay commented 9 years ago

Snackbar with undo looks great. On iOS it would be easy to wrap up the functionality in a closure/block, send it over to a delay and let the snackbar do its thing. I'm not sure how drop in simple it would be on Android, but a delay for a call would be an easy way to get around having server code to handle undos.

dosiecki commented 9 years ago

Two thoughts:

I think either the snackbar or the undotoast look fine, so long as we keep them nice and clean within the existing NewsBlur styles. One caveat is that we will have to make sure we don't double or triple the size of our install by accidentally pulling in too many compat libs.

Also, while deferring and subsequently cancelling the API call is doable (this is exactly why I implemented the ReadingAction class a few months back), the trickier part is undoing the local UI/state changes. For example: the action for marking a feed read only contains the feed ID. The user marks the feed read, and they then see the feed count for that feed go down, and are presented with the undo button. If they press the button, the API call is cancelled. However, now we have to also locally undo the action by un-marking the affeted stories and restoring the counts. This (the affected surface of the dataset) is not yet stored anywhere and the original call parameters (feedID) are insufficient to undo all effects of the call. I think this latter half will be the tricky part of the implementation.

manderson23 commented 9 years ago

Initial research gives two options for implementing SnackBar

https://github.com/nispok/snackbar and https://github.com/MrEngineer13/SnackBar

Looking at both the gradle files the first requires a min SDK of 15 and support-appcompat-v7 while the second is min SDK 8 and doesn't seem to have any support dependencies.

Give the above and your comment should I take a look at the second option?

dosiecki commented 9 years ago

Looking at their code, both impls are essentially a single classfile with a supporting layout resource. Depending on the liscense, is there any reason we can't just CASE and tweak one into our own little lib? Seems weird to pull in a whole external dep just to draw a rectangle with some text?

manderson23 commented 9 years ago

The first is MIT license and the second in Apache 2.0 so no issues there.

Is it not more maintainable going forward to just include it as a dependency? e.g. we want to upgrade to the next version but now there are X classes to copy over? The whole point of a build system like gradle is to take care of all this for you. No?

samuelclay commented 9 years ago

Both look good in terms of design, so pick whichever is easier to work with.

dosiecki commented 9 years ago

Is it not more maintainable going forward to just include it as a dependency? e.g. we want to upgrade to the next version but now there are X classes to copy over?

If you want to build a jar and version that in /libs/ (like we do with gson), I would be fine. I wouldn't recommend using the Android project-to-project build dependency system, though. It tends to add a ton of overhead to build times. (we used to depend on ABS from source and it took several times as long to build a release than it does today)

manderson23 commented 9 years ago

OK. My initial plan is to build a jar file, add the snackbar in and see how everyone feels about the look.

No point embarking on the backend changes if we aren't happy with the look etc.

manderson23 commented 9 years ago

FYI I still think this would be a nice improvement but had held off thinking about looking into it while the whole SQL vs Realm debate was ongoing.

If we are still considering Realm (even in 6 month + timeframe) it is questionable whether it would be worth investing any time in the back end changes required to support undo. Any refactoring/redesign for Realm could perhaps be performed with consideration to supporting undo.

Thoughts?

dosiecki commented 9 years ago

I think this is roughly do-able regarless of DB solution, now. Either way, we will have to build the same components:

1) a delay system for remote actions 2) a journal-ish system for local actions so they can be un-done 3) the UI

The most difficult part is the second. It is insufficient to say "undo mark-read folder Foo" to the DB, because the DB doesn't know what stories were unread before the mark. Rather, we have to temporarily store all the hashes that were marked by the action, so they can be locally un-done when the remote action is cancelled from the delay queue. Alternatively, we could make the 'read' status of stories a tombstone-like ternary flag, rather than a binary, and require a confirmation write in order to flip them permanantly.

Part 1 is mostly done, as the offline system was built with delayed actions in mind. The UI is as much work as we want to make it.

All told, it is a lot of moving parts. I would personally triage anything that increases complexity of the app this much a low-to-lowest priority, at least until we are caught up on bugs. Do we have feedback from users that the confirmation dialogue is causing problems?

manderson23 commented 9 years ago

As with most of the work I have contributed this is pretty much something I'd like to see. I feel that the confirmation dialog just interrupts the flow when using the app. When I press to mark a folder read I'm pretty sure I want to mark it read. No other action to mark multiple items read (e.g. on a feed) has a confirmation though obviously the folder action is going to effect most stories (apart from perhaps older/newer on a folder). It is also quite jarring UI compared to other apps where the undo action is available.

Undo could obviously be applied to any action that marks multiple items as read and gives a way to back out any accidental mark read actions.

dosiecki commented 9 years ago

I noticed that undo has received a bit of attention on getsatisfaction lately and had a thought today:

Implementing this on Android would be much easier if we made one small concession. If the localy-visible rollback of the mark-read was delayed until the next feed metadata refresh, we could eliminate most of the nasty bookkeeping that makes this complex. We could even trigger the refresh in a lightweight manner so the delay would be small, provided the user is online.

I personally think this would be an acceptable compromise given that the downside of accidentally marking a feed read causes huge data loss and that the confirmation dialogue on folders causes some grief.

I think I could bang this out in a few hours.

manderson23 commented 9 years ago

Sounds good.

samuelclay commented 9 years ago

I disagree that marking a feed as read is data loss. The workflow of a reader is to mark stories as read. And if you accidentally mark a feed as read, you can go back and read it.

I'm not convinced that we should build this and I think this is going to cause issues. If you mark as read on Android and then check the web, it's still unread on the web. That's not a good solution. This needs to be on the backend and I just don't see it happening.

dosiecki commented 9 years ago

If you mark as read on Android and then check the web, it's still unread on the web

I was envisioning the timeout for this being something like 5-10 seconds, tops. More quickly than one can generally get over to a browser window.

I disagree that marking a feed as read is data loss.

Might not be the right term. It is definitely a daily frustration for me that I will accidentally mark all read in a feed where I had been carefully keeping track of my reading progress in a chronological manner. A quick "oh, crap" button would relieve me from then having to go back into the feed and scroll around to figure out where I was in the list to un-mark it or quickly read all the newer stories. I will admit that I have very unsteady fingers on my phone, though.

samuelclay commented 9 years ago

This is why I want to ship #177. We have this on iOS and it's great. If i do accidentally mark a feed as read, I can go right back into it. I definitely want to ship #177 before this.

manderson23 commented 9 years ago

My original reason for filing this was nothing to do with accidentally marking as read but was more about getting rid of the mark folder as read confirmation and replacing it with nicer UI.

Being able to undo bulk unreads would be a further benefit. I don't think I've ever done it for a folder/feed but there are a few times where I would have found the undo option useful for mark older/newer read operations.

manderson23 commented 9 years ago

Another case where undo would be useful https://getsatisfaction.com/newsblur/topics/android-move-mark-stories-as-read-away-from-save-story-in-menu

samuelclay commented 9 years ago

For that I'd prefer to move the Save Story button. Can we move that to below "Send To" and above "Mark as unread"?