diff --git a/app/src/main/java/org/mozilla/fenix/library/LibrarySiteItemView.kt b/app/src/main/java/org/mozilla/fenix/library/LibrarySiteItemView.kt index 2a6243ee8..239b9036e 100644 --- a/app/src/main/java/org/mozilla/fenix/library/LibrarySiteItemView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/LibrarySiteItemView.kt @@ -13,11 +13,45 @@ import android.widget.TextView import androidx.constraintlayout.widget.ConstraintLayout import androidx.core.view.isVisible import kotlinx.android.synthetic.main.library_site_item.view.* +import mozilla.components.browser.menu.BrowserMenu +import mozilla.components.browser.menu.BrowserMenuBuilder import org.mozilla.fenix.R import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.increaseTapArea import org.mozilla.fenix.ext.loadIntoView +/** + * Interactor for items that can be selected on the bookmarks and history screens. + */ +interface SelectionInteractor { + /** + * Called when an item is tapped to open it. + * @param item the tapped item to open. + */ + fun open(item: T) + + /** + * Called when an item is long pressed and selection mode is started, + * or when selection mode has already started an an item is tapped. + * @param item the item to select. + */ + fun select(item: T) + + /** + * Called when a selected item is tapped in selection mode and should no longer be selected. + * @param item the item to deselect. + */ + fun deselect(item: T) +} + +interface SelectionHolder { + val selectedItems: Set +} + +interface LibraryItemMenu { + val menuBuilder: BrowserMenuBuilder +} + class LibrarySiteItemView @JvmOverloads constructor( context: Context, attrs: AttributeSet? = null, @@ -63,6 +97,43 @@ class LibrarySiteItemView @JvmOverloads constructor( context.components.core.icons.loadIntoView(favicon, url) } + fun attachMenu(menu: LibraryItemMenu) { + overflow_menu.setOnClickListener { + menu.menuBuilder.build(context).show( + anchor = it, + orientation = BrowserMenu.Orientation.DOWN + ) + } + } + + fun setSelectionInteractor(item: T, holder: SelectionHolder, interactor: SelectionInteractor) { + setOnClickListener { + val selected = holder.selectedItems + when { + selected.isEmpty() -> interactor.open(item) + item in selected -> interactor.deselect(item) + else -> interactor.select(item) + } + } + + setOnLongClickListener { + if (holder.selectedItems.isEmpty()) { + interactor.select(item) + true + } else { + false + } + } + + favicon.setOnClickListener { + if (item in holder.selectedItems) { + interactor.deselect(item) + } else { + interactor.select(item) + } + } + } + enum class ItemType { SITE, FOLDER, SEPARATOR; } 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 7c4047b86..7a7fbca0c 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 @@ -8,24 +8,25 @@ import android.view.View import android.view.ViewGroup import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.view.ViewGroup.LayoutParams.WRAP_CONTENT +import androidx.core.view.isVisible import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType 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() { + RecyclerView.Adapter(), SelectionHolder { private var tree: List = listOf() private var mode: BookmarkState.Mode = BookmarkState.Mode.Normal - val selected: Set - get() = (mode as? BookmarkState.Mode.Selecting)?.selectedItems ?: setOf() + override val selectedItems: Set get() = mode.selectedItems private var isFirstRun = true fun updateData(tree: BookmarkNode?, mode: BookmarkState.Mode) { @@ -40,7 +41,7 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto this.tree = tree?.children ?: listOf() isFirstRun = if (isFirstRun) false else { - emptyView.visibility = if (this.tree.isEmpty()) View.VISIBLE else View.GONE + emptyView.isVisible = this.tree.isEmpty() false } this.mode = mode @@ -57,15 +58,8 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = old[oldItemPosition].guid == new[newItemPosition].guid - override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean { - val oldSelected = (oldMode as? BookmarkState.Mode.Selecting)?.selectedItems ?: setOf() - val newSelected = (newMode as? BookmarkState.Mode.Selecting)?.selectedItems ?: setOf() - val modesEqual = oldMode::class == newMode::class - val selectedEqual = - ((oldSelected.contains(old[oldItemPosition]) && newSelected.contains(new[newItemPosition])) || - (!oldSelected.contains(old[oldItemPosition]) && !newSelected.contains(new[newItemPosition]))) - return modesEqual && selectedEqual - } + override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean = + old[oldItemPosition] in oldMode.selectedItems == new[newItemPosition] in newMode.selectedItems override fun getOldListSize(): Int = old.size override fun getNewListSize(): Int = new.size @@ -77,12 +71,9 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto } return when (viewType) { - LibrarySiteItemView.ItemType.SITE.ordinal -> - BookmarkItemViewHolder(view, interactor) - LibrarySiteItemView.ItemType.FOLDER.ordinal -> - BookmarkFolderViewHolder(view, interactor) - LibrarySiteItemView.ItemType.SEPARATOR.ordinal -> - BookmarkSeparatorViewHolder(view, interactor) + LibrarySiteItemView.ItemType.SITE.ordinal -> BookmarkItemViewHolder(view, interactor, this) + LibrarySiteItemView.ItemType.FOLDER.ordinal -> BookmarkFolderViewHolder(view, interactor, this) + LibrarySiteItemView.ItemType.SEPARATOR.ordinal -> BookmarkSeparatorViewHolder(view, interactor) else -> throw IllegalStateException("ViewType $viewType does not match to a ViewHolder") } } @@ -99,11 +90,7 @@ class BookmarkAdapter(val emptyView: View, val interactor: BookmarkViewInteracto override fun getItemCount(): Int = tree.size override fun onBindViewHolder(holder: BookmarkNodeViewHolder, position: Int) { - holder.bind( - tree[position], - mode, - tree[position] in selected - ) + holder.bind(tree[position]) } } 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 index 1f96d43f2..98b483db8 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -153,7 +153,7 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou } override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { - when (val mode = bookmarkView.mode) { + when (val mode = bookmarkStore.state.mode) { BookmarkState.Mode.Normal -> { inflater.inflate(R.menu.bookmarks_menu, menu) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt index 57da3da32..75c4041c2 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt @@ -51,24 +51,26 @@ class BookmarkFragmentInteractor( } override fun open(item: BookmarkNode) { - require(item.type == BookmarkNodeType.ITEM) - item.url?.let { url -> - activity!! - .openToBrowserAndLoad( - searchTermOrURL = url, - newTab = true, - from = BrowserDirection.FromBookmarks + when (item.type) { + BookmarkNodeType.ITEM -> { + item.url?.let { url -> + activity!! + .openToBrowserAndLoad( + searchTermOrURL = url, + newTab = true, + from = BrowserDirection.FromBookmarks + ) + } + metrics.track(Event.OpenedBookmark) + } + BookmarkNodeType.FOLDER -> { + navController.nav( + R.id.bookmarkFragment, + BookmarkFragmentDirections.actionBookmarkFragmentSelf(item.guid) ) + } + BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot open separators") } - metrics.track(Event.OpenedBookmark) - } - - override fun expand(folder: BookmarkNode) { - require(folder.type == BookmarkNodeType.FOLDER) - navController.nav( - R.id.bookmarkFragment, - BookmarkFragmentDirections.actionBookmarkFragmentSelf(folder.guid) - ) } override fun switchMode(mode: BookmarkState.Mode) { @@ -83,16 +85,16 @@ class BookmarkFragmentInteractor( ) } - override fun select(node: BookmarkNode) { - if (node.inRoots()) { + override fun select(item: BookmarkNode) { + if (item.inRoots()) { snackbarPresenter.present(context.getString(R.string.bookmark_cannot_edit_root)) return } - bookmarkStore.dispatch(BookmarkAction.Select(node)) + bookmarkStore.dispatch(BookmarkAction.Select(item)) } - override fun deselect(node: BookmarkNode) { - bookmarkStore.dispatch(BookmarkAction.Deselect(node)) + override fun deselect(item: BookmarkNode) { + bookmarkStore.dispatch(BookmarkAction.Deselect(item)) } override fun deselectAll() { @@ -148,23 +150,14 @@ class BookmarkFragmentInteractor( } } - override fun delete(node: BookmarkNode) { - val eventType = when (node.type) { - BookmarkNodeType.ITEM -> { - Event.RemoveBookmark - } - BookmarkNodeType.FOLDER -> { - Event.RemoveBookmarkFolder - } - BookmarkNodeType.SEPARATOR -> { - throw IllegalStateException("Cannot delete separators") - } + override fun delete(nodes: Set) { + val eventType = when (nodes.singleOrNull()?.type) { + BookmarkNodeType.ITEM -> Event.RemoveBookmark + BookmarkNodeType.FOLDER -> Event.RemoveBookmarkFolder + BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot delete separators") + null -> Event.RemoveBookmarks } - deleteBookmarkNodes(setOf(node), eventType) - } - - override fun deleteMulti(nodes: Set) { - deleteBookmarkNodes(nodes, Event.RemoveBookmarks) + deleteBookmarkNodes(nodes, eventType) } override fun backPressed() { diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt index 31c43562b..6b5f81e8a 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt @@ -11,12 +11,13 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import org.mozilla.fenix.R import org.mozilla.fenix.ThemeManager +import org.mozilla.fenix.library.LibraryItemMenu class BookmarkItemMenu( private val context: Context, private val item: BookmarkNode, private val onItemTapped: (BookmarkItemMenu.Item) -> Unit = {} -) { +) : LibraryItemMenu { sealed class Item { object Edit : Item() @@ -28,7 +29,7 @@ class BookmarkItemMenu( object Delete : Item() } - val menuBuilder by lazy { BrowserMenuBuilder(menuItems) } + override val menuBuilder by lazy { BrowserMenuBuilder(menuItems) } private val menuItems by lazy { listOfNotNull( diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt index 0bedb0ffb..9c8f993ad 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt @@ -13,13 +13,14 @@ import mozilla.components.concept.storage.BookmarkNode import mozilla.components.support.base.feature.BackHandler import org.mozilla.fenix.R import org.mozilla.fenix.library.LibraryPageView +import org.mozilla.fenix.library.SelectionInteractor /** * Interface for the Bookmarks view. * This interface is implemented by objects that want to respond to user interaction on the bookmarks management UI. */ @SuppressWarnings("TooManyFunctions") -interface BookmarkViewInteractor { +interface BookmarkViewInteractor : SelectionInteractor { /** * Swaps the head of the bookmarks tree, replacing it with a new, updated bookmarks tree. @@ -28,20 +29,6 @@ interface BookmarkViewInteractor { */ fun change(node: BookmarkNode) - /** - * Opens a tab for a bookmark item. - * - * @param item the bookmark item to open - */ - fun open(item: BookmarkNode) - - /** - * Expands a bookmark folder in the bookmarks tree, providing a view of a different folder elsewhere in the tree. - * - * @param folder the bookmark folder to expand - */ - fun expand(folder: BookmarkNode) - /** * Switches the current bookmark multi-selection mode. * @@ -56,20 +43,6 @@ interface BookmarkViewInteractor { */ fun edit(node: BookmarkNode) - /** - * Selects a bookmark node in multi-selection. - * - * @param node the bookmark node to select - */ - fun select(node: BookmarkNode) - - /** - * De-selects a bookmark node in multi-selection. - * - * @param node the bookmark node to deselect - */ - fun deselect(node: BookmarkNode) - /** * De-selects all bookmark nodes, clearing the multi-selection mode. * @@ -105,18 +78,11 @@ interface BookmarkViewInteractor { fun openInPrivateTab(item: BookmarkNode) /** - * Deletes a bookmark node. + * Deletes a set of bookmark node. * - * @param node the bookmark node to delete + * @param nodes the bookmark nodes to delete */ - fun delete(node: BookmarkNode) - - /** - * Deletes a set of bookmark nodes. - * - * @param nodes the set of bookmark nodes to delete - */ - fun deleteMulti(nodes: Set) + fun delete(nodes: Set) /** * Handles back presses for the bookmark screen, so navigation up the tree is possible. @@ -133,10 +99,8 @@ class BookmarkView( val view: View = LayoutInflater.from(container.context) .inflate(R.layout.component_bookmark, container, true) - var mode: BookmarkState.Mode = BookmarkState.Mode.Normal - private set - var tree: BookmarkNode? = null - private set + private var mode: BookmarkState.Mode = BookmarkState.Mode.Normal + private var tree: BookmarkNode? = null private var canGoBack = false private val bookmarkAdapter: BookmarkAdapter @@ -157,7 +121,7 @@ class BookmarkView( } bookmarkAdapter.updateData(state.tree, mode) - when (state.mode) { + when (mode) { is BookmarkState.Mode.Normal -> setUiForNormalMode(state.tree) is BookmarkState.Mode.Selecting -> 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 524ab1734..627d39202 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 @@ -10,7 +10,7 @@ import mozilla.components.concept.storage.BookmarkNode import org.jetbrains.anko.image import org.mozilla.fenix.R import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.bookmarks.BookmarkState +import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor import org.mozilla.fenix.library.bookmarks.inRoots @@ -19,14 +19,15 @@ import org.mozilla.fenix.library.bookmarks.inRoots */ class BookmarkFolderViewHolder( view: LibrarySiteItemView, - interactor: BookmarkViewInteractor + interactor: BookmarkViewInteractor, + private val selectionHolder: SelectionHolder ) : BookmarkNodeViewHolder(view, interactor) { - override fun bind(item: BookmarkNode, mode: BookmarkState.Mode, selected: Boolean) { + override fun bind(item: BookmarkNode) { containerView.displayAs(LibrarySiteItemView.ItemType.FOLDER) - setClickListeners(mode, item, selected) + setSelectionListeners(item, selectionHolder) if (!item.inRoots()) { setupMenu(item) @@ -34,31 +35,10 @@ class BookmarkFolderViewHolder( containerView.overflowView.visibility = View.GONE } - containerView.changeSelected(selected) + containerView.changeSelected(item in selectionHolder.selectedItems) containerView.iconView.image = containerView.context.getDrawable(R.drawable.ic_folder_icon)?.apply { setTint(ContextCompat.getColor(containerView.context, R.color.primary_text_light_theme)) } containerView.titleView.text = item.title } - - private fun setClickListeners( - mode: BookmarkState.Mode, - item: BookmarkNode, - selected: Boolean - ) { - containerView.setOnClickListener { - when { - mode == BookmarkState.Mode.Normal -> interactor.expand(item) - selected -> interactor.deselect(item) - else -> interactor.select(item) - } - } - - containerView.setOnLongClickListener { - if (mode == BookmarkState.Mode.Normal && !item.inRoots()) { - interactor.select(item) - true - } else false - } - } } 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 15e8fffb5..2fb1d2a7c 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 @@ -6,7 +6,7 @@ package org.mozilla.fenix.library.bookmarks.viewholders import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.bookmarks.BookmarkState +import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor /** @@ -14,10 +14,11 @@ import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor */ class BookmarkItemViewHolder( view: LibrarySiteItemView, - interactor: BookmarkViewInteractor + interactor: BookmarkViewInteractor, + private val selectionHolder: SelectionHolder ) : BookmarkNodeViewHolder(view, interactor) { - override fun bind(item: BookmarkNode, mode: BookmarkState.Mode, selected: Boolean) { + override fun bind(item: BookmarkNode) { containerView.displayAs(LibrarySiteItemView.ItemType.SITE) @@ -25,8 +26,9 @@ class BookmarkItemViewHolder( containerView.titleView.text = if (item.title.isNullOrBlank()) item.url else item.title containerView.urlView.text = item.url - setClickListeners(mode, item, selected) - containerView.changeSelected(selected) + setSelectionListeners(item, selectionHolder) + + containerView.changeSelected(item in selectionHolder.selectedItems) setColorsAndIcons(item.url) } @@ -37,32 +39,4 @@ class BookmarkItemViewHolder( containerView.iconView.setImageDrawable(null) } } - - private fun setClickListeners( - mode: BookmarkState.Mode, - item: BookmarkNode, - selected: Boolean - ) { - containerView.setOnClickListener { - when { - mode == BookmarkState.Mode.Normal -> interactor.open(item) - selected -> interactor.deselect(item) - else -> interactor.select(item) - } - } - - containerView.setOnLongClickListener { - if (mode == BookmarkState.Mode.Normal) { - interactor.select(item) - true - } else false - } - - containerView.iconView.setOnClickListener({ - when { - selected -> interactor.deselect(item) - else -> interactor.select(item) - } - }) - } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt index 15dc36431..3780a50cf 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt @@ -6,11 +6,10 @@ package org.mozilla.fenix.library.bookmarks.viewholders import androidx.recyclerview.widget.RecyclerView import kotlinx.android.extensions.LayoutContainer -import mozilla.components.browser.menu.BrowserMenu import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.library.LibrarySiteItemView +import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu -import org.mozilla.fenix.library.bookmarks.BookmarkState import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor /** @@ -18,11 +17,15 @@ import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor */ abstract class BookmarkNodeViewHolder( override val containerView: LibrarySiteItemView, - val interactor: BookmarkViewInteractor + private val interactor: BookmarkViewInteractor ) : RecyclerView.ViewHolder(containerView), LayoutContainer { - abstract fun bind(item: BookmarkNode, mode: BookmarkState.Mode, selected: Boolean) + abstract fun bind(item: BookmarkNode) + + protected fun setSelectionListeners(item: BookmarkNode, selectionHolder: SelectionHolder) { + containerView.setSelectionInteractor(item, selectionHolder, interactor) + } protected fun setupMenu(item: BookmarkNode) { val bookmarkItemMenu = BookmarkItemMenu(containerView.context, item) { @@ -33,15 +36,10 @@ abstract class BookmarkNodeViewHolder( BookmarkItemMenu.Item.Share -> interactor.share(item) BookmarkItemMenu.Item.OpenInNewTab -> interactor.openInNewTab(item) BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.openInPrivateTab(item) - BookmarkItemMenu.Item.Delete -> interactor.delete(item) + BookmarkItemMenu.Item.Delete -> interactor.delete(setOf(item)) } } - containerView.overflowView.setOnClickListener { - bookmarkItemMenu.menuBuilder.build(containerView.context).show( - anchor = it, - orientation = BrowserMenu.Orientation.DOWN - ) - } + containerView.attachMenu(bookmarkItemMenu) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkSeparatorViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkSeparatorViewHolder.kt index 2cebc3be9..83ee21fcc 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkSeparatorViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkSeparatorViewHolder.kt @@ -6,7 +6,6 @@ package org.mozilla.fenix.library.bookmarks.viewholders import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.library.LibrarySiteItemView -import org.mozilla.fenix.library.bookmarks.BookmarkState import org.mozilla.fenix.library.bookmarks.BookmarkViewInteractor /** @@ -17,7 +16,7 @@ class BookmarkSeparatorViewHolder( interactor: BookmarkViewInteractor ) : BookmarkNodeViewHolder(view, interactor) { - override fun bind(item: BookmarkNode, mode: BookmarkState.Mode, selected: Boolean) { + override fun bind(item: BookmarkNode) { containerView.displayAs(LibrarySiteItemView.ItemType.SEPARATOR) setupMenu(item) } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt index 2e34e9ec4..061bc8799 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt @@ -11,6 +11,7 @@ import android.view.ViewGroup import androidx.paging.PagedListAdapter import androidx.recyclerview.widget.DiffUtil import org.mozilla.fenix.R +import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.history.viewholders.HistoryListItemViewHolder import java.util.Calendar import java.util.Date @@ -28,14 +29,16 @@ enum class HistoryItemTimeGroup { class HistoryAdapter( private val historyInteractor: HistoryInteractor -) : PagedListAdapter(historyDiffCallback) { +) : PagedListAdapter(historyDiffCallback), SelectionHolder { + private var mode: HistoryState.Mode = HistoryState.Mode.Normal + override val selectedItems get() = mode.selectedItems override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): HistoryListItemViewHolder { val view = LayoutInflater.from(parent.context).inflate(viewType, parent, false) - return HistoryListItemViewHolder(view, historyInteractor) + return HistoryListItemViewHolder(view, historyInteractor, this) } fun updateMode(mode: HistoryState.Mode) { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index d9c0ba35a..77fa10e7f 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -94,10 +94,11 @@ class HistoryFragment : LibraryPageFragment(), BackHandler { private fun deleteHistoryItems(items: Set) { lifecycleScope.launch { - val storage = context?.components?.core?.historyStorage - for (item in items) { - context?.components?.analytics?.metrics?.track(Event.HistoryItemRemoved) - storage?.deleteVisit(item.url, item.visitedAt) + context?.components?.run { + for (item in items) { + analytics.metrics.track(Event.HistoryItemRemoved) + core.historyStorage.deleteVisit(item.url, item.visitedAt) + } } viewModel.invalidate() historyStore.dispatch(HistoryAction.ExitDeletionMode) @@ -134,7 +135,7 @@ class HistoryFragment : LibraryPageFragment(), BackHandler { if (mode is HistoryState.Mode.Editing) { menu.findItem(R.id.share_history_multi_select)?.run { - isVisible = mode.selectedItems.isNotEmpty() + isVisible = true icon.colorFilter = PorterDuffColorFilter( ContextCompat.getColor(context!!, R.color.white_color), SRC_IN @@ -212,7 +213,7 @@ class HistoryFragment : LibraryPageFragment(), BackHandler { ) } - fun displayDeleteAllDialog() { + private fun displayDeleteAllDialog() { activity?.let { activity -> AlertDialog.Builder(activity).apply { setMessage(R.string.history_delete_all_dialog) diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt index b5f279d3a..c44c86944 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt @@ -15,29 +15,16 @@ class HistoryInteractor( private val invalidateOptionsMenu: () -> Unit, private val deleteHistoryItems: (Set) -> Unit ) : HistoryViewInteractor { - override fun onItemPress(item: HistoryItem) { - when (val mode = store.state.mode) { - is HistoryState.Mode.Normal -> openToBrowser(item) - is HistoryState.Mode.Editing -> { - val isSelected = mode.selectedItems.contains(item) - - if (isSelected) { - store.dispatch(HistoryAction.RemoveItemForRemoval(item)) - } else { - store.dispatch(HistoryAction.AddItemForRemoval(item)) - } - } - } + override fun open(item: HistoryItem) { + openToBrowser(item) } - override fun onItemLongPress(item: HistoryItem) { - val isSelected = store.state.mode.selectedItems.contains(item) + override fun select(item: HistoryItem) { + store.dispatch(HistoryAction.AddItemForRemoval(item)) + } - if (isSelected) { - store.dispatch(HistoryAction.RemoveItemForRemoval(item)) - } else { - store.dispatch(HistoryAction.AddItemForRemoval(item)) - } + override fun deselect(item: HistoryItem) { + store.dispatch(HistoryAction.RemoveItemForRemoval(item)) } override fun onBackPressed(): Boolean { @@ -57,10 +44,6 @@ class HistoryInteractor( displayDeleteAll.invoke() } - override fun onDeleteOne(item: HistoryItem) { - deleteHistoryItems.invoke(setOf(item)) - } - override fun onDeleteSome(items: Set) { deleteHistoryItems.invoke(items) } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemMenu.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemMenu.kt index aadcd33b6..5f89829df 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemMenu.kt @@ -9,16 +9,17 @@ import mozilla.components.browser.menu.BrowserMenuBuilder import mozilla.components.browser.menu.item.SimpleBrowserMenuItem import org.mozilla.fenix.R import org.mozilla.fenix.ThemeManager +import org.mozilla.fenix.library.LibraryItemMenu class HistoryItemMenu( private val context: Context, private val onItemTapped: (Item) -> Unit = {} -) { +) : LibraryItemMenu { sealed class Item { object Delete : Item() } - val menuBuilder by lazy { BrowserMenuBuilder(menuItems) } + override val menuBuilder by lazy { BrowserMenuBuilder(menuItems) } private val menuItems by lazy { listOf( diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt index ba27f66f1..cfbfa52df 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt @@ -15,21 +15,13 @@ import kotlinx.android.synthetic.main.component_history.view.* import mozilla.components.support.base.feature.BackHandler import org.mozilla.fenix.R import org.mozilla.fenix.library.LibraryPageView +import org.mozilla.fenix.library.SelectionInteractor /** * Interface for the HistoryViewInteractor. This interface is implemented by objects that want * to respond to user interaction on the HistoryView */ -interface HistoryViewInteractor { - /** - * Called when a user taps a history item - */ - fun onItemPress(item: HistoryItem) - - /** - * Called when a user long clicks a user - */ - fun onItemLongPress(item: HistoryItem) +interface HistoryViewInteractor : SelectionInteractor { /** * Called on backpressed to exit edit mode @@ -46,12 +38,6 @@ interface HistoryViewInteractor { */ fun onDeleteAll() - /** - * Called when one history item is deleted - * @param item the history item to delete - */ - fun onDeleteOne(item: HistoryItem) - /** * Called when multiple history items are deleted * @param items the history items to delete diff --git a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt index 763cc240a..14851dea5 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt @@ -7,8 +7,8 @@ package org.mozilla.fenix.library.history.viewholders import android.view.View import androidx.recyclerview.widget.RecyclerView import kotlinx.android.synthetic.main.history_list_item.view.* -import mozilla.components.browser.menu.BrowserMenu import org.mozilla.fenix.R +import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.history.HistoryInteractor import org.mozilla.fenix.library.history.HistoryItem import org.mozilla.fenix.library.history.HistoryItemMenu @@ -17,34 +17,21 @@ import org.mozilla.fenix.library.history.HistoryState class HistoryListItemViewHolder( view: View, - private val historyInteractor: HistoryInteractor + private val historyInteractor: HistoryInteractor, + private val selectionHolder: SelectionHolder ) : RecyclerView.ViewHolder(view) { private var item: HistoryItem? = null - private var mode: HistoryState.Mode = HistoryState.Mode.Normal init { setupMenu() - itemView.history_layout.setOnLongClickListener { - item?.also(historyInteractor::onItemLongPress) - true - } - - itemView.history_layout.setOnClickListener { - item?.also(historyInteractor::onItemPress) - } - - itemView.history_layout.iconView.setOnClickListener { - item?.apply { - historyInteractor.onItemLongPress(this) - } - } - itemView.delete_button.setOnClickListener { - when (val mode = this.mode) { - HistoryState.Mode.Normal -> historyInteractor.onDeleteAll() - is HistoryState.Mode.Editing -> historyInteractor.onDeleteSome(mode.selectedItems) + val selected = selectionHolder.selectedItems + if (selected.isEmpty()) { + historyInteractor.onDeleteAll() + } else { + historyInteractor.onDeleteSome(selected) } } } @@ -56,24 +43,24 @@ class HistoryListItemViewHolder( mode: HistoryState.Mode ) { this.item = item - this.mode = mode itemView.history_layout.titleView.text = item.title itemView.history_layout.urlView.text = item.url - toggleDeleteButton(showDeleteButton, mode) + toggleDeleteButton(showDeleteButton, mode === HistoryState.Mode.Normal) val headerText = timeGroup?.humanReadable(itemView.context) toggleHeader(headerText) - itemView.history_layout.changeSelected(item in mode.selectedItems) + itemView.history_layout.setSelectionInteractor(item, selectionHolder, historyInteractor) + itemView.history_layout.changeSelected(item in selectionHolder.selectedItems) itemView.history_layout.loadFavicon(item.url) } - private fun toggleHeader(text: String?) { - if (text != null) { + private fun toggleHeader(headerText: String?) { + if (headerText != null) { itemView.header_title.visibility = View.VISIBLE - itemView.header_title.text = text + itemView.header_title.text = headerText } else { itemView.header_title.visibility = View.GONE } @@ -81,18 +68,18 @@ class HistoryListItemViewHolder( private fun toggleDeleteButton( showDeleteButton: Boolean, - mode: HistoryState.Mode + isNormalMode: Boolean ) { if (showDeleteButton) { itemView.delete_button.run { visibility = View.VISIBLE - if (mode === HistoryState.Mode.Deleting || mode.selectedItems.isNotEmpty()) { - isEnabled = false - alpha = DELETE_BUTTON_DISABLED_ALPHA - } else { + if (isNormalMode) { isEnabled = true alpha = 1f + } else { + isEnabled = false + alpha = DELETE_BUTTON_DISABLED_ALPHA } } } else { @@ -102,17 +89,13 @@ class HistoryListItemViewHolder( private fun setupMenu() { val historyMenu = HistoryItemMenu(itemView.context) { + val item = this.item ?: return@HistoryItemMenu when (it) { - HistoryItemMenu.Item.Delete -> item?.also(historyInteractor::onDeleteOne) + HistoryItemMenu.Item.Delete -> historyInteractor.onDeleteSome(setOf(item)) } } - itemView.history_layout.overflowView.setOnClickListener { - historyMenu.menuBuilder.build(itemView.context).show( - anchor = it, - orientation = BrowserMenu.Orientation.DOWN - ) - } + itemView.history_layout.attachMenu(historyMenu) } companion object { diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt index d50d65adf..00ec827c0 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt @@ -117,7 +117,7 @@ class BookmarkFragmentInteractorTest { @Test fun `expand a level of bookmarks`() { - interactor.expand(tree) + interactor.open(tree) verify { navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid)) @@ -236,7 +236,7 @@ class BookmarkFragmentInteractorTest { @Test fun `delete a bookmark item`() { - interactor.delete(item) + interactor.delete(setOf(item)) verify { deleteBookmarkNodes(setOf(item), Event.RemoveBookmark) @@ -245,7 +245,7 @@ class BookmarkFragmentInteractorTest { @Test fun `delete a bookmark folder`() { - interactor.delete(subfolder) + interactor.delete(setOf(subfolder)) verify { deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder) @@ -254,7 +254,7 @@ class BookmarkFragmentInteractorTest { @Test fun `delete multiple bookmarks`() { - interactor.deleteMulti(setOf(item, subfolder)) + interactor.delete(setOf(item, subfolder)) verify { deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt index a341c12ac..5e660b00e 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt @@ -31,7 +31,7 @@ class HistoryInteractorTest { mockk() ) - interactor.onItemPress(historyItem) + interactor.open(historyItem) assertEquals(historyItem, historyItemReceived) } @@ -51,7 +51,7 @@ class HistoryInteractorTest { mockk() ) - interactor.onItemPress(historyItem) + interactor.select(historyItem) verify { store.dispatch(HistoryAction.AddItemForRemoval(historyItem)) @@ -74,7 +74,7 @@ class HistoryInteractorTest { mockk() ) - interactor.onItemPress(historyItem) + interactor.deselect(historyItem) verify { store.dispatch(HistoryAction.RemoveItemForRemoval(historyItem)) @@ -135,22 +135,6 @@ class HistoryInteractorTest { assertEquals(true, deleteAllDialogShown) } - @Test - fun onDeleteOne() { - var itemsToDelete: Set? = null - val historyItem = HistoryItem(0, "title", "url", 0.toLong()) - val interactor = - HistoryInteractor( - mockk(), - mockk(), - mockk(), - mockk(), - { itemsToDelete = it } - ) - interactor.onDeleteOne(historyItem) - assertEquals(itemsToDelete, setOf(historyItem)) - } - @Test fun onDeleteSome() { var itemsToDelete: Set? = null