Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
Revert "For issue #9949 - Bookmarks/History deletion inconsistencies"
Browse files Browse the repository at this point in the history
This reverts commit 3feab90.
  • Loading branch information
ekager authored Jun 18, 2020
1 parent 4270c83 commit 1c83dc4
Show file tree
Hide file tree
Showing 14 changed files with 89 additions and 206 deletions.
2 changes: 0 additions & 2 deletions app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ class HistoryTest {
}.openHistory {
}.openThreeDotMenu {
}.clickDelete {
verifyDeleteSnackbarText("Deleted")
verifyEmptyHistoryView()
}
}
Expand All @@ -176,7 +175,6 @@ class HistoryTest {
clickDeleteHistoryButton()
verifyDeleteConfirmationMessage()
confirmDeleteAllHistory()
verifyDeleteSnackbarText("Browsing data deleted")
verifyEmptyHistoryView()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ class HistoryRobot {
.click()
}

fun verifyDeleteSnackbarText(text: String) = assertSnackBarText(text)

class Transition {
fun goBack(interact: HistoryRobot.() -> Unit): Transition {
goBackButton().click()
Expand Down Expand Up @@ -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))))
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ interface BookmarkController {
fun handleBookmarkSharing(item: BookmarkNode)
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>)
fun handleBookmarkFolderDeletion(node: BookmarkNode)
fun handleBackPressed()
}

Expand All @@ -45,7 +45,7 @@ class DefaultBookmarkController(
private val navController: NavController,
private val showSnackbar: (String) -> Unit,
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
private val deleteBookmarkFolder: (BookmarkNode) -> Unit,
private val invokePendingDeletion: () -> Unit
) : BookmarkController {

Expand Down Expand Up @@ -94,8 +94,8 @@ class DefaultBookmarkController(
deleteBookmarkNodes(nodes, eventType)
}

override fun handleBookmarkFolderDeletion(nodes: Set<BookmarkNode>) {
deleteBookmarkFolder(nodes)
override fun handleBookmarkFolderDeletion(node: BookmarkNode) {
deleteBookmarkFolder(node)
}

override fun handleBackPressed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,17 +274,13 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
}

private fun deleteMulti(selected: Set<BookmarkNode>, 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 -> {
Expand All @@ -305,16 +301,9 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
)
}

private fun getRemoveBookmarksSnackBarMessage(
selected: Set<BookmarkNode>,
containsFolders: Boolean
): String {
private fun getRemoveBookmarksSnackBarMessage(selected: Set<BookmarkNode>): 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(
Expand All @@ -325,38 +314,29 @@ 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() {
super.onDestroyView()
_bookmarkInteractor = null
}

private fun showRemoveFolderDialog(selected: Set<BookmarkNode>) {
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)
)
Expand All @@ -373,6 +353,14 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), 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<BookmarkNode>) {
pendingBookmarksToDelete.removeAll(selected)
pendingBookmarkDeletionJob = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ class HistoryAdapter(

private var mode: HistoryFragmentState.Mode = HistoryFragmentState.Mode.Normal
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

Expand All @@ -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<Long>) {
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 {
Expand Down
Loading

0 comments on commit 1c83dc4

Please sign in to comment.