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 <mihai.badea@softvision.ro>
releases/v79.1.0
Mihai-Eduard Badea 4 years ago committed by GitHub
parent eed20b43b9
commit 1823fdb66d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -159,6 +159,7 @@ class HistoryTest {
}.openHistory { }.openHistory {
}.openThreeDotMenu { }.openThreeDotMenu {
}.clickDelete { }.clickDelete {
verifyDeleteSnackbarText("Deleted")
verifyEmptyHistoryView() verifyEmptyHistoryView()
} }
} }
@ -175,6 +176,7 @@ class HistoryTest {
clickDeleteHistoryButton() clickDeleteHistoryButton()
verifyDeleteConfirmationMessage() verifyDeleteConfirmationMessage()
confirmDeleteAllHistory() confirmDeleteAllHistory()
verifyDeleteSnackbarText("Browsing data deleted")
verifyEmptyHistoryView() verifyEmptyHistoryView()
} }
} }

@ -82,6 +82,8 @@ class HistoryRobot {
.click() .click()
} }
fun verifyDeleteSnackbarText(text: String) = assertSnackBarText(text)
class Transition { class Transition {
fun closeMenu(interact: HistoryRobot.() -> Unit): Transition { fun closeMenu(interact: HistoryRobot.() -> Unit): Transition {
closeButton().click() closeButton().click()
@ -158,3 +160,6 @@ private fun assertDeleteConfirmationMessage() =
.check(matches(isDisplayed())) .check(matches(isDisplayed()))
private fun assertCopySnackBarText() = snackBarText().check(matches(withText("URL copied"))) private fun assertCopySnackBarText() = snackBarText().check(matches(withText("URL copied")))
private fun assertSnackBarText(text: String) =
snackBarText().check(matches(withText(Matchers.containsString(text))))

@ -42,7 +42,7 @@ interface BookmarkController {
fun handleBookmarkSharing(item: BookmarkNode) fun handleBookmarkSharing(item: BookmarkNode)
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event) fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
fun handleBookmarkFolderDeletion(node: BookmarkNode) fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>)
fun handleRequestSync() fun handleRequestSync()
fun handleBackPressed() fun handleBackPressed()
} }
@ -58,7 +58,7 @@ class DefaultBookmarkController(
private val loadBookmarkNode: suspend (String) -> BookmarkNode?, private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
private val showSnackbar: (String) -> Unit, private val showSnackbar: (String) -> Unit,
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit, private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
private val deleteBookmarkFolder: (BookmarkNode) -> Unit, private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
private val invokePendingDeletion: () -> Unit private val invokePendingDeletion: () -> Unit
) : BookmarkController { ) : BookmarkController {
@ -133,8 +133,8 @@ class DefaultBookmarkController(
deleteBookmarkNodes(nodes, eventType) deleteBookmarkNodes(nodes, eventType)
} }
override fun handleBookmarkFolderDeletion(node: BookmarkNode) { override fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>) {
deleteBookmarkFolder(node) deleteBookmarkFolder(nodes)
} }
override fun handleRequestSync() { override fun handleRequestSync() {

@ -283,13 +283,17 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
} }
private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) { private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) {
selected.forEach { if (it.type == BookmarkNodeType.FOLDER) {
showRemoveFolderDialog(selected)
return
} }
updatePendingBookmarksToDelete(selected) updatePendingBookmarksToDelete(selected)
pendingBookmarkDeletionJob = getDeleteOperation(eventType) pendingBookmarkDeletionJob = getDeleteOperation(eventType)
val message = when (eventType) { val message = when (eventType) {
is Event.RemoveBookmarks -> { is Event.RemoveBookmarks -> {
getRemoveBookmarksSnackBarMessage(selected) getRemoveBookmarksSnackBarMessage(selected, containsFolders = false)
} }
is Event.RemoveBookmarkFolder, is Event.RemoveBookmarkFolder,
is Event.RemoveBookmark -> { is Event.RemoveBookmark -> {
@ -310,9 +314,16 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
) )
} }
private fun getRemoveBookmarksSnackBarMessage(selected: Set<BookmarkNode>): String { private fun getRemoveBookmarksSnackBarMessage(
selected: Set<BookmarkNode>,
containsFolders: Boolean
): String {
return if (selected.size > 1) { 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 { } else {
val bookmarkNode = selected.first() val bookmarkNode = selected.first()
getString( getString(
@ -323,29 +334,38 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
} }
} }
private fun getDialogConfirmationMessage(selected: Set<BookmarkNode>): 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() { override fun onDestroyView() {
super.onDestroyView() super.onDestroyView()
_bookmarkInteractor = null _bookmarkInteractor = null
} }
private fun showRemoveFolderDialog(selected: BookmarkNode) { private fun showRemoveFolderDialog(selected: Set<BookmarkNode>) {
activity?.let { activity -> activity?.let { activity ->
AlertDialog.Builder(activity).apply { 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, _ -> setNegativeButton(R.string.delete_browsing_data_prompt_cancel) { dialog: DialogInterface, _ ->
dialog.cancel() dialog.cancel()
} }
setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ -> setPositiveButton(R.string.delete_browsing_data_prompt_allow) { dialog: DialogInterface, _ ->
updatePendingBookmarksToDelete(setOf(selected)) updatePendingBookmarksToDelete(selected)
pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder) pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder)
dialog.dismiss() dialog.dismiss()
val message = getDeleteDialogString(selected) val snackbarMessage = getRemoveBookmarksSnackBarMessage(selected, containsFolders = true)
viewLifecycleOwner.lifecycleScope.allowUndo( viewLifecycleOwner.lifecycleScope.allowUndo(
requireView(), requireView(),
message, snackbarMessage,
getString(R.string.bookmark_undo_deletion), getString(R.string.bookmark_undo_deletion),
{ {
undoPendingDeletion(setOf(selected)) undoPendingDeletion(selected)
}, },
operation = getDeleteOperation(Event.RemoveBookmarkFolder) operation = getDeleteOperation(Event.RemoveBookmarkFolder)
) )
@ -362,14 +382,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
bookmarkInteractor.onBookmarksChanged(bookmarkTree) 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<BookmarkNode>) { private suspend fun undoPendingDeletion(selected: Set<BookmarkNode>) {
pendingBookmarksToDelete.removeAll(selected) pendingBookmarksToDelete.removeAll(selected)
pendingBookmarkDeletionJob = null pendingBookmarkDeletionJob = null

@ -88,7 +88,7 @@ class BookmarkFragmentInteractor(
null -> Event.RemoveBookmarks null -> Event.RemoveBookmarks
} }
if (eventType == Event.RemoveBookmarkFolder) { if (eventType == Event.RemoveBookmarkFolder) {
bookmarksController.handleBookmarkFolderDeletion(nodes.first()) bookmarksController.handleBookmarkFolderDeletion(nodes)
} else { } else {
bookmarksController.handleBookmarkDeletion(nodes, eventType) bookmarksController.handleBookmarkDeletion(nodes, eventType)
} }

@ -33,6 +33,8 @@ class HistoryAdapter(
private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
override val selectedItems get() = mode.selectedItems override val selectedItems get() = mode.selectedItems
var pendingDeletionIds = emptySet<Long>()
private val itemsWithHeaders: MutableMap<HistoryItemTimeGroup, Int> = mutableMapOf()
override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID override fun getItemViewType(position: Int): Int = HistoryListItemViewHolder.LAYOUT_ID
@ -48,13 +50,33 @@ class HistoryAdapter(
} }
override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) { override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) {
val previous = if (position == 0) null else getItem(position - 1)
val current = getItem(position) ?: return 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)
}
val previousHeader = previous?.let(::timeGroupForHistoryItem) fun updatePendingDeletionIds(pendingDeletionIds: Set<Long>) {
val currentHeader = timeGroupForHistoryItem(current) this.pendingDeletionIds = pendingDeletionIds
val timeGroup = if (currentHeader != previousHeader) currentHeader else null
holder.bind(current, timeGroup, position == 0, mode)
} }
companion object { companion object {

@ -17,8 +17,11 @@ import android.view.ViewGroup
import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AlertDialog
import androidx.lifecycle.Observer import androidx.lifecycle.Observer
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import androidx.navigation.NavDirections
import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.findNavController
import kotlinx.android.synthetic.main.fragment_history.view.* 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.Dispatchers.Main
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@ -31,7 +34,6 @@ import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.addons.showSnackBar import org.mozilla.fenix.addons.showSnackBar
import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.Components
import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.components.StoreProvider
import org.mozilla.fenix.components.history.createSynchronousPagedHistoryProvider 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.showToolbar
import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.ext.toShortUrl
import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.library.LibraryPageFragment
import org.mozilla.fenix.utils.allowUndo
@SuppressWarnings("TooManyFunctions", "LargeClass") @SuppressWarnings("TooManyFunctions", "LargeClass")
class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandler { class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandler {
@ -49,6 +52,8 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
private lateinit var historyView: HistoryView private lateinit var historyView: HistoryView
private lateinit var historyInteractor: HistoryInteractor private lateinit var historyInteractor: HistoryInteractor
private lateinit var viewModel: HistoryViewModel private lateinit var viewModel: HistoryViewModel
private var undoScope: CoroutineScope? = null
private var pendingHistoryDeletionJob: (suspend () -> Unit)? = null
override fun onCreateView( override fun onCreateView(
inflater: LayoutInflater, inflater: LayoutInflater,
@ -59,7 +64,10 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
historyStore = StoreProvider.get(this) { historyStore = StoreProvider.get(this) {
HistoryFragmentStore( HistoryFragmentStore(
HistoryFragmentState( HistoryFragmentState(
items = listOf(), mode = HistoryFragmentState.Mode.Normal items = listOf(),
mode = HistoryFragmentState.Mode.Normal,
pendingDeletionIds = emptySet(),
isDeletingItems = false
) )
) )
} }
@ -111,18 +119,18 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
} }
private fun deleteHistoryItems(items: Set<HistoryItem>) { private fun deleteHistoryItems(items: Set<HistoryItem>) {
val message = getMultiSelectSnackBarMessage(items)
viewLifecycleOwner.lifecycleScope.launch { updatePendingHistoryToDelete(items)
context?.components?.run { undoScope = CoroutineScope(IO)
for (item in items) { undoScope?.allowUndo(
analytics.metrics.track(Event.HistoryItemRemoved) requireView(),
core.historyStorage.deleteVisit(item.url, item.visitedAt) getMultiSelectSnackBarMessage(items),
} getString(R.string.bookmark_undo_deletion),
} {
viewModel.invalidate() undoPendingDeletion(items)
showSnackBar(requireView(), message) },
historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode) getDeleteHistoryItemsOperation(items)
} )
} }
@ExperimentalCoroutinesApi @ExperimentalCoroutinesApi
@ -146,8 +154,8 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) {
val menuRes = when (historyStore.state.mode) { val menuRes = when (historyStore.state.mode) {
HistoryFragmentState.Mode.Normal -> R.menu.library_menu HistoryFragmentState.Mode.Normal -> R.menu.library_menu
is HistoryFragmentState.Mode.Syncing -> R.menu.library_menu
is HistoryFragmentState.Mode.Editing -> R.menu.history_select_multi is HistoryFragmentState.Mode.Editing -> R.menu.history_select_multi
else -> return
} }
inflater.inflate(menuRes, menu) inflater.inflate(menuRes, menu)
@ -166,13 +174,8 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
true true
} }
R.id.delete_history_multi_select -> { R.id.delete_history_multi_select -> {
val message = getMultiSelectSnackBarMessage(selectedItems) deleteHistoryItems(historyStore.state.mode.selectedItems)
viewLifecycleOwner.lifecycleScope.launch(Main) { historyStore.dispatch(HistoryFragmentAction.ExitEditMode)
deleteSelectedHistory(historyStore.state.mode.selectedItems, requireComponents)
viewModel.invalidate()
historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode)
showSnackBar(requireView(), message)
}
true true
} }
R.id.open_history_in_new_tabs_multi_select -> { R.id.open_history_in_new_tabs_multi_select -> {
@ -181,8 +184,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
selectedItem.url selectedItem.url
} }
nav( navigate(
R.id.historyFragment,
HistoryFragmentDirections.actionGlobalTabTrayDialogFragment() HistoryFragmentDirections.actionGlobalTabTrayDialogFragment()
) )
true true
@ -197,8 +199,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
browsingModeManager.mode = BrowsingMode.Private browsingModeManager.mode = BrowsingMode.Private
supportActionBar?.hide() supportActionBar?.hide()
} }
nav( navigate(
R.id.historyFragment,
HistoryFragmentDirections.actionGlobalTabTrayDialogFragment() HistoryFragmentDirections.actionGlobalTabTrayDialogFragment()
) )
true true
@ -218,7 +219,15 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), 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) { private fun openItem(item: HistoryItem, mode: BrowsingMode? = null) {
requireComponents.analytics.metrics.track(Event.HistoryItemOpened) requireComponents.analytics.metrics.track(Event.HistoryItemOpened)
@ -258,23 +267,58 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
} }
} }
private suspend fun deleteSelectedHistory(
selected: Set<HistoryItem>,
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<ShareData>) { private fun share(data: List<ShareData>) {
requireComponents.analytics.metrics.track(Event.HistoryItemShared) requireComponents.analytics.metrics.track(Event.HistoryItemShared)
val directions = HistoryFragmentDirections.actionGlobalShareFragment( val directions = HistoryFragmentDirections.actionGlobalShareFragment(
data = data.toTypedArray() 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<HistoryItem>): (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<HistoryItem>) {
pendingHistoryDeletionJob = getDeleteHistoryItemsOperation(items)
val ids = items.map { item -> item.visitedAt }.toSet()
historyStore.dispatch(HistoryFragmentAction.AddPendingDeletionSet(ids))
}
private fun undoPendingDeletion(items: Set<HistoryItem>) {
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() { private suspend fun syncHistory() {

@ -30,6 +30,8 @@ sealed class HistoryFragmentAction : Action {
object ExitEditMode : HistoryFragmentAction() object ExitEditMode : HistoryFragmentAction()
data class AddItemForRemoval(val item: HistoryItem) : HistoryFragmentAction() data class AddItemForRemoval(val item: HistoryItem) : HistoryFragmentAction()
data class RemoveItemForRemoval(val item: HistoryItem) : HistoryFragmentAction() data class RemoveItemForRemoval(val item: HistoryItem) : HistoryFragmentAction()
data class AddPendingDeletionSet(val itemIds: Set<Long>) : HistoryFragmentAction()
data class UndoPendingDeletionSet(val itemIds: Set<Long>) : HistoryFragmentAction()
object EnterDeletionMode : HistoryFragmentAction() object EnterDeletionMode : HistoryFragmentAction()
object ExitDeletionMode : HistoryFragmentAction() object ExitDeletionMode : HistoryFragmentAction()
object StartSync : HistoryFragmentAction() object StartSync : HistoryFragmentAction()
@ -41,12 +43,16 @@ sealed class HistoryFragmentAction : Action {
* @property items List of HistoryItem to display * @property items List of HistoryItem to display
* @property mode Current Mode of History * @property mode Current Mode of History
*/ */
data class HistoryFragmentState(val items: List<HistoryItem>, val mode: Mode) : State { data class HistoryFragmentState(
val items: List<HistoryItem>,
val mode: Mode,
val pendingDeletionIds: Set<Long>,
val isDeletingItems: Boolean
) : State {
sealed class Mode { sealed class Mode {
open val selectedItems = emptySet<HistoryItem>() open val selectedItems = emptySet<HistoryItem>()
object Normal : Mode() object Normal : Mode()
object Deleting : Mode()
object Syncing : Mode() object Syncing : Mode()
data class Editing(override val selectedItems: Set<HistoryItem>) : Mode() data class Editing(override val selectedItems: Set<HistoryItem>) : Mode()
} }
@ -73,9 +79,17 @@ private fun historyStateReducer(
) )
} }
is HistoryFragmentAction.ExitEditMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) is HistoryFragmentAction.ExitEditMode -> state.copy(mode = HistoryFragmentState.Mode.Normal)
is HistoryFragmentAction.EnterDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Deleting) is HistoryFragmentAction.EnterDeletionMode -> state.copy(isDeletingItems = true)
is HistoryFragmentAction.ExitDeletionMode -> state.copy(mode = HistoryFragmentState.Mode.Normal) is HistoryFragmentAction.ExitDeletionMode -> state.copy(isDeletingItems = false)
is HistoryFragmentAction.StartSync -> state.copy(mode = HistoryFragmentState.Mode.Syncing) is HistoryFragmentAction.StartSync -> state.copy(mode = HistoryFragmentState.Mode.Syncing)
is HistoryFragmentAction.FinishSync -> state.copy(mode = HistoryFragmentState.Mode.Normal) 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
)
} }
} }

@ -90,7 +90,6 @@ class HistoryView(
val view: View = LayoutInflater.from(container.context) val view: View = LayoutInflater.from(container.context)
.inflate(R.layout.component_history, container, true) .inflate(R.layout.component_history, container, true)
private var items: List<HistoryItem> = listOf()
var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
private set private set
@ -116,13 +115,16 @@ class HistoryView(
fun update(state: HistoryFragmentState) { fun update(state: HistoryFragmentState) {
val oldMode = mode 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.isRefreshing = state.mode === HistoryFragmentState.Mode.Syncing
view.swipe_refresh.isEnabled = view.swipe_refresh.isEnabled =
state.mode === HistoryFragmentState.Mode.Normal || state.mode === HistoryFragmentState.Mode.Syncing state.mode === HistoryFragmentState.Mode.Normal || state.mode === HistoryFragmentState.Mode.Syncing
items = state.items
mode = state.mode mode = state.mode
historyAdapter.updatePendingDeletionIds(state.pendingDeletionIds)
updateEmptyState(state.pendingDeletionIds.size != historyAdapter.currentList?.size)
historyAdapter.updateMode(state.mode) historyAdapter.updateMode(state.mode)
val first = layoutManager.findFirstVisibleItemPosition() val first = layoutManager.findFirstVisibleItemPosition()
val last = layoutManager.findLastVisibleItemPosition() + 1 val last = layoutManager.findLastVisibleItemPosition() + 1

@ -11,13 +11,13 @@ import kotlinx.android.synthetic.main.library_site_item.view.*
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.hideAndDisable import org.mozilla.fenix.ext.hideAndDisable
import org.mozilla.fenix.ext.showAndEnable import org.mozilla.fenix.ext.showAndEnable
import org.mozilla.fenix.utils.Do
import org.mozilla.fenix.library.SelectionHolder import org.mozilla.fenix.library.SelectionHolder
import org.mozilla.fenix.library.history.HistoryFragmentState import org.mozilla.fenix.library.history.HistoryFragmentState
import org.mozilla.fenix.library.history.HistoryInteractor import org.mozilla.fenix.library.history.HistoryInteractor
import org.mozilla.fenix.library.history.HistoryItem import org.mozilla.fenix.library.history.HistoryItem
import org.mozilla.fenix.library.history.HistoryItemMenu import org.mozilla.fenix.library.history.HistoryItemMenu
import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.library.history.HistoryItemTimeGroup
import org.mozilla.fenix.utils.Do
class HistoryListItemViewHolder( class HistoryListItemViewHolder(
view: View, view: View,
@ -44,8 +44,15 @@ class HistoryListItemViewHolder(
item: HistoryItem, item: HistoryItem,
timeGroup: HistoryItemTimeGroup?, timeGroup: HistoryItemTimeGroup?,
showDeleteButton: Boolean, 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.titleView.text = item.title
itemView.history_layout.urlView.text = item.url itemView.history_layout.urlView.text = item.url

@ -566,6 +566,8 @@
<string name="bookmark_select_folder">Select folder</string> <string name="bookmark_select_folder">Select folder</string>
<!-- Confirmation message for a dialog confirming if the user wants to delete the selected folder --> <!-- Confirmation message for a dialog confirming if the user wants to delete the selected folder -->
<string name="bookmark_delete_folder_confirmation_dialog">Are you sure you want to delete this folder?</string> <string name="bookmark_delete_folder_confirmation_dialog">Are you sure you want to delete this folder?</string>
<!-- Confirmation message for a dialog confirming if the user wants to delete multiple items including folders. Parameter will be replaced by app name. -->
<string name="bookmark_delete_multiple_folders_confirmation_dialog">%s will delete the selected items.</string>
<!-- Snackbar title shown after a folder has been deleted. This first parameter is the name of the deleted folder --> <!-- Snackbar title shown after a folder has been deleted. This first parameter is the name of the deleted folder -->
<string name="bookmark_delete_folder_snackbar">Deleted %1$s</string> <string name="bookmark_delete_folder_snackbar">Deleted %1$s</string>
<!-- Screen title for adding a bookmarks folder --> <!-- Screen title for adding a bookmarks folder -->
@ -620,8 +622,10 @@
<!-- Bookmark snackbar message on deletion <!-- Bookmark snackbar message on deletion
The first parameter is the host part of the URL of the bookmark deleted, if any --> The first parameter is the host part of the URL of the bookmark deleted, if any -->
<string name="bookmark_deletion_snackbar_message">Deleted %1$s</string> <string name="bookmark_deletion_snackbar_message">Deleted %1$s</string>
<!-- Bookmark snackbar message on deleting multiple bookmarks --> <!-- Bookmark snackbar message on deleting multiple bookmarks not including folders-->
<string name="bookmark_deletion_multiple_snackbar_message_2">Bookmarks deleted</string> <string name="bookmark_deletion_multiple_snackbar_message_2">Bookmarks deleted</string>
<!-- Bookmark snackbar message on deleting multiple bookmarks including folders-->
<string name="bookmark_deletion_multiple_snackbar_message_3">Deleting selected folders</string>
<!-- Bookmark undo button for deletion snackbar action --> <!-- Bookmark undo button for deletion snackbar action -->
<string name="bookmark_undo_deletion">UNDO</string> <string name="bookmark_undo_deletion">UNDO</string>

@ -55,7 +55,7 @@ class BookmarkControllerTest {
private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true) private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
private val showSnackbar: (String) -> Unit = mockk(relaxed = true) private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true) private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
private val deleteBookmarkFolder: (BookmarkNode) -> Unit = mockk(relaxed = true) private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = mockk(relaxed = true)
private val invokePendingDeletion: () -> Unit = mockk(relaxed = true) private val invokePendingDeletion: () -> Unit = mockk(relaxed = true)
private val homeActivity: HomeActivity = mockk(relaxed = true) private val homeActivity: HomeActivity = mockk(relaxed = true)
@ -304,10 +304,10 @@ class BookmarkControllerTest {
@Test @Test
fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() { fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() {
controller.handleBookmarkFolderDeletion(subfolder) controller.handleBookmarkFolderDeletion(setOf(subfolder))
verify { verify {
deleteBookmarkFolder(subfolder) deleteBookmarkFolder(setOf(subfolder))
} }
} }

@ -180,7 +180,7 @@ class BookmarkFragmentInteractorTest {
interactor.onDelete(setOf(subfolder)) interactor.onDelete(setOf(subfolder))
verify { verify {
bookmarkController.handleBookmarkFolderDeletion(subfolder) bookmarkController.handleBookmarkFolderDeletion(setOf(subfolder))
} }
} }

@ -60,7 +60,9 @@ class HistoryFragmentStoreTest {
fun finishSync() = runBlocking { fun finishSync() = runBlocking {
val initialState = HistoryFragmentState( val initialState = HistoryFragmentState(
items = listOf(), items = listOf(),
mode = HistoryFragmentState.Mode.Syncing mode = HistoryFragmentState.Mode.Syncing,
pendingDeletionIds = emptySet(),
isDeletingItems = false
) )
val store = HistoryFragmentStore(initialState) val store = HistoryFragmentStore(initialState)
@ -71,16 +73,22 @@ class HistoryFragmentStoreTest {
private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState( private fun emptyDefaultState(): HistoryFragmentState = HistoryFragmentState(
items = listOf(), items = listOf(),
mode = HistoryFragmentState.Mode.Normal mode = HistoryFragmentState.Mode.Normal,
pendingDeletionIds = emptySet(),
isDeletingItems = false
) )
private fun oneItemEditState(): HistoryFragmentState = HistoryFragmentState( private fun oneItemEditState(): HistoryFragmentState = HistoryFragmentState(
items = listOf(), items = listOf(),
mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)) mode = HistoryFragmentState.Mode.Editing(setOf(historyItem)),
pendingDeletionIds = emptySet(),
isDeletingItems = false
) )
private fun twoItemEditState(): HistoryFragmentState = HistoryFragmentState( private fun twoItemEditState(): HistoryFragmentState = HistoryFragmentState(
items = listOf(), items = listOf(),
mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)) mode = HistoryFragmentState.Mode.Editing(setOf(historyItem, newHistoryItem)),
pendingDeletionIds = emptySet(),
isDeletingItems = false
) )
} }

Loading…
Cancel
Save