From 58e24b81aa1df3a628856e3fdd29aebccd089f61 Mon Sep 17 00:00:00 2001 From: Mihai Eduard Badea Date: Tue, 14 Jul 2020 11:42:02 +0300 Subject: [PATCH] For issue #12400 - Refresh swiped collection tab view Item is now refreshed by calling notifyDataSetChanged on the adapter when the last tab from the collection has been swiped away and the user cancels the deletion by pressing the cancel button from the dialog. Also added a "wasSwiped" flag to onCollectionRemoveTab in order to check if the tab was deleted from a swipe action and not by pressing the "X" button. --- .../org/mozilla/fenix/home/HomeFragment.kt | 18 +++++++++++-- .../SessionControlController.kt | 19 ++++++++----- .../SessionControlInteractor.kt | 6 ++--- .../sessioncontrol/SwipeToDeleteCallback.kt | 2 +- .../viewholders/TabInCollectionViewHolder.kt | 2 +- .../DefaultSessionControlControllerTest.kt | 27 ++++++++++++------- .../home/SessionControlInteractorTest.kt | 4 +-- 7 files changed, 54 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index d6e01c7c5..7f6aebfa7 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -211,7 +211,8 @@ class HomeFragment : Fragment() { hideOnboarding = ::hideOnboardingAndOpenSearch, registerCollectionStorageObserver = ::registerCollectionStorageObserver, showDeleteCollectionPrompt = ::showDeleteCollectionPrompt, - showTabTray = ::openTabTray + showTabTray = ::openTabTray, + handleSwipedItemDeletionCancel = ::handleSwipedItemDeletionCancel ) ) updateLayout(view) @@ -557,12 +558,21 @@ class HomeFragment : Fragment() { } } - private fun showDeleteCollectionPrompt(tabCollection: TabCollection, title: String?, message: String) { + private fun showDeleteCollectionPrompt( + tabCollection: TabCollection, + title: String?, + message: String, + wasSwiped: Boolean, + handleSwipedItemDeletionCancel: () -> Unit + ) { val context = context ?: return AlertDialog.Builder(context).apply { setTitle(title) setMessage(message) setNegativeButton(R.string.tab_collection_dialog_negative) { dialog: DialogInterface, _ -> + if (wasSwiped) { + handleSwipedItemDeletionCancel() + } dialog.cancel() } setPositiveButton(R.string.tab_collection_dialog_positive) { dialog: DialogInterface, _ -> @@ -951,6 +961,10 @@ class HomeFragment : Fragment() { view?.add_tabs_to_collections_button?.isVisible = tabCount > 0 } + private fun handleSwipedItemDeletionCancel() { + view?.sessionControlRecyclerView?.adapter?.notifyDataSetChanged() + } + companion object { const val ALL_NORMAL_TABS = "all_normal" const val ALL_PRIVATE_TABS = "all_private" diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index 85c267842..c9df30ee2 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -63,7 +63,7 @@ interface SessionControlController { /** * @see [CollectionInteractor.onCollectionRemoveTab] */ - fun handleCollectionRemoveTab(collection: TabCollection, tab: ComponentTab) + fun handleCollectionRemoveTab(collection: TabCollection, tab: ComponentTab, wasSwiped: Boolean) /** * @see [CollectionInteractor.onCollectionShareTabsClicked] @@ -160,8 +160,15 @@ class DefaultSessionControlController( private val viewLifecycleScope: CoroutineScope, private val hideOnboarding: () -> Unit, private val registerCollectionStorageObserver: () -> Unit, - private val showDeleteCollectionPrompt: (tabCollection: TabCollection, title: String?, message: String) -> Unit, - private val showTabTray: () -> Unit + private val showDeleteCollectionPrompt: ( + tabCollection: TabCollection, + title: String?, + message: String, + wasSwiped: Boolean, + handleSwipedItemDeletionCancel: () -> Unit + ) -> Unit, + private val showTabTray: () -> Unit, + private val handleSwipedItemDeletionCancel: () -> Unit ) : SessionControlController { override fun handleCollectionAddTabTapped(collection: TabCollection) { @@ -206,7 +213,7 @@ class DefaultSessionControlController( metrics.track(Event.CollectionAllTabsRestored) } - override fun handleCollectionRemoveTab(collection: TabCollection, tab: ComponentTab) { + override fun handleCollectionRemoveTab(collection: TabCollection, tab: ComponentTab, wasSwiped: Boolean) { metrics.track(Event.CollectionTabRemoved) if (collection.tabs.size == 1) { @@ -216,7 +223,7 @@ class DefaultSessionControlController( ) val message = activity.resources.getString(R.string.delete_tab_and_collection_dialog_message) - showDeleteCollectionPrompt(collection, title, message) + showDeleteCollectionPrompt(collection, title, message, wasSwiped, handleSwipedItemDeletionCancel) } else { viewLifecycleScope.launch(Dispatchers.IO) { tabCollectionStorage.removeTabFromCollection(collection, tab) @@ -232,7 +239,7 @@ class DefaultSessionControlController( override fun handleDeleteCollectionTapped(collection: TabCollection) { val message = activity.resources.getString(R.string.tab_collection_dialog_message, collection.title) - showDeleteCollectionPrompt(collection, null, message) + showDeleteCollectionPrompt(collection, null, message, false, handleSwipedItemDeletionCancel) } override fun handleOpenInPrivateTabClicked(topSite: TopSite) { diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt index f89881d4f..644178f38 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt @@ -54,7 +54,7 @@ interface CollectionInteractor { * @param collection The collection of tabs that will be modified. * @param tab The tab to remove from the tab collection. */ - fun onCollectionRemoveTab(collection: TabCollection, tab: Tab) + fun onCollectionRemoveTab(collection: TabCollection, tab: Tab, wasSwiped: Boolean) /** * Shares the tabs in the given tab collection. Called when a user clicks on the Collection @@ -189,8 +189,8 @@ class SessionControlInteractor( controller.handleCollectionOpenTabsTapped(collection) } - override fun onCollectionRemoveTab(collection: TabCollection, tab: Tab) { - controller.handleCollectionRemoveTab(collection, tab) + override fun onCollectionRemoveTab(collection: TabCollection, tab: Tab, wasSwiped: Boolean) { + controller.handleCollectionRemoveTab(collection, tab, wasSwiped) } override fun onCollectionShareTabsClicked(collection: TabCollection) { diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SwipeToDeleteCallback.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SwipeToDeleteCallback.kt index 990fca0c1..8c488cfd6 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SwipeToDeleteCallback.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SwipeToDeleteCallback.kt @@ -29,7 +29,7 @@ class SwipeToDeleteCallback( override fun onSwiped(viewHolder: RecyclerView.ViewHolder, direction: Int) { when (viewHolder) { is TabInCollectionViewHolder -> { - interactor.onCollectionRemoveTab(viewHolder.collection, viewHolder.tab) + interactor.onCollectionRemoveTab(viewHolder.collection, viewHolder.tab, wasSwiped = true) } } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt index 7f374285d..9fbb41e71 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/TabInCollectionViewHolder.kt @@ -53,7 +53,7 @@ class TabInCollectionViewHolder( list_item_action_button.increaseTapArea(buttonIncreaseDps) list_item_action_button.setOnClickListener { - interactor.onCollectionRemoveTab(collection, tab) + interactor.onCollectionRemoveTab(collection, tab, wasSwiped = false) } } diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index dc1ea1e96..201515011 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -32,7 +32,6 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.searchEngineManager import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController @@ -55,12 +54,17 @@ class DefaultSessionControlControllerTest { private val tabCollectionStorage: TabCollectionStorage = mockk(relaxed = true) private val topSiteStorage: TopSiteStorage = mockk(relaxed = true) private val tabsUseCases: TabsUseCases = mockk(relaxed = true) - private val hideOnboarding: () -> Unit = mockk(relaxed = true) private val registerCollectionStorageObserver: () -> Unit = mockk(relaxed = true) private val showTabTray: () -> Unit = mockk(relaxed = true) - private val showDeleteCollectionPrompt: (tabCollection: TabCollection, title: String?, message: String) -> Unit = - mockk(relaxed = true) + private val handleSwipedItemDeletionCancel: () -> Unit = mockk(relaxed = true) + private val showDeleteCollectionPrompt: ( + tabCollection: TabCollection, + title: String?, + message: String, + wasSwiped: Boolean, + handleSwipedItemDeletionCancel: () -> Unit + ) -> Unit = mockk(relaxed = true) private val searchEngine = mockk(relaxed = true) private val searchEngineManager = mockk(relaxed = true) private val settings: Settings = mockk(relaxed = true) @@ -102,7 +106,8 @@ class DefaultSessionControlControllerTest { hideOnboarding = hideOnboarding, registerCollectionStorageObserver = registerCollectionStorageObserver, showDeleteCollectionPrompt = showDeleteCollectionPrompt, - showTabTray = showTabTray + showTabTray = showTabTray, + handleSwipedItemDeletionCancel = handleSwipedItemDeletionCancel ) } @@ -181,14 +186,16 @@ class DefaultSessionControlControllerTest { activity.resources.getString(R.string.delete_tab_and_collection_dialog_message) } returns "Deleting this tab will delete everything." - controller.handleCollectionRemoveTab(collection, tab) + controller.handleCollectionRemoveTab(collection, tab, false) verify { metrics.track(Event.CollectionTabRemoved) } verify { showDeleteCollectionPrompt( collection, "Delete Collection?", - "Deleting this tab will delete everything." + "Deleting this tab will delete everything.", + false, + handleSwipedItemDeletionCancel ) } } @@ -197,7 +204,7 @@ class DefaultSessionControlControllerTest { fun `handleCollectionRemoveTab multiple tabs`() { val collection: TabCollection = mockk(relaxed = true) val tab: ComponentTab = mockk(relaxed = true) - controller.handleCollectionRemoveTab(collection, tab) + controller.handleCollectionRemoveTab(collection, tab, false) verify { metrics.track(Event.CollectionTabRemoved) } } @@ -231,7 +238,9 @@ class DefaultSessionControlControllerTest { showDeleteCollectionPrompt( collection, null, - "Are you sure you want to delete Collection?" + "Are you sure you want to delete Collection?", + false, + handleSwipedItemDeletionCancel ) } } diff --git a/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt index c51bd81d6..63bc74661 100644 --- a/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt @@ -49,8 +49,8 @@ class SessionControlInteractorTest { fun onCollectionRemoveTab() { val collection: TabCollection = mockk(relaxed = true) val tab: Tab = mockk(relaxed = true) - interactor.onCollectionRemoveTab(collection, tab) - verify { controller.handleCollectionRemoveTab(collection, tab) } + interactor.onCollectionRemoveTab(collection, tab, false) + verify { controller.handleCollectionRemoveTab(collection, tab, false) } } @Test