permissions-dispatcher / PermissionsDispatcher

A declarative API to handle Android runtime permissions.
https://github.com/permissions-dispatcher/PermissionsDispatcher
Apache License 2.0
11.22k stars 1.44k forks source link

OnShowRationale API signature change #562

Closed hotchemi closed 5 years ago

hotchemi commented 5 years ago

Issue

resolves https://github.com/permissions-dispatcher/PermissionsDispatcher/issues/434

Overview

This API change is based on https://github.com/permissions-dispatcher/PermissionsDispatcher/issues/434#issuecomment-447257123 discussion.

1. Attach annotations

@RuntimePermissions
class MainActivity : AppCompatActivity(), View.OnClickListener {

    @NeedsPermission(Manifest.permission.CAMERA)
    fun showCamera() {
        supportFragmentManager.beginTransaction()
                .replace(R.id.sample_content_fragment, CameraPreviewFragment.newInstance())
                .addToBackStack("camera")
                .commitAllowingStateLoss()
    }

    @OnShowRationale(Manifest.permission.CAMERA)
    fun showRationaleForCamera() {
        AlertDialog.Builder(this)
            .setMessage(R.string.permission_camera_rationale)
            .setPositiveButton(R.string.button_allow, {dialog, button -> /* TODO */ })
            .setNegativeButton(R.string.button_deny, {dialog, button -> /* TODO */ })
            .show()
    }

    @OnPermissionDenied(Manifest.permission.CAMERA)
    fun onCameraDenied() {
        Toast.makeText(this, R.string.permission_camera_denied, Toast.LENGTH_SHORT).show()
    }

    @OnNeverAskAgain(Manifest.permission.CAMERA)
    fun onCameraNeverAskAgain() {
        Toast.makeText(this, R.string.permission_camera_never_askagain, Toast.LENGTH_SHORT).show()
    }
}

2. Delegate to generated functions

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    setContentView(R.layout.activity_main)
    findViewById(R.id.button_camera).setOnClickListener {
        // delegate the permission handling to generated function
        showCameraWithPermissionCheck()
    }
}

@OnShowRationale(Manifest.permission.CAMERA)
fun showRationaleForCamera() {
    AlertDialog.Builder(this)
       .setMessage(R.string.permission_camera_rationale)
       .setPositiveButton(R.string.button_allow, {dialog, button -> proceedShowCameraPermissionRequest() })
       .setNegativeButton(R.string.button_deny, {dialog, button -> cancelShowCameraPermissionRequest() })
       .show()
}

override fun onRequestPermissionsResult(requestCode: Int, permissions: Array<String>, grantResults: IntArray) {
    super.onRequestPermissionsResult(requestCode, permissions, grantResults)
    // delegate the permission handling to generated function
    onRequestPermissionsResult(requestCode, grantResults)
}
mannodermaus commented 5 years ago

I'm a little sad to let go of the PermissionRequest interface, because I liked the straightforwardness of calling proceed() and cancel() directly. Usually, our generated class names are quite long, so this introduces two more instances where we utilize long calls.

What do you think about keeping both? I mean, the PermissionRequest could still keep its simple API, and we still generate the additional static methods, to which the request would delegate. This is the best of both worlds, however it does raise the complexity of the generated code, since @OnShowRationale's parameter would become optional...

hotchemi commented 5 years ago

Ooh your proposal seems better:D I didn't come up with that somehow but then we can avoid loosing backward compatibility! @mannodermaus

hotchemi commented 5 years ago

Let me create another PR since the diff is so huge .

hotchemi commented 5 years ago

https://github.com/permissions-dispatcher/PermissionsDispatcher/pull/569