For issue #8648
parent
18bd93ce05
commit
02bd0cc203
|
@ -445,8 +445,14 @@ class BookmarksTest {
|
|||
getInstrumentation().waitForIdleSync()
|
||||
}.openThreeDotMenu("1") {
|
||||
}.clickDelete {
|
||||
verifyDeleteFolderConfirmationMessage()
|
||||
confirmFolderDeletion()
|
||||
verifyDeleteSnackBarText()
|
||||
}.openThreeDotMenu("2") {
|
||||
}.clickDelete {
|
||||
verifyDeleteFolderConfirmationMessage()
|
||||
confirmFolderDeletion()
|
||||
verifyDeleteSnackBarText()
|
||||
verifyFolderTitle("3")
|
||||
}.goBack {
|
||||
}
|
||||
|
|
|
@ -13,6 +13,7 @@ import androidx.test.espresso.action.ViewActions.longClick
|
|||
import androidx.test.espresso.action.ViewActions.typeText
|
||||
import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
|
||||
import androidx.test.espresso.assertion.ViewAssertions.matches
|
||||
import androidx.test.espresso.matcher.RootMatchers
|
||||
import androidx.test.espresso.matcher.ViewMatchers
|
||||
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
|
||||
import androidx.test.espresso.matcher.ViewMatchers.withChild
|
||||
|
@ -113,6 +114,8 @@ class BookmarksRobot {
|
|||
|
||||
fun verifySignInToSyncButton() = signInToSyncButton().check(matches(isDisplayed()))
|
||||
|
||||
fun verifyDeleteFolderConfirmationMessage() = assertDeleteFolderConfirmationMessage()
|
||||
|
||||
fun createFolder(name: String) {
|
||||
clickAddFolderButton()
|
||||
addNewFolderName(name)
|
||||
|
@ -169,6 +172,13 @@ class BookmarksRobot {
|
|||
|
||||
fun longTapDesktopFolder(title: String) = onView(withText(title)).perform(longClick())
|
||||
|
||||
fun confirmFolderDeletion() {
|
||||
onView(withText(R.string.delete_browsing_data_prompt_allow))
|
||||
.inRoot(RootMatchers.isDialog())
|
||||
.check(matches(isDisplayed()))
|
||||
.click()
|
||||
}
|
||||
|
||||
class Transition {
|
||||
fun goBack(interact: HomeScreenRobot.() -> Unit): HomeScreenRobot.Transition {
|
||||
goBackButton().click()
|
||||
|
@ -335,3 +345,8 @@ private fun assertShareBookmarkFavicon() =
|
|||
|
||||
private fun assertShareBookmarkUrl() =
|
||||
onView(withId(R.id.share_tab_url)).check(matches(isDisplayed()))
|
||||
|
||||
private fun assertDeleteFolderConfirmationMessage() =
|
||||
onView(withText(R.string.bookmark_delete_folder_confirmation_dialog))
|
||||
.inRoot(RootMatchers.isDialog())
|
||||
.check(matches(isDisplayed()))
|
||||
|
|
|
@ -759,7 +759,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
|
|||
withContext(Main) {
|
||||
nav(
|
||||
R.id.browserFragment,
|
||||
BrowserFragmentDirections.actionGlobalBookmarkEditFragment(existing.guid)
|
||||
BrowserFragmentDirections.actionGlobalBookmarkEditFragment(existing.guid, true)
|
||||
)
|
||||
}
|
||||
} else {
|
||||
|
@ -784,7 +784,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session
|
|||
.setAction(getString(R.string.edit_bookmark_snackbar_action)) {
|
||||
nav(
|
||||
R.id.browserFragment,
|
||||
BrowserFragmentDirections.actionGlobalBookmarkEditFragment(guid)
|
||||
BrowserFragmentDirections.actionGlobalBookmarkEditFragment(guid, true)
|
||||
)
|
||||
}
|
||||
.show()
|
||||
|
|
|
@ -35,6 +35,7 @@ interface BookmarkController {
|
|||
fun handleBookmarkSharing(item: BookmarkNode)
|
||||
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
|
||||
fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
|
||||
fun handleBookmarkFolderDeletion(node: BookmarkNode)
|
||||
fun handleBackPressed()
|
||||
}
|
||||
|
||||
|
@ -44,6 +45,7 @@ class DefaultBookmarkController(
|
|||
private val navController: NavController,
|
||||
private val showSnackbar: (String) -> Unit,
|
||||
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
|
||||
private val deleteBookmarkFolder: (BookmarkNode) -> Unit,
|
||||
private val invokePendingDeletion: () -> Unit
|
||||
) : BookmarkController {
|
||||
|
||||
|
@ -92,6 +94,10 @@ class DefaultBookmarkController(
|
|||
deleteBookmarkNodes(nodes, eventType)
|
||||
}
|
||||
|
||||
override fun handleBookmarkFolderDeletion(node: BookmarkNode) {
|
||||
deleteBookmarkFolder(node)
|
||||
}
|
||||
|
||||
override fun handleBackPressed() {
|
||||
invokePendingDeletion.invoke()
|
||||
navController.popBackStack()
|
||||
|
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
package org.mozilla.fenix.library.bookmarks
|
||||
|
||||
import android.content.DialogInterface
|
||||
import android.os.Bundle
|
||||
import android.view.LayoutInflater
|
||||
import android.view.Menu
|
||||
|
@ -11,6 +12,7 @@ import android.view.MenuInflater
|
|||
import android.view.MenuItem
|
||||
import android.view.View
|
||||
import android.view.ViewGroup
|
||||
import androidx.appcompat.app.AlertDialog
|
||||
import androidx.fragment.app.activityViewModels
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.lifecycleScope
|
||||
|
@ -42,8 +44,8 @@ import org.mozilla.fenix.ext.bookmarkStorage
|
|||
import org.mozilla.fenix.ext.components
|
||||
import org.mozilla.fenix.ext.minus
|
||||
import org.mozilla.fenix.ext.nav
|
||||
import org.mozilla.fenix.ext.toShortUrl
|
||||
import org.mozilla.fenix.ext.requireComponents
|
||||
import org.mozilla.fenix.ext.toShortUrl
|
||||
import org.mozilla.fenix.library.LibraryPageFragment
|
||||
import org.mozilla.fenix.utils.allowUndo
|
||||
|
||||
|
@ -85,6 +87,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
|
|||
navController = findNavController(),
|
||||
showSnackbar = ::showSnackBarWithText,
|
||||
deleteBookmarkNodes = ::deleteMulti,
|
||||
deleteBookmarkFolder = ::showRemoveFolderDialog,
|
||||
invokePendingDeletion = ::invokePendingDeletion
|
||||
),
|
||||
metrics = metrics!!
|
||||
|
@ -261,24 +264,13 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
|
|||
}
|
||||
|
||||
private fun deleteMulti(selected: Set<BookmarkNode>, eventType: Event = Event.RemoveBookmarks) {
|
||||
pendingBookmarksToDelete.addAll(selected)
|
||||
updatePendingBookmarksToDelete(selected)
|
||||
|
||||
val bookmarkTree = sharedViewModel.selectedFolder!! - pendingBookmarksToDelete
|
||||
bookmarkInteractor.onBookmarksChanged(bookmarkTree)
|
||||
|
||||
val deleteOperation: (suspend () -> Unit) = {
|
||||
deleteSelectedBookmarks(pendingBookmarksToDelete)
|
||||
pendingBookmarkDeletionJob = null
|
||||
// Since this runs in a coroutine, we can't depend upon the fragment still being attached
|
||||
metrics?.track(Event.RemoveBookmarks)
|
||||
refreshBookmarks()
|
||||
}
|
||||
|
||||
pendingBookmarkDeletionJob = deleteOperation
|
||||
pendingBookmarkDeletionJob = getDeleteOperation(eventType)
|
||||
|
||||
val message = when (eventType) {
|
||||
is Event.RemoveBookmarks -> {
|
||||
getString(R.string.bookmark_deletion_multiple_snackbar_message)
|
||||
getRemoveBookmarksSnackBarMessage(selected)
|
||||
}
|
||||
is Event.RemoveBookmarkFolder,
|
||||
is Event.RemoveBookmark -> {
|
||||
|
@ -294,18 +286,87 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
|
|||
viewLifecycleOwner.lifecycleScope.allowUndo(
|
||||
view!!, message,
|
||||
getString(R.string.bookmark_undo_deletion), {
|
||||
pendingBookmarksToDelete.removeAll(selected)
|
||||
pendingBookmarkDeletionJob = null
|
||||
refreshBookmarks()
|
||||
}, operation = deleteOperation
|
||||
undoPendingDeletion(selected)
|
||||
}, operation = getDeleteOperation(eventType)
|
||||
)
|
||||
}
|
||||
|
||||
private fun getRemoveBookmarksSnackBarMessage(selected: Set<BookmarkNode>): String {
|
||||
return if (selected.size > 1) {
|
||||
getString(R.string.bookmark_deletion_multiple_snackbar_message)
|
||||
} else {
|
||||
val bookmarkNode = selected.first()
|
||||
getString(
|
||||
R.string.bookmark_deletion_snackbar_message,
|
||||
bookmarkNode.url?.toShortUrl(context!!.components.publicSuffixList)
|
||||
?: bookmarkNode.title
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
override fun onDestroyView() {
|
||||
super.onDestroyView()
|
||||
_bookmarkInteractor = null
|
||||
}
|
||||
|
||||
private fun showRemoveFolderDialog(selected: BookmarkNode) {
|
||||
activity?.let { activity ->
|
||||
AlertDialog.Builder(activity).apply {
|
||||
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(setOf(selected))
|
||||
pendingBookmarkDeletionJob = getDeleteOperation(Event.RemoveBookmarkFolder)
|
||||
dialog.dismiss()
|
||||
val message = getDeleteDialogString(selected)
|
||||
lifecycleScope.allowUndo(
|
||||
view!!,
|
||||
message,
|
||||
getString(R.string.bookmark_undo_deletion),
|
||||
{
|
||||
undoPendingDeletion(setOf(selected))
|
||||
},
|
||||
operation = getDeleteOperation(Event.RemoveBookmarkFolder)
|
||||
)
|
||||
}
|
||||
create()
|
||||
}
|
||||
.show()
|
||||
}
|
||||
}
|
||||
|
||||
private fun updatePendingBookmarksToDelete(selected: Set<BookmarkNode>) {
|
||||
pendingBookmarksToDelete.addAll(selected)
|
||||
val bookmarkTree = sharedViewModel.selectedFolder!! - pendingBookmarksToDelete
|
||||
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
|
||||
refreshBookmarks()
|
||||
}
|
||||
|
||||
private fun getDeleteOperation(event: Event): (suspend () -> Unit) {
|
||||
return {
|
||||
deleteSelectedBookmarks(pendingBookmarksToDelete)
|
||||
pendingBookmarkDeletionJob = null
|
||||
// Since this runs in a coroutine, we can't depend upon the fragment still being attached
|
||||
metrics?.track(event)
|
||||
refreshBookmarks()
|
||||
}
|
||||
}
|
||||
|
||||
private fun invokePendingDeletion() {
|
||||
pendingBookmarkDeletionJob?.let {
|
||||
viewLifecycleOwner.lifecycleScope.launch {
|
||||
|
|
|
@ -89,7 +89,11 @@ class BookmarkFragmentInteractor(
|
|||
BookmarkNodeType.FOLDER -> Event.RemoveBookmarkFolder
|
||||
null -> Event.RemoveBookmarks
|
||||
}
|
||||
bookmarksController.handleBookmarkDeletion(nodes, eventType)
|
||||
if (eventType == Event.RemoveBookmarkFolder) {
|
||||
bookmarksController.handleBookmarkFolderDeletion(nodes.first())
|
||||
} else {
|
||||
bookmarksController.handleBookmarkDeletion(nodes, eventType)
|
||||
}
|
||||
}
|
||||
|
||||
override fun onBackPressed() {
|
||||
|
|
|
@ -166,7 +166,7 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) {
|
|||
bookmarkNode?.let { bookmark ->
|
||||
FenixSnackbar.make(
|
||||
view = activity.getRootView()!!,
|
||||
isDisplayedWithBrowserToolbar = true
|
||||
isDisplayedWithBrowserToolbar = args.requiresSnackbarPaddingForToolbar
|
||||
)
|
||||
.setText(
|
||||
getString(
|
||||
|
|
|
@ -28,6 +28,7 @@ import mozilla.components.support.base.feature.UserInteractionHandler
|
|||
import org.mozilla.fenix.BrowserDirection
|
||||
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
|
||||
|
@ -38,6 +39,7 @@ import org.mozilla.fenix.ext.components
|
|||
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
|
||||
|
||||
@SuppressWarnings("TooManyFunctions", "LargeClass")
|
||||
|
@ -106,6 +108,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
|
|||
}
|
||||
|
||||
private fun deleteHistoryItems(items: Set<HistoryItem>) {
|
||||
val message = getMultiSelectSnackBarMessage(items)
|
||||
viewLifecycleOwner.lifecycleScope.launch {
|
||||
context?.components?.run {
|
||||
for (item in items) {
|
||||
|
@ -114,6 +117,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
|
|||
}
|
||||
}
|
||||
viewModel.invalidate()
|
||||
showSnackBar(view!!, message)
|
||||
historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode)
|
||||
}
|
||||
}
|
||||
|
@ -155,10 +159,12 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), 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)
|
||||
}
|
||||
true
|
||||
}
|
||||
|
@ -193,6 +199,17 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
|
|||
else -> super.onOptionsItemSelected(item)
|
||||
}
|
||||
|
||||
private fun getMultiSelectSnackBarMessage(historyItems: Set<HistoryItem>): String {
|
||||
return if (historyItems.size > 1) {
|
||||
getString(R.string.history_delete_multiple_items_snackbar)
|
||||
} else {
|
||||
getString(
|
||||
R.string.history_delete_single_item_snackbar,
|
||||
historyItems.first().url.toShortUrl(requireComponents.publicSuffixList)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
override fun onBackPressed(): Boolean = historyView.onBackPressed()
|
||||
|
||||
private fun openItem(item: HistoryItem, mode: BrowsingMode? = null) {
|
||||
|
@ -222,6 +239,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
|
|||
launch(Main) {
|
||||
viewModel.invalidate()
|
||||
historyStore.dispatch(HistoryFragmentAction.ExitDeletionMode)
|
||||
showSnackBar(requireView(), getString(R.string.preferences_delete_browsing_data_snackbar))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -241,6 +241,10 @@
|
|||
<action
|
||||
android:id="@+id/action_bookmarkEditFragment_to_bookmarkSelectFolderFragment"
|
||||
app:destination="@id/bookmarkSelectFolderFragment" />
|
||||
<argument
|
||||
android:name="requiresSnackbarPaddingForToolbar"
|
||||
app:argType="boolean"
|
||||
android:defaultValue="false" />
|
||||
</fragment>
|
||||
|
||||
<fragment
|
||||
|
|
|
@ -466,6 +466,10 @@
|
|||
<string name="history_delete_all">Delete history</string>
|
||||
<!-- Text for the dialog to confirm clearing all history -->
|
||||
<string name="history_delete_all_dialog">Are you sure you want to clear your history?</string>
|
||||
<!-- Text for the snackbar to confirm that multiple browsing history items has been deleted -->
|
||||
<string name="history_delete_multiple_items_snackbar">History Deleted</string>
|
||||
<!-- Text for the snackbar to confirm that a single browsing history item has been deleted -->
|
||||
<string name="history_delete_single_item_snackbar">Deleted %1$s</string>
|
||||
<!-- Text for positive action to delete history in deleting history dialog -->
|
||||
<string name="history_clear_dialog">Clear</string>
|
||||
<!-- History overflow menu copy button -->
|
||||
|
@ -520,6 +524,10 @@
|
|||
<string name="bookmark_edit">Edit bookmark</string>
|
||||
<!-- Screen title for selecting a bookmarks folder -->
|
||||
<string name="bookmark_select_folder">Select folder</string>
|
||||
<!-- 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>
|
||||
<!-- Snackbar title shown after a folder has been deleted -->
|
||||
<string name="bookmark_delete_folder_snackbar">Deleted %1$s</string>
|
||||
<!-- Screen title for adding a bookmarks folder -->
|
||||
<string name="bookmark_add_folder">Add folder</string>
|
||||
<!-- deprecated: Snackbar title shown after a bookmark has been created. -->
|
||||
|
@ -573,7 +581,7 @@
|
|||
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>
|
||||
<!-- Bookmark snackbar message on deleting multiple bookmarks -->
|
||||
<string name="bookmark_deletion_multiple_snackbar_message">Deleting selected bookmarks</string>
|
||||
<string name="bookmark_deletion_multiple_snackbar_message">Bookmarks deleted</string>
|
||||
<!-- Bookmark undo button for deletion snackbar action -->
|
||||
<string name="bookmark_undo_deletion">UNDO</string>
|
||||
|
||||
|
|
|
@ -58,7 +58,7 @@ class CrashReporterControllerTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
fun `"handle close and remove tab`() {
|
||||
fun `handle close and remove tab`() {
|
||||
val controller = CrashReporterController(crash, session, navContoller, components, settings)
|
||||
controller.handleCloseAndRemove(sendCrash = false)?.joinBlocking()
|
||||
|
||||
|
|
|
@ -42,6 +42,7 @@ class BookmarkControllerTest {
|
|||
private val navController: NavController = mockk(relaxed = true)
|
||||
private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
|
||||
private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> 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)
|
||||
|
@ -92,6 +93,7 @@ class BookmarkControllerTest {
|
|||
navController = navController,
|
||||
showSnackbar = showSnackbar,
|
||||
deleteBookmarkNodes = deleteBookmarkNodes,
|
||||
deleteBookmarkFolder = deleteBookmarkFolder,
|
||||
invokePendingDeletion = invokePendingDeletion
|
||||
)
|
||||
}
|
||||
|
@ -237,11 +239,11 @@ class BookmarkControllerTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
fun `handleBookmarkDeletion for a folder should properly call a passed in delegate`() {
|
||||
controller.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder)
|
||||
fun `handleBookmarkDeletion for a folder should properly call the delete folder delegate`() {
|
||||
controller.handleBookmarkFolderDeletion(subfolder)
|
||||
|
||||
verify {
|
||||
deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder)
|
||||
deleteBookmarkFolder(subfolder)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -196,7 +196,7 @@ class BookmarkFragmentInteractorTest {
|
|||
interactor.onDelete(setOf(subfolder))
|
||||
|
||||
verify {
|
||||
bookmarkController.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder)
|
||||
bookmarkController.handleBookmarkFolderDeletion(subfolder)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue