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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Instructions for manual enabling of permission #10

Merged
merged 7 commits into from Sep 14, 2018

Conversation

PLNech
Copy link
Member

@PLNech PLNech commented Sep 12, 2018

Feature

Flow for manual enabling of recording permission.

  • Implementation in app, with default UI:
override fun onRequestPermissionsResult(requestCode: Int, permissions: Array<out String>, grantResults: IntArray) {
    super.onRequestPermissionsResult(requestCode, permissions, grantResults)
    if (Voice.isRecordPermissionWithResults(requestCode, grantResults)) {
        when {
            Voice.isPermissionGranted(grantResults) -> showVoiceDialog()
            Voice.shouldExplainPermission(this) -> Voice.showPermissionRationale(getPermissionView(), this)
            else -> Voice.showPermissionManualInstructions(getPermissionView())
        }
    }
}
  • Users can customize the rationale's message/CTA (whyAllow, buttonAllow), and the manual instructions' message/CTA/instructions (whyEnable, buttonEnable, howEnable)
  • If one wants a different UI, they can call Voice.openAppSettings() from their own UI logic

Questions

  • As showPermissionRationale and showPermissionManualInstructions have two overloads for String and StringRes parameters, default arguments caused an Overload resolution ambiguity. The solution I found was a third overload with the minimum amount of arguments, but let me know if you have a better approach!

  • WDYT of refactoring all those as Activity.* extension methods? I feel it improves DX by allowing this.showX() instead of Voice.showX(this), but might pollute the scope/prevent discoverability (for example with Activity.openAppSettings()). @q-litzler what's your take on this?

  • Documenting overloads results in duplicated documentation. Is there a way to avoid that or is it a necessary evil? 馃

Copy link
Contributor

@q-litzler q-litzler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, let's offer only one method for showPermissionManualInstructions as a clearer way to present our helpers method.

This looks good. To answer your question, I'm in favour of using function extensions rather that an Object with static methods. It is the idiomatic way to add functionalities to a class in Kotlin.

I don't see any solutions for duplicated documentation.

@PLNech PLNech merged commit 07582c0 into master Sep 14, 2018
@PLNech PLNech deleted the feat/manualEnable branch September 14, 2018 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants