Contextual menu for multi selecting episodes #5130

peakvalleytech · 2021-04-23T02:27:53Z

Here is a working build I did implementing your mockup: https://drive.google.com/file/d/1A0fMp7SemlpHHltod-x0MQsDRn9TZDwg/view

I'm not close to done, but I wanted to get your feedback on it before I proceed further.

What I added:

Integrated multi select on same existing episodes screen using contextual menu (https://developer.android.com/guide/topics/ui/menus.html#context-menu)
Moved speed dial to episodes screen
Ported existing multi select actions to existing screen
Have NOT added new multi select actions

Lint fixes

ByteHamster · 2021-04-23T06:16:01Z

Oh, I thought you would do the subscriptions screen, not the episodes screen :) I have not tried it yet but I skimmed the code. Some thoughts:

I think it would be good to still show the context menu on long press. Only when clicking a new "select" entry there (or, alternatively, when clicking it in the 3-dot-menu), then enter the ActionMode. Reason is that it would otherwise make the more common action (changing a single episode) need a lot more clicks.
When having the ActionMode, I don't think we need the FAB anymore. The actions can directly be added to the ActionMode.

peakvalleytech · 2021-04-23T08:26:54Z

idk, it seems odd to have it has an option in the context menu. How about adding a 3-dot like button next to the play button. That is very common UI pattern. With this approach, we will be implementing 2 common patterns versus 1. It seems like a very good choice to me.

Regarding the FAB. I think with our current design we should keep it. Current users are already familiar with it. And, it is better than having it at the top where users need to reach for it.

If we're deciding whether to add a FAB, I would have been against it, as like you said, it is unnecessary with the action mode. But having an already implemented FAB. Why get rid of the work just to regress to a poorer UX?

I vote to keep it.

ByteHamster · 2021-04-23T18:38:30Z

How about adding a 3-dot like button next to the play button.

I think this will clutter the screen too much. Please use the long-press menu for now. Changing that later would be pretty easy, so I would rather go with as few muscle-memory breaking changes for now.

Regarding the FAB. I think with our current design we should keep it. Current users are already familiar with it. And, it is better than having it at the top where users need to reach for it.

Okay 👍 I think it will be slightly harder to implement than the ActionMode but I agree :)

peakvalleytech · 2021-04-24T09:38:56Z

After thinking it over, I agree that having it in the context menu wouldn't be such a bad idea. So let's do that.

Thanks, it actually isn't too hard. I've already implemented it, and works it works nicely.

peakvalleytech · 2021-04-25T23:52:15Z

@ByteHamster i am ready for you to review my latest commit. The multi-select is not working on FeedItemListFragment. If you approve of it, I will proceed with migrating the rest of the views that use episode multi-select.
To reduce code, I suggest that all fragments that use episode multi-select should have common base classes containing the multi-select logic. Is that ok with you?

ByteHamster · 2021-04-26T05:50:23Z

To reduce code, I suggest that all fragments that use episode multi-select should have common base classes containing the multi-select logic. Is that ok with you?

This makes it harder to modularize and test the code. I think it would be better to do it the other way around: Extract the common parts (like ActionBinding) to their own classes. Then these things can be included in all classes that need them. They can then also be tested independently of the UI.

app/src/main/java/de/danoeh/antennapod/adapter/EpisodeItemListAdapter.java

app/src/main/java/de/danoeh/antennapod/fragment/FeedItemlistFragment.java

app/src/main/java/de/danoeh/antennapod/view/EpisodeItemListRecyclerView.java

app/src/main/res/menu/episodes.xml

ByteHamster · 2021-04-26T06:05:06Z

app/src/main/res/menu/episodes_apply_action_options.xml

@@ -9,17 +9,17 @@
        app:showAsAction="always">
        <menu>
            <item android:id="@+id/sort_title_a_z"
-                  android:title="@string/sort_title_a_z"/>
+                android:title="@string/sort_title_a_z"/>


Please don't reformat the file, unless you really need to change it. We don't have clear guidelines for xml formatting currently, so it should just be kept as it is.

Still unrelated changes here :)

core/src/main/res/values/attrs.xml

peakvalleytech · 2021-04-26T07:19:50Z

I'll take a look at the comments soon. Thanks for the reply.

keunes · 2021-04-27T20:41:07Z

I'm downloading a bunch of 'debug' artifects to try out - let me know when I can/should do that for this PR. Eager to try it out :)

peakvalleytech · 2021-04-29T04:42:48Z

I'm downloading a bunch of 'debug' artifects to try out - let me know when I can/should do that for this PR. Eager to try it out :)

I should have something ready soon.

peakvalleytech · 2021-05-02T21:26:42Z

Hey @ByteHamster,
I just pushed a new commit. Still waiting for your response. Hope all is well.

ByteHamster · 2021-05-05T21:55:19Z

Still waiting for your response. Hope all is well.

Sorry, I can't check AntennaPod every day (it's done in my free time). Could you please clean up some of the code here? It contains a lot of commented code and changes to unrelated files. That would make it a lot easier to review.

I just played around with it. It does feel pretty nice. There seems to be a problem though, where wrong episodes can get changed. Also, pressing the "Select all" button twice brings everything into a broken state that always affects all episodes, regardless of what you select.

Other issues I found:

The ActionBar icons are in fact wrong in the light theme (there is already a comment about this above)
The items no longer have a Ripple effect when they are normally pressed. I think selection would be easier to handle using a state-list-drawable
The selected color is a bit too strong, in my opinion. Maybe @keunes has an idea for what color to use instead of the accent blue.
When pressing the back button, the ActionMode closes but the FAB stays

peakvalleytech · 2021-05-06T01:07:55Z

Sorry, I can't check AntennaPod every day (it's done in my free time). Could you please clean up some of the code here? It contains a lot of commented code and changes to unrelated files. That would make it a lot easier to review.

Ok that is fine. I'll go and send a new commit soon.

keunes · 2021-05-06T11:38:30Z

I'll have a look at the colour later (probably over the weekend)

peakvalleytech · 2021-05-09T21:06:45Z

I'll have a look at the colour later (probably over the weekend)

@keunes have you had the chance to look at this PR yet?

ByteHamster · 2021-05-09T21:20:47Z

I was thinking maybe we could use the action button to indicate selection state. When an episode is not selected, just keep it empty. When an episode is selected, display a check mark (blue copy of the drawable). Then we don't have problems with the contrast when setting a backgroubd. Also, the action button still showing "download" could be confusing otherwise.

peakvalleytech · 2021-05-09T23:19:01Z

I was thinking maybe we could use the action button to indicate selection state. When an episode is not selected, just keep it empty. When an episode is selected, display a check mark (blue copy of the drawable). Then we don't have problems with the contrast when setting a backgroubd. Also, the action button still showing "download" could be confusing otherwise.

Yeah that will work nicely. So should I proceed with porting it to the other screens (Queue, Downloads, ... etc.)? Let me know if you need any more changes, especially related to the architecture.

peakvalleytech · 2021-05-09T23:48:05Z

@ByteHamster Actually, after looking at the code, I don't think the actionbutton can be changed easilty. I suggest hiding it while in multi select mode.

I think we can make it work if we change the color. Maybe to a light gray for light theme and dark gray for dark theme.

That is how it is usally done by other apps. It will be better UX.

If you want to still do the checkmark approach. I can add a checkmark view to the EpisodeItemViewHolder. Would that be ok with you?

ByteHamster · 2021-06-19T09:10:37Z

I just removed 2 of the 3 lists that store which episodes are selected and also did a bit of cleanup. I think I am happy with the code structure now. Please have another look again to see if I overlooked something.

peakvalleytech

I just removed 2 of the 3 lists that store which episodes are selected and also did a bit of cleanup. I think I am happy with the code structure now. Please have another look again to see if I overlooked something.

@ByteHamster I looked at the code a bit, but haven't tested it. I didn't code it like that on a whim. This was the result of a lot of manual testing. Let's hope you didn't break anything. :) After I test it, i'll give you an update.

peakvalleytech · 2021-06-17T07:05:57Z

app/src/main/res/layout/feeditemlist_item.xml

+                    android:id="@+id/selectCheckBox"
+                    android:layout_width="0dp"
+                    android:layout_height="match_parent"
+                    android:layout_weight="1" />


I'll remove it.

peakvalleytech · 2021-06-17T07:07:33Z

app/src/main/res/menu/feeditemlist_context.xml

@@ -1,6 +1,12 @@
 <?xml version="1.0" encoding="utf-8"?>

 <menu xmlns:android="http://schemas.android.com/apk/res/android">
+    <item
+        android:id="@+id/episode_actions"


I thought the name was amibiguous too. It was from the original author. Just wasn't sure if you would approve of the rename. I'll rename it as per suggestion.

peakvalleytech · 2021-06-17T07:07:51Z

app/src/main/res/layout/feed_item_list_fragment.xml

+                android:visibility="gone"
+                app:sdMainFabClosedSrc="@drawable/ic_fab_edit"
+                app:sdOverlayLayout="@id/fabSDOverlay" />
+    </ScrollView>


app/src/main/java/de/danoeh/antennapod/view/EpisodeItemListRecyclerView.java

peakvalleytech · 2021-06-19T23:40:13Z

@ByteHamster Ok, i've reviewed your changes. And I am also happy with the code structure.

Just added one last commit to remove the previous multi-select option.

Now that we have implemented it for FeedItemListFragment. We just need to implement it for the rest of the screens that need episode multi select.

I believe those are (at least those that currently have multi-select)
CompletedDownloadsFragment
QueueFragment

Should we merge and close this PR or wait till we have it implemented for the rest of the screens?
I think we should merge this and open a new PR for better organization.

ByteHamster · 2021-06-20T06:57:29Z

I think we should merge this and open a new PR for better organization.

Yeah

I just noticed a little problem before merging this. All screens (eg playback history) now show the multi-select context menu item, even if they don't support multi select.

peakvalleytech · 2021-06-20T20:44:56Z

I think we should merge this and open a new PR for better organization.

Yeah

Ok, once we get this merged we can start work on implementing it for the other fragments.
These are QeueFragment and CompletedDownloadsFragment, right?

I just noticed a little problem before merging this. All screens (eg playback history) now show the multi-select context menu item, even if they don't support multi select.

I'll fix this today.

…creens that use it.

ByteHamster · 2021-06-27T19:22:10Z

Your most recent commit breaks long-pressing in select mode (for "select all above")

ByteHamster · 2021-06-29T20:01:14Z

Thanks :)

peakvalleytech · 2021-06-30T09:07:20Z

Thanks :)

My pleasure. Now to start on migrating the other screens to use this multi select. Any suggestions on how this should be done?

The two other screens that use multi select are the QeueFragment and thd CompletedDownloadsFragment. I know the UI is about to under go major changes so I want to avoid doing work that will end up being thrown away anyway. The only screen I know for certain will not be changed is the Queue. Should i start with that?

antennapod-bot · 2021-09-05T21:12:00Z

This pull request has been mentioned on AntennaPod Forum. There might be relevant details there:

https://forum.antennapod.org/t/antennapod-2-4-release-notes/1300/1

antennapod-bot · 2021-09-06T21:16:09Z

This pull request has been mentioned on AntennaPod Forum. There might be relevant details there:

https://forum.antennapod.org/t/antennapod-2-4-release-notes/1300/3

ByteHamster and others added 3 commits April 21, 2021 22:45

Merge pull request AntennaPod#5121 from ByteHamster/lint

c91705f

Lint fixes

Add new contextual action mode for multi selecting episodes.

3fdc593

Merge branch 'develop' into contextual-multi-episode-select

a66ef09

peakvalleytech added 3 commits April 25, 2021 15:08

Add new contextual action mode for multi selecting episodes.

988ce0b

Implement context action mode for episodes.

d15a916

Implement context action mode for episodes.

23fc162

ByteHamster reviewed Apr 26, 2021

View reviewed changes

Revised prototype for episode multi select.

05e901e

peakvalleytech requested a review from ByteHamster May 6, 2021 01:04

Cleanup code, fix bugs.

949dc0a

peakvalleytech added 2 commits May 6, 2021 19:55

Resolve PR issues and add to implmentation.

7f97c83

Checkstyle erors.

ebd373f

Replace selection indicator with checkbox.

183a9b2

peakvalleytech and others added 9 commits June 15, 2021 13:43

Circleci.

95e784c

Remove unrelated whitespace from FeedItemlistFragment.

b8f6e92

Revert changes to unrelated file.

ebd514e

Remove weight attribute from feeditemlist_item.xml

2c50e62

Cleanup holding selection state 3 times

976989e

Translatable toolbar title

0e09a8b

Move multi select speed dial to its own file

427d3c5

Small cleanup

7a4e3c6

Merge branch 'develop' into contextual-multi-episode-select

a54a1c8

peakvalleytech commented Jun 19, 2021

View reviewed changes

Remove old multi-select action option from FeedItemlistFragment.

2b6781b

Modify EpiosodeItemListAdapter context menu to show multi select on s…

cdbdfaf

…creens that use it.

ByteHamster mentioned this pull request Jun 25, 2021

Adapt 'apply actions' filter & view #1834

Closed

peakvalleytech and others added 4 commits June 27, 2021 20:36

Fix bug breaking select above/below options in select mode.

fad4d83

Fix xml codestyle.

0887aca

Code style changes

3f6fc45

Small cleanup

00e6105

ByteHamster merged commit 323f1f6 into AntennaPod:develop Jun 29, 2021

peakvalleytech mentioned this pull request Jul 1, 2021

Migrate other screens to new episode multi-select #5250

Closed

peakvalleytech deleted the contextual-multi-episode-select branch August 21, 2021 00:17

keunes mentioned this pull request Dec 14, 2021

Multi Select no longer allows selection of played episodes in Downloads #5599

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contextual menu for multi selecting episodes #5130

Contextual menu for multi selecting episodes #5130

peakvalleytech commented Apr 23, 2021

ByteHamster commented Apr 23, 2021

peakvalleytech commented Apr 23, 2021

ByteHamster commented Apr 23, 2021

peakvalleytech commented Apr 24, 2021

peakvalleytech commented Apr 25, 2021

ByteHamster commented Apr 26, 2021

ByteHamster Apr 26, 2021

ByteHamster Jun 5, 2021

peakvalleytech commented Apr 26, 2021

keunes commented Apr 27, 2021 •

edited

peakvalleytech commented Apr 29, 2021

peakvalleytech commented May 2, 2021

ByteHamster commented May 5, 2021

peakvalleytech commented May 6, 2021

keunes commented May 6, 2021

peakvalleytech commented May 9, 2021

ByteHamster commented May 9, 2021

peakvalleytech commented May 9, 2021

peakvalleytech commented May 9, 2021

ByteHamster commented Jun 19, 2021

peakvalleytech left a comment

peakvalleytech Jun 17, 2021

peakvalleytech Jun 17, 2021

peakvalleytech Jun 17, 2021

peakvalleytech commented Jun 19, 2021 •

edited

ByteHamster commented Jun 20, 2021

peakvalleytech commented Jun 20, 2021 •

edited

ByteHamster commented Jun 27, 2021

ByteHamster commented Jun 29, 2021

peakvalleytech commented Jun 30, 2021

antennapod-bot commented Sep 5, 2021

antennapod-bot commented Sep 6, 2021

Contextual menu for multi selecting episodes #5130

Contextual menu for multi selecting episodes #5130

Conversation

peakvalleytech commented Apr 23, 2021

ByteHamster commented Apr 23, 2021

peakvalleytech commented Apr 23, 2021

ByteHamster commented Apr 23, 2021

peakvalleytech commented Apr 24, 2021

peakvalleytech commented Apr 25, 2021

ByteHamster commented Apr 26, 2021

ByteHamster Apr 26, 2021

Choose a reason for hiding this comment

ByteHamster Jun 5, 2021

Choose a reason for hiding this comment

peakvalleytech commented Apr 26, 2021

keunes commented Apr 27, 2021 • edited

peakvalleytech commented Apr 29, 2021

peakvalleytech commented May 2, 2021

ByteHamster commented May 5, 2021

peakvalleytech commented May 6, 2021

keunes commented May 6, 2021

peakvalleytech commented May 9, 2021

ByteHamster commented May 9, 2021

peakvalleytech commented May 9, 2021

peakvalleytech commented May 9, 2021

ByteHamster commented Jun 19, 2021

peakvalleytech left a comment

Choose a reason for hiding this comment

peakvalleytech Jun 17, 2021

Choose a reason for hiding this comment

peakvalleytech Jun 17, 2021

Choose a reason for hiding this comment

peakvalleytech Jun 17, 2021

Choose a reason for hiding this comment

peakvalleytech commented Jun 19, 2021 • edited

ByteHamster commented Jun 20, 2021

peakvalleytech commented Jun 20, 2021 • edited

ByteHamster commented Jun 27, 2021

ByteHamster commented Jun 29, 2021

peakvalleytech commented Jun 30, 2021

antennapod-bot commented Sep 5, 2021

antennapod-bot commented Sep 6, 2021

keunes commented Apr 27, 2021 •

edited

peakvalleytech commented Jun 19, 2021 •

edited

peakvalleytech commented Jun 20, 2021 •

edited