From 1ad9da09b0df0e91da52816c4268a6dcdde626fd Mon Sep 17 00:00:00 2001 From: Kainalu Hagiwara Date: Thu, 6 Aug 2020 12:22:53 -0700 Subject: [PATCH] Remove selection holder from bookmark viewholder constructors. Now that we're passing the mode to the viewholders in their bind methods, there's no real need to pass them into their constructors. This also allows us to remove the indirection of having the adapter implement the SelectionHolder interface and have the mode implement it directly. --- .../fenix/library/bookmarks/BookmarkAdapter.kt | 10 ++++------ .../library/bookmarks/BookmarkFragmentStore.kt | 5 +++-- .../viewholders/BookmarkFolderViewHolder.kt | 8 +++----- .../viewholders/BookmarkItemViewHolder.kt | 8 +++----- .../viewholders/BookmarkFolderViewHolderTest.kt | 6 +----- .../viewholders/BookmarkItemViewHolderTest.kt | 17 +++++++---------- 6 files changed, 21 insertions(+), 33 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 index e191bffa3..a91bc6277 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkAdapter.kt @@ -16,18 +16,16 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import org.mozilla.fenix.R import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkFolderViewHolder import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkItemViewHolder import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkNodeViewHolder import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkSeparatorViewHolder -class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteractor) : - RecyclerView.Adapter(), SelectionHolder { +class BookmarkAdapter(private val emptyView: View, private val interactor: BookmarkViewInteractor) : + RecyclerView.Adapter() { private var tree: List = listOf() private var mode: BookmarkFragmentState.Mode = BookmarkFragmentState.Mode.Normal() - override val selectedItems: Set get() = mode.selectedItems private var isFirstRun = true fun updateData(tree: BookmarkNode?, mode: BookmarkFragmentState.Mode) { @@ -85,8 +83,8 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto .inflate(R.layout.bookmark_list_item, parent, false) as LibrarySiteItemView return when (viewType) { - LibrarySiteItemView.ItemType.SITE.ordinal -> BookmarkItemViewHolder(view, interactor, this) - LibrarySiteItemView.ItemType.FOLDER.ordinal -> BookmarkFolderViewHolder(view, interactor, this) + LibrarySiteItemView.ItemType.SITE.ordinal -> BookmarkItemViewHolder(view, interactor) + LibrarySiteItemView.ItemType.FOLDER.ordinal -> BookmarkFolderViewHolder(view, interactor) LibrarySiteItemView.ItemType.SEPARATOR.ordinal -> BookmarkSeparatorViewHolder(view, interactor) else -> throw IllegalStateException("ViewType $viewType does not match to a ViewHolder") } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt index e1c4a8f43..a14a5c023 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt @@ -9,6 +9,7 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store +import org.mozilla.fenix.library.SelectionHolder class BookmarkFragmentStore( initialState: BookmarkFragmentState @@ -32,8 +33,8 @@ data class BookmarkFragmentState( val isLoading: Boolean = true, val isSwipeToRefreshEnabled: Boolean = true ) : State { - sealed class Mode { - open val selectedItems = emptySet() + sealed class Mode : SelectionHolder { + override val selectedItems = emptySet() data class Normal(val showMenu: Boolean = true) : Mode() data class Selecting(override val selectedItems: Set) : Mode() diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolder.kt index 70e418b98..8c6eda9af 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolder.kt @@ -12,7 +12,6 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState import org.mozilla.fenix.library.bookmarks.BookmarkPayload import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor @@ -23,8 +22,7 @@ import org.mozilla.fenix.library.bookmarks.inRoots */ class BookmarkFolderViewHolder( view: LibrarySiteItemView, - interactor: BookmarkViewInteractor, - private val selectionHolder: SelectionHolder + interactor: BookmarkViewInteractor ) : BookmarkNodeViewHolder(view, interactor) { override var item: BookmarkNode? = null @@ -43,7 +41,7 @@ class BookmarkFolderViewHolder( override fun bind(item: BookmarkNode, mode: BookmarkFragmentState.Mode, payload: BookmarkPayload) { this.item = item - setSelectionListeners(item, selectionHolder) + setSelectionListeners(item, mode) if (!item.inRoots()) { setupMenu(item) @@ -59,7 +57,7 @@ class BookmarkFolderViewHolder( } if (payload.selectedChanged) { - containerView.changeSelected(item in selectionHolder.selectedItems) + containerView.changeSelected(item in mode.selectedItems) } containerView.iconView.setImageDrawable( diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolder.kt index b97fe86c7..ec1a9a5b4 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolder.kt @@ -9,7 +9,6 @@ import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState import org.mozilla.fenix.library.bookmarks.BookmarkPayload import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor @@ -19,8 +18,7 @@ import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor */ class BookmarkItemViewHolder( view: LibrarySiteItemView, - interactor: BookmarkViewInteractor, - private val selectionHolder: SelectionHolder + interactor: BookmarkViewInteractor ) : BookmarkNodeViewHolder(view, interactor) { override var item: BookmarkNode? = null @@ -50,7 +48,7 @@ class BookmarkItemViewHolder( } if (payload.selectedChanged) { - containerView.changeSelected(item in selectionHolder.selectedItems) + containerView.changeSelected(item in mode.selectedItems) } if (payload.titleChanged) { @@ -64,7 +62,7 @@ class BookmarkItemViewHolder( setColorsAndIcons(item.url) } - setSelectionListeners(item, selectionHolder) + setSelectionListeners(item, mode) } @VisibleForTesting diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolderTest.kt index e622dbf20..64d1c914e 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkFolderViewHolderTest.kt @@ -18,7 +18,6 @@ import org.junit.Test import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkFragmentInteractor import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState import org.mozilla.fenix.library.bookmarks.BookmarkPayload @@ -29,8 +28,6 @@ class BookmarkFolderViewHolderTest { private lateinit var interactor: BookmarkFragmentInteractor @MockK(relaxed = true) private lateinit var siteItemView: LibrarySiteItemView - @MockK(relaxed = true) - private lateinit var selectionHolder: SelectionHolder private lateinit var holder: BookmarkFolderViewHolder private val folder = BookmarkNode( @@ -50,7 +47,7 @@ class BookmarkFolderViewHolderTest { mockkStatic(AppCompatResources::class) every { AppCompatResources.getDrawable(any(), any()) } returns mockk(relaxed = true) - holder = BookmarkFolderViewHolder(siteItemView, interactor, selectionHolder) + holder = BookmarkFolderViewHolder(siteItemView, interactor) } @Test @@ -63,7 +60,6 @@ class BookmarkFolderViewHolderTest { siteItemView.changeSelected(false) } - every { selectionHolder.selectedItems } returns setOf(folder) holder.bind(folder, BookmarkFragmentState.Mode.Selecting(setOf(folder))) verify { diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolderTest.kt index 36d64172b..0a1cd2f2d 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkItemViewHolderTest.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.library.bookmarks.viewholders import io.mockk.MockKAnnotations -import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.verify import mozilla.components.concept.storage.BookmarkNode @@ -15,7 +14,6 @@ import org.junit.Test import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkFragmentInteractor import org.mozilla.fenix.library.bookmarks.BookmarkFragmentState import org.mozilla.fenix.library.bookmarks.BookmarkPayload @@ -28,8 +26,6 @@ class BookmarkItemViewHolderTest { @MockK(relaxed = true) private lateinit var siteItemView: LibrarySiteItemView - @MockK(relaxed = true) - private lateinit var selectionHolder: SelectionHolder private lateinit var holder: BookmarkItemViewHolder private val item = BookmarkNode( @@ -45,15 +41,16 @@ class BookmarkItemViewHolderTest { @Before fun setup() { MockKAnnotations.init(this) - holder = BookmarkItemViewHolder(siteItemView, interactor, selectionHolder) + holder = BookmarkItemViewHolder(siteItemView, interactor) } @Test fun `binds views for unselected item`() { - holder.bind(item, BookmarkFragmentState.Mode.Normal()) + val mode = BookmarkFragmentState.Mode.Normal() + holder.bind(item, mode) verify { - siteItemView.setSelectionInteractor(item, selectionHolder, interactor) + siteItemView.setSelectionInteractor(item, mode, interactor) siteItemView.titleView.text = item.title siteItemView.urlView.text = item.url siteItemView.overflowView.showAndEnable() @@ -64,11 +61,11 @@ class BookmarkItemViewHolderTest { @Test fun `binds views for selected item`() { - every { selectionHolder.selectedItems } returns setOf(item) - holder.bind(item, BookmarkFragmentState.Mode.Selecting(setOf(item))) + val mode = BookmarkFragmentState.Mode.Selecting(setOf(item)) + holder.bind(item, mode) verify { - siteItemView.setSelectionInteractor(item, selectionHolder, interactor) + siteItemView.setSelectionInteractor(item, mode, interactor) siteItemView.titleView.text = item.title siteItemView.urlView.text = item.url siteItemView.overflowView.hideAndDisable()