Browse Source

Merge pull request #11736 from nextcloud/fix/singlefilefragment-actions

Fix file actions available in BottomSheet
Andy Scherzinger 2 years ago
parent
commit
c98df3285d

+ 1 - 1
app/detekt.yml

@@ -69,7 +69,7 @@ complexity:
     excludes: ['**/androidTest/**']
   LongParameterList:
     active: true
-    functionThreshold: 6
+    functionThreshold: 7
     constructorThreshold: 6
     ignoreDefaultParameters: false
   MethodOverloading:

+ 75 - 0
app/src/androidTest/java/com/owncloud/android/files/FileMenuFilterIT.kt

@@ -269,6 +269,81 @@ class FileMenuFilterIT : AbstractIT() {
         }
     }
 
+    @Test
+    fun filter_select_all() {
+        configureCapability(OCCapability())
+
+        // not in single file fragment -> multi selection is possible under certain circumstances
+
+        launchActivity<TestActivity>().use {
+            it.onActivity { activity ->
+                val filterFactory = FileMenuFilter.Factory(mockStorageManager, activity, editorUtils)
+
+                val files = listOf(OCFile("/foo.bin"), OCFile("/bar.bin"), OCFile("/baz.bin"))
+
+                // single file, not in multi selection
+                // *Select all* and *Deselect all* should stay hidden
+                var sut = filterFactory.newInstance(files.first(), mockComponentsGetter, true, user)
+
+                var toHide = sut.getToHide(false)
+                assertTrue(toHide.contains(R.id.action_select_all_action_menu))
+                assertTrue(toHide.contains(R.id.action_deselect_all_action_menu))
+
+                // multiple files, all selected in multi selection
+                // *Deselect all* shown, *Select all* not
+                sut = filterFactory.newInstance(files.size, files, mockComponentsGetter, false, user)
+
+                toHide = sut.getToHide(false)
+                assertTrue(toHide.contains(R.id.action_select_all_action_menu))
+                assertFalse(toHide.contains(R.id.action_deselect_all_action_menu))
+
+                // multiple files, all but one selected
+                // both *Select all* and *Deselect all* should be shown
+                sut = filterFactory.newInstance(files.size + 1, files, mockComponentsGetter, false, user)
+
+                toHide = sut.getToHide(false)
+                assertFalse(toHide.contains(R.id.action_select_all_action_menu))
+                assertFalse(toHide.contains(R.id.action_deselect_all_action_menu))
+            }
+        }
+    }
+
+    fun filter_select_all_singleFileFragment() {
+        configureCapability(OCCapability())
+
+        // in single file fragment (e.g. FileDetailFragment or PreviewImageFragment), selecting multiple files
+        // is not possible -> *Select all* and *Deselect all* options should be hidden
+
+        launchActivity<TestActivity>().use {
+            it.onActivity { activity ->
+                val filterFactory = FileMenuFilter.Factory(mockStorageManager, activity, editorUtils)
+
+                val files = listOf(OCFile("/foo.bin"), OCFile("/bar.bin"), OCFile("/baz.bin"))
+
+                // single file
+                var sut = filterFactory.newInstance(files.first(), mockComponentsGetter, true, user)
+
+                var toHide = sut.getToHide(true)
+                assertTrue(toHide.contains(R.id.action_select_all_action_menu))
+                assertTrue(toHide.contains(R.id.action_deselect_all_action_menu))
+
+                // multiple files, all selected
+                sut = filterFactory.newInstance(files.size, files, mockComponentsGetter, false, user)
+
+                toHide = sut.getToHide(true)
+                assertTrue(toHide.contains(R.id.action_select_all_action_menu))
+                assertTrue(toHide.contains(R.id.action_deselect_all_action_menu))
+
+                // multiple files, all but one selected
+                sut = filterFactory.newInstance(files.size + 1, files, mockComponentsGetter, false, user)
+
+                toHide = sut.getToHide(true)
+                assertTrue(toHide.contains(R.id.action_select_all_action_menu))
+                assertTrue(toHide.contains(R.id.action_deselect_all_action_menu))
+            }
+        }
+    }
+
     private data class ExpectedLockVisibilities(
         val lockFile: Boolean,
         val unlockFile: Boolean

+ 5 - 3
app/src/main/java/com/nextcloud/ui/fileactions/FileActionsBottomSheet.kt

@@ -324,7 +324,7 @@ class FileActionsBottomSheet : BottomSheetDialogFragment(), Injectable {
             @IdRes
             additionalToHide: List<Int>? = null
         ): FileActionsBottomSheet {
-            return newInstance(1, listOf(file), isOverflow, additionalToHide)
+            return newInstance(1, listOf(file), isOverflow, additionalToHide, true)
         }
 
         @JvmStatic
@@ -334,13 +334,15 @@ class FileActionsBottomSheet : BottomSheetDialogFragment(), Injectable {
             files: Collection<OCFile>,
             isOverflow: Boolean,
             @IdRes
-            additionalToHide: List<Int>? = null
+            additionalToHide: List<Int>? = null,
+            inSingleFileFragment: Boolean = false
         ): FileActionsBottomSheet {
             return FileActionsBottomSheet().apply {
                 val argsBundle = bundleOf(
                     FileActionsViewModel.ARG_ALL_FILES_COUNT to numberOfAllFiles,
                     FileActionsViewModel.ARG_FILES to ArrayList<OCFile>(files),
-                    FileActionsViewModel.ARG_IS_OVERFLOW to isOverflow
+                    FileActionsViewModel.ARG_IS_OVERFLOW to isOverflow,
+                    FileActionsViewModel.ARG_IN_SINGLE_FILE_FRAGMENT to inSingleFileFragment
                 )
                 additionalToHide?.let {
                     argsBundle.putIntArray(FileActionsViewModel.ARG_ADDITIONAL_FILTER, additionalToHide.toIntArray())

+ 13 - 6
app/src/main/java/com/nextcloud/ui/fileactions/FileActionsViewModel.kt

@@ -69,17 +69,21 @@ class FileActionsViewModel @Inject constructor(
         @IdRes
         get() = _clickActionId
 
-    fun load(arguments: Bundle, componentsGetter: ComponentsGetter) {
+    fun load(
+        arguments: Bundle,
+        componentsGetter: ComponentsGetter
+    ) {
         val files: List<OCFile>? = arguments.getParcelableArrayList(ARG_FILES)
         val numberOfAllFiles: Int = arguments.getInt(ARG_ALL_FILES_COUNT, 1)
         val isOverflow = arguments.getBoolean(ARG_IS_OVERFLOW, false)
         val additionalFilter: IntArray? = arguments.getIntArray(ARG_ADDITIONAL_FILTER)
+        val inSingleFileFragment = arguments.getBoolean(ARG_IN_SINGLE_FILE_FRAGMENT)
 
         if (files.isNullOrEmpty()) {
             logger.d(TAG, "No valid files argument for loading actions")
             _uiState.postValue(UiState.Error)
         } else {
-            load(componentsGetter, files.toList(), numberOfAllFiles, isOverflow, additionalFilter)
+            load(componentsGetter, files.toList(), numberOfAllFiles, isOverflow, additionalFilter, inSingleFileFragment)
         }
     }
 
@@ -88,10 +92,11 @@ class FileActionsViewModel @Inject constructor(
         files: Collection<OCFile>,
         numberOfAllFiles: Int?,
         isOverflow: Boolean?,
-        additionalFilter: IntArray?
+        additionalFilter: IntArray?,
+        inSingleFileFragment: Boolean = false
     ) {
         viewModelScope.launch(Dispatchers.IO) {
-            val toHide = getHiddenActions(componentsGetter, numberOfAllFiles, files, isOverflow)
+            val toHide = getHiddenActions(componentsGetter, numberOfAllFiles, files, isOverflow, inSingleFileFragment)
             val availableActions = getActionsToShow(additionalFilter, toHide)
             updateStateLoaded(files, availableActions)
         }
@@ -101,7 +106,8 @@ class FileActionsViewModel @Inject constructor(
         componentsGetter: ComponentsGetter,
         numberOfAllFiles: Int?,
         files: Collection<OCFile>,
-        isOverflow: Boolean?
+        isOverflow: Boolean?,
+        inSingleFileFragment: Boolean
     ): List<Int> {
         return filterFactory.newInstance(
             numberOfAllFiles ?: 1,
@@ -110,7 +116,7 @@ class FileActionsViewModel @Inject constructor(
             isOverflow ?: false,
             currentAccountProvider.user
         )
-            .getToHide(false)
+            .getToHide(inSingleFileFragment)
     }
 
     private fun getActionsToShow(
@@ -161,6 +167,7 @@ class FileActionsViewModel @Inject constructor(
         const val ARG_FILES = "FILES"
         const val ARG_IS_OVERFLOW = "OVERFLOW"
         const val ARG_ADDITIONAL_FILTER = "ADDITIONAL_FILTER"
+        const val ARG_IN_SINGLE_FILE_FRAGMENT = "IN_SINGLE_FILE_FRAGMENT"
 
         private val TAG = FileActionsViewModel::class.simpleName!!
     }

+ 16 - 11
app/src/main/java/com/owncloud/android/files/FileMenuFilter.java

@@ -185,23 +185,19 @@ public class FileMenuFilter {
         return toHide;
     }
 
+
     private void filterShareFile(List<Integer> toHide, OCCapability capability) {
-        if (containsEncryptedFile() || (!isShareViaLinkAllowed() && !isShareWithUsersAllowed()) ||
-            !isSingleSelection() || !isShareApiEnabled(capability) || !files.iterator().next().canReshare()
-            || overflowMenu) {
+        if (!isSingleSelection() || containsEncryptedFile() ||
+            (!isShareViaLinkAllowed() && !isShareWithUsersAllowed()) ||
+            !isShareApiEnabled(capability) || !files.iterator().next().canReshare()) {
             toHide.add(R.id.action_send_share_file);
         }
     }
 
     private void filterSendFiles(List<Integer> toHide, boolean inSingleFileFragment) {
-        boolean show = true;
-        if (overflowMenu || SEND_OFF.equalsIgnoreCase(context.getString(R.string.send_files_to_other_apps)) || containsEncryptedFile()) {
-            show = false;
-        }
-        if (!inSingleFileFragment && (isSingleSelection() || !anyFileDown())) {
-            show = false;
-        }
-        if (!show) {
+        if ((overflowMenu || SEND_OFF.equalsIgnoreCase(context.getString(R.string.send_files_to_other_apps)) || containsEncryptedFile()) ||
+            (!inSingleFileFragment && (isSingleSelection() || !allFileDown())) ||
+            !toHide.contains(R.id.action_send_share_file)) {
             toHide.add(R.id.action_send_file);
         }
     }
@@ -543,6 +539,15 @@ public class FileMenuFilter {
         return false;
     }
 
+    private boolean allFileDown() {
+        for (OCFile file: files) {
+            if(!file.isDown()) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     private boolean allFavorites() {
         for (OCFile file : files) {
             if (!file.isFavorite()) {

+ 2 - 1
app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java

@@ -282,7 +282,8 @@ public class FileDetailFragment extends FileFragment implements OnClickListener,
                 R.id.action_copy,
                 R.id.action_stream_media,
                 R.id.action_send_share_file,
-                R.id.action_select_all_action_menu));
+                R.id.action_pin_to_homescreen
+                         ));
         if (getFile().isFolder()) {
             additionalFilter.add(R.id.action_send_file);
             additionalFilter.add(R.id.action_sync_file);

+ 2 - 2
app/src/main/java/com/owncloud/android/ui/preview/PreviewImageFragment.java

@@ -375,11 +375,11 @@ public class PreviewImageFragment extends FileFragment implements Injectable {
             Arrays.asList(
                 R.id.action_rename_file,
                 R.id.action_sync_file,
-                R.id.action_select_all,
                 R.id.action_move,
                 R.id.action_copy,
                 R.id.action_favorite,
-                R.id.action_unset_favorite
+                R.id.action_unset_favorite,
+                R.id.action_pin_to_homescreen
                          ));
         if (getFile() != null && getFile().isSharedWithMe() && !getFile().canReshare()) {
             additionalFilter.add(R.id.action_send_share_file);

+ 0 - 1
app/src/main/java/com/owncloud/android/ui/preview/PreviewMediaFragment.java

@@ -424,7 +424,6 @@ public class PreviewMediaFragment extends FileFragment implements OnTouchListene
             Arrays.asList(
                 R.id.action_rename_file,
                 R.id.action_sync_file,
-                R.id.action_select_all,
                 R.id.action_move,
                 R.id.action_copy,
                 R.id.action_favorite,

+ 2 - 2
app/src/main/java/com/owncloud/android/ui/preview/PreviewTextFileFragment.java

@@ -300,11 +300,11 @@ public class PreviewTextFileFragment extends PreviewTextFragment {
             Arrays.asList(
                 R.id.action_rename_file,
                 R.id.action_sync_file,
-                R.id.action_select_all,
                 R.id.action_move,
                 R.id.action_copy,
                 R.id.action_favorite,
-                R.id.action_unset_favorite
+                R.id.action_unset_favorite,
+                R.id.action_pin_to_homescreen
                          ));
         if (getFile() != null && getFile().isSharedWithMe() && !getFile().canReshare()) {
             additionalFilter.add(R.id.action_send_share_file);

+ 1 - 1
scripts/analysis/lint-results.txt

@@ -1,2 +1,2 @@
 DO NOT TOUCH; GENERATED BY DRONE
-      <span class="mdl-layout-title">Lint Report: 77 warnings</span>
+      <span class="mdl-layout-title">Lint Report: 79 warnings</span>