Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚡ Simplify 'permissionsBuilder()' ext fun by merging the params into a single 'vararg' param #379

Closed
RahulSDeshpande opened this issue Aug 17, 2023 · 6 comments
Labels
enhancement Introduces a new feature, a behavior change or a performance change

Comments

@RahulSDeshpande
Copy link

RahulSDeshpande commented Aug 17, 2023

Description

Simplify permissionsBuilder() extension function by merging both the params into one.
In short, add and extra remove firstPermission: String & only keep a single param as vararg permissions: String.

Motivation

I am using 3.4.0 version of this library.
This feature would be very useful, as the developer will have to only pass the param as vararg in array format (or even a list).

Example

Currently we have to do it like this:

val permissionRequest = 
    permissionsBuilder(
        Manifest.permission.CAMERA,
        Manifest.permission.READ_SMS
    ).build()

We cannot do this:

val permissions =
    arrayOf(
        Manifest.permission.CAMERA,
        Manifest.permission.READ_SMS
    )
val permissionRequest = permissionsBuilder(permissions).build()

This is what I am suggesting as a part of this feature request.

I really loved how you did for this ext function in CheckPermissionsStatus.kt:

internal fun Activity.checkRuntimePermissionsStatus(permissions: List<String>): List<PermissionStatus> =
    permissions.map { permission ->
        if (isPermissionGranted(permission)) {
            return@map PermissionStatus.Granted(permission)
        }
        if (ActivityCompat.shouldShowRequestPermissionRationale(this, permission)) {
            PermissionStatus.Denied.ShouldShowRationale(permission)
        } else {
            PermissionStatus.RequestRequired(permission)
        }
    }

I know you have a pretty good architecture setup for this.
This would impact many places, primarily at:

  • PermissionRequestBuilder
  • BasePermissionRequestBuilder
  • checkPermissionsStatus()

So, my suggestion is also applicable exactly for the checkPermissionsStatus() ext fun.

Please let me know. ✋🏼


❤️ Amazing library btw.
❤️ I would really love to contribute, please let me know.

@RahulSDeshpande RahulSDeshpande added the enhancement Introduces a new feature, a behavior change or a performance change label Aug 17, 2023
@RahulSDeshpande RahulSDeshpande changed the title ⚡ Simplify 'permissionsBuilder()' ext fun by merging the params into one ⚡ Simplify 'permissionsBuilder()' ext fun by merging the params into a single 'vararg' param Aug 17, 2023
@fondesa
Copy link
Owner

fondesa commented Oct 11, 2023

I've been busy sorry, so I'm pretty late in answering this.

add and extra remove firstPermission: String

This was done primarly to avoid to pass an empty vararg.

We cannot do this:

Your suggestion makes sense, to avoid to make breaking changes though, I'm adding the possibility to pass a List, still supporting the vararg functions which are already there.

Hence, to make an example, there will be these two functions:

fun Type.permissionsBuilder(firstPermission: String, vararg others: String)
fun Type.permissionsBuilder(permissions: List<String>)

Thanks for the suggestion!

@fondesa
Copy link
Owner

fondesa commented Oct 13, 2023

PR to implement this: #393

The next release will contain this, thanks again for the suggestion.

@fondesa fondesa closed this as completed Oct 13, 2023
@RahulSDeshpande
Copy link
Author

This looks perfect!!

Welcome & thanks. ❤️💯🚀🚀

@RahulSDeshpande
Copy link
Author

@fondesa
Buddy, any plans to releasing the latest lib builds including this change??

@fondesa
Copy link
Owner

fondesa commented Feb 22, 2024

So much sorry @RahulSDeshpande, I didn't used much Github lately. I released the version 3.5.0 with this new API!

@RahulSDeshpande
Copy link
Author

Thanks @fondesa bro!! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Introduces a new feature, a behavior change or a performance change
Projects
None yet
Development

No branches or pull requests

2 participants