Delete messages by swiping and add snackbar for undoing deletion. #99
Conversation
When swiping away a message it is only removed from the local message lists and a snackbar appears. When the snackbar is dismissed or the user deletes another message the delete request is sent to the server. If the user presses "undo" on the snackbar the message is reinserted into the local lists at its previous position.
leopoldsedev
requested review from
jmattheis and
eternal-flame-AD
and removed request for
eternal-flame-AD
There was a problem hiding this comment.
Looks pretty good! I don't really know how useful the undo deletion is. I see messages as events short lived events with a small lifetime, but I guess having this feature doesn't hurt.
I've found one bug and have suggested some minor improvements. Feel free to argue against them if they are stupid (:.
The commits are great, nicely grouped!
app/src/main/java/com/github/gotify/messages/MessagesActivity.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/github/gotify/messages/provider/MessageFacade.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java
Outdated
Show resolved
Hide resolved
Is there anybody outside using (working with) this programm or just developer who want to have fun? ;-) Of course the "delete button" should stay! Tapping 5 times on the "delete button" deletes the last 5 messages. It takes 1.5 Seconds (for all 5 messages). Deleting 5 Messages with swiping: It takes 15 seconds. 10 times more time! I agree, swiping is cooler, more modern and "latest hot stuff", but slower and more complicated!! Let me explain: You should be able to configure it from my side: Slow swiping or fast deleting with trashcan icon. ;-) Thanks. |
|
Good point @pm-pm, I agree. Regarding if this is a pet project: There isn't much feedback on the usage of the app and different people have different opinions on how things should work. #38 has 5 upvotes which is a lot for this repo. But yeah, removing functionality shouldn't be done without any notice. Maybe changes like this could be proposed inside our matrix chatroom for discussion. Tho, I think that mostly developers use the app because of its nature and feature set :D. |
|
Hello jmattheis, thank you for your understanding. :-) Please let me tell you something: I am also family admin and there are some older people who have problems understanding changes in the software. You and I understand a change. You may like the changes or not, but we both can handle it. Other people have a lot of problems with that. Think of your parents: Imagine the operation of a microwave oven would change every 4 weeks or in a car the buttons would be moved every 2 months. grrr You can only understand this if you have really worked with older people or people who are not technically minded on computers. That is not easy. Excuse me for this explanation, but everyone should think about that when designing software for other people. I like innovations and software has to be developed further, but you have to think about all users ;-) Make it configurable. :-) Many greetings. :-) :-) |
This seems to cause an issue where onSwipe is not being called anymore afterwards. Steps to reproduce: 1. Delete a message 2. Scroll a little bit down 3. Undo delete 4. Try delete again -> Swiping works, but callback is not being called.
Previously the MessageFacade and MessageStateHolder both had their own state of the last deleted message, which was redundant. Now only MessageStateHolder governs the state of a pending deletion and MessageFacade handles commiting the deletion to the server.
This reverts commit b0e8562.
|
Since a settings dialog would be out of scope here I simply readded the delete button for now. |
app/src/main/java/com/github/gotify/messages/provider/MessageStateHolder.java
Outdated
Show resolved
Hide resolved
Firstly the ListView is replaced by the more modern RecyclerView. The swiping functionality is implemented and consequently the delete button removed. Lastly, a snackbar is implemented for undoing the last deletion. To achieve that I had to extend the MessageFacade and MessageStateHolder a bit. I hope I covered all cases.
The removal of the delete button is debatable, but I think the UI looks cleaner without it (especially since it is the same element for every list item).
Bonus question: Why are all of the server operations in MessagesActivity wrapped in an AsyncTask if they end up in a Retrofit
enqueuecall anyway? I thinkenqueuecan be called from the UI thread.