From 6ed412d4707eaa471de5a87fd2c6ad7692853441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey?= Date: Fri, 21 Oct 2022 17:29:19 +0200 Subject: [PATCH] Fix creation and result logic for StoragePermissionDialogFragment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents a crash on dialog recreation due to it not having default constructor, seen in Google Play crash records Signed-off-by: Álvaro Brey --- .../dialog/StoragePermissionDialogFragment.kt | 53 ++++++++++++++----- .../owncloud/android/utils/PermissionUtil.kt | 52 +++++++++--------- 2 files changed, 68 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt b/app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt index 18fd276fe2..c3cddfcc30 100644 --- a/app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt +++ b/app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt @@ -23,35 +23,43 @@ package com.owncloud.android.ui.dialog import android.app.Dialog import android.os.Build import android.os.Bundle +import android.os.Parcelable import android.view.View import androidx.annotation.RequiresApi import androidx.appcompat.app.AlertDialog +import androidx.core.os.bundleOf import androidx.fragment.app.DialogFragment import com.google.android.material.dialog.MaterialAlertDialogBuilder import com.nextcloud.client.di.Injectable import com.owncloud.android.R import com.owncloud.android.databinding.StoragePermissionDialogBinding -import com.owncloud.android.ui.dialog.StoragePermissionDialogFragment.Listener import com.owncloud.android.utils.theme.ViewThemeUtils +import kotlinx.parcelize.Parcelize import javax.inject.Inject /** * Dialog that shows permission options in SDK >= 30 * * Allows choosing "full access" (MANAGE_ALL_FILES) or "read-only media" (READ_EXTERNAL_STORAGE) - * - * @param listener a [Listener] for button clicks. The dialog will auto-dismiss after the callback is called. - * @param permissionRequired Whether the permission is absolutely required by the calling component. - * This changes the texts to a more strict version. */ @RequiresApi(Build.VERSION_CODES.R) -class StoragePermissionDialogFragment(val listener: Listener, val permissionRequired: Boolean = false) : +class StoragePermissionDialogFragment : DialogFragment(), Injectable { + private lateinit var binding: StoragePermissionDialogBinding + private var permissionRequired = false + @Inject lateinit var viewThemeUtils: ViewThemeUtils + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + arguments?.let { + permissionRequired = it.getBoolean(ARG_PERMISSION_REQUIRED, false) + } + } + override fun onStart() { super.onStart() dialog?.let { @@ -75,12 +83,12 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ // Setup layout viewThemeUtils.material.colorMaterialButtonPrimaryFilled(binding.btnFullAccess) binding.btnFullAccess.setOnClickListener { - listener.onClickFullAccess() + setResult(Result.FULL_ACCESS) dismiss() } viewThemeUtils.platform.colorTextButtons(binding.btnReadOnly) binding.btnReadOnly.setOnClickListener { - listener.onClickMediaReadOnly() + setResult(Result.MEDIA_READ_ONLY) dismiss() } @@ -94,7 +102,7 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ .setTitle(titleResource) .setView(view) .setNegativeButton(R.string.common_cancel) { _, _ -> - listener.onCancel() + setResult(Result.CANCEL) dismiss() } @@ -103,9 +111,28 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ return builder.create() } - interface Listener { - fun onCancel() - fun onClickFullAccess() - fun onClickMediaReadOnly() + private fun setResult(result: Result) { + parentFragmentManager.setFragmentResult(REQUEST_KEY, bundleOf(RESULT_KEY to result)) + } + + @Parcelize + enum class Result : Parcelable { + CANCEL, FULL_ACCESS, MEDIA_READ_ONLY + } + + companion object { + private const val ARG_PERMISSION_REQUIRED = "ARG_PERMISSION_REQUIRED" + const val REQUEST_KEY = "REQUEST_KEY_STORAGE_PERMISSION" + const val RESULT_KEY = "RESULT" + + /** + * @param permissionRequired Whether the permission is absolutely required by the calling component. + * This changes the texts to a more strict version. + */ + fun newInstance(permissionRequired: Boolean): StoragePermissionDialogFragment { + return StoragePermissionDialogFragment().apply { + arguments = bundleOf(ARG_PERMISSION_REQUIRED to permissionRequired) + } + } } } diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index ca919a233d..6dde844a96 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -209,33 +209,37 @@ object PermissionUtil { viewThemeUtils: ViewThemeUtils ) { val preferences: AppPreferences = AppPreferencesImpl.fromContext(activity) - - if (!preferences.isStoragePermissionRequested || permissionRequired) { - if (activity.supportFragmentManager.findFragmentByTag(PERMISSION_CHOICE_DIALOG_TAG) == null) { - val listener = object : StoragePermissionDialogFragment.Listener { - override fun onCancel() { - preferences.isStoragePermissionRequested = true - } - - override fun onClickFullAccess() { - preferences.isStoragePermissionRequested = true - val intent = getManageAllFilesIntent(activity) - activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES) - preferences.isStoragePermissionRequested = true - } - - override fun onClickMediaReadOnly() { - preferences.isStoragePermissionRequested = true - requestStoragePermission( - activity, - Manifest.permission.READ_EXTERNAL_STORAGE, - viewThemeUtils - ) + val shouldRequestPermission = !preferences.isStoragePermissionRequested || permissionRequired + if (shouldRequestPermission && + activity.supportFragmentManager.findFragmentByTag(PERMISSION_CHOICE_DIALOG_TAG) == null + ) { + activity.supportFragmentManager.setFragmentResultListener( + StoragePermissionDialogFragment.REQUEST_KEY, + activity + ) { _, resultBundle -> + val result: StoragePermissionDialogFragment.Result? = + resultBundle.getParcelable(StoragePermissionDialogFragment.RESULT_KEY) + if (result != null) { + preferences.isStoragePermissionRequested = true + when (result) { + StoragePermissionDialogFragment.Result.FULL_ACCESS -> { + val intent = getManageAllFilesIntent(activity) + activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES) + } + StoragePermissionDialogFragment.Result.MEDIA_READ_ONLY -> { + requestStoragePermission( + activity, + Manifest.permission.READ_EXTERNAL_STORAGE, + viewThemeUtils + ) + } + StoragePermissionDialogFragment.Result.CANCEL -> {} } } - val dialogFragment = StoragePermissionDialogFragment(listener, permissionRequired) - dialogFragment.show(activity.supportFragmentManager, PERMISSION_CHOICE_DIALOG_TAG) } + + val dialogFragment = StoragePermissionDialogFragment.newInstance(permissionRequired) + dialogFragment.show(activity.supportFragmentManager, PERMISSION_CHOICE_DIALOG_TAG) } }