For #12831 - Disable SwipeRefreshLayout while swiping a bookmark.

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -45,6 +45,8 @@ interface BookmarkController {
     fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>)
     fun handleRequestSync()
     fun handleBackPressed()
+    fun handleStartSwipingItem()
+    fun handleStopSwipingItem()
 }
 
 @Suppress("TooManyFunctions")
@@ -169,6 +171,14 @@ class DefaultBookmarkController(
         }
     }
 
+    override fun handleStartSwipingItem() {
+        store.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(false))
+    }
+
+    override fun handleStopSwipingItem() {
+        store.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(true))
+    }
+
     private fun openInNewTab(
         searchTermOrURL: String,
         newTab: Boolean,

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt

@@ -120,4 +120,12 @@ class BookmarkFragmentInteractor(
     override fun onRequestSync() {
         bookmarksController.handleRequestSync()
     }
+
+    override fun onStartSwipingItem() {
+        bookmarksController.handleStartSwipingItem()
+    }
+
+    override fun onStopSwipingItem() {
+        bookmarksController.handleStopSwipingItem()
+    }
 }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt

@@ -23,12 +23,14 @@ class BookmarkFragmentStore(
  * @property guidBackstack A set of guids for bookmark nodes we have visited. Used to traverse back
  *                  up the tree after a sync.
  * @property isLoading true if bookmarks are still being loaded from disk
+ * @property isSwipeToRefreshEnabled true if swipe to refresh should be enabled
  */
 data class BookmarkFragmentState(
     val tree: BookmarkNode?,
     val mode: Mode = Mode.Normal(),
     val guidBackstack: List<String> = emptyList(),
-    val isLoading: Boolean = true
+    val isLoading: Boolean = true,
+    val isSwipeToRefreshEnabled: Boolean = true
 ) : State {
     sealed class Mode {
         open val selectedItems = emptySet<BookmarkNode>()
@@ -49,6 +51,7 @@ sealed class BookmarkFragmentAction : Action {
     object DeselectAll : BookmarkFragmentAction()
     object StartSync : BookmarkFragmentAction()
     object FinishSync : BookmarkFragmentAction()
+    data class SwipeRefreshAvailabilityChanged(val enabled: Boolean) : BookmarkFragmentAction()
 }
 
 /**
@@ -71,31 +74,37 @@ private fun bookmarkFragmentStateReducer(
             } + action.tree.guid
 
             val items = state.mode.selectedItems.filter { it in action.tree }
+            val mode = when {
+                state.mode is BookmarkFragmentState.Mode.Syncing -> {
+                    BookmarkFragmentState.Mode.Syncing
+                }
+                items.isEmpty() -> {
+                    BookmarkFragmentState.Mode.Normal(shouldShowMenu(action.tree.guid))
+                }
+                else -> BookmarkFragmentState.Mode.Selecting(items.toSet())
+            }
             state.copy(
                 tree = action.tree,
-                mode = when {
+                mode = mode,
-                    state.mode is BookmarkFragmentState.Mode.Syncing -> {
-                        BookmarkFragmentState.Mode.Syncing
-                    }
-                    items.isEmpty() -> {
-                        BookmarkFragmentState.Mode.Normal(shouldShowMenu(action.tree.guid))
-                    }
-                    else -> BookmarkFragmentState.Mode.Selecting(items.toSet())
-                },
                 guidBackstack = backstack,
-                isLoading = false
+                isLoading = false,
+                isSwipeToRefreshEnabled = mode !is BookmarkFragmentState.Mode.Selecting
             )
         }
-        is BookmarkFragmentAction.Select ->
+        is BookmarkFragmentAction.Select -> state.copy(
-            state.copy(mode = BookmarkFragmentState.Mode.Selecting(state.mode.selectedItems + action.item))
+                mode = BookmarkFragmentState.Mode.Selecting(state.mode.selectedItems + action.item),
+                isSwipeToRefreshEnabled = false
+            )
         is BookmarkFragmentAction.Deselect -> {
             val items = state.mode.selectedItems - action.item
+            val mode = if (items.isEmpty()) {
+                BookmarkFragmentState.Mode.Normal()
+            } else {
+                BookmarkFragmentState.Mode.Selecting(items)
+            }
             state.copy(
-                mode = if (items.isEmpty()) {
+                mode = mode,
-                    BookmarkFragmentState.Mode.Normal()
+                isSwipeToRefreshEnabled = mode !is BookmarkFragmentState.Mode.Selecting
-                } else {
-                    BookmarkFragmentState.Mode.Selecting(items)
-                }
             )
         }
         is BookmarkFragmentAction.DeselectAll ->
@@ -104,18 +113,22 @@ private fun bookmarkFragmentStateReducer(
                     BookmarkFragmentState.Mode.Syncing
                 } else {
                     BookmarkFragmentState.Mode.Normal()
-                }
+                },
-            )
+                isSwipeToRefreshEnabled = true
-        is BookmarkFragmentAction.StartSync ->
-            state.copy(
-                mode = BookmarkFragmentState.Mode.Syncing
-            )
-        is BookmarkFragmentAction.FinishSync ->
-            state.copy(
-                mode = BookmarkFragmentState.Mode.Normal(
-                    showMenu = shouldShowMenu(state.tree?.guid)
-                )
             )
+        is BookmarkFragmentAction.StartSync -> state.copy(
+            mode = BookmarkFragmentState.Mode.Syncing,
+            isSwipeToRefreshEnabled = true
+        )
+        is BookmarkFragmentAction.FinishSync -> state.copy(
+            mode = BookmarkFragmentState.Mode.Normal(
+                showMenu = shouldShowMenu(state.tree?.guid)
+            ),
+            isSwipeToRefreshEnabled = true
+        )
+        is BookmarkFragmentAction.SwipeRefreshAvailabilityChanged -> state.copy(
+            isSwipeToRefreshEnabled = action.enabled && state.mode !is BookmarkFragmentState.Mode.Selecting
+        )
     }
 }
 

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkTouchHelper.kt

@@ -116,6 +116,15 @@ class BookmarkTouchCallback(
         target: RecyclerView.ViewHolder
     ): Boolean = false
 
+    override fun onSelectedChanged(viewHolder: RecyclerView.ViewHolder?, actionState: Int) {
+        super.onSelectedChanged(viewHolder, actionState)
+        if (actionState == ItemTouchHelper.ACTION_STATE_SWIPE) {
+            interactor.onStartSwipingItem()
+        } else {
+            interactor.onStopSwipingItem()
+        }
+    }
+
     private fun setBounds(
         background: Drawable,
         backgroundBounds: Rect,

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt

@@ -98,6 +98,16 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
      *
      */
     fun onRequestSync()
+
+    /**
+     * Handles the start of a swipe on a bookmark.
+     */
+    fun onStartSwipingItem()
+
+    /**
+     * Handles the end of a swipe on a bookmark.
+     */
+    fun onStopSwipingItem()
 }
 
 class BookmarkView(
@@ -152,8 +162,7 @@ class BookmarkView(
             }
         }
         view.bookmarks_progress_bar.isVisible = state.isLoading
-        view.swipe_refresh.isEnabled =
+        view.swipe_refresh.isEnabled = state.isSwipeToRefreshEnabled
-            state.mode is BookmarkFragmentState.Mode.Normal || state.mode is BookmarkFragmentState.Mode.Syncing
         view.swipe_refresh.isRefreshing = state.mode is BookmarkFragmentState.Mode.Syncing
     }
 

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -337,4 +337,22 @@ class BookmarkControllerTest {
             navController.popBackStack()
         }
     }
+
+    @Test
+    fun `handleStartSwipingItem disables swipe to refresh`() {
+        controller.handleStartSwipingItem()
+
+        verify {
+            bookmarkStore.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(false))
+        }
+    }
+
+    @Test
+    fun `handleStopSwipingItem attempts to enable swipe to refresh`() {
+        controller.handleStopSwipingItem()
+
+        verify {
+            bookmarkStore.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(true))
+        }
+    }
 }

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt

@@ -210,4 +210,22 @@ class BookmarkFragmentInteractorTest {
             bookmarkController.handleRequestSync()
         }
     }
+
+    @Test
+    fun `start swiping an item`() {
+        interactor.onStartSwipingItem()
+
+        verify {
+            bookmarkController.handleStartSwipingItem()
+        }
+    }
+
+    @Test
+    fun `stop swiping an item`() {
+        interactor.onStopSwipingItem()
+
+        verify {
+            bookmarkController.handleStopSwipingItem()
+        }
+    }
 }

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStoreTest.kt

@@ -80,32 +80,63 @@ class BookmarkFragmentStoreTest {
 
     @Test
     fun `ensure selected items remain selected after a tree change`() = runBlocking {
-        val initialState = BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Selecting(setOf(item, subfolder)))
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Selecting(setOf(item, subfolder)),
+            isLoading = false,
+            isSwipeToRefreshEnabled = false
+        )
         val store = BookmarkFragmentStore(initialState)
 
         store.dispatch(BookmarkFragmentAction.Change(newTree)).join()
 
-        assertEquals(store.state.tree, newTree)
+        assertEquals(
-        assertEquals(store.state.mode, BookmarkFragmentState.Mode.Selecting(setOf(subfolder)))
+            store.state,
+            BookmarkFragmentState(
+                newTree,
+                BookmarkFragmentState.Mode.Selecting(setOf(subfolder)),
+                guidBackstack = listOf(tree.guid),
+                isLoading = false,
+                isSwipeToRefreshEnabled = false
+            )
+        )
     }
 
     @Test
-    fun `select and deselect bookmarks changes the mode`() = runBlocking {
+    fun `select and deselect a single bookmark changes the mode and swipe to refresh state`() = runBlocking {
         val initialState = BookmarkFragmentState(tree)
         val store = BookmarkFragmentStore(initialState)
 
         store.dispatch(BookmarkFragmentAction.Select(childItem)).join()
 
-        assertEquals(store.state, BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Selecting(setOf(childItem))))
+        assertEquals(
+            store.state,
+            BookmarkFragmentState(
+                tree,
+                BookmarkFragmentState.Mode.Selecting(setOf(childItem)),
+                isSwipeToRefreshEnabled = false
+            )
+        )
 
         store.dispatch(BookmarkFragmentAction.Deselect(childItem)).join()
 
-        assertEquals(store.state, BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Normal()))
+        assertEquals(
+            store.state,
+            BookmarkFragmentState(
+                tree,
+                BookmarkFragmentState.Mode.Normal(),
+                isSwipeToRefreshEnabled = true
+            )
+        )
     }
 
     @Test
     fun `selecting the same item twice does nothing`() = runBlocking {
-        val initialState = BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Selecting(setOf(item, subfolder)))
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Selecting(setOf(item, subfolder)),
+            isSwipeToRefreshEnabled = false
+        )
         val store = BookmarkFragmentStore(initialState)
 
         store.dispatch(BookmarkFragmentAction.Select(item)).join()
@@ -115,7 +146,11 @@ class BookmarkFragmentStoreTest {
 
     @Test
     fun `deselecting an unselected bookmark does nothing`() = runBlocking {
-        val initialState = BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Selecting(setOf(childItem)))
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Selecting(setOf(childItem)),
+            isSwipeToRefreshEnabled = false
+        )
         val store = BookmarkFragmentStore(initialState)
 
         store.dispatch(BookmarkFragmentAction.Deselect(item)).join()
@@ -134,14 +169,25 @@ class BookmarkFragmentStoreTest {
     }
 
     @Test
-    fun `deselect all bookmarks changes the mode`() = runBlocking {
+    fun `deselect all bookmarks changes the mode and updates swipe to refresh state`() =
-        val initialState = BookmarkFragmentState(tree, BookmarkFragmentState.Mode.Selecting(setOf(item, childItem)))
+        runBlocking {
-        val store = BookmarkFragmentStore(initialState)
+            val initialState = BookmarkFragmentState(
+                tree,
+                BookmarkFragmentState.Mode.Selecting(setOf(item, childItem)),
+                isSwipeToRefreshEnabled = false
+            )
+            val store = BookmarkFragmentStore(initialState)
 
-        store.dispatch(BookmarkFragmentAction.DeselectAll).join()
+            store.dispatch(BookmarkFragmentAction.DeselectAll).join()
 
-        assertEquals(store.state, initialState.copy(mode = BookmarkFragmentState.Mode.Normal()))
+            assertEquals(
-    }
+                store.state,
+                initialState.copy(
+                    mode = BookmarkFragmentState.Mode.Normal(),
+                    isSwipeToRefreshEnabled = true
+                )
+            )
+        }
 
     @Test
     fun `deselect all bookmarks when none are selected`() = runBlocking {
@@ -214,6 +260,45 @@ class BookmarkFragmentStoreTest {
         assertEquals(BookmarkFragmentState.Mode.Syncing, store.state.mode)
     }
 
+    @Test
+    fun `enabling swipe to refresh in Normal mode works`() = runBlocking {
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Normal(),
+            isSwipeToRefreshEnabled = false
+        )
+        val store = BookmarkFragmentStore(initialState)
+
+        store.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(true)).join()
+        assertEquals(true, store.state.isSwipeToRefreshEnabled)
+    }
+
+    @Test
+    fun `enabling swipe to refresh in Syncing mode works`() = runBlocking {
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Syncing,
+            isSwipeToRefreshEnabled = false
+        )
+        val store = BookmarkFragmentStore(initialState)
+
+        store.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(true)).join()
+        assertEquals(true, store.state.isSwipeToRefreshEnabled)
+    }
+
+    @Test
+    fun `enabling swipe to refresh in Selecting mode does not work`() = runBlocking {
+        val initialState = BookmarkFragmentState(
+            tree,
+            BookmarkFragmentState.Mode.Selecting(emptySet()),
+            isSwipeToRefreshEnabled = false
+        )
+        val store = BookmarkFragmentStore(initialState)
+
+        store.dispatch(BookmarkFragmentAction.SwipeRefreshAvailabilityChanged(true)).join()
+        assertEquals(false, store.state.isSwipeToRefreshEnabled)
+    }
+
     private val item = BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
     private val separator = BookmarkNode(BookmarkNodeType.SEPARATOR, "789", "123", 1, null, null, null)
     private val subfolder = BookmarkNode(BookmarkNodeType.FOLDER, "987", "123", 0, "Subfolder", null, listOf())