Move to top/bottom

[TomHennen]

This changes adds the menu options 'Move to top' and 'Move to bottom' to items in the Episode queue. This change enhances usability by making it easier to quickly change what you will (or won't) be listening to yet.

Let me know if any changes are needed.

[danieloeh]

I think i understand why this feature can be useful when organizing larger queues. Here are some things that i have noticed that should be changed:

• The moveQueueItemToTop/Bottom methods use a DBReader method at the beginning, which is executed on the same thread. This means that moveQueueItemToTop/Bottom method is accessing the DB on the GUI thread. Both methods should be exectuted on the internal ExecutorService instead. When doing this you also have to make sure not to wait for the completion of any other DBWriter methods (like moveQueueItem) in order to avoid deadlocks (because the Executor is a SingleThreadExecutorService). I would suggest moving the content of moveQueueItem's Runnable into a private helper method and use this one instead.
• The methods aren't using the broadcastUpdate argument. It should probably be used when calling moveQueueItem

Apart from that, i am not sure if the 'move to top' and 'move to bottom' buttons should be placed in the context menu of the queue item in the main screen. Since this action is moving an item in the queue, the 'organize queue' screen might be a better place for this action. But perhaps this would make the feature too difficult to find and use?

[TomHennen]

Ah, didn't realize there was that restriction (and I probably didn't fully understand what context everything was executing in anyways). I'll make those changes.

For button placement: my workflow is basically:

• Wake up
• Open AntennaPod
• Check on podcasts
• If there is a new podcast I want to listen to on the way to work
  • Move it to the top of the list

Putting the buttons on the context menu make that workflow faster (don't have to change modes).

Of course, I originally found the organize queue method a little difficult to find, so it would probably be hard for people to find the move top/bottom there as well. Perhaps there's something else that could be done to make it more discoverable/easier to access (in the future).

[TomHennen]

I've made the changes you suggested. Let me know if there's anything else.

[danieloeh]

I can see your point about the accessability of the "organize queue" screen. The only reason why i have created it in the first place was that there is no drag & drop for ExpandableListViews, so i had to create this extra screen with a normal ListView. In the long run, the "Organize Queue" screen should be removed.

Your changes work quite well, but there are two more things that should be changed in my opinion:

1. The context items "move to top" and "move to bottom" make sense when they are shown in the queue screen, but the user might not know what they mean when they are visible somehwere else (for example in the FeedItemlist). I think the context items should only be shown when selecting an item from the queue ListView (the one that is shown in the main Activity).
2. This one is kind of related to the first one: The "move to top" and "move to bottom" items are also visible for items that aren't in the queue at all. Clicking on one of the context items does nothing in this case. I think these two context items should not be visible for items that are not in the queue.

I'd suggest taking a look at the onCreateContextMenu-method in the EpisodesFragment in case you want to implement these changes. If you add the menu items in this method (instead of FeedItemMenuHandler) and check if the selected FeedItem is in the queue, it should work as described above.

[TomHennen]

Ah, you're right of course. I didn't realize it was occurring in those places.

I'll make the changes you suggest.

[TomHennen]

Ok, I've made the changes. It also doesn't show move to top/bottom if it's already at the top/bottom.

Not sure what you think generally about using helper functions or fixing spacing issues when they're encountered. I tried to keep my changes minimal.

Let me know if there's anything else.

[danieloeh]

Works perfectly! I have merged your changes into develop. There was a minor issue that i have fixed: In moveQueueItem you called moveQueueItem again instead of moveQueueItemHelper.

[TomHennen]

Ahck, that was a dumb mistake. Thanks for merging it and making such a cool
product.

Written on a tiny keyboard.
On Sep 12, 2013 7:54 AM, "danieloeh" notifications@github.com wrote:

Works perfectly! I have merged your changes into develop. There was a
minor issue that i have fixed: In moveQueueItem you called moveQueueItem
again instead of moveQueueItemHelper.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/258#issuecomment-24312503
.

[ortylp]

Could this new API be used for sorting Audio/Video to top/bottom? The functionality I have implemented (see #192) in my fork is no more applicable in 0.9.7.5.