From 1823fdb66dec1be0f8b7e85b23f945ec48b171e3 Mon Sep 17 00:00:00 2001 From: Mihai-Eduard Badea Date: Fri, 17 Jul 2020 23:13:18 +0300 Subject: [PATCH] For issue #9949 - Bookmarks/History deletion inconsistencies (#12630) - Added the undo action for deleting individual history items by creating a new field to the history state containing the id's of the history items that are pending for deletion; This field is used inside the update function from the view to show/hide the items. - Added a new check inside the "deleteMulti" method from BookmarkFragment that calls the showRemoveFoldersDialog to prevent the user from being able to delete one or more bookmark folders without being asked for confirmation, as in #8648. Co-authored-by: Mihai Eduard Badea --- .../java/org/mozilla/fenix/ui/HistoryTest.kt | 2 + .../mozilla/fenix/ui/robots/HistoryRobot.kt | 5 + .../library/bookmarks/BookmarkController.kt | 8 +- .../library/bookmarks/BookmarkFragment.kt | 46 ++++--- .../bookmarks/BookmarkFragmentInteractor.kt | 2 +- .../fenix/library/history/HistoryAdapter.kt | 32 ++++- .../fenix/library/history/HistoryFragment.kt | 122 ++++++++++++------ .../library/history/HistoryFragmentStore.kt | 22 +++- .../fenix/library/history/HistoryView.kt | 8 +- .../viewholders/HistoryListItemViewHolder.kt | 11 +- app/src/main/res/values/strings.xml | 6 +- .../bookmarks/BookmarkControllerTest.kt | 6 +- .../BookmarkFragmentInteractorTest.kt | 2 +- .../history/HistoryFragmentStoreTest.kt | 16 ++- 14 files changed, 204 insertions(+), 84 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt index 248584e3e..db54d7480 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt @@ -159,6 +159,7 @@ class HistoryTest { }.openHistory { }.openThreeDotMenu { }.clickDelete { + verifyDeleteSnackbarText("Deleted") verifyEmptyHistoryView() } } @@ -175,6 +176,7 @@ class HistoryTest { clickDeleteHistoryButton() verifyDeleteConfirmationMessage() confirmDeleteAllHistory() + verifyDeleteSnackbarText("Browsing data deleted") verifyEmptyHistoryView() } } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt index 792ad6e6e..2debc3999 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HistoryRobot.kt @@ -82,6 +82,8 @@ class HistoryRobot { .click() } + fun verifyDeleteSnackbarText(text: String) = assertSnackBarText(text) + class Transition { fun closeMenu(interact: HistoryRobot.() -> Unit): Transition { closeButton().click() @@ -158,3 +160,6 @@ private fun assertDeleteConfirmationMessage() = .check(matches(isDisplayed())) private fun assertCopySnackBarText() = snackBarText().check(matches(withText("URL copied"))) + +private fun assertSnackBarText(text: String) = + snackBarText().check(matches(withText(Matchers.containsString(text)))) diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt index 63dcd716e..1aca89890 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt @@ -42,7 +42,7 @@ interface BookmarkController { fun handleBookmarkSharing(item: BookmarkNode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleBookmarkDeletion(nodes: Set, eventType: Event) - fun handleBookmarkFolderDeletion(node: BookmarkNode) + fun handleBookmarkFolderDeletion(nodes: Set) fun handleRequestSync() fun handleBackPressed() } @@ -58,7 +58,7 @@ class DefaultBookmarkController( private val loadBookmarkNode: suspend (String) -> BookmarkNode?, private val showSnackbar: (String) -> Unit, private val deleteBookmarkNodes: (Set, Event) -> Unit, - private val deleteBookmarkFolder: (BookmarkNode) -> Unit, + private val deleteBookmarkFolder: (Set) -> Unit, private val invokePendingDeletion: () -> Unit ) : BookmarkController { @@ -133,8 +133,8 @@ class DefaultBookmarkController( deleteBookmarkNodes(nodes, eventType) } - override fun handleBookmarkFolderDeletion(node: BookmarkNode) { - deleteBookmarkFolder(node) + override fun handleBookmarkFolderDeletion(nodes: Set) { + deleteBookmarkFolder(nodes) } override fun handleRequestSync() { 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 51d8e12de..9b76c072f 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 @@ -283,13 +283,17 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan } private fun deleteMulti(selected: Set, eventType: Event = Event.RemoveBookmarks) { + selected.forEach { if (it.type == BookmarkNodeType.FOLDER) { + showRemoveFolderDialog(selected) + return + } } updatePendingBookmarksToDelete(selected) pendingBookmarkDeletionJob = getDeleteOperation(eventType) val message = when (eventType) { is Event.RemoveBookmarks -> { - getRemoveBookmarksSnackBarMessage(selected) + getRemoveBookmarksSnackBarMessage(selected, containsFolders = false) } is Event.RemoveBookmarkFolder, is Event.RemoveBookmark -> { @@ -310,9 +314,16 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan ) } - private fun getRemoveBookmarksSnackBarMessage(selected: Set): String { + private fun getRemoveBookmarksSnackBarMessage( + selected: Set, + containsFolders: Boolean + ): String { return if (selected.size > 1) { - getString(R.string.bookmark_deletion_multiple_snackbar_message_2) + return if (containsFolders) { + getString(R.string.bookmark_deletion_multiple_snackbar_message_3) + } else { + getString(R.string.bookmark_deletion_multiple_snackbar_message_2) + } } else { val bookmarkNode = selected.first() getString( @@ -323,29 +334,38 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan } } + private fun getDialogConfirmationMessage(selected: Set): String { + return if (selected.size > 1) { + getString(R.string.bookmark_delete_multiple_folders_confirmation_dialog, getString(R.string.app_name)) + } else { + getString(R.string.bookmark_delete_folder_confirmation_dialog) + } + } + override fun onDestroyView() { super.onDestroyView() _bookmarkInteractor = null } - private fun showRemoveFolderDialog(selected: BookmarkNode) { + private fun showRemoveFolderDialog(selected: Set) { activity?.let { activity -> AlertDialog.Builder(activity).apply { - setMessage(R.string.bookmark_delete_folder_confirmation_dialog) + val dialogConfirmationMessage = getDialogConfirmationMessage(selected) + setMessage(dialogConfirmationMessage) setNegativeButton(R.string.delete_browsing_data_prompt_cancel) { dialog: DialogInterface, _ -> dialog.cancel() } setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ -> - updatePendingBookmarksToDelete(setOf(selected)) + updatePendingBookmarksToDelete(selected) pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder) dialog.dismiss() - val message = getDeleteDialogString(selected) + val snackbarMessage = getRemoveBookmarksSnackBarMessage(selected, containsFolders = true) viewLifecycleOwner.lifecycleScope.allowUndo( requireView(), - message, + snackbarMessage, getString(R.string.bookmark_undo_deletion), { - undoPendingDeletion(setOf(selected)) + undoPendingDeletion(selected) }, operation = getDeleteOperation(Event.RemoveBookmarkFolder) ) @@ -362,14 +382,6 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan bookmarkInteractor.onBookmarksChanged(bookmarkTree) } - private fun getDeleteDialogString(selected: BookmarkNode): String { - return getString( - R.string.bookmark_deletion_snackbar_message, - context?.components?.publicSuffixList?.let { selected.url?.toShortUrl(it) } - ?: selected.title - ) - } - private suspend fun undoPendingDeletion(selected: Set) { pendingBookmarksToDelete.removeAll(selected) pendingBookmarkDeletionJob = null 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 1a9cc0842..4f5e757fc 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 @@ -88,7 +88,7 @@ class BookmarkFragmentInteractor( null -> Event.RemoveBookmarks } if (eventType == Event.RemoveBookmarkFolder) { - bookmarksController.handleBookmarkFolderDeletion(nodes.first()) + bookmarksController.handleBookmarkFolderDeletion(nodes) } else { bookmarksController.handleBookmarkDeletion(nodes, eventType) } 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 5f970334a..234082b74 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 @@ -33,6 +33,8 @@ class HistoryAdapter( private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal override val selectedItems get() = mode.selectedItems + var pendingDeletionIds = emptySet() + private val itemsWithHeaders: MutableMap = mutableMapOf() override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID @@ -48,13 +50,33 @@ class HistoryAdapter( } override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) { - val previous = if (position == 0) null else getItem(position - 1) val current = getItem(position) ?: return + val headerForCurrentItem = timeGroupForHistoryItem(current) + val isPendingDeletion = pendingDeletionIds.contains(current.visitedAt) + var timeGroup: HistoryItemTimeGroup? = null - val previousHeader = previous?.let(::timeGroupForHistoryItem) - val currentHeader = timeGroupForHistoryItem(current) - val timeGroup = if (currentHeader != previousHeader) currentHeader else null - holder.bind(current, timeGroup, position == 0, mode) + // Add or remove the header and position to the map depending on it's deletion status + if (itemsWithHeaders.containsKey(headerForCurrentItem)) { + if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] == position) { + itemsWithHeaders.remove(headerForCurrentItem) + } else if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] != position) { + // do nothing + } else { + if (position <= itemsWithHeaders[headerForCurrentItem] as Int) { + itemsWithHeaders[headerForCurrentItem] = position + timeGroup = headerForCurrentItem + } + } + } else if (!isPendingDeletion) { + itemsWithHeaders[headerForCurrentItem] = position + timeGroup = headerForCurrentItem + } + + holder.bind(current, timeGroup, position == 0, mode, isPendingDeletion) + } + + fun updatePendingDeletionIds(pendingDeletionIds: Set) { + this.pendingDeletionIds = pendingDeletionIds } companion object { 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 868ac3668..a228d12c7 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 @@ -17,8 +17,11 @@ import android.view.ViewGroup import androidx.appcompat.app.AlertDialog import androidx.lifecycle.Observer import androidx.lifecycle.lifecycleScope +import androidx.navigation.NavDirections import androidx.navigation.fragment.findNavController import kotlinx.android.synthetic.main.fragment_history.view.* +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch @@ -31,7 +34,6 @@ 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.Components import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.history.createSynchronousPagedHistoryProvider @@ -42,6 +44,7 @@ import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.library.LibraryPageFragment +import org.mozilla.fenix.utils.allowUndo @SuppressWarnings("TooManyFunctions", "LargeClass") class HistoryFragment : LibraryPageFragment(), UserInteractionHandler { @@ -49,6 +52,8 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl private lateinit var historyView: HistoryView private lateinit var historyInteractor: HistoryInteractor private lateinit var viewModel: HistoryViewModel + private var undoScope: CoroutineScope? = null + private var pendingHistoryDeletionJob: (suspend () -> Unit)? = null override fun onCreateView( inflater: LayoutInflater, @@ -59,7 +64,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl historyStore = StoreProvider.get(this) { HistoryFragmentStore( HistoryFragmentState( - items = listOf(), mode = HistoryFragmentState.Mode.Normal + items = listOf(), + mode = HistoryFragmentState.Mode.Normal, + pendingDeletionIds = emptySet(), + isDeletingItems = false ) ) } @@ -111,18 +119,18 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl } private fun deleteHistoryItems(items: Set) { - val message = getMultiSelectSnackBarMessage(items) - viewLifecycleOwner.lifecycleScope.launch { - context?.components?.run { - for (item in items) { - analytics.metrics.track(Event.HistoryItemRemoved) - core.historyStorage.deleteVisit(item.url, item.visitedAt) - } - } - viewModel.invalidate() - showSnackBar(requireView(), message) - historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) - } + + updatePendingHistoryToDelete(items) + undoScope = CoroutineScope(IO) + undoScope?.allowUndo( + requireView(), + getMultiSelectSnackBarMessage(items), + getString(R.string.bookmark_undo_deletion), + { + undoPendingDeletion(items) + }, + getDeleteHistoryItemsOperation(items) + ) } @ExperimentalCoroutinesApi @@ -146,8 +154,8 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { val menuRes = when (historyStore.state.mode) { HistoryFragmentState.Mode.Normal -> R.menu.library_menu + is HistoryFragmentState.Mode.Syncing -> R.menu.library_menu is HistoryFragmentState.Mode.Editing -> R.menu.history_select_multi - else -> return } inflater.inflate(menuRes, menu) @@ -166,13 +174,8 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl true } R.id.delete_history_multi_select -> { - val message = getMultiSelectSnackBarMessage(selectedItems) - viewLifecycleOwner.lifecycleScope.launch(Main) { - deleteSelectedHistory(historyStore.state.mode.selectedItems, requireComponents) - viewModel.invalidate() - historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) - showSnackBar(requireView(), message) - } + deleteHistoryItems(historyStore.state.mode.selectedItems) + historyStore.dispatch(HistoryFragmentAction.ExitEditMode) true } R.id.open_history_in_new_tabs_multi_select -> { @@ -181,8 +184,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl selectedItem.url } - nav( - R.id.historyFragment, + navigate( HistoryFragmentDirections.actionGlobalTabTrayDialogFragment() ) true @@ -197,8 +199,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl browsingModeManager.mode = BrowsingMode.Private supportActionBar?.hide() } - nav( - R.id.historyFragment, + navigate( HistoryFragmentDirections.actionGlobalTabTrayDialogFragment() ) true @@ -218,7 +219,15 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl } } - override fun onBackPressed(): Boolean = historyView.onBackPressed() + override fun onPause() { + invokePendingDeletion() + super.onPause() + } + + override fun onBackPressed(): Boolean { + invokePendingDeletion() + return historyView.onBackPressed() + } private fun openItem(item: HistoryItem, mode: BrowsingMode? = null) { requireComponents.analytics.metrics.track(Event.HistoryItemOpened) @@ -258,23 +267,58 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl } } - private suspend fun deleteSelectedHistory( - selected: Set, - components: Components = requireComponents - ) { - requireComponents.analytics.metrics.track(Event.HistoryItemRemoved) - val storage = components.core.historyStorage - for (item in selected) { - storage.deleteVisit(item.url, item.visitedAt) - } - } - private fun share(data: List) { requireComponents.analytics.metrics.track(Event.HistoryItemShared) val directions = HistoryFragmentDirections.actionGlobalShareFragment( data = data.toTypedArray() ) - nav(R.id.historyFragment, directions) + navigate(directions) + } + + private fun navigate(directions: NavDirections) { + invokePendingDeletion() + findNavController().nav( + R.id.historyFragment, + directions + ) + } + + private fun getDeleteHistoryItemsOperation(items: Set): (suspend () -> Unit) { + return { + CoroutineScope(IO).launch { + historyStore.dispatch(HistoryFragmentAction.EnterDeletionMode) + context?.components?.run { + for (item in items) { + analytics.metrics.track(Event.HistoryItemRemoved) + core.historyStorage.deleteVisit(item.url, item.visitedAt) + } + } + historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) + pendingHistoryDeletionJob = null + } + } + } + + private fun updatePendingHistoryToDelete(items: Set) { + pendingHistoryDeletionJob = getDeleteHistoryItemsOperation(items) + val ids = items.map { item -> item.visitedAt }.toSet() + historyStore.dispatch(HistoryFragmentAction.AddPendingDeletionSet(ids)) + } + + private fun undoPendingDeletion(items: Set) { + pendingHistoryDeletionJob = null + val ids = items.map { item -> item.visitedAt }.toSet() + historyStore.dispatch(HistoryFragmentAction.UndoPendingDeletionSet(ids)) + } + + private fun invokePendingDeletion() { + pendingHistoryDeletionJob?.let { + viewLifecycleOwner.lifecycleScope.launch { + it.invoke() + }.invokeOnCompletion { + pendingHistoryDeletionJob = null + } + } } private suspend fun syncHistory() { 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 5b16b200c..f19ffc41c 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 @@ -30,6 +30,8 @@ sealed class HistoryFragmentAction : Action { object ExitEditMode : HistoryFragmentAction() data class AddItemForRemoval(val item: HistoryItem) : HistoryFragmentAction() data class RemoveItemForRemoval(val item: HistoryItem) : HistoryFragmentAction() + data class AddPendingDeletionSet(val itemIds: Set) : HistoryFragmentAction() + data class UndoPendingDeletionSet(val itemIds: Set) : HistoryFragmentAction() object EnterDeletionMode : HistoryFragmentAction() object ExitDeletionMode : HistoryFragmentAction() object StartSync : HistoryFragmentAction() @@ -41,12 +43,16 @@ sealed class HistoryFragmentAction : Action { * @property items List of HistoryItem to display * @property mode Current Mode of History */ -data class HistoryFragmentState(val items: List, val mode: Mode) : State { +data class HistoryFragmentState( + val items: List, + val mode: Mode, + val pendingDeletionIds: Set, + val isDeletingItems: Boolean +) : State { sealed class Mode { open val selectedItems = emptySet() object Normal : Mode() - object Deleting : Mode() object Syncing : Mode() data class Editing(override val selectedItems: Set) : Mode() } @@ -73,9 +79,17 @@ private fun historyStateReducer( ) } is HistoryFragmentAction.ExitEditMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) - is HistoryFragmentAction.EnterDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Deleting) - is HistoryFragmentAction.ExitDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) + is HistoryFragmentAction.EnterDeletionMode -> state.copy(isDeletingItems = true) + 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 + ) } } 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 f831f3305..09be8a3d7 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 @@ -90,7 +90,6 @@ class HistoryView( val view: View = LayoutInflater.from(container.context) .inflate(R.layout.component_history, container, true) - private var items: List = listOf() var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal private set @@ -116,13 +115,16 @@ class HistoryView( fun update(state: HistoryFragmentState) { val oldMode = mode - view.progress_bar.isVisible = state.mode === HistoryFragmentState.Mode.Deleting + view.progress_bar.isVisible = state.isDeletingItems view.swipe_refresh.isRefreshing = state.mode === HistoryFragmentState.Mode.Syncing view.swipe_refresh.isEnabled = state.mode === HistoryFragmentState.Mode.Normal || state.mode === HistoryFragmentState.Mode.Syncing - items = state.items mode = state.mode + historyAdapter.updatePendingDeletionIds(state.pendingDeletionIds) + + updateEmptyState(state.pendingDeletionIds.size != historyAdapter.currentList?.size) + historyAdapter.updateMode(state.mode) val first = layoutManager.findFirstVisibleItemPosition() val last = layoutManager.findLastVisibleItemPosition() + 1 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 685081167..c14be966b 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 @@ -11,13 +11,13 @@ import kotlinx.android.synthetic.main.library_site_item.view.* import org.mozilla.fenix.R import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.showAndEnable -import org.mozilla.fenix.utils.Do import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.history.HistoryFragmentState import org.mozilla.fenix.library.history.HistoryInteractor import org.mozilla.fenix.library.history.HistoryItem import org.mozilla.fenix.library.history.HistoryItemMenu import org.mozilla.fenix.library.history.HistoryItemTimeGroup +import org.mozilla.fenix.utils.Do class HistoryListItemViewHolder( view: View, @@ -44,8 +44,15 @@ class HistoryListItemViewHolder( item: HistoryItem, timeGroup: HistoryItemTimeGroup?, showDeleteButton: Boolean, - mode: HistoryFragmentState.Mode + mode: HistoryFragmentState.Mode, + isPendingDeletion: Boolean = false ) { + if (isPendingDeletion) { + itemView.history_layout.visibility = View.GONE + } else { + itemView.history_layout.visibility = View.VISIBLE + } + itemView.history_layout.titleView.text = item.title itemView.history_layout.urlView.text = item.url diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f8a03c6af..505fe2935 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -566,6 +566,8 @@ Select folder Are you sure you want to delete this folder? + + %s will delete the selected items. Deleted %1$s @@ -620,8 +622,10 @@ Deleted %1$s - + Bookmarks deleted + + Deleting selected folders UNDO 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 df484d0cb..d55eb3818 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 @@ -55,7 +55,7 @@ class BookmarkControllerTest { private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true) private val showSnackbar: (String) -> Unit = mockk(relaxed = true) private val deleteBookmarkNodes: (Set, Event) -> Unit = mockk(relaxed = true) - private val deleteBookmarkFolder: (BookmarkNode) -> Unit = mockk(relaxed = true) + private val deleteBookmarkFolder: (Set) -> Unit = mockk(relaxed = true) private val invokePendingDeletion: () -> Unit = mockk(relaxed = true) private val homeActivity: HomeActivity = mockk(relaxed = true) @@ -304,10 +304,10 @@ class BookmarkControllerTest { @Test fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() { - controller.handleBookmarkFolderDeletion(subfolder) + controller.handleBookmarkFolderDeletion(setOf(subfolder)) verify { - deleteBookmarkFolder(subfolder) + deleteBookmarkFolder(setOf(subfolder)) } } 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 6aba5b365..5f0a409cc 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 @@ -180,7 +180,7 @@ class BookmarkFragmentInteractorTest { interactor.onDelete(setOf(subfolder)) verify { - bookmarkController.handleBookmarkFolderDeletion(subfolder) + bookmarkController.handleBookmarkFolderDeletion(setOf(subfolder)) } } 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 ee68f7b6c..a7b9470d5 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 @@ -60,7 +60,9 @@ class HistoryFragmentStoreTest { fun finishSync() = runBlocking { val initialState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Syncing + mode = HistoryFragmentState.Mode.Syncing, + pendingDeletionIds = emptySet(), + isDeletingItems = false ) val store = HistoryFragmentStore(initialState) @@ -71,16 +73,22 @@ class HistoryFragmentStoreTest { private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Normal + mode = HistoryFragmentState.Mode.Normal, + pendingDeletionIds = emptySet(), + isDeletingItems = false ) private fun oneItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)) + mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)), + pendingDeletionIds = emptySet(), + isDeletingItems = false ) private fun twoItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)) + mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)), + pendingDeletionIds = emptySet(), + isDeletingItems = false ) }