From 44ff29bdc06cb6df70fefb1e130c49a591a68cd3 Mon Sep 17 00:00:00 2001 From: Emily Kager Date: Thu, 18 Jun 2020 18:49:27 -0400 Subject: [PATCH] Revert "For issue #9949 - Bookmarks/History deletion inconsistencies" This reverts commit 3feab90b19ed6bd2b373bb915e04294aa189830a. --- .../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 | 129 ++++++------------ .../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, 89 insertions(+), 206 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 3c1d7b65a..77b00a3c7 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt @@ -159,7 +159,6 @@ class HistoryTest { }.openHistory { }.openThreeDotMenu { }.clickDelete { - verifyDeleteSnackbarText("Deleted") verifyEmptyHistoryView() } } @@ -176,7 +175,6 @@ 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 61f5e4d9b..d88b6f277 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 @@ -83,8 +83,6 @@ class HistoryRobot { .click() } - fun verifyDeleteSnackbarText(text: String) = assertSnackBarText(text) - class Transition { fun goBack(interact: HistoryRobot.() -> Unit): Transition { goBackButton().click() @@ -154,6 +152,3 @@ 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 19c64df51..5a52f148c 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 @@ -35,7 +35,7 @@ interface BookmarkController { fun handleBookmarkSharing(item: BookmarkNode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleBookmarkDeletion(nodes: Set, eventType: Event) - fun handleBookmarkFolderDeletion(nodes: Set) + fun handleBookmarkFolderDeletion(node: BookmarkNode) fun handleBackPressed() } @@ -45,7 +45,7 @@ class DefaultBookmarkController( private val navController: NavController, private val showSnackbar: (String) -> Unit, private val deleteBookmarkNodes: (Set, Event) -> Unit, - private val deleteBookmarkFolder: (Set) -> Unit, + private val deleteBookmarkFolder: (BookmarkNode) -> Unit, private val invokePendingDeletion: () -> Unit ) : BookmarkController { @@ -94,8 +94,8 @@ class DefaultBookmarkController( deleteBookmarkNodes(nodes, eventType) } - override fun handleBookmarkFolderDeletion(nodes: Set) { - deleteBookmarkFolder(nodes) + override fun handleBookmarkFolderDeletion(node: BookmarkNode) { + deleteBookmarkFolder(node) } override fun handleBackPressed() { 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 21262394b..c6b26e770 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 @@ -274,17 +274,13 @@ 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, containsFolders = false) + getRemoveBookmarksSnackBarMessage(selected) } is Event.RemoveBookmarkFolder, is Event.RemoveBookmark -> { @@ -305,16 +301,9 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan ) } - private fun getRemoveBookmarksSnackBarMessage( - selected: Set, - containsFolders: Boolean - ): String { + private fun getRemoveBookmarksSnackBarMessage(selected: Set): String { return if (selected.size > 1) { - return if (containsFolders) { - getString(R.string.bookmark_deletion_multiple_snackbar_message_3) - } else { - getString(R.string.bookmark_deletion_multiple_snackbar_message_2) - } + getString(R.string.bookmark_deletion_multiple_snackbar_message_2) } else { val bookmarkNode = selected.first() getString( @@ -325,38 +314,29 @@ 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: Set) { + private fun showRemoveFolderDialog(selected: BookmarkNode) { activity?.let { activity -> AlertDialog.Builder(activity).apply { - val dialogConfirmationMessage = getDialogConfirmationMessage(selected) - setMessage(dialogConfirmationMessage) + setMessage(R.string.bookmark_delete_folder_confirmation_dialog) setNegativeButton(R.string.delete_browsing_data_prompt_cancel) { dialog: DialogInterface, _ -> dialog.cancel() } setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ -> - updatePendingBookmarksToDelete(selected) + updatePendingBookmarksToDelete(setOf(selected)) pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder) dialog.dismiss() - val snackbarMessage = getRemoveBookmarksSnackBarMessage(selected, containsFolders = true) + val message = getDeleteDialogString(selected) viewLifecycleOwner.lifecycleScope.allowUndo( requireView(), - snackbarMessage, + message, getString(R.string.bookmark_undo_deletion), { - undoPendingDeletion(selected) + undoPendingDeletion(setOf(selected)) }, operation = getDeleteOperation(Event.RemoveBookmarkFolder) ) @@ -373,6 +353,14 @@ 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 395c4f106..e8fac64b4 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 @@ -90,7 +90,7 @@ class BookmarkFragmentInteractor( null -> Event.RemoveBookmarks } if (eventType == Event.RemoveBookmarkFolder) { - bookmarksController.handleBookmarkFolderDeletion(nodes) + bookmarksController.handleBookmarkFolderDeletion(nodes.first()) } 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 234082b74..5f970334a 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,8 +33,6 @@ 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 @@ -50,33 +48,13 @@ 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 - // 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 + val previousHeader = previous?.let(::timeGroupForHistoryItem) + val currentHeader = timeGroupForHistoryItem(current) + val timeGroup = if (currentHeader != previousHeader) currentHeader else null + holder.bind(current, timeGroup, position == 0, mode) } 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 35f1a1281..ffc327cec 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,11 +17,8 @@ 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 @@ -34,18 +31,17 @@ 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 import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.getStringWithArgSafe import org.mozilla.fenix.ext.nav 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 { @@ -53,8 +49,6 @@ 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, @@ -65,10 +59,7 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl historyStore = StoreProvider.get(this) { HistoryFragmentStore( HistoryFragmentState( - items = listOf(), - mode = HistoryFragmentState.Mode.Normal, - pendingDeletionIds = emptySet(), - isDeletingItems = false + items = listOf(), mode = HistoryFragmentState.Mode.Normal ) ) } @@ -120,18 +111,18 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl } private fun deleteHistoryItems(items: Set) { - - updatePendingHistoryToDelete(items) - undoScope = CoroutineScope(IO) - undoScope?.allowUndo( - requireView(), - getMultiSelectSnackBarMessage(items), - getString(R.string.bookmark_undo_deletion), - { - undoPendingDeletion(items) - }, - getDeleteHistoryItemsOperation(items) - ) + 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) + } } @ExperimentalCoroutinesApi @@ -155,8 +146,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) @@ -171,8 +162,13 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl true } R.id.delete_history_multi_select -> { - deleteHistoryItems(historyStore.state.mode.selectedItems) - historyStore.dispatch(HistoryFragmentAction.ExitEditMode) + val message = getMultiSelectSnackBarMessage(selectedItems) + viewLifecycleOwner.lifecycleScope.launch(Main) { + deleteSelectedHistory(historyStore.state.mode.selectedItems, requireComponents) + viewModel.invalidate() + historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) + showSnackBar(requireView(), message) + } true } R.id.open_history_in_new_tabs_multi_select -> { @@ -181,7 +177,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl selectedItem.url } - navigate(HistoryFragmentDirections.actionGlobalHome()) + nav( + R.id.historyFragment, + HistoryFragmentDirections.actionGlobalHome() + ) true } R.id.open_history_in_private_tabs_multi_select -> { @@ -194,7 +193,10 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl browsingModeManager.mode = BrowsingMode.Private supportActionBar?.hide() } - navigate(HistoryFragmentDirections.actionGlobalHome()) + nav( + R.id.historyFragment, + HistoryFragmentDirections.actionGlobalHome() + ) true } else -> super.onOptionsItemSelected(item) @@ -204,22 +206,14 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandl return if (historyItems.size > 1) { getString(R.string.history_delete_multiple_items_snackbar) } else { - requireContext().getStringWithArgSafe( + getString( R.string.history_delete_single_item_snackbar, historyItems.first().url.toShortUrl(requireComponents.publicSuffixList) ) } } - override fun onPause() { - invokePendingDeletion() - super.onPause() - } - - override fun onBackPressed(): Boolean { - invokePendingDeletion() - return historyView.onBackPressed() - } + override fun onBackPressed(): Boolean = historyView.onBackPressed() private fun openItem(item: HistoryItem, mode: BrowsingMode? = null) { requireComponents.analytics.metrics.track(Event.HistoryItemOpened) @@ -259,58 +253,23 @@ 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() ) - 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.UndoRemovePendingDeletionSet(ids)) - } - - private fun invokePendingDeletion() { - pendingHistoryDeletionJob?.let { - viewLifecycleOwner.lifecycleScope.launch { - it.invoke() - }.invokeOnCompletion { - pendingHistoryDeletionJob = null - } - } + nav(R.id.historyFragment, directions) } 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 cd50df173..5b16b200c 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,8 +30,6 @@ 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 UndoRemovePendingDeletionSet(val itemIds: Set) : HistoryFragmentAction() object EnterDeletionMode : HistoryFragmentAction() object ExitDeletionMode : HistoryFragmentAction() object StartSync : HistoryFragmentAction() @@ -43,16 +41,12 @@ 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, - val pendingDeletionIds: Set, - val isDeletingItems: Boolean -) : State { +data class HistoryFragmentState(val items: List, val mode: Mode) : 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() } @@ -79,17 +73,9 @@ private fun historyStateReducer( ) } is HistoryFragmentAction.ExitEditMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) - is HistoryFragmentAction.EnterDeletionMode -> state.copy(isDeletingItems = true) - is HistoryFragmentAction.ExitDeletionMode -> state.copy(isDeletingItems = false) + is HistoryFragmentAction.EnterDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Deleting) + is HistoryFragmentAction.ExitDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) 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.UndoRemovePendingDeletionSet -> - 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 8e461b54a..b26f3ad99 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,6 +90,7 @@ 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 @@ -115,16 +116,13 @@ class HistoryView( fun update(state: HistoryFragmentState) { val oldMode = mode - view.progress_bar.isVisible = state.isDeletingItems + view.progress_bar.isVisible = state.mode === HistoryFragmentState.Mode.Deleting 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 c14be966b..685081167 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,15 +44,8 @@ class HistoryListItemViewHolder( item: HistoryItem, timeGroup: HistoryItemTimeGroup?, showDeleteButton: Boolean, - mode: HistoryFragmentState.Mode, - isPendingDeletion: Boolean = false + mode: HistoryFragmentState.Mode ) { - 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 077fb2e70..9b72708d4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -566,8 +566,6 @@ Select folder Are you sure you want to delete this folder? - - %s will delete the selected items. Deleted %1$s @@ -622,10 +620,8 @@ 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 8faf7e314..71cf57e16 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 @@ -42,7 +42,7 @@ class BookmarkControllerTest { private val navController: NavController = mockk(relaxed = true) private val showSnackbar: (String) -> Unit = mockk(relaxed = true) private val deleteBookmarkNodes: (Set, Event) -> Unit = mockk(relaxed = true) - private val deleteBookmarkFolder: (Set) -> Unit = mockk(relaxed = true) + private val deleteBookmarkFolder: (BookmarkNode) -> Unit = mockk(relaxed = true) private val invokePendingDeletion: () -> Unit = mockk(relaxed = true) private val homeActivity: HomeActivity = mockk(relaxed = true) @@ -240,10 +240,10 @@ class BookmarkControllerTest { @Test fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() { - controller.handleBookmarkFolderDeletion(setOf(subfolder)) + controller.handleBookmarkFolderDeletion(subfolder) verify { - deleteBookmarkFolder(setOf(subfolder)) + deleteBookmarkFolder(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 2ffbe3f4b..c0129e961 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 @@ -196,7 +196,7 @@ class BookmarkFragmentInteractorTest { interactor.onDelete(setOf(subfolder)) verify { - bookmarkController.handleBookmarkFolderDeletion(setOf(subfolder)) + bookmarkController.handleBookmarkFolderDeletion(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 a7b9470d5..ee68f7b6c 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,9 +60,7 @@ class HistoryFragmentStoreTest { fun finishSync() = runBlocking { val initialState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Syncing, - pendingDeletionIds = emptySet(), - isDeletingItems = false + mode = HistoryFragmentState.Mode.Syncing ) val store = HistoryFragmentStore(initialState) @@ -73,22 +71,16 @@ class HistoryFragmentStoreTest { private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Normal, - pendingDeletionIds = emptySet(), - isDeletingItems = false + mode = HistoryFragmentState.Mode.Normal ) private fun oneItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)), - pendingDeletionIds = emptySet(), - isDeletingItems = false + mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)) ) private fun twoItemEditState(): HistoryFragmentState = HistoryFragmentState( items = listOf(), - mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)), - pendingDeletionIds = emptySet(), - isDeletingItems = false + mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)) ) }