[Needs improvement] Playlist dialog: Add checkmark to indicate if song is in playlist

[Sogolumbo]

The "Add to playlist" dialog will show a checkmark behind a playlist if the song is already part of that playlist.
This feature will only be shown if only one song is selected for addition.

In the following screenshot the song is already part of the "Example playlist":

[kabouzeid]

I think this won't perform well on larger playlists, since it's doing an new SQL query for every single item.

[Sogolumbo]

@kabouzeid Yes, there might be a problem with very big playlists. But I just tested it on my Android device. My biggest playlist contains all of my 1663 tracks and I can't see any difference between the normal app and my fork version.

[Sogolumbo]

@kabouzeid Do you plan to include this feature? I would love to hear that because think there is no reason not to. The delay may be a drawback. But if the smartphone needs noticeable time to check whether a song is already in a playlist, imagine how long a human would need...
Do you know how big the most playlists of the users are? If they aren't much bigger than 2000 songs I don't think that there will be a noticeable delay.
If you still have reservations about this feature I would suggest adding the possibility to switch it on and off in the options. This feature is a key functionality of any music player which will save the users hours of their time (if they want to have playlist without duplicates).

[arkon]

An alternative solution would be to allow the user to prune a playlist of duplicates, so it would just be a query within a playlist to check for duplicate IDs.

[kabouzeid]

@Sogolumbo Sorry for the delay, I have very little time for this project currently. Also I don't have any large playlist to test against. Could you measure the time in millis on your 1663 playlist how long the for loop actually takes?

[kabouzeid]

Instead of displaying a checkmark I would prefer to popup a dialog, in case the playlist already contains at least one song and allow to exclude those songs from adding again to the playlist.

[kabouzeid]

@arkon this could be added either ways

[Sogolumbo]

I just tested how long the for loop takes. Here are the results:
The loop which checks whether the song is already in the playlists takes 91ms ±10ms.

Could you measure the time in millis on your 1663 playlist how long the for loop actually takes?

@kabouzeid What do you mean with "on your 1663 playlist"?
The checkmarks only get shown if i try to add exactly one song to a playlist.

In the test environment (my phone) there are 7 playlists. The longest one contains 1663 songs. The other playlists are smaller (180, 3, 253, 188, 22, 91 songs) (all playlists added up: 2400 songs). Device: honor 6C

[Sogolumbo]

Instead of displaying a checkmark I would prefer to popup a dialog, in case the playlist already contains at least one song and allow to exclude those songs from adding again to the playlist.

@kabouzeid I think that is the right solution if the user tries to add multiple songs / a playlist / an album to other playlists. For just one song I prefer my solution because it creates a faster workflow (for the user).
Also the App is missing a possibility to check whether a song is already contained in a playlist. (And the playlists are not ordered alphabetically)

[kabouzeid]

@Sogolumbo I apologize, I just notice that I've completely misunderstood your code before. Somehow I thought you would make an SQL query for every song in the playlist. Obviously this is not the case at all. Now I'm even more surprised that this takes 90ms.
I don't think the workflow should be different when you add one or many songs to a playlist. This just leads to confusion.

[kabouzeid]

Also if we display the checkmark additionally when the user tries to add exactly one song, it should be a proper material one. In this case we have to add an image view to the items where we display the checkmark. https://material.io/icons/#ic_done

[touficbatache]

Yes a material design icon would look better! Wouldn't it be even better if the icon is on the right end? What do you think @kabouzeid?

[kabouzeid]

@touficbatache absolutely

[touficbatache]

@kabouzeid How can we add lyrics to music?

[GeoZac]

@touficbatache You could add lyrics to tags or add *.LRC file to the same folder as music file location.

[touficbatache]

Thanks!

[ghost]

Any update or is the fork the best we're going to get?

[Sogolumbo]

@dronewizard Well the authors of the Phonograph seem to care a lot about the design of the app. Unfortunately I have absolutely no Idea how the material design works and I don't have the time learn it.
I have fully implemented all the business logic I can. Now either we include this feature as-is or we need someone to take care of the design.

[ghost]

@dronewizard Well the authors of the Phonograph seem to care a lot about the design of the app. Unfortunately I have absolutely no Idea how the material design works and I don't have the time learn it.
I have fully implemented all the business logic I can. Now either we include this feature as-is or we need someone to take care of the design.

I'll look into it.

[kabouzeid]

No ascii checkmark please. The checkmark should be an image (ic_done) and right aligned.

[tralph3]

Can I highjack the thread for a sec? How do you test the app? I want to make some changes for a PR but I don't know how to make a reasonable testing environment. You know, so I can change some code, and check out how it works on the fly.