From caa53eb6e942942527c22b4b1ed62075e138e013 Mon Sep 17 00:00:00 2001 From: mike a Date: Mon, 25 Apr 2022 13:46:15 -0700 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/24513: add undo snackbar to history group screen --- .../fenix/components/appstate/AppAction.kt | 9 ++ .../fenix/components/appstate/AppState.kt | 4 + .../components/appstate/AppStoreReducer.kt | 7 +- .../fenix/library/history/HistoryAdapter.kt | 77 +++++++++++-- .../library/history/HistoryController.kt | 53 ++++++++- .../fenix/library/history/HistoryFragment.kt | 88 +++++++-------- .../library/history/HistoryFragmentStore.kt | 36 ++++--- .../fenix/library/history/HistoryView.kt | 31 +++--- .../library/history/PendingDeletionHistory.kt | 58 ++++++++++ .../viewholders/HistoryListItemViewHolder.kt | 21 ++-- .../HistoryMetadataGroupFragment.kt | 102 +++++++++++++++--- .../HistoryMetadataGroupFragmentStore.kt | 20 +++- .../HistoryMetadataGroupController.kt | 64 ++++++++--- .../view/HistoryMetadataGroupAdapter.kt | 51 ++++++++- .../HistoryMetadataGroupItemViewHolder.kt | 8 +- .../view/HistoryMetadataGroupView.kt | 53 +++++++-- app/src/main/res/values/strings.xml | 9 ++ .../bookmarks/BookmarkControllerTest.kt | 12 --- .../library/history/HistoryControllerTest.kt | 16 ++- .../history/HistoryFragmentStoreTest.kt | 43 +++++++- .../HistoryMetadataGroupFragmentStoreTest.kt | 27 ++++- .../HistoryMetadataGroupControllerTest.kt | 18 +++- .../HistoryMetadataGroupItemViewHolderTest.kt | 2 +- 23 files changed, 643 insertions(+), 166 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/library/history/PendingDeletionHistory.kt diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt index 9a015ad36f..ccaa3db611 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppAction.kt @@ -17,6 +17,7 @@ import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentsyncedtabs.RecentSyncedTabState import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem +import org.mozilla.fenix.library.history.PendingDeletionHistory import org.mozilla.fenix.gleanplumb.Message import org.mozilla.fenix.gleanplumb.MessagingState @@ -57,6 +58,14 @@ sealed class AppAction : Action { data class DeselectPocketStoriesCategory(val categoryName: String) : AppAction() data class PocketStoriesShown(val storiesShown: List) : AppAction() data class PocketStoriesChange(val pocketStories: List) : AppAction() + /** + * Adds a set of items marked for removal to the app state, to be hidden in the UI. + */ + data class AddPendingDeletionSet(val historyItems: Set) : AppAction() + /** + * Removes a set of items, previously marked for removal, to be displayed again in the UI. + */ + data class UndoPendingDeletionSet(val historyItems: Set) : AppAction() data class PocketStoriesCategoriesChange(val storiesCategories: List) : AppAction() data class PocketStoriesCategoriesSelectionsChange( diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt index 5a1305c3b0..dcd88801ac 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppState.kt @@ -18,6 +18,7 @@ import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentsyncedtabs.RecentSyncedTabState import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem +import org.mozilla.fenix.library.history.PendingDeletionHistory import org.mozilla.fenix.gleanplumb.MessagingState /** @@ -39,6 +40,8 @@ import org.mozilla.fenix.gleanplumb.MessagingState * @property pocketStories The list of currently shown [PocketRecommendedStory]s. * @property pocketStoriesCategories All [PocketRecommendedStory] categories. * @property messaging State related messages. + * @property pendingDeletionHistoryItems The set of History items marked for removal in the UI, + * awaiting to be removed once the Undo snackbar hides away. * Also serves as an in memory cache of all stories mapped by category allowing for quick stories filtering. */ data class AppState( @@ -57,4 +60,5 @@ data class AppState( val pocketStoriesCategories: List = emptyList(), val pocketStoriesCategoriesSelections: List = emptyList(), val messaging: MessagingState = MessagingState(), + val pendingDeletionHistoryItems: Set = emptySet(), ) : State diff --git a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt index 39e508b042..a55fe11d3b 100644 --- a/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt +++ b/app/src/main/java/org/mozilla/fenix/components/appstate/AppStoreReducer.kt @@ -94,7 +94,7 @@ internal object AppStoreReducer { ) is AppAction.DisbandSearchGroupAction -> state.copy( recentHistory = state.recentHistory.filterNot { - it is RecentlyVisitedItem.RecentHistoryGroup && ( + it is RecentHistoryGroup && ( it.title.equals(action.searchTerm, true) || it.title.equals(state.recentSearchGroup?.searchTerm, true) ) @@ -173,6 +173,11 @@ internal object AppStoreReducer { state.copy(pocketStoriesCategories = updatedCategories) } + is AppAction.AddPendingDeletionSet -> + state.copy(pendingDeletionHistoryItems = state.pendingDeletionHistoryItems + action.historyItems) + + is AppAction.UndoPendingDeletionSet -> + state.copy(pendingDeletionHistoryItems = state.pendingDeletionHistoryItems - action.historyItems) } } 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 59531c2ff8..351bb2c2d2 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 @@ -16,13 +16,21 @@ import org.mozilla.fenix.library.history.viewholders.HistoryListItemViewHolder */ class HistoryAdapter( private val historyInteractor: HistoryInteractor, + private val onEmptyStateChanged: (Boolean) -> Unit, ) : PagingDataAdapter(historyDiffCallback), SelectionHolder { private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal - override val selectedItems get() = mode.selectedItems - var pendingDeletionIds = emptySet() + private var pendingDeletionItems = emptySet() private val itemsWithHeaders: MutableMap = mutableMapOf() + // A flag to track the empty state of the list. Items are not being deleted immediately, + // but hidden from the UI until the Undo snackbar will execute the delayed operation. + // Whether the adapter has actually zero items or all present items are hidden, + // the screen should be updated into proper empty/not empty state. + private var isEmpty = true + + override val selectedItems + get() = mode.selectedItems override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID @@ -38,10 +46,45 @@ class HistoryAdapter( if (itemCount > 0) notifyItemChanged(0) } + @Suppress("ComplexMethod") override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) { val current = getItem(position) ?: return - val isPendingDeletion = pendingDeletionIds.contains(current.visitedAt) + var isPendingDeletion = false + var groupPendingDeletionCount = 0 var timeGroup: HistoryItemTimeGroup? = null + if (position == 0) { + isEmpty = true + } + + if (pendingDeletionItems.isNotEmpty()) { + when (current) { + is History.Regular -> { + isPendingDeletion = pendingDeletionItems.find { + it is PendingDeletionHistory.Item && it.visitedAt == current.visitedAt + } != null + } + is History.Group -> { + isPendingDeletion = pendingDeletionItems.find { + it is PendingDeletionHistory.Group && it.visitedAt == current.visitedAt + } != null + + if (!isPendingDeletion) { + groupPendingDeletionCount = current.items.count { historyMetadata -> + pendingDeletionItems.find { + it is PendingDeletionHistory.MetaData && + it.key == historyMetadata.historyMetadataKey && + it.visitedAt == historyMetadata.visitedAt + } != null + }.also { + if (it == current.items.size) { + isPendingDeletion = true + } + } + } + } + else -> {} + } + } // Add or remove the header and position to the map depending on it's deletion status if (itemsWithHeaders.containsKey(current.historyTimeGroup)) { @@ -60,11 +103,33 @@ class HistoryAdapter( timeGroup = current.historyTimeGroup } - holder.bind(current, timeGroup, position == 0, mode, isPendingDeletion) + // If there is a single visible item, it's enough to change the empty state of the view. + if (isEmpty && !isPendingDeletion) { + isEmpty = false + onEmptyStateChanged.invoke(isEmpty) + } else if (position + 1 == itemCount) { + // If we reached the bottom of the list and there still has been zero visible items, + // we can can change the History view state to empty. + if (isEmpty) { + onEmptyStateChanged.invoke(isEmpty) + } + } + + holder.bind( + current, + timeGroup, + position == 0, + mode, + isPendingDeletion, + groupPendingDeletionCount + ) } - fun updatePendingDeletionIds(pendingDeletionIds: Set) { - this.pendingDeletionIds = pendingDeletionIds + /** + * @param pendingDeletionItems is used to filter out the items that should not be displayed. + */ + fun updatePendingDeletionItems(pendingDeletionItems: Set) { + this.pendingDeletionItems = pendingDeletionItems } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt index fc893697fc..0d959f34dd 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt @@ -4,14 +4,21 @@ package org.mozilla.fenix.library.history +import android.content.Context import androidx.navigation.NavController import androidx.navigation.NavOptions import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import mozilla.telemetry.glean.private.NoExtras +import mozilla.components.browser.state.action.HistoryMetadataAction +import mozilla.components.service.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.R +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.GleanMetrics.History as GleanHistory @@ -29,15 +36,21 @@ interface HistoryController { fun handleEnterRecentlyClosed() } -@Suppress("TooManyFunctions") +@Suppress("TooManyFunctions", "LongParameterList") class DefaultHistoryController( private val store: HistoryFragmentStore, + private val appStore: AppStore, + private var historyProvider: DefaultPagedHistoryProvider, private val navController: NavController, private val scope: CoroutineScope, private val openToBrowser: (item: History.Regular) -> Unit, private val displayDeleteAll: () -> Unit, private val invalidateOptionsMenu: () -> Unit, - private val deleteHistoryItems: (Set) -> Unit, + private val deleteSnackbar: ( + items: Set, + undo: suspend (Set) -> Unit, + delete: (Set) -> suspend (context: Context) -> Unit + ) -> Unit, private val syncHistory: suspend () -> Unit, private val metrics: MetricController ) : HistoryController { @@ -95,7 +108,39 @@ class DefaultHistoryController( } override fun handleDeleteSome(items: Set) { - deleteHistoryItems.invoke(items) + val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() + appStore.dispatch(AppAction.AddPendingDeletionSet(pendingDeletionItems)) + deleteSnackbar.invoke(items, ::undo, ::delete) + } + + private fun undo(items: Set) { + val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() + appStore.dispatch(AppAction.UndoPendingDeletionSet(pendingDeletionItems)) + } + + private fun delete(items: Set): suspend (context: Context) -> Unit { + return { context -> + CoroutineScope(Dispatchers.IO).launch { + store.dispatch(HistoryFragmentAction.EnterDeletionMode) + for (item in items) { + GleanHistory.removed.record(NoExtras()) + + when (item) { + is History.Regular -> context.components.core.historyStorage.deleteVisitsFor(item.url) + is History.Group -> { + // NB: If we have non-search groups, this logic needs to be updated. + historyProvider.deleteMetadataSearchGroup(item) + context.components.core.store.dispatch( + HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = item.title) + ) + } + // We won't encounter individual metadata entries outside of groups. + is History.Metadata -> {} + } + } + store.dispatch(HistoryFragmentAction.ExitDeletionMode) + } + } } override fun handleRequestSync() { 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 575c5afd75..0aaa2e7407 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 @@ -15,7 +15,6 @@ import android.view.MenuItem import android.view.View import android.view.ViewGroup import androidx.appcompat.app.AlertDialog -import androidx.lifecycle.MutableLiveData import androidx.lifecycle.lifecycleScope import androidx.navigation.NavDirections import androidx.navigation.fragment.findNavController @@ -25,15 +24,16 @@ import androidx.paging.PagingData import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch import mozilla.components.browser.state.action.EngineAction -import mozilla.components.browser.state.action.HistoryMetadataAction import mozilla.components.browser.state.action.RecentlyClosedAction import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.lib.state.ext.consumeFrom +import mozilla.components.lib.state.ext.flowScoped import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.telemetry.glean.private.NoExtras @@ -63,7 +63,6 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { private lateinit var historyInteractor: HistoryInteractor private lateinit var historyProvider: DefaultPagedHistoryProvider - private var userHasHistory = MutableLiveData(true) private var history: Flow> = Pager( PagingConfig(PAGE_SIZE), null @@ -91,19 +90,22 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Normal, - pendingDeletionIds = emptySet(), + pendingDeletionItems = emptySet(), + isEmpty = false, isDeletingItems = false ) ) } val historyController: HistoryController = DefaultHistoryController( store = historyStore, + appStore = requireContext().components.appStore, + historyProvider = historyProvider, navController = findNavController(), scope = lifecycleScope, openToBrowser = ::openItem, displayDeleteAll = ::displayDeleteAllDialog, invalidateOptionsMenu = ::invalidateOptionsMenu, - deleteHistoryItems = ::deleteHistoryItems, + deleteSnackbar = :: deleteSnackbar, syncHistory = ::syncHistory, metrics = requireComponents.analytics.metrics ) @@ -113,7 +115,16 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { _historyView = HistoryView( binding.historyLayout, historyInteractor, - onZeroItemsLoaded = { userHasHistory.value = false } + onZeroItemsLoaded = { + historyStore.dispatch( + HistoryFragmentAction.ChangeEmptyState(isEmpty = true) + ) + }, + onEmptyStateChanged = { + historyStore.dispatch( + HistoryFragmentAction.ChangeEmptyState(it) + ) + } ) return view @@ -145,16 +156,19 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { setHasOptionsMenu(true) } - private fun deleteHistoryItems(items: Set) { - updatePendingHistoryToDelete(items) + private fun deleteSnackbar( + items: Set, + undo: suspend (items: Set) -> Unit, + delete: (Set) -> suspend (context: Context) -> Unit + ) { CoroutineScope(IO).allowUndo( requireActivity().getRootView()!!, getMultiSelectSnackBarMessage(items), - getString(R.string.bookmark_undo_deletion), + getString(R.string.snackbar_deleted_undo), { - undoPendingDeletion(items) + undo.invoke(items) }, - getDeleteHistoryItemsOperation(items) + delete(items) ) } @@ -165,10 +179,13 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { historyView.update(it) } - userHasHistory.observe( - viewLifecycleOwner, - historyView::updateEmptyState - ) + requireContext().components.appStore.flowScoped(viewLifecycleOwner) { flow -> + flow.mapNotNull { state -> state.pendingDeletionHistoryItems }.collect { items -> + historyStore.dispatch( + HistoryFragmentAction.UpdatePendingDeletionItems(pendingDeletionItems = items) + ) + } + } lifecycleScope.launch { history.collect { @@ -228,7 +245,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { true } R.id.delete_history_multi_select -> { - deleteHistoryItems(historyStore.state.mode.selectedItems) + historyInteractor.onDeleteSome(historyStore.state.mode.selectedItems) historyStore.dispatch(HistoryFragmentAction.ExitEditMode) true } @@ -362,47 +379,14 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { ) } - private fun getDeleteHistoryItemsOperation(items: Set): (suspend (context: Context) -> Unit) { - return { context -> - CoroutineScope(IO).launch { - historyStore.dispatch(HistoryFragmentAction.EnterDeletionMode) - for (item in items) { - GleanHistory.removed.record(NoExtras()) - - when (item) { - is History.Regular -> context.components.core.historyStorage.deleteVisitsFor(item.url) - is History.Group -> { - // NB: If we have non-search groups, this logic needs to be updated. - historyProvider.deleteMetadataSearchGroup(item) - context.components.core.store.dispatch( - HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = item.title) - ) - } - // We won't encounter individual metadata entries outside of groups. - is History.Metadata -> {} - } - } - historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) - } - } - } - - private fun updatePendingHistoryToDelete(items: Set) { - val ids = items.map { item -> item.visitedAt }.toSet() - historyStore.dispatch(HistoryFragmentAction.AddPendingDeletionSet(ids)) - } - - private fun undoPendingDeletion(items: Set) { - val ids = items.map { item -> item.visitedAt }.toSet() - historyStore.dispatch(HistoryFragmentAction.UndoPendingDeletionSet(ids)) - } - + @Suppress("UnusedPrivateMember") private suspend fun syncHistory() { val accountManager = requireComponents.backgroundServices.accountManager accountManager.syncNow(SyncReason.User) historyView.historyAdapter.refresh() } + @Suppress("UnusedPrivateMember") companion object { private const val PAGE_SIZE = 25 } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt index d407af1648..77658df859 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt @@ -33,7 +33,8 @@ sealed class History : Parcelable { * @property historyTimeGroup [HistoryItemTimeGroup] of the history item. * @property selected Whether or not the history item is selected. */ - @Parcelize data class Regular( + @Parcelize + data class Regular( override val position: Int, override val title: String, val url: String, @@ -55,7 +56,8 @@ sealed class History : Parcelable { * was opened from history. * @property selected Whether or not the history metadata item is selected. */ - @Parcelize data class Metadata( + @Parcelize + data class Metadata( override val position: Int, override val title: String, val url: String, @@ -76,7 +78,8 @@ sealed class History : Parcelable { * @property items List of history metadata items associated with the group. * @property selected Whether or not the history group is selected. */ - @Parcelize data class Group( + @Parcelize + data class Group( override val position: Int, override val title: String, override val visitedAt: Long, @@ -115,8 +118,16 @@ sealed class HistoryFragmentAction : Action { object ExitEditMode : HistoryFragmentAction() data class AddItemForRemoval(val item: History) : HistoryFragmentAction() data class RemoveItemForRemoval(val item: History) : HistoryFragmentAction() - data class AddPendingDeletionSet(val itemIds: Set) : HistoryFragmentAction() - data class UndoPendingDeletionSet(val itemIds: Set) : HistoryFragmentAction() + /** + * Updates the empty state of [org.mozilla.fenix.library.history.HistoryView]. + */ + data class ChangeEmptyState(val isEmpty: Boolean) : HistoryFragmentAction() + /** + * Updates the set of items marked for removal from the [org.mozilla.fenix.components.AppStore] + * to the [HistoryFragmentStore], to be hidden from the UI. + */ + data class UpdatePendingDeletionItems(val pendingDeletionItems: Set) : + HistoryFragmentAction() object EnterDeletionMode : HistoryFragmentAction() object ExitDeletionMode : HistoryFragmentAction() object StartSync : HistoryFragmentAction() @@ -131,7 +142,8 @@ sealed class HistoryFragmentAction : Action { data class HistoryFragmentState( val items: List, val mode: Mode, - val pendingDeletionIds: Set, + val pendingDeletionItems: Set, + val isEmpty: Boolean, val isDeletingItems: Boolean ) : State { sealed class Mode { @@ -168,13 +180,9 @@ private fun historyStateReducer( is HistoryFragmentAction.ExitDeletionMode -> state.copy(isDeletingItems = false) is HistoryFragmentAction.StartSync -> state.copy(mode = HistoryFragmentState.Mode.Syncing) is HistoryFragmentAction.FinishSync -> state.copy(mode = HistoryFragmentState.Mode.Normal) - is HistoryFragmentAction.AddPendingDeletionSet -> - state.copy( - pendingDeletionIds = state.pendingDeletionIds + action.itemIds - ) - is HistoryFragmentAction.UndoPendingDeletionSet -> - state.copy( - pendingDeletionIds = state.pendingDeletionIds - action.itemIds - ) + is HistoryFragmentAction.ChangeEmptyState -> state.copy(isEmpty = action.isEmpty) + is HistoryFragmentAction.UpdatePendingDeletionItems -> state.copy( + pendingDeletionItems = action.pendingDeletionItems + ) } } 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 6f67f2891a..f0f19ea5ad 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 @@ -6,6 +6,7 @@ package org.mozilla.fenix.library.history import android.view.LayoutInflater import android.view.ViewGroup +import androidx.core.view.isInvisible import androidx.core.view.isVisible import androidx.paging.LoadState import androidx.recyclerview.widget.LinearLayoutManager @@ -23,7 +24,8 @@ import org.mozilla.fenix.theme.ThemeManager class HistoryView( container: ViewGroup, val interactor: HistoryInteractor, - val onZeroItemsLoaded: () -> Unit + val onZeroItemsLoaded: () -> Unit, + val onEmptyStateChanged: (Boolean) -> Unit ) : LibraryPageView(container), UserInteractionHandler { val binding = ComponentHistoryBinding.inflate( @@ -33,7 +35,9 @@ class HistoryView( var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal private set - val historyAdapter = HistoryAdapter(interactor).apply { + val historyAdapter = HistoryAdapter(interactor) { isEmpty -> + onEmptyStateChanged(isEmpty) + }.apply { addLoadStateListener { // First call will always have itemCount == 0, but we want to keep adapterItemCount // as null until we can distinguish an empty list from populated, so updateEmptyState() @@ -59,8 +63,7 @@ class HistoryView( (itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false } - val primaryTextColor = - ThemeManager.resolveAttribute(R.attr.textPrimary, context) + val primaryTextColor = ThemeManager.resolveAttribute(R.attr.textPrimary, context) binding.swipeRefresh.setColorSchemeColors(primaryTextColor) binding.swipeRefresh.setOnRefreshListener { interactor.onRequestSync() @@ -76,12 +79,14 @@ class HistoryView( state.mode === HistoryFragmentState.Mode.Normal || state.mode === HistoryFragmentState.Mode.Syncing mode = state.mode - historyAdapter.updatePendingDeletionIds(state.pendingDeletionIds) + historyAdapter.updatePendingDeletionItems(state.pendingDeletionItems) - updateEmptyState(state.pendingDeletionIds.size != adapterItemCount) + updateEmptyState(userHasHistory = !state.isEmpty) historyAdapter.updateMode(state.mode) - val first = layoutManager.findFirstVisibleItemPosition() + // We want to update the one item above the upper border of the screen, because + // RecyclerView won't redraw it on scroll and onBindViewHolder() method won't be called. + val first = layoutManager.findFirstVisibleItemPosition() - 1 val last = layoutManager.findLastVisibleItemPosition() + 1 historyAdapter.notifyItemRangeChanged(first, last - first) @@ -89,14 +94,6 @@ class HistoryView( interactor.onModeSwitched() } - if (state.mode is HistoryFragmentState.Mode.Editing) { - val unselectedItems = oldMode.selectedItems - state.mode.selectedItems - - state.mode.selectedItems.union(unselectedItems).forEach { item -> - historyAdapter.notifyItemChanged(item.position) - } - } - when (val mode = state.mode) { is HistoryFragmentState.Mode.Normal -> { setUiForNormalMode( @@ -114,8 +111,8 @@ class HistoryView( } } - fun updateEmptyState(userHasHistory: Boolean) { - binding.historyList.isVisible = userHasHistory + private fun updateEmptyState(userHasHistory: Boolean) { + binding.historyList.isInvisible = !userHasHistory binding.historyEmptyView.isVisible = !userHasHistory with(binding.recentlyClosedNavEmpty) { recentlyClosedNav.setOnClickListener { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/PendingDeletionHistory.kt b/app/src/main/java/org/mozilla/fenix/library/history/PendingDeletionHistory.kt new file mode 100644 index 0000000000..2e015e9359 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/history/PendingDeletionHistory.kt @@ -0,0 +1,58 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.library.history + +import mozilla.components.concept.storage.HistoryMetadataKey + +/** + * Wrapper for the data of the history item that has been marked for removal. Undo snackbar delays + * the actual removal, while this class is used to match History items that should be hidden in the + * UI. + */ +sealed class PendingDeletionHistory { + + /** + * This class represents a single, separate item in the history list. + */ + data class Item( + val visitedAt: Long, + val url: String + ) : PendingDeletionHistory() + + /** + * This class represents a group in the history list. + */ + data class Group( + val visitedAt: Long, + val historyMetadata: List + ) : PendingDeletionHistory() + + /** + * This class represents an item inside a group in the group history list + */ + data class MetaData( + val visitedAt: Long, + val key: HistoryMetadataKey + ) : PendingDeletionHistory() +} + +/** + * Maps an instance of [History] to an instance of [PendingDeletionHistory]. + */ +fun History.toPendingDeletionHistory(): PendingDeletionHistory { + return when (this) { + is History.Regular -> PendingDeletionHistory.Item(visitedAt = visitedAt, url = url) + is History.Group -> PendingDeletionHistory.Group( + visitedAt = visitedAt, + historyMetadata = items.map { historyMetadata -> + PendingDeletionHistory.MetaData( + historyMetadata.visitedAt, + historyMetadata.historyMetadataKey + ) + } + ) + is History.Metadata -> PendingDeletionHistory.MetaData(visitedAt, historyMetadataKey) + } +} 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 4fec50eb4e..2939e04910 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 @@ -43,18 +43,25 @@ class HistoryListItemViewHolder( } } + /** + * Displays the data of the given history record. + * @param timeGroup used to form headers for different time frames, like today, yesterday, etc. + * @param showTopContent enables the Recent tab button. + * @param mode switches between editing and regular modes. + * @param isPendingDeletion hides the item unless an undo snackbar action is evoked. + * @param groupPendingDeletionCount allows to properly display the number of items inside a + * history group, taking into account pending removal of items inside. + */ + @Suppress("LongParameterList") fun bind( item: History, timeGroup: HistoryItemTimeGroup?, showTopContent: Boolean, mode: HistoryFragmentState.Mode, - isPendingDeletion: Boolean = false, + isPendingDeletion: Boolean, + groupPendingDeletionCount: Int ) { - if (isPendingDeletion) { - binding.historyLayout.visibility = View.GONE - } else { - binding.historyLayout.visibility = View.VISIBLE - } + binding.historyLayout.isVisible = !isPendingDeletion binding.historyLayout.titleView.text = item.title @@ -62,7 +69,7 @@ class HistoryListItemViewHolder( is History.Regular -> item.url is History.Metadata -> item.url is History.Group -> { - val numChildren = item.items.size + val numChildren = item.items.size - groupPendingDeletionCount val stringId = if (numChildren == 1) { R.string.history_search_group_site } else { diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt index a23f59c590..e09869334d 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragment.kt @@ -4,6 +4,8 @@ package org.mozilla.fenix.library.historymetadata +import android.content.Context +import android.content.DialogInterface import android.os.Bundle import android.text.SpannableString import android.view.LayoutInflater @@ -12,13 +14,19 @@ import android.view.MenuInflater import android.view.MenuItem import android.view.View import android.view.ViewGroup -import androidx.lifecycle.lifecycleScope +import androidx.appcompat.app.AlertDialog import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map import mozilla.components.lib.state.ext.consumeFrom +import mozilla.components.lib.state.ext.flowScoped import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R +import org.mozilla.fenix.addons.showSnackBar import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.databinding.FragmentHistoryMetadataGroupBinding @@ -27,16 +35,19 @@ import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.setTextColor import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.historymetadata.controller.DefaultHistoryMetadataGroupController import org.mozilla.fenix.library.historymetadata.interactor.DefaultHistoryMetadataGroupInteractor import org.mozilla.fenix.library.historymetadata.interactor.HistoryMetadataGroupInteractor import org.mozilla.fenix.library.historymetadata.view.HistoryMetadataGroupView +import org.mozilla.fenix.utils.allowUndo /** * Displays a list of history metadata items for a history metadata search group. */ +@SuppressWarnings("TooManyFunctions") class HistoryMetadataGroupFragment : LibraryPageFragment(), UserInteractionHandler { @@ -51,6 +62,9 @@ class HistoryMetadataGroupFragment : private val args by navArgs() + override val selectedItems: Set + get() = historyMetadataGroupStore.state.items.filter { it.selected }.toSet() + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setHasOptionsMenu(true) @@ -63,10 +77,13 @@ class HistoryMetadataGroupFragment : ): View { _binding = FragmentHistoryMetadataGroupBinding.inflate(inflater, container, false) + val historyItems = args.historyMetadataItems.filterIsInstance() historyMetadataGroupStore = StoreProvider.get(this) { HistoryMetadataGroupFragmentStore( HistoryMetadataGroupFragmentState( - items = args.historyMetadataItems.filterIsInstance() + items = historyItems, + pendingDeletionItems = requireContext().components.appStore.state.pendingDeletionHistoryItems, + isEmpty = historyItems.isEmpty() ) ) } @@ -75,18 +92,26 @@ class HistoryMetadataGroupFragment : controller = DefaultHistoryMetadataGroupController( historyStorage = (activity as HomeActivity).components.core.historyStorage, browserStore = (activity as HomeActivity).components.core.store, + appStore = requireContext().components.appStore, store = historyMetadataGroupStore, selectOrAddUseCase = requireComponents.useCases.tabsUseCases.selectOrAddTab, navController = findNavController(), - scope = lifecycleScope, - searchTerm = args.title + searchTerm = args.title, + deleteSnackbar = :: deleteSnackbar, + promptDeleteAll = :: promptDeleteAll, + allDeletedSnackbar = ::allDeletedSnackbar, ) ) _historyMetadataGroupView = HistoryMetadataGroupView( container = binding.historyMetadataGroupLayout, interactor = interactor, - title = args.title + title = args.title, + onEmptyStateChanged = { + historyMetadataGroupStore.dispatch( + HistoryMetadataGroupFragmentAction.ChangeEmptyState(it) + ) + } ) return binding.root @@ -97,6 +122,16 @@ class HistoryMetadataGroupFragment : historyMetadataGroupView.update(state) activity?.invalidateOptionsMenu() } + + requireContext().components.appStore.flowScoped(viewLifecycleOwner) { flow -> + flow.map { state -> state.pendingDeletionHistoryItems }.collect { items -> + historyMetadataGroupStore.dispatch( + HistoryMetadataGroupFragmentAction.UpdatePendingDeletionItems( + pendingDeletionItems = items + ) + ) + } + } } override fun onResume() { @@ -104,6 +139,14 @@ class HistoryMetadataGroupFragment : showToolbar(args.title) } + override fun onDestroyView() { + super.onDestroyView() + _historyMetadataGroupView = null + _binding = null + } + + override fun onBackPressed(): Boolean = interactor.onBackPressed(selectedItems) + override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { if (selectedItems.isNotEmpty()) { inflater.inflate(R.menu.history_select_multi, menu) @@ -157,17 +200,42 @@ class HistoryMetadataGroupFragment : } } - override fun onDestroyView() { - super.onDestroyView() - _historyMetadataGroupView = null - _binding = null + private fun deleteSnackbar( + items: Set, + undo: suspend (items: Set) -> Unit, + delete: (Set) -> suspend (context: Context) -> Unit + ) { + CoroutineScope(Dispatchers.IO).allowUndo( + requireView(), + getSnackBarMessage(items), + getString(R.string.snackbar_deleted_undo), + { + undo.invoke(items) + }, + delete(items) + ) } - override val selectedItems: Set - get() = - historyMetadataGroupStore.state.items.filter { it.selected }.toSet() + private fun promptDeleteAll(delete: () -> Unit) { + AlertDialog.Builder(requireContext()).apply { + setMessage(R.string.delete_history_group_prompt_message) + setNegativeButton(R.string.delete_history_group_prompt_cancel) { dialog: DialogInterface, _ -> + dialog.cancel() + } + setPositiveButton(R.string.delete_history_group_prompt_allow) { dialog: DialogInterface, _ -> + delete.invoke() + dialog.dismiss() + } + create() + }.show() + } - override fun onBackPressed(): Boolean = interactor.onBackPressed(selectedItems) + private fun allDeletedSnackbar() { + showSnackBar( + requireView(), + getString(R.string.delete_history_group_snackbar) + ) + } private fun showTabTray() { findNavController().nav( @@ -175,4 +243,12 @@ class HistoryMetadataGroupFragment : HistoryMetadataGroupFragmentDirections.actionGlobalTabsTrayFragment() ) } + + private fun getSnackBarMessage(historyItems: Set): String { + val historyItem = historyItems.first() + return String.format( + requireContext().getString(R.string.history_delete_single_item_snackbar), + historyItem.url.toShortUrl(requireComponents.publicSuffixList) + ) + } } diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStore.kt index b267f1f707..38e6d979f8 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStore.kt @@ -8,6 +8,7 @@ import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.PendingDeletionHistory /** * The [Store] for holding the [HistoryMetadataGroupFragmentState] and applying @@ -28,9 +29,19 @@ sealed class HistoryMetadataGroupFragmentAction : Action { HistoryMetadataGroupFragmentAction() data class Select(val item: History.Metadata) : HistoryMetadataGroupFragmentAction() data class Deselect(val item: History.Metadata) : HistoryMetadataGroupFragmentAction() + /** + * Updates the set of items marked for removal from the [org.mozilla.fenix.components.AppStore] + * to the [HistoryMetadataGroupFragmentStore], to be hidden from the UI. + */ + data class UpdatePendingDeletionItems(val pendingDeletionItems: Set) : + HistoryMetadataGroupFragmentAction() object DeselectAll : HistoryMetadataGroupFragmentAction() data class Delete(val item: History.Metadata) : HistoryMetadataGroupFragmentAction() object DeleteAll : HistoryMetadataGroupFragmentAction() + /** + * Updates the empty state of [org.mozilla.fenix.library.historymetadata.view.HistoryMetadataGroupView]. + */ + data class ChangeEmptyState(val isEmpty: Boolean) : HistoryMetadataGroupFragmentAction() } /** @@ -39,7 +50,9 @@ sealed class HistoryMetadataGroupFragmentAction : Action { * @property items The list of [History.Metadata] to display. */ data class HistoryMetadataGroupFragmentState( - val items: List = emptyList() + val items: List, + val pendingDeletionItems: Set, + val isEmpty: Boolean, ) : State /** @@ -91,5 +104,10 @@ private fun historyStateReducer( } is HistoryMetadataGroupFragmentAction.DeleteAll -> state.copy(items = emptyList()) + is HistoryMetadataGroupFragmentAction.UpdatePendingDeletionItems -> + state.copy(pendingDeletionItems = action.pendingDeletionItems) + is HistoryMetadataGroupFragmentAction.ChangeEmptyState -> state.copy( + isEmpty = action.isEmpty + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt index 11a538c84f..5bffef545d 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupController.kt @@ -4,17 +4,23 @@ package org.mozilla.fenix.library.historymetadata.controller +import android.content.Context import androidx.navigation.NavController import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.launch import mozilla.components.browser.state.action.HistoryMetadataAction import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.storage.sync.PlacesHistoryStorage import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.feature.tabs.TabsUseCases -import mozilla.components.service.glean.private.NoExtras import org.mozilla.fenix.R +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.appstate.AppAction +import org.mozilla.fenix.ext.components +import mozilla.components.service.glean.private.NoExtras import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.toPendingDeletionHistory import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentAction import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentDirections import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentStore @@ -79,11 +85,18 @@ interface HistoryMetadataGroupController { class DefaultHistoryMetadataGroupController( private val historyStorage: PlacesHistoryStorage, private val browserStore: BrowserStore, + private val appStore: AppStore, private val store: HistoryMetadataGroupFragmentStore, private val selectOrAddUseCase: TabsUseCases.SelectOrAddUseCase, private val navController: NavController, - private val scope: CoroutineScope, private val searchTerm: String, + private val deleteSnackbar: ( + items: Set, + undo: suspend (Set) -> Unit, + delete: (Set) -> suspend (context: Context) -> Unit + ) -> Unit, + private val promptDeleteAll: (() -> Unit) -> Unit, + private val allDeletedSnackbar: () -> Unit ) : HistoryMetadataGroupController { override fun handleOpen(item: History.Metadata) { @@ -118,25 +131,42 @@ class DefaultHistoryMetadataGroupController( } override fun handleDelete(items: Set) { - scope.launch { - val isDeletingLastItem = items.containsAll(store.state.items) - items.forEach { - store.dispatch(HistoryMetadataGroupFragmentAction.Delete(it)) - historyStorage.deleteVisitsFor(it.url) - GleanHistory.searchTermGroupRemoveTab.record(NoExtras()) - } - // The method is called for both single and multiple items. - // In case all items have been deleted, we have to disband the search group. - if (isDeletingLastItem) { - browserStore.dispatch( - HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = searchTerm) - ) + val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() + appStore.dispatch(AppAction.AddPendingDeletionSet(pendingDeletionItems)) + deleteSnackbar.invoke(items, ::undo, ::delete) + } + + private fun undo(items: Set) { + val pendingDeletionItems = items.map { it.toPendingDeletionHistory() }.toSet() + appStore.dispatch(AppAction.UndoPendingDeletionSet(pendingDeletionItems)) + } + + private fun delete(items: Set): suspend (context: Context) -> Unit { + return { context -> + CoroutineScope(IO).launch { + val isDeletingLastItem = items.containsAll(store.state.items) + items.forEach { + store.dispatch(HistoryMetadataGroupFragmentAction.Delete(it)) + context.components.core.historyStorage.deleteVisitsFor(it.url) + GleanHistory.searchTermGroupRemoveTab.record(NoExtras()) + } + // The method is called for both single and multiple items. + // In case all items have been deleted, we have to disband the search group. + if (isDeletingLastItem) { + context.components.core.store.dispatch( + HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = searchTerm) + ) + } } } } override fun handleDeleteAll() { - scope.launch { + promptDeleteAll.invoke(::deleteAll) + } + + private fun deleteAll() { + CoroutineScope(IO).launch { store.dispatch(HistoryMetadataGroupFragmentAction.DeleteAll) store.state.items.forEach { historyStorage.deleteVisitsFor(it.url) @@ -145,6 +175,8 @@ class DefaultHistoryMetadataGroupController( HistoryMetadataAction.DisbandSearchGroupAction(searchTerm = searchTerm) ) GleanHistory.searchTermGroupRemoveAll.record(NoExtras()) + allDeletedSnackbar.invoke() + navController.popBackStack() } } } diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt index 524b784055..74587c0beb 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt @@ -9,6 +9,7 @@ import android.view.ViewGroup import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.ListAdapter import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.PendingDeletionHistory import org.mozilla.fenix.library.historymetadata.interactor.HistoryMetadataGroupInteractor import org.mozilla.fenix.selection.SelectionHolder @@ -16,11 +17,14 @@ import org.mozilla.fenix.selection.SelectionHolder * Adapter for a list of history metadata items to be displayed. */ class HistoryMetadataGroupAdapter( - private val interactor: HistoryMetadataGroupInteractor + private val interactor: HistoryMetadataGroupInteractor, + private val onEmptyStateChanged: (Boolean) -> Unit, ) : ListAdapter(DiffCallback), SelectionHolder { private var selectedHistoryItems: Set = emptySet() + private var pendingDeletionItems = emptySet() + private var isEmpty = true override val selectedItems: Set get() = selectedHistoryItems @@ -34,16 +38,55 @@ class HistoryMetadataGroupAdapter( return HistoryMetadataGroupItemViewHolder(view, interactor, this) } + override fun getItemId(position: Int): Long { + return getItem(position).visitedAt + } + override fun onBindViewHolder(holder: HistoryMetadataGroupItemViewHolder, position: Int) { - holder.bind(getItem(position)) + val current = getItem(position) ?: return + if (position == 0) { + isEmpty = true + } + + val isPendingDeletion = pendingDeletionItems.any { + it is PendingDeletionHistory.MetaData && + it.key == current.historyMetadataKey && + it.visitedAt == current.visitedAt + } + + // If there is a single visible item, it's enough to change the empty state of the view. + if (isEmpty && !isPendingDeletion) { + isEmpty = false + onEmptyStateChanged.invoke(isEmpty) + } else if (position + 1 == itemCount) { + // If we reached the bottom of the list and there still has been zero visible items, + // we can can change the History Group view state to empty. + if (isEmpty) { + onEmptyStateChanged.invoke(isEmpty) + } + } + + holder.bind(getItem(position), isPendingDeletion) } fun updateData(items: List) { - this.selectedHistoryItems = items.filter { it.selected }.toSet() - notifyItemRangeChanged(0, items.size) submitList(items) } + /** + * @param selectedHistoryItems is used to keep track of the items selected by the user. + */ + fun updateSelectedItems(selectedHistoryItems: Set) { + this.selectedHistoryItems = selectedHistoryItems + } + + /** + * @param pendingDeletionItems is used to keep track of the items selected by the user. + */ + fun updatePendingDeletionItems(pendingDeletionItems: Set) { + this.pendingDeletionItems = pendingDeletionItems + } + internal object DiffCallback : DiffUtil.ItemCallback() { override fun areContentsTheSame(oldItem: History.Metadata, newItem: History.Metadata): Boolean = oldItem.position == newItem.position diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolder.kt index e721a461bb..1090829235 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolder.kt @@ -5,6 +5,7 @@ package org.mozilla.fenix.library.historymetadata.view import android.view.View +import androidx.core.view.isVisible import androidx.recyclerview.widget.RecyclerView import org.mozilla.fenix.R import org.mozilla.fenix.databinding.HistoryMetadataGroupListItemBinding @@ -38,7 +39,12 @@ class HistoryMetadataGroupItemViewHolder( } } - fun bind(item: History.Metadata) { + /** + * Displays the data of the given history record. + * @param isPendingDeletion hides the item unless user evokes Undo snackbar action. + */ + fun bind(item: History.Metadata, isPendingDeletion: Boolean) { + binding.historyLayout.isVisible = !isPendingDeletion binding.historyLayout.titleView.text = item.title binding.historyLayout.urlView.text = item.url diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupView.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupView.kt index 896b1ca306..98c01fc0e3 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupView.kt @@ -6,8 +6,10 @@ package org.mozilla.fenix.library.historymetadata.view import android.view.LayoutInflater import android.view.ViewGroup +import androidx.core.view.isInvisible import androidx.core.view.isVisible import androidx.recyclerview.widget.LinearLayoutManager +import androidx.recyclerview.widget.SimpleItemAnimator import org.mozilla.fenix.R import org.mozilla.fenix.databinding.ComponentHistoryMetadataGroupBinding import org.mozilla.fenix.library.LibraryPageView @@ -20,19 +22,28 @@ import org.mozilla.fenix.library.historymetadata.interactor.HistoryMetadataGroup class HistoryMetadataGroupView( container: ViewGroup, val interactor: HistoryMetadataGroupInteractor, - val title: String + val title: String, + val onEmptyStateChanged: (Boolean) -> Unit, ) : LibraryPageView(container) { private val binding = ComponentHistoryMetadataGroupBinding.inflate( LayoutInflater.from(container.context), container, true ) - private val historyMetadataGroupAdapter = HistoryMetadataGroupAdapter(interactor) + private val historyMetadataGroupAdapter = HistoryMetadataGroupAdapter(interactor) { isEmpty -> + onEmptyStateChanged(isEmpty) + }.apply { + setHasStableIds(true) + } + private var layoutManager: LinearLayoutManager init { binding.historyMetadataGroupList.apply { - layoutManager = LinearLayoutManager(containerView.context) + layoutManager = LinearLayoutManager(containerView.context).also { + this@HistoryMetadataGroupView.layoutManager = it + } adapter = historyMetadataGroupAdapter + (itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false } } @@ -41,18 +52,44 @@ class HistoryMetadataGroupView( * [HistoryMetadataGroupFragmentState]. */ fun update(state: HistoryMetadataGroupFragmentState) { - binding.historyMetadataGroupList.isVisible = state.items.isNotEmpty() - binding.historyMetadataGroupEmptyView.isVisible = state.items.isEmpty() + binding.historyMetadataGroupList.isInvisible = state.isEmpty + binding.historyMetadataGroupEmptyView.isVisible = state.isEmpty + val selectedHistoryItems = state.items.filter { + it.selected + }.toSet() + + historyMetadataGroupAdapter.updatePendingDeletionItems(state.pendingDeletionItems) + historyMetadataGroupAdapter.updateSelectedItems(selectedHistoryItems) historyMetadataGroupAdapter.updateData(state.items) - val selectedItems = state.items.filter { it.selected } + var first = layoutManager.findFirstVisibleItemPosition() + val last = layoutManager.findLastVisibleItemPosition() - if (selectedItems.isEmpty()) { + // We want to adjust the position of the first visible in order to update the one item above + // the edge of the screen. It's an edge case, when the item partially visible is being + // removed. Currently, Undo action won't make it visible again. + // This block should be above the itemCount calculation, otherwise bottom partially visible + // item won't be updated. + if (first > 0) { + --first + } + + // In case there are no visible items, we still have to request updating two items, to cover + // the case when the last item has been removed and Undo action was called. + val itemCount = if (last != -1) { + (last - first) + 1 + } else { + 2 + } + + historyMetadataGroupAdapter.notifyItemRangeChanged(first, itemCount) + + if (selectedHistoryItems.isEmpty()) { setUiForNormalMode(title) } else { setUiForSelectingMode( - context.getString(R.string.history_multi_select_title, selectedItems.size) + context.getString(R.string.history_multi_select_title, selectedHistoryItems.size) ) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 314038f902..44228ca379 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1138,6 +1138,15 @@ Deleting browsing data… + + This will delete all items. + + Cancel + + Delete + + Group deleted + Firefox Preview is now Firefox Nightly diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt index 9b93b125c4..85be62b366 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -405,18 +405,6 @@ class BookmarkControllerTest { } } - @Test - fun `WHEN onSearch is called with BookmarkFragment THEN navigate to BookmarkSearchDialogFragment`() { - val controller = createController() - - controller.handleSearch() - verify { - navController.navigate( - BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkSearchDialogFragment() - ) - } - } - @Suppress("LongParameterList") private fun createController( loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null }, diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt index 776ad305f5..a6ae306003 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt @@ -18,18 +18,28 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R +import org.mozilla.fenix.components.AppStore +import org.mozilla.fenix.components.history.DefaultPagedHistoryProvider import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) class HistoryControllerTest { - private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) + private val historyItem = History.Regular( + 0, + "title", + "url", + 0.toLong(), + HistoryItemTimeGroup.timeGroupForTimestamp(0) + ) private val scope = TestCoroutineScope() private val store: HistoryFragmentStore = mockk(relaxed = true) + private val appStore: AppStore = mockk(relaxed = true) private val state: HistoryFragmentState = mockk(relaxed = true) private val navController: NavController = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) + private val historyProvider: DefaultPagedHistoryProvider = mockk(relaxed = true) @Before fun setUp() { @@ -183,12 +193,14 @@ class HistoryControllerTest { ): HistoryController { return DefaultHistoryController( store, + appStore, + historyProvider, navController, scope, openInBrowser, displayDeleteAll, invalidateOptionsMenu, - deleteHistoryItems, + { items, _, _ -> deleteHistoryItems.invoke(items) }, syncHistory, metrics ) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt index 4c4588280a..a5f1cf891e 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt @@ -6,12 +6,15 @@ package org.mozilla.fenix.library.history import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotSame +import org.junit.Assert.assertTrue import org.junit.Test class HistoryFragmentStoreTest { private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) private val newHistoryItem = History.Regular(1, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) + private val pendingDeletionItem = historyItem.toPendingDeletionHistory() @Test fun exitEditMode() = runBlocking { @@ -61,7 +64,8 @@ class HistoryFragmentStoreTest { val initialState = HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Syncing, - pendingDeletionIds = emptySet(), + pendingDeletionItems = emptySet(), + isEmpty = false, isDeletingItems = false ) val store = HistoryFragmentStore(initialState) @@ -71,24 +75,55 @@ class HistoryFragmentStoreTest { assertEquals(HistoryFragmentState.Mode.Normal, store.state.mode) } + @Test + fun changeEmptyState() = runBlocking { + val initialState = emptyDefaultState() + val store = HistoryFragmentStore(initialState) + + store.dispatch(HistoryFragmentAction.ChangeEmptyState(true)).join() + assertNotSame(initialState, store.state) + assertTrue(store.state.isEmpty) + + store.dispatch(HistoryFragmentAction.ChangeEmptyState(false)).join() + assertNotSame(initialState, store.state) + assertFalse(store.state.isEmpty) + } + + @Test + fun updatePendingDeletionItems() = runBlocking { + val initialState = emptyDefaultState() + val store = HistoryFragmentStore(initialState) + + store.dispatch(HistoryFragmentAction.UpdatePendingDeletionItems(setOf(pendingDeletionItem))).join() + assertNotSame(initialState, store.state) + assertEquals(setOf(pendingDeletionItem), store.state.pendingDeletionItems) + + store.dispatch(HistoryFragmentAction.UpdatePendingDeletionItems(emptySet())).join() + assertNotSame(initialState, store.state) + assertEquals(emptySet(), store.state.pendingDeletionItems) + } + private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Normal, - pendingDeletionIds = emptySet(), + pendingDeletionItems = emptySet(), + isEmpty = false, isDeletingItems = false ) private fun oneItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)), - pendingDeletionIds = emptySet(), + pendingDeletionItems = emptySet(), + isEmpty = false, isDeletingItems = false ) private fun twoItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)), - pendingDeletionIds = emptySet(), + pendingDeletionItems = emptySet(), + isEmpty = false, isDeletingItems = false ) } diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt index fca2949d94..c8ca5ccf85 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt @@ -13,6 +13,8 @@ import org.junit.Before import org.junit.Test import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryItemTimeGroup +import org.mozilla.fenix.library.history.PendingDeletionHistory +import org.mozilla.fenix.library.history.toPendingDeletionHistory class HistoryMetadataGroupFragmentStoreTest { @@ -37,10 +39,15 @@ class HistoryMetadataGroupFragmentStoreTest { totalViewTime = 0, historyMetadataKey = HistoryMetadataKey("http://www.firefox.com", "mozilla", null) ) + private val pendingDeletionItem = mozillaHistoryMetadataItem.toPendingDeletionHistory() @Before fun setup() { - state = HistoryMetadataGroupFragmentState() + state = HistoryMetadataGroupFragmentState( + items = emptyList(), + pendingDeletionItems = emptySet(), + isEmpty = true + ) store = HistoryMetadataGroupFragmentStore(state) } @@ -106,4 +113,22 @@ class HistoryMetadataGroupFragmentStoreTest { assertEquals(0, store.state.items.size) } + + @Test + fun `Test changing the empty state of HistoryMetadataGroupFragmentStore`() = runBlocking { + store.dispatch(HistoryMetadataGroupFragmentAction.ChangeEmptyState(false)).join() + assertFalse(store.state.isEmpty) + + store.dispatch(HistoryMetadataGroupFragmentAction.ChangeEmptyState(true)).join() + assertTrue(store.state.isEmpty) + } + + @Test + fun `Test updating pending deletion items in HistoryMetadataGroupFragmentStore`() = runBlocking { + store.dispatch(HistoryMetadataGroupFragmentAction.UpdatePendingDeletionItems(setOf(pendingDeletionItem))).join() + assertEquals(setOf(pendingDeletionItem), store.state.pendingDeletionItems) + + store.dispatch(HistoryMetadataGroupFragmentAction.UpdatePendingDeletionItems(setOf())).join() + assertEquals(emptySet(), store.state.pendingDeletionItems) + } } diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt index 18e2ce2a27..5d92063a67 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt @@ -4,11 +4,13 @@ package org.mozilla.fenix.library.historymetadata.controller +import android.content.Context import androidx.navigation.NavController import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestCoroutineScope import kotlinx.coroutines.test.runBlockingTest import mozilla.components.browser.state.action.HistoryMetadataAction @@ -31,6 +33,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R +import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.directionsEq import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @@ -52,6 +55,8 @@ class HistoryMetadataGroupControllerTest { private val scope = TestCoroutineScope(testDispatcher) private val activity: HomeActivity = mockk(relaxed = true) + private val context: Context = mockk(relaxed = true) + private val appStore: AppStore = mockk(relaxed = true) private val store: HistoryMetadataGroupFragmentStore = mockk(relaxed = true) private val browserStore: BrowserStore = mockk(relaxed = true) private val selectOrAddUseCase: TabsUseCases.SelectOrAddUseCase = mockk(relaxed = true) @@ -89,14 +94,23 @@ class HistoryMetadataGroupControllerTest { controller = DefaultHistoryMetadataGroupController( historyStorage = historyStorage, browserStore = browserStore, + appStore = appStore, store = store, selectOrAddUseCase = selectOrAddUseCase, navController = navController, - scope = scope, - searchTerm = "mozilla" + searchTerm = "mozilla", + deleteSnackbar = { items, _, delete -> + scope.launch { + delete(items).invoke(context) + } + }, + promptDeleteAll = { deleteAll -> deleteAll.invoke() }, + allDeletedSnackbar = {} ) every { activity.components.core.historyStorage } returns historyStorage + every { context.components.core.store } returns browserStore + every { context.components.core.historyStorage } returns historyStorage every { store.state.items } returns getMetadataItemsList() } diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt index f2f26b724f..5c61408f6c 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt @@ -50,7 +50,7 @@ class HistoryMetadataGroupItemViewHolderTest { @Test fun `GIVEN a history metadata item on bind THEN set the title and url text`() { every { testContext.components.core.icons } returns BrowserIcons(testContext, mockk(relaxed = true)) - HistoryMetadataGroupItemViewHolder(binding.root, interactor, selectionHolder).bind(item) + HistoryMetadataGroupItemViewHolder(binding.root, interactor, selectionHolder).bind(item, isPendingDeletion = false) assertEquals(item.title, binding.historyLayout.titleView.text) assertEquals(item.url, binding.historyLayout.urlView.text)