-
Notifications
You must be signed in to change notification settings - Fork 730
Dialog for handling duplicate-addition to playlist #375
base: master
Are you sure you want to change the base?
Conversation
|
||
@NonNull | ||
public static AddDuplicateToPlaylistDialog create(Song song) { | ||
ArrayList<Song> list = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should try to see if you can get this to just be a list of IDs since there is a restriction on the max bundle size.
int title; | ||
final CharSequence content; | ||
title = R.string.add_duplicate_title; | ||
content = Html.fromHtml(getString(R.string.add_duplicate_msg, songs.get(0).title)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it only handles the first duplicate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently for every duplicate a dialog is shown - So actually only one create-Method is required as the ArrayList will never get bigger than 1.
The downside to this is that adding an Album a second time will open Album.size dialogs.
final int basecopy = base; | ||
for(final Song song: songs) { | ||
if (doPlaylistContains(context, playlistId, song.id)) { | ||
//Toast.makeText(context, song.title+" already in Playlist!", Toast.LENGTH_SHORT).show(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this.
@@ -168,6 +187,17 @@ public static void removeFromPlaylist(@NonNull final Context context, @NonNull f | |||
} | |||
} | |||
|
|||
public static void removeFromPlaylist(@NonNull final Context context, @NonNull final Song song, int playlistId, int startID) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain what the start ID is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startID gets the value of base in case a insertion of a duplicate should be reverted. If you just call the function with a Song every occurence of the song will be deleted. The startID can be passed as the base of the insertion so it is guaranteed that only the recently added song, not the one already inside the playlist will be deleted. (MediaStore.Audio.Playlists.Members.PLAY_ORDER + " >=?" is the query to also find the recently added song in the delete-call)
This could be made the standard-parameter-set for removeFromPlaylist, startID could be 0 to do the same as it does currently without the startID. But it would increase the overhead in the classic deletion process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about the deletion, it might actually be possible that if you delete a song afterwards all songs with that id will be deleted from the playlist, I will have to test this.
@@ -92,6 +92,9 @@ | |||
<string name="clear_playlist_title">Clear playlist</string> | |||
<string name="save_playlist_title">Save as file</string> | |||
<string name="add_playlist_title">"Add to playlist"</string> | |||
<string name="add_duplicate_title">"Duplicate in Playlist"</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need the quotes.
selListener = null; | ||
} | ||
|
||
public void setSelListener(SelectionListener SelListener) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the full name: setSelectionListener
public Dialog onCreateDialog(Bundle savedInstanceState) { | ||
//noinspection unchecked | ||
final ArrayList<Song> songs = getArguments().getParcelableArrayList("songs"); | ||
int title; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the assignments in the same line.
@Override | ||
public void onClick(@NonNull MaterialDialog dialog, @NonNull DialogAction which) { | ||
if (getActivity() == null) return; | ||
selection = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you just directly call the listener? I'm a bit confused as to why it's done this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onDismiss has to be called anyways and by calling the listener in onDismiss it is possible to easily handle cancellation on the dialog e. g. by tapping beside it.
Fixes Issue #109 by notifying the user about duplicate addition and offers to remove it.