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 c897917a2c..2ca98130bf 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 @@ -71,6 +71,8 @@ class BookmarkFragment : Fragment(), BackHandler, AccountObserver { getManagedEmitter().onNext(BookmarkChange.ClearSelection) } lateinit var initialJob: Job + private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null + private var pendingBookmarksToDelete: MutableSet = HashSet() override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.fragment_bookmark, container, false) @@ -249,21 +251,16 @@ class BookmarkFragment : Fragment(), BackHandler, AccountObserver { } } is BookmarkAction.Delete -> { - context?.let { context -> - getManagedEmitter() - .onNext(BookmarkChange.Change(currentRoot - it.item.guid)) - - lifecycleScope.allowUndo( - view!!, - getString( - R.string.bookmark_deletion_snackbar_message, - it.item.url?.urlToTrimmedHost(context) - ), - getString(R.string.bookmark_undo_deletion), { refreshBookmarks() } - ) { - context.bookmarkStorage()?.deleteNode(it.item.guid) - metrics()?.track(Event.RemoveBookmark) - refreshBookmarks() + val bookmarkItem = it.item + if (pendingBookmarkDeletionJob == null) { + removeBookmarkWithUndo(bookmarkItem) + } else { + pendingBookmarkDeletionJob?.let { + viewLifecycleOwner.lifecycleScope.launch { + it.invoke() + }.invokeOnCompletion { + removeBookmarkWithUndo(bookmarkItem) + } } } } @@ -286,6 +283,42 @@ class BookmarkFragment : Fragment(), BackHandler, AccountObserver { } } + private fun removeBookmarkWithUndo(bookmarkNode: BookmarkNode) { + val bookmarkStorage = context.bookmarkStorage() + pendingBookmarksToDelete.add(bookmarkNode) + + var bookmarkTree = currentRoot + pendingBookmarksToDelete.forEach { + bookmarkTree -= it.guid + } + + getManagedEmitter().onNext(BookmarkChange.Change(bookmarkTree!!)) + + val deleteOperation: (suspend () -> Unit) = { + bookmarkStorage?.deleteNode(bookmarkNode.guid) + metrics()?.track(Event.RemoveBookmark) + pendingBookmarkDeletionJob = null + refreshBookmarks() + } + + pendingBookmarkDeletionJob = deleteOperation + + lifecycleScope.allowUndo( + view!!, + getString( + R.string.bookmark_deletion_snackbar_message, + bookmarkNode.url?.urlToTrimmedHost(context!!) + ), + getString(R.string.bookmark_undo_deletion), + onCancel = { + pendingBookmarkDeletionJob = null + pendingBookmarksToDelete.remove(bookmarkNode) + refreshBookmarks() + }, + operation = deleteOperation + ) + } + override fun onOptionsItemSelected(item: MenuItem): Boolean { return when (item.itemId) { R.id.libraryClose -> { @@ -330,17 +363,32 @@ class BookmarkFragment : Fragment(), BackHandler, AccountObserver { } R.id.delete_bookmarks_multi_select -> { val selectedBookmarks = getSelectedBookmarks() - getManagedEmitter().onNext(BookmarkChange.Change(currentRoot - selectedBookmarks)) + pendingBookmarksToDelete.addAll(selectedBookmarks) - lifecycleScope.allowUndo( - view!!, getString(R.string.bookmark_deletion_multiple_snackbar_message), - getString(R.string.bookmark_undo_deletion), { refreshBookmarks() } - ) { + var bookmarkTree = currentRoot + pendingBookmarksToDelete.forEach { + bookmarkTree -= it.guid + } + getManagedEmitter().onNext(BookmarkChange.Change(bookmarkTree!!)) + + val deleteOperation: (suspend () -> Unit) = { deleteSelectedBookmarks(selectedBookmarks) + pendingBookmarkDeletionJob = null // Since this runs in a coroutine, we can't depend on the fragment still being attached. metrics()?.track(Event.RemoveBookmarks) refreshBookmarks() } + + pendingBookmarkDeletionJob = deleteOperation + + lifecycleScope.allowUndo( + view!!, getString(R.string.bookmark_deletion_multiple_snackbar_message), + getString(R.string.bookmark_undo_deletion), { + pendingBookmarksToDelete.removeAll(selectedBookmarks) + pendingBookmarkDeletionJob = null + refreshBookmarks() + }, operation = deleteOperation + ) true } else -> super.onOptionsItemSelected(item) @@ -380,10 +428,29 @@ class BookmarkFragment : Fragment(), BackHandler, AccountObserver { private suspend fun refreshBookmarks() { context?.bookmarkStorage()?.getTree(currentRoot!!.guid, false).withOptionalDesktopFolders(context) ?.let { node -> - getManagedEmitter().onNext(BookmarkChange.Change(node)) + var rootNode = node + pendingBookmarksToDelete.forEach { + rootNode -= it.guid + } + getManagedEmitter().onNext(BookmarkChange.Change(rootNode)) } } + override fun onPause() { + invokePendingDeletion() + super.onPause() + } + + private fun invokePendingDeletion() { + pendingBookmarkDeletionJob?.let { + viewLifecycleOwner.lifecycleScope.launch { + it.invoke() + }.invokeOnCompletion { + pendingBookmarkDeletionJob = null + } + } + } + private fun BookmarkNode.copyUrl(context: Context) { context.getSystemService()?.apply { primaryClip = ClipData.newPlainText(url, url) diff --git a/app/src/main/res/layout/component_bookmark.xml b/app/src/main/res/layout/component_bookmark.xml index 291fdb42d9..6507fe3fc0 100644 --- a/app/src/main/res/layout/component_bookmark.xml +++ b/app/src/main/res/layout/component_bookmark.xml @@ -5,7 +5,7 @@ + android:layout_height="wrap_content"> - diff --git a/app/src/main/res/layout/fragment_bookmark.xml b/app/src/main/res/layout/fragment_bookmark.xml index 0f11c18ce8..c3456fc7a2 100644 --- a/app/src/main/res/layout/fragment_bookmark.xml +++ b/app/src/main/res/layout/fragment_bookmark.xml @@ -2,10 +2,14 @@ - + tools:context="org.mozilla.fenix.library.bookmarks.BookmarkFragment"> + +