For mozilla-mobile#3213: Fix multi-select bookmarks on return to frag…

…ment
colintheshots · Jun 13, 2019 · e542ba9 · e542ba9
1 parent 5eb2bc6
commit e542ba9
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkAdapter.kt
@@ -34,19 +34,19 @@ class BookmarkAdapter(val emptyView: View, val actionEmitter: Observer<BookmarkA
 
     private var tree: List<BookmarkNode> = listOf()
     private var mode: BookmarkState.Mode = BookmarkState.Mode.Normal
-    var selected = setOf<BookmarkNode>()
+    val selected: Set<BookmarkNode>
+        get() = (mode as? BookmarkState.Mode.Selecting)?.selectedItems ?: setOf()
     private var isFirstRun = true
 
     lateinit var job: Job
 
     fun updateData(tree: BookmarkNode?, mode: BookmarkState.Mode) {
-        this.tree = tree?.children?.filterNotNull() ?: listOf()
+        this.tree = tree?.children ?: listOf()
         isFirstRun = if (isFirstRun) false else {
             emptyView.visibility = if (this.tree.isEmpty()) View.VISIBLE else View.GONE
             false
         }
         this.mode = mode
-        this.selected = if (mode is BookmarkState.Mode.Selecting) mode.selectedItems else setOf()
         notifyDataSetChanged()
     }
 
@@ -137,9 +137,9 @@ class BookmarkAdapter(val emptyView: View, val actionEmitter: Observer<BookmarkA
             val shiftTwoDp = TypedValue.applyDimension(
                 TypedValue.COMPLEX_UNIT_DIP, TWO_DIGIT_MARGIN, containerView!!.context.resources.displayMetrics
             ).toInt()
-            val params = bookmark_title.getLayoutParams() as ViewGroup.MarginLayoutParams
+            val params = bookmark_title.layoutParams as ViewGroup.MarginLayoutParams
             params.topMargin = shiftTwoDp
-            bookmark_title.setLayoutParams(params)
+            bookmark_title.layoutParams = params
 
             bookmark_favicon.visibility = View.VISIBLE
             bookmark_title.visibility = View.VISIBLE

diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkComponent.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkComponent.kt
@@ -56,6 +56,7 @@ sealed class BookmarkAction : Action {
     data class Delete(val item: BookmarkNode) : BookmarkAction()
     object BackPressed : BookmarkAction()
     object SwitchMode : BookmarkAction()
+    object DeselectAll : BookmarkAction()
 }
 
 sealed class BookmarkChange : Change {

diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt
@@ -275,11 +275,10 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
                         }
                     }
                     is BookmarkAction.SwitchMode -> {
-                        if ((bookmarkComponent.uiView as BookmarkUIView).mode is BookmarkState.Mode.Normal) {
-                            getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.ClearSelection)
-                        }
                         activity?.invalidateOptionsMenu()
                     }
+                    is BookmarkAction.DeselectAll ->
+                        getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.ClearSelection)
                 }
             }
 

diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkUIView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkUIView.kt
@@ -65,20 +65,16 @@ class BookmarkUIView(
             mode = it.mode
             actionEmitter.onNext(BookmarkAction.SwitchMode)
         }
-        bookmarkAdapter.updateData(it.tree, it.mode)
-        when (val modeCopy = mode) {
+        when (val modeCopy = it.mode) {
             is BookmarkState.Mode.Normal -> setUIForNormalMode(it.tree)
-            is BookmarkState.Mode.Selecting -> setUIForSelectingMode(modeCopy)
+            is BookmarkState.Mode.Selecting -> setUIForSelectingMode(it.tree, modeCopy)
         }
     }
 
     override fun onBackPressed(): Boolean {
         return when {
             mode is BookmarkState.Mode.Selecting -> {
-                mode = BookmarkState.Mode.Normal
-                bookmarkAdapter.updateData(tree, mode)
-                setUIForNormalMode(tree)
-                actionEmitter.onNext(BookmarkAction.SwitchMode)
+                actionEmitter.onNext(BookmarkAction.DeselectAll)
                 true
             }
             canGoBack -> {
@@ -107,8 +103,10 @@ class BookmarkUIView(
     }
 
     private fun setUIForSelectingMode(
+        root: BookmarkNode?,
         mode: BookmarkState.Mode.Selecting
     ) {
+        bookmarkAdapter.updateData(root, mode)
         activity?.title =
             context.getString(R.string.bookmarks_multi_select_title, mode.selectedItems.size)
         setToolbarColors(
@@ -118,6 +116,7 @@ class BookmarkUIView(
     }
 
     private fun setUIForNormalMode(root: BookmarkNode?) {
+        bookmarkAdapter.updateData(root, BookmarkState.Mode.Normal)
         setTitle(root)
         setToolbarColors(
             R.attr.primaryText.getColorIntFromAttr(context!!),