Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Labels indicating feeds and items "read"/"unread"/"in progress" counts and states #46

Merged
merged 12 commits into from
Nov 16, 2012

Conversation

patheticpat
Copy link
Contributor

The changes in this branch all revolve about the concept of episodes that were started listening to but aren't finished yet.

  • The feed list shows the count of "in progress" episodes in addition to the count of the "new" episodes, both with a colored label so they can easily be distinguished
  • The feed item list shows a colored label indicating the "read"/"unread"/"in progress" state. This makes it a lot easier to find "this one episode I think I started listening to but I can't really remember which one it was"
  • Episodes that are currently "in progress" can be marked either "new" (and will consequently start from the beginning the next time they are played) or "read"

@danieloeh
Copy link
Member

Great idea! And i really like the implementation of it, too. There are some small issues that i noticed:

  1. If you mark an "in progress"-item as read, you change its media-position, but it's not actually saved in the database because the "markItemRead" methods in FeedManager only save the feeditem and not its media (so if you restart the app, the item will still be marked as "in progress"). But that can be easily fixed, i could just add a method to save both objects.
  2. In the setRead method of feeditem you change the playback completion date, which is problematic because this item would then be displayed in the playback history (after restarting the app) and the user probably wouldn't expect that. The playback completion date should only change after the player has reached the end of the episode. Would the feature you implemented still work if you removed the calls to setPlaybackCompletionDate in setRead?

Thank you very much for implementing this feature!

@patheticpat
Copy link
Contributor Author

  1. Oh, you're right, I will look into this. I also wondered if it would be necessary to reset the media-position in the markAllItemsRead method. But since it only acts on unread episodes, this shouldn't be needed, right?
  2. Again you're right, I didn't think of that. It should be no problem at all to leave the playback completion date unchanged.

This was referenced Oct 27, 2012
Don't change the playback completion date when marking an episode as
"read" or "unread"
The unreadItems list contains items that are "in progress", so we need
to save the media.
@danieloeh
Copy link
Member

Great, it looks like saving seems to work now. I found two more aspects which might be worth thinking about.

  1. There currently seem to be 4 different states which a feeditem can be in:
    • unread and not in progress ('new')
    • unread and in progress
    • read and in progress
    • read and not in progress

In my opinion the 'unread and in progress' state is not really very useful, because an item can only be considered 'unread' if you have not listened to it yet or haven't started listening. The user also can't distinguish between the 'read and in progress' and the 'unread and in progress' state which can be a bit confusing. So i think the 'unread and in progress' state should be 'removed' by marking an item as 'read' as soon as you start listening to it.
And as a consequence, markAllItemsRead will not change the position of an item anymore because it will only change unread items which are not in progress.

  1. The user shouldn't be able to mark an item as 'read' or 'unread' if he is currently listening to the item's episode, because setting the position to 0 wouldn't actually do anything in that case (because the position will be overwritten later by the playback service and the item will switch back to the 'in progress' state). Instead, the currently playing episode should have different controls and probably have a differently colored indicator than the other items.

@danieloeh
Copy link
Member

Update: I have added some methods to check if an item is currently being played in commit 0762e7b.

@patheticpat
Copy link
Contributor Author

  1. In my opinion there are 3 states that make sense: New, in progress and old. The 4th state is introduced only because of the current implementation looking at the read flag and the media position. Maybe it would be cleaner to hide this inside a method like this:
public enum State { NEW, IN_PROGRESS, READ }

public State getState() {
    if (media != null && media.isInProgress())
        return State.IN_PROGRESS;
    else
        return (read ? State.READ : State.NEW);
}

What I am undecided about is the following: Should the new items pane contain episodes that are in progress (I think I would prefer this) or not?

  1. Absolutley right, those operations don't make sense on an item that is currently playing. I'll see if I can think of a nice color

Conflicts:
	src/de/danoeh/antennapod/feed/FeedItem.java
@danieloeh
Copy link
Member

I agree. And maybe there should also be a PLAYING state in the 'State' enum (I have added a method called isPlaying() to the FeedItem class which makes it easy to check for that).
The new items list currently contains all items which are unread, so it also contains items which are in progress. I use this list like this: If i want to listen to one of the new episodes, i usually add it to the queue and then mark it as read. Then i use 'mark all as read' button to remove all the other items from the list which i don't want to listen to.

But on the other hand it also makes sense to leave an item in the new list when a user has started listening to it, because he might not have added it to the queue yet and then he won't find it again later where he would expect to find it.
But here is one thing i am concerned about: If this user has started listening to the episode in the new list (so that it is in progress) and then maybe decides to add it to the queue and marks it as read to remove it from the new list, the position is reset to zero and in my opinion this behavior is unexpected in this case.

I think there are two solutions for that problem:

  1. make setRead() not change the media position and introduce a new button ('mark as unlistened') instead that does exactly that (this method should only reset the media position and not set a playback completion date).
  2. Make the new list contain only new items and remove it from the new list when the user starts listening to it and maybe find another way to keep track of in progress items (in my opinion that is what the queue was made for). I would prefer this solution because it makes sure that a feeditem can only be in one of those 3(or 4 if you add PLAYING) states.

Which solution do you think is better?

@patheticpat
Copy link
Contributor Author

I had a look at your isPlaying() method and noticed one little flaw: It doesn't check if there is any media playing at all, so it will also return true if the item is the last played media but finished playing already.

About the use of the new items list and the queue I am still not sure. Before I discovered AntennaPod I used DoggCatcher. The way it works there is the following:

  • There's your list of feeds, showing the number of new and in progress items and in each feed's episode view, the items are marked as new, in progress or old, very much like in AntennaPod.
  • Then there is a list of downloaded episodes. When you download an item, it is added to this list and stays there until you finished listening to it or the media gets deleted (either manually or automatically). Basically this is the same as AntennaPod's queue if you enable automatic queuing of downloaded items.

The things I am not sure about are:

  • Does it make sense to download an item and not add it to the queue?
  • Is the new items list really useful enough to deserve this prominent position in the ui? You can easily find your new episodes in the feeds list or (after they are downloaded) in the queue. When I was still using DoggCatcher, I never wished there was a compact list of new items. But other users may disagree of course.

@danieloeh
Copy link
Member

You are right, i didn't think of that case. I added a separate sharedPreference that will set the id of the currently playing media when the user has started playback and that will be reset if playback has finished. So the isPlaying() method should now work as expected (but an item will still be in playing state if it is currently paused because in that case 'mark as (un)read' would also have the same unexpected effects.)

  • I also think that downloading an episode without adding it to the queue makes much sense. That is why i have the auto-queue option enabled on my device and i might make this the default behavior in version 1.0 and remove that preference but i am not sure if i will do that.
  • Yes, i think there should be a list like this and it should be on the main screen. The main advantage of having such a list is not necessarily that you can easily see if there are new items (you can also see this in the feedlist) but you can decide what to do with your items in one place. For example, if you want to add all new items to the queue you can do it there. I just think that having a list of all items which are in my opinion the most interesting ones is a lot more convenient than having to navigate into various feeds to find these items.

But of course that depends on the users' preferences and i might reorganize the main screen in the future but at the moment i don't know how this new main screen should look like.

@dander
Copy link

dander commented Nov 4, 2012

Hi, I recently discovered AntennaPod while searching for a replacement to Google Listen, and I really like it so far. I'd like to contribute to the project, though I have very limited in time lately. At any rate, I had a comment regarding a question in this thread.

Should the new items pane contain episodes that are in progress (I think I would prefer this) or not?
...
But on the other hand it also makes sense to leave an item in the new list when a user has started listening to it, because he might not have added it to the queue yet and then he won't find it again later where he would expect to find it.

The automatic queuing feature was the first thing I enabled, so my opinion is also biased towards being a heavy user. That being said, I would suggest that starting an episode should remove it from the new list, and add it to the top of the queue. Once I'm done with it, I generally want to go to the next item on my list anyway, so it seems natural to me.

@danieloeh
Copy link
Member

I like the idea of showing the the currently playing episode in the queue, but what should happen if the user first starts an episode (which would then be added to the queue), but then switches to another episode before actually finishing it? I think it would then make sense to not show this episode in the queue anymore (unless you have added it directly) because the user might not want to listen to it anymore.

@patheticpat I think it would be great if your pull request was in the upcoming release (0.9.6). Are you working on any more commits on this branch or can i merge it now?

@dander
Copy link

dander commented Nov 14, 2012

I agree with that. I would even extend it to the general case of removing an episode from the queue even if it was manually added. I can think of two cases where this could be a little problematic.

  • accidentally switching to another episode before being finished
  • some people may want to switch episodes mid-way through and come back later

Conceptually, I think the difference is between saying "I'm done with this", and "I don't want to listen to this now, but I want to come back to it later". I don't know how this would be best represented, but I think the "I'm done with this" action seems to be missing. In other applications I would use the next-track button.

@patheticpat
Copy link
Contributor Author

@danieloeh I am not working on this branch right now, so if you want to merge it, that's fine by me.

@danieloeh danieloeh merged commit b154865 into AntennaPod:develop Nov 16, 2012
@danieloeh
Copy link
Member

OK, i have now merged the changes in this branch so they are going to be in the next release.

I think the queue definitely has to be improved in the near future. In my opinion this list is supposed to show all items the user wants to listen to in the future or is currently listening to (i.e. 'in progress' items), but the queue currently only shows items the user is planning to listen to. So there should be an additional list section that shows these 'in progress' items so that the user can switch to another episode before finishing it and come back later very easily.

Also, the 'mark as read' button is pretty much the "i'm done with this" button but it doesn't always behave as you would expect it (for example it would make sense to remove an item from the queue if you are done with it). That is because this button wasn't made for this purpose but for marking a 'new' item as read and not actually an 'in progress' item. So maybe there should be a new button for marking something as listened?

Please post any ideas you have on this topic into a new issue instead of posting it here, since this pull request is closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants