Skip to content
This repository has been archived by the owner on Feb 4, 2023. It is now read-only.

Dialog for handling duplicate-addition to playlist #375

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Dialog for handling duplicate-addition to playlist #375

wants to merge 3 commits into from

Conversation

dremerb
Copy link

@dremerb dremerb commented Dec 15, 2017

Fixes Issue #109 by notifying the user about duplicate addition and offers to remove it.


@NonNull
public static AddDuplicateToPlaylistDialog create(Song song) {
ArrayList<Song> list = new ArrayList<>();
Copy link
Collaborator

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));
Copy link
Collaborator

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?

Copy link
Author

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();
Copy link
Collaborator

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) {
Copy link
Collaborator

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?

Copy link
Author

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.

Copy link
Author

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>
Copy link
Collaborator

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) {
Copy link
Collaborator

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;
Copy link
Collaborator

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;
Copy link
Collaborator

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.

Copy link
Author

@dremerb dremerb Dec 17, 2017

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.

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

Successfully merging this pull request may close these issues.

2 participants