diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListener.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListener.kt new file mode 100644 index 000000000..fabc8b5b5 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListener.kt @@ -0,0 +1,47 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.library.bookmarks + +import android.os.Bundle +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleObserver +import androidx.lifecycle.OnLifecycleEvent +import androidx.navigation.NavController +import androidx.navigation.NavDestination +import org.mozilla.fenix.R + +class BookmarkDeselectNavigationListener( + private val navController: NavController, + private val viewModel: BookmarksSharedViewModel, + private val bookmarkInteractor: BookmarkViewInteractor +) : NavController.OnDestinationChangedListener, LifecycleObserver { + + @OnLifecycleEvent(Lifecycle.Event.ON_RESUME) + fun onResume() { + navController.addOnDestinationChangedListener(this) + } + + @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY) + fun onDestroy() { + navController.removeOnDestinationChangedListener(this) + } + + /** + * Deselects all items when the user navigates to a different fragment or a different folder. + */ + override fun onDestinationChanged(controller: NavController, destination: NavDestination, arguments: Bundle?) { + if (destination.id != R.id.bookmarkFragment || differentFromSelectedFolder(arguments)) { + bookmarkInteractor.onAllBookmarksDeselected() + } + } + + /** + * Returns true if the currentRoot listed in the [arguments] is different from the current selected folder. + */ + private fun differentFromSelectedFolder(arguments: Bundle?): Boolean { + return arguments != null && + BookmarkFragmentArgs.fromBundle(arguments).currentRoot != viewModel.selectedFolder?.guid + } +} 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 552c82053..4cc5770e1 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 @@ -16,7 +16,6 @@ import androidx.fragment.app.activityViewModels import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.lifecycleScope -import androidx.navigation.NavController import androidx.navigation.NavDirections import androidx.navigation.fragment.findNavController import kotlinx.android.synthetic.main.fragment_bookmark.view.* @@ -62,15 +61,6 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou } var currentRoot: BookmarkNode? = null - private val navigation by lazy { findNavController() } - private val onDestinationChangedListener = - NavController.OnDestinationChangedListener { _, destination, args -> - if (destination.id != R.id.bookmarkFragment || - args != null && BookmarkFragmentArgs.fromBundle(args).currentRoot != currentRoot?.guid) { - - bookmarkInteractor.onAllBookmarksDeselected() - } - } lateinit var initialJob: Job private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null private var pendingBookmarksToDelete: MutableSet = mutableSetOf() @@ -101,6 +91,15 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor) signInView = SignInView(view.bookmarkLayout, bookmarkInteractor) + + viewLifecycleOwner.lifecycle.addObserver( + BookmarkDeselectNavigationListener( + findNavController(), + sharedViewModel, + bookmarkInteractor + ) + ) + return view } @@ -131,7 +130,6 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou (activity as? AppCompatActivity)?.supportActionBar?.show() checkIfSignedIn() - navigation.addOnDestinationChangedListener(onDestinationChangedListener) val currentGuid = BookmarkFragmentArgs.fromBundle(arguments!!).currentRoot.ifEmpty { BookmarkRoot.Mobile.id } initialJob = loadInitialBookmarkFolder(currentGuid) @@ -158,11 +156,6 @@ class BookmarkFragment : LibraryPageFragment(), BackHandler, Accou } } - override fun onDestroyView() { - super.onDestroyView() - navigation.removeOnDestinationChangedListener(onDestinationChangedListener) - } - override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) { when (val mode = bookmarkStore.state.mode) { BookmarkFragmentState.Mode.Normal -> { diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListenerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListenerTest.kt new file mode 100644 index 000000000..632589960 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkDeselectNavigationListenerTest.kt @@ -0,0 +1,95 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.library.bookmarks + +import androidx.navigation.NavController +import androidx.navigation.NavDestination +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ObsoleteCoroutinesApi +import mozilla.appservices.places.BookmarkRoot +import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.R +import org.mozilla.fenix.TestApplication +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@ObsoleteCoroutinesApi +@RunWith(RobolectricTestRunner::class) +@Config(application = TestApplication::class) +class BookmarkDeselectNavigationListenerTest { + + private val basicNode = BookmarkNode( + BookmarkNodeType.ITEM, + BookmarkRoot.Root.id, + parentGuid = null, + position = 0, + title = null, + url = null, + children = null + ) + + @Test + fun `add listener on resume and remove on destroy`() { + val navController: NavController = mockk(relaxed = true) + val listener = BookmarkDeselectNavigationListener(navController, mockk(), mockk()) + + listener.onResume() + verify { navController.addOnDestinationChangedListener(listener) } + + listener.onDestroy() + verify { navController.removeOnDestinationChangedListener(listener) } + } + + @Test + fun `deselect when navigating to a different fragment`() { + val destination: NavDestination = mockk() + every { destination.id } returns R.id.homeFragment + + val interactor: BookmarkViewInteractor = mockk(relaxed = true) + val listener = BookmarkDeselectNavigationListener(mockk(), mockk(), interactor) + + listener.onDestinationChanged(mockk(), destination, mockk()) + verify { interactor.onAllBookmarksDeselected() } + } + + @Test + fun `deselect when navigating to a different folder`() { + val arguments = BookmarkFragmentArgs(currentRoot = "mock-guid").toBundle() + val destination: NavDestination = mockk() + every { destination.id } returns R.id.bookmarkFragment + + val viewModel: BookmarksSharedViewModel = mockk() + val interactor: BookmarkViewInteractor = mockk(relaxed = true) + val listener = BookmarkDeselectNavigationListener(mockk(), viewModel, interactor) + + every { viewModel.selectedFolder } returns null + listener.onDestinationChanged(mockk(), destination, arguments) + verify { interactor.onAllBookmarksDeselected() } + + every { viewModel.selectedFolder } returns basicNode.copy(guid = "some-other-guid") + listener.onDestinationChanged(mockk(), destination, arguments) + verify { interactor.onAllBookmarksDeselected() } + } + + @Test + fun `do not deselect when navigating to the same folder`() { + val arguments = BookmarkFragmentArgs(currentRoot = "mock-guid").toBundle() + val destination: NavDestination = mockk() + every { destination.id } returns R.id.bookmarkFragment + + val viewModel: BookmarksSharedViewModel = mockk() + val interactor: BookmarkViewInteractor = mockk(relaxed = true) + val listener = BookmarkDeselectNavigationListener(mockk(), viewModel, interactor) + + every { viewModel.selectedFolder } returns basicNode.copy(guid = "mock-guid") + listener.onDestinationChanged(mockk(), destination, arguments) + verify(exactly = 0) { interactor.onAllBookmarksDeselected() } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt index 6160b5972..74494a221 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt @@ -12,10 +12,8 @@ import io.mockk.just import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ObsoleteCoroutinesApi -import kotlinx.coroutines.launch import kotlinx.coroutines.newSingleThreadContext import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runBlockingTest @@ -125,9 +123,7 @@ class DefaultDeleteBrowsingDataControllerTest { controller = DefaultDeleteBrowsingDataController(context, coroutineContext) every { context.components.core.permissionStorage.deleteAllSitePermissions() } just Runs - launch(IO) { - controller.deleteSitePermissions() - } + controller.deleteSitePermissions() verify { context.components.core.engine.clearData(Engine.BrowsingData.select(Engine.BrowsingData.ALL_SITE_SETTINGS))