Playlist thumbnail

[Abhinavreddy-B]

Functionality to pick custom thumbnails for playlist

[Abhinavreddy-B]

Moderators,
please do check this PR thoroughly for any mistakes, since this is my first contribution.

[Abhinavreddy-B]

btw, i used this api for photo picker

[fast4x]

Thanks, but app must remain FOSS and isn't possible to use Google Play Service api, instead try a solution without it.

[Abhinavreddy-B]

Updated, now i am using ActivityResultContracts.GetContent. removed the dependency from the manifest. i hope now it is fine!

[knighthat]

some suggestions to make the code easier to read and understand

[knighthat]

These two are identical, except for the title. Having two similar blocks of code makes it harder to read and harder to maintain.

I can suggest a solution

var title: Int? = null
if( onEditThumbnail != null )
    title = R.string.edit_thumbnail
if( onResetThumbnail != null )
    title = R.string.reset_thumbnail

title ?: return // Or something that skip next line

GridMenuItem(
    icon = R.drawable.image,
    title = title,
    colorIcon = colorPalette.text,
    colorText = colorPalette.text,
    onClick = {
        onDismiss()
        onEditThumbnail()
    }
)

[Abhinavreddy-B]

1. This code snippet doesn't allow both options simultaneously, right?
2. They differ in the callback function too, not just the title.
3. It's kind of repetitive, I agree, but thats how all the other options were written

[knighthat]

Same thing.

Even better, make a separate function handles this particular thing

[Abhinavreddy-B]

I understand that they are kind of repetitive. But, Since all the other menu items are listed this way, putting only these 2 items in a separate function and then calling them here will lead to more confusion is what I feel.

[knighthat]

use rememberLauncherForActivityResult. Example HERE

[Abhinavreddy-B]

But these are operations on file that are stored in internal storage of the apps, so there is no need to ask user consent right? Hence, rememberLauncherForActivityResult is unnecessary right?

[fast4x]

@Abhinavreddy-B Two improvements:

• Replace Log.d with Timber.d
• Save images to folder so all thumbnails will be inside a specific folder, obviously always inside app files folder.

[Abhinavreddy-B]

@Abhinavreddy-B Two improvements:

• Replace Log.d with Timber.d
• Save images to folder so all thumbnails will be inside a specific folder, obviously always inside app files folder.

Resolved in 29e21f6

[fast4x]

Update your repo and resolve conflict in your branch please, i can't merge it.

[Abhinavreddy-B]

Resolved conflicts now :)

[fast4x]

Thanks for patience

[fast4x]

@knighthat it's ok, isn't necessary add other extreme code optimizations.

@Abhinavreddy-B merge in progress...

[fast4x]

Please fix image in playlists list when custom image is present...

[knighthat]

@fast4x not extreme but eaiser down the road to read and fix bugs. I'm fixing tons of ridiculous errors in the code base and you couldn't believe the amount of duplicate/nested code

[fast4x]

Please don’t change the code, because it can indirectly generate bugs. Focus on other things please, code it's ok, not necessary change it.
Thank you for your contribution.