From bd45f0e942c549d4ccbc1b35541391882b0d6262 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 13 Nov 2024 18:04:37 +0530 Subject: [PATCH] Showing storage selection dialog at first time when copying/moving ZIM file --- .../library/CopyMoveFileHandler.kt | 96 +++++++++++++------ .../library/LocalLibraryFragment.kt | 6 +- .../library/OnlineLibraryFragment.kt | 1 + .../localLibrary/CopyMoveFileHandlerTest.kt | 16 ++-- .../utils/storage/StorageSelectDialog.kt | 7 +- .../utils/storage/adapter/StorageDelegate.kt | 4 +- .../storage/adapter/StorageViewHolder.kt | 4 +- .../core/utils/SharedPreferenceUtil.kt | 8 +- .../core/utils/dialog/KiwixDialog.kt | 1 + core/src/main/res/values/strings.xml | 1 + 10 files changed, 98 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt index 765bfd03d..ab2846119 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/CopyMoveFileHandler.kt @@ -30,6 +30,11 @@ import android.widget.TextView import androidx.appcompat.app.AlertDialog import androidx.core.net.toUri import androidx.documentfile.provider.DocumentFile +import androidx.fragment.app.FragmentManager +import eu.mhutti1.utils.storage.STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE +import eu.mhutti1.utils.storage.StorageDevice +import eu.mhutti1.utils.storage.StorageDeviceUtils +import eu.mhutti1.utils.storage.StorageSelectDialog import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import kotlinx.coroutines.CoroutineScope @@ -43,6 +48,8 @@ import org.kiwix.kiwixmobile.core.extensions.deleteFile import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.settings.StorageCalculator +import org.kiwix.kiwixmobile.core.utils.EXTERNAL_SELECT_POSITION +import org.kiwix.kiwixmobile.core.utils.INTERNAL_SELECT_POSITION import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog @@ -74,6 +81,10 @@ class CopyMoveFileHandler @Inject constructor( var isMoveOperation = false var shouldValidateZimFile: Boolean = false private var fileSystemDisposable: Disposable? = null + private lateinit var fragmentManager: FragmentManager + private val storageDeviceList by lazy { + StorageDeviceUtils.getWritableStorage(activity) + } private val copyMoveTitle: String by lazy { if (isMoveOperation) { @@ -93,13 +104,24 @@ class CopyMoveFileHandler @Inject constructor( fun showMoveFileToPublicDirectoryDialog( uri: Uri? = null, documentFile: DocumentFile? = null, - shouldValidateZimFile: Boolean = false + shouldValidateZimFile: Boolean = false, + fragmentManager: FragmentManager ) { this.shouldValidateZimFile = shouldValidateZimFile + this.fragmentManager = fragmentManager setSelectedFileAndUri(uri, documentFile) - if (!sharedPreferenceUtil.copyMoveZimFilePermissionDialog) { - showMoveToPublicDirectoryPermissionDialog() + if (sharedPreferenceUtil.shouldShowStorageSelectionDialog && storageDeviceList.size > 1) { + // Show dialog to select storage if more than one storage device is available, and user + // have not configured the storage yet. + showCopyMoveDialog(true) } else { + if (storageDeviceList.size == 1) { + // If only internal storage is currently available, set shouldShowStorageSelectionDialog + // to true. This allows the storage configuration dialog to be shown again if the + // user removes an external storage device (like an SD card) and then reinserts it. + // This ensures they are prompted to configure storage settings upon SD card reinsertion. + sharedPreferenceUtil.shouldShowStorageSelectionDialog = true + } if (validateZimFileCanCopyOrMove()) { showCopyMoveDialog() } @@ -107,8 +129,8 @@ class CopyMoveFileHandler @Inject constructor( } fun setSelectedFileAndUri(uri: Uri?, documentFile: DocumentFile?) { - selectedFileUri = uri - selectedFile = documentFile + uri?.let { selectedFileUri = it } + documentFile?.let { selectedFile = it } } fun setFileCopyMoveCallback(fileCopyMoveCallback: FileCopyMoveCallback?) { @@ -119,22 +141,34 @@ class CopyMoveFileHandler @Inject constructor( lifecycleScope = coroutineScope } - private fun showMoveToPublicDirectoryPermissionDialog() { - alertDialogShower.show( - KiwixDialog.MoveFileToPublicDirectoryPermissionDialog, - { - sharedPreferenceUtil.copyMoveZimFilePermissionDialog = true - if (validateZimFileCanCopyOrMove()) { - performCopyOperation() - } - }, - { - sharedPreferenceUtil.copyMoveZimFilePermissionDialog = true - if (validateZimFileCanCopyOrMove()) { - performMoveOperation() - } - } + private fun showStorageSelectDialog() = StorageSelectDialog() + .apply { + onSelectAction = ::copyMoveZIMFileInSelectedStorage + titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE + setStorageDeviceList(storageDeviceList) + setShouldShowCheckboxSelected(false) + } + .show( + fragmentManager, + activity.getString(R.string.choose_storage_to_copy_move_zim_file) ) + + fun copyMoveZIMFileInSelectedStorage(storageDevice: StorageDevice) { + sharedPreferenceUtil.apply { + shouldShowStorageSelectionDialog = false + putPrefStorage(sharedPreferenceUtil.getPublicDirectoryPath(storageDevice.name)) + putStoragePosition( + if (storageDevice.isInternal) INTERNAL_SELECT_POSITION + else EXTERNAL_SELECT_POSITION + ) + } + if (validateZimFileCanCopyOrMove()) { + if (isMoveOperation) { + performMoveOperation() + } else { + performCopyOperation() + } + } } fun isBookLessThan4GB(): Boolean = @@ -192,22 +226,30 @@ class CopyMoveFileHandler @Inject constructor( } } - fun showCopyMoveDialog() { + fun showCopyMoveDialog(showStorageSelectionDialog: Boolean = false) { alertDialogShower.show( KiwixDialog.CopyMoveFileToPublicDirectoryDialog, - ::performCopyOperation, - ::performMoveOperation + { performCopyOperation(showStorageSelectionDialog) }, + { performMoveOperation(showStorageSelectionDialog) } ) } - fun performCopyOperation() { + fun performCopyOperation(showStorageSelectionDialog: Boolean = false) { isMoveOperation = false - copyZimFileToPublicAppDirectory() + if (showStorageSelectionDialog) { + showStorageSelectDialog() + } else { + copyZimFileToPublicAppDirectory() + } } - fun performMoveOperation() { + fun performMoveOperation(showStorageSelectionDialog: Boolean = false) { isMoveOperation = true - moveZimFileToPublicAppDirectory() + if (showStorageSelectionDialog) { + showStorageSelectDialog() + } else { + moveZimFileToPublicAppDirectory() + } } private fun copyZimFileToPublicAppDirectory() { diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt index 87fb27811..f201eca2b 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/LocalLibraryFragment.kt @@ -431,7 +431,8 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal uri, documentFile, // pass if fileName is null then we will validate it after copying/moving - fileName == null + fileName == null, + parentFragmentManager ) } else { getZimFileFromUri(uri)?.let(::navigateToReaderFragment) @@ -669,6 +670,7 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal .apply { onSelectAction = ::storeDeviceInPreferences setStorageDeviceList(storageDeviceList) + setShouldShowCheckboxSelected(true) } .show(parentFragmentManager, getString(string.pref_storage)) @@ -683,6 +685,6 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal else EXTERNAL_SELECT_POSITION ) // after selecting the storage try to copy/move the zim file. - copyMoveFileHandler?.showMoveFileToPublicDirectoryDialog() + copyMoveFileHandler?.copyMoveZIMFileInSelectedStorage(storageDevice) } } diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt index e2297e571..caa877827 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt @@ -577,6 +577,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { onSelectAction = ::storeDeviceInPreferences titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE setStorageDeviceList(storageDeviceList) + setShouldShowCheckboxSelected(false) } .show(parentFragmentManager, getString(string.choose_storage_to_download_book)) diff --git a/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt b/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt index 124436ae1..5ae4c0061 100644 --- a/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt +++ b/app/src/test/java/org/kiwix/kiwixmobile/localLibrary/CopyMoveFileHandlerTest.kt @@ -230,15 +230,15 @@ class CopyMoveFileHandlerTest { fileHandler.showMoveFileToPublicDirectoryDialog() every { fileHandler.validateZimFileCanCopyOrMove() } returns true - every { fileHandler.performCopyOperation() } just Runs + every { fileHandler.performCopyOperation(showStorageSelectionDialog) } just Runs positiveButtonClickSlot.captured.invoke() - verify { fileHandler.performCopyOperation() } + verify { fileHandler.performCopyOperation(showStorageSelectionDialog) } every { sharedPreferenceUtil.copyMoveZimFilePermissionDialog } returns false - every { fileHandler.performMoveOperation() } just Runs + every { fileHandler.performMoveOperation(showStorageSelectionDialog) } just Runs negativeButtonClickSlot.captured.invoke() - verify { fileHandler.performMoveOperation() } + verify { fileHandler.performMoveOperation(showStorageSelectionDialog) } verify { sharedPreferenceUtil.copyMoveZimFilePermissionDialog = true } } @@ -275,14 +275,14 @@ class CopyMoveFileHandlerTest { every { fileHandler.validateZimFileCanCopyOrMove() } returns true fileHandler.showMoveFileToPublicDirectoryDialog() - every { fileHandler.performCopyOperation() } just Runs + every { fileHandler.performCopyOperation(showStorageSelectionDialog) } just Runs positiveButtonClickSlot.captured.invoke() - verify { fileHandler.performCopyOperation() } - every { fileHandler.performMoveOperation() } just Runs + verify { fileHandler.performCopyOperation(showStorageSelectionDialog) } + every { fileHandler.performMoveOperation(showStorageSelectionDialog) } just Runs negativeButtonClickSlot.captured.invoke() - verify { fileHandler.performMoveOperation() } + verify { fileHandler.performMoveOperation(showStorageSelectionDialog) } } private fun prepareFileSystemAndFileForMockk( diff --git a/core/src/main/java/eu/mhutti1/utils/storage/StorageSelectDialog.kt b/core/src/main/java/eu/mhutti1/utils/storage/StorageSelectDialog.kt index 3e562fa9c..193d5456d 100644 --- a/core/src/main/java/eu/mhutti1/utils/storage/StorageSelectDialog.kt +++ b/core/src/main/java/eu/mhutti1/utils/storage/StorageSelectDialog.kt @@ -50,13 +50,14 @@ class StorageSelectDialog : DialogFragment() { private var aTitle: String? = null private var storageSelectDialogViewBinding: StorageSelectDialogBinding? = null private val storageDeviceList = arrayListOf() + private var shouldShowCheckboxSelected: Boolean = true private val storageAdapter: StorageAdapter by lazy { StorageAdapter( StorageDelegate( storageCalculator, sharedPreferenceUtil, - aTitle == getString(R.string.choose_storage_to_download_book) + shouldShowCheckboxSelected ) { onSelectAction?.invoke(it) dismiss() @@ -68,6 +69,10 @@ class StorageSelectDialog : DialogFragment() { this.storageDeviceList.addAll(storageDeviceList) } + fun setShouldShowCheckboxSelected(shouldShowCheckboxSelected: Boolean) { + this.shouldShowCheckboxSelected = shouldShowCheckboxSelected + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, diff --git a/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageDelegate.kt b/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageDelegate.kt index bd8a07ea8..c809f8476 100644 --- a/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageDelegate.kt +++ b/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageDelegate.kt @@ -30,7 +30,7 @@ import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil class StorageDelegate( private val storageCalculator: StorageCalculator, private val sharedPreferenceUtil: SharedPreferenceUtil, - private val isShowingStorageOptionForFirstDownload: Boolean, + private val shouldShowCheckboxSelected: Boolean, private val onClickAction: (StorageDevice) -> Unit ) : AdapterDelegate { override fun createViewHolder(parent: ViewGroup): ViewHolder { @@ -38,7 +38,7 @@ class StorageDelegate( parent.viewBinding(ItemStoragePreferenceBinding::inflate, false), storageCalculator, sharedPreferenceUtil, - isShowingStorageOptionForFirstDownload, + shouldShowCheckboxSelected, onClickAction ) } diff --git a/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageViewHolder.kt b/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageViewHolder.kt index ad686e5ba..05f6492d8 100644 --- a/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageViewHolder.kt +++ b/core/src/main/java/eu/mhutti1/utils/storage/adapter/StorageViewHolder.kt @@ -44,7 +44,7 @@ internal class StorageViewHolder( private val itemStoragePreferenceBinding: ItemStoragePreferenceBinding, private val storageCalculator: StorageCalculator, private val sharedPreferenceUtil: SharedPreferenceUtil, - private val isShowingStorageOptionForFirstDownload: Boolean, + private val shouldShowCheckboxSelected: Boolean, private val onClickAction: (StorageDevice) -> Unit ) : BaseViewHolder(itemStoragePreferenceBinding.root) { @@ -60,7 +60,7 @@ internal class StorageViewHolder( ) ) - radioButton.isChecked = !isShowingStorageOptionForFirstDownload && + radioButton.isChecked = shouldShowCheckboxSelected && adapterPosition == sharedPreferenceUtil.storagePosition freeSpace.apply { text = item.getFreeSpace(root.context, storageCalculator) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.kt index 29795da46..10dabe249 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.kt @@ -257,11 +257,11 @@ class SharedPreferenceUtil @Inject constructor(val context: Context) { _textZooms.offer(textZoom) } - var copyMoveZimFilePermissionDialog: Boolean - get() = sharedPreferences.getBoolean(PREF_COPY_MOVE_PERMISSION, false) + var shouldShowStorageSelectionDialog: Boolean + get() = sharedPreferences.getBoolean(PREF_SHOW_COPY_MOVE_STORAGE_SELECTION_DIALOG, true) set(value) { sharedPreferences.edit { - putBoolean(PREF_COPY_MOVE_PERMISSION, value) + putBoolean(PREF_SHOW_COPY_MOVE_STORAGE_SELECTION_DIALOG, value) } } @@ -329,7 +329,7 @@ class SharedPreferenceUtil @Inject constructor(val context: Context) { const val PREF_HISTORY_MIGRATED = "pref_history_migrated" const val PREF_NOTES_MIGRATED = "pref_notes_migrated" const val PREF_APP_DIRECTORY_TO_PUBLIC_MIGRATED = "pref_app_directory_to_public_migrated" - const val PREF_COPY_MOVE_PERMISSION = "pref_copy_move_permission" + const val PREF_SHOW_COPY_MOVE_STORAGE_SELECTION_DIALOG = "pref_show_copy_move_storage_dialog" private const val PREF_LATER_CLICKED_MILLIS = "pref_later_clicked_millis" const val PREF_LAST_DONATION_POPUP_SHOWN_IN_MILLISECONDS = "pref_last_donation_shown_in_milliseconds" diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt index 8f2aa2b32..1676041df 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/KiwixDialog.kt @@ -123,6 +123,7 @@ sealed class KiwixDialog( R.string.copy_move_files_dialog_description, R.string.action_copy, R.string.move, + neutralMessage = R.string.cancel, cancelable = false ) diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml index fb7f7c8c7..866cc936f 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -331,6 +331,7 @@ Error in copying the ZIM file: %s. Move/Copy files to app public directory? Due to Google Play policies on Android 11 and above, our app can no longer directly access files stored elsewhere on your device. To let you view your selected files, we need to move or copy them into a special folder within our application directory. This allows us to access and open the files. Do you agree to this? + Choose storage to copy/move ZIM file Error in moving the ZIM file: %s. How to update content? To update content (a ZIM file) you need to download the full latest version of this very same content. You can do that via the download section.