diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt index 380291709..f05497971 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt @@ -91,7 +91,7 @@ class BookmarksTest { }.openLibrary { }.openBookmarks { verifyBookmarkedURL(defaultWebPage.url) - verifyBookmarkFavicon() + verifyBookmarkFavicon(defaultWebPage.url) } } @@ -132,7 +132,7 @@ class BookmarksTest { }.openThreeDotMenu { }.openLibrary { }.openBookmarks { - }.openThreeDotMenu { + }.openThreeDotMenu(defaultWebPage.url) { }.clickEdit { verifyEditBookmarksView() verifyBookmarkNameEditBox() @@ -152,7 +152,7 @@ class BookmarksTest { }.openThreeDotMenu { }.openLibrary { }.openBookmarks { - }.openThreeDotMenu { + }.openThreeDotMenu(defaultWebPage.url) { }.clickCopy { verifyCopySnackBarText() } @@ -167,7 +167,7 @@ class BookmarksTest { }.openThreeDotMenu { }.openLibrary { }.openBookmarks { - }.openThreeDotMenu { + }.openThreeDotMenu(defaultWebPage.url) { }.clickOpenInNewTab { verifyPageContent(defaultWebPage.content) }.openHomeScreen { @@ -184,7 +184,7 @@ class BookmarksTest { }.openThreeDotMenu { }.openLibrary { }.openBookmarks { - }.openThreeDotMenu { + }.openThreeDotMenu(defaultWebPage.url) { }.clickOpenInPrivateTab { verifyPageContent(defaultWebPage.content) }.openHomeScreen { @@ -201,7 +201,7 @@ class BookmarksTest { }.openThreeDotMenu { }.openLibrary { }.openBookmarks { - }.openThreeDotMenu { + }.openThreeDotMenu(defaultWebPage.url) { }.clickDelete { verifyDeleteSnackBarText() } @@ -220,7 +220,7 @@ class BookmarksTest { } multipleSelectionToolbar { - verifyMultiSelectionCheckmark() + verifyMultiSelectionCheckmark(defaultWebPage.url) verifyMultiSelectionCounter() verifyShareBookmarksButton() verifyCloseToolbarButton() diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt index c26e995ac..23263a618 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt @@ -38,7 +38,7 @@ class BookmarksRobot { fun verifyEmptyBookmarksList() = assertEmptyBookmarksList() - fun verifyBookmarkFavicon() = assertBookmarkFavicon() + fun verifyBookmarkFavicon(forUrl: Uri) = assertBookmarkFavicon(forUrl) fun verifyBookmarkedURL(url: Uri) = assertBookmarkURL(url) @@ -114,6 +114,13 @@ class BookmarksRobot { ThreeDotMenuBookmarksRobot().interact() return ThreeDotMenuBookmarksRobot.Transition() } + + fun openThreeDotMenu(bookmarkUrl: Uri, interact: ThreeDotMenuBookmarksRobot.() -> Unit): ThreeDotMenuBookmarksRobot.Transition { + threeDotMenu(bookmarkUrl).click() + + ThreeDotMenuBookmarksRobot().interact() + return ThreeDotMenuBookmarksRobot.Transition() + } } } @@ -124,9 +131,14 @@ fun bookmarksMenu(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transitio private fun goBackButton() = onView(withContentDescription("Navigate up")) -private fun bookmarkFavicon() = onView(withId(R.id.favicon)) +private fun bookmarkFavicon(url: String) = onView(allOf( + withId(R.id.favicon), + withParent(withParent( + withChild(allOf(withId(R.id.url), withText(url)))) + )) +) -private fun bookmarkURL() = onView(withId(R.id.url)) +private fun bookmarkURL(url: String) = onView(allOf(withId(R.id.url), withText(url))) private fun folderTitle() = onView(withId(R.id.title)) @@ -136,14 +148,21 @@ private fun addFolderTitleField() = onView(withId(R.id.bookmarkNameEdit)) private fun saveFolderButton() = onView(withId(R.id.confirm_add_folder_button)) -private fun threeDotMenu(folderName: String) = onView( +private fun threeDotMenu(bookmarkUrl: Uri) = onView( allOf( withId(R.id.overflow_menu), - withParent(withChild(allOf(withId(R.id.title), withText(folderName)))) + withParent(withChild(allOf(withId(R.id.url), withText(bookmarkUrl.toString())))) ) ) -private fun threeDotMenu() = onView(withId(R.id.overflow_menu)) +private fun threeDotMenu(bookmarkTitle: String) = onView( + allOf( + withId(R.id.overflow_menu), + withParent(withChild(allOf(withId(R.id.title), withText(bookmarkTitle)))) + ) +) + +private fun threeDotMenu() = onView(withId(R.id.overflow_menu)).check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) private fun snackBarText() = onView(withId(R.id.snackbar_text)) @@ -160,7 +179,7 @@ private fun assertBookmarksView() { private fun assertEmptyBookmarksList() = onView(withId(R.id.bookmarks_empty_view)).check(matches(withText("No bookmarks here"))) -private fun assertBookmarkFavicon() = bookmarkFavicon().check( +private fun assertBookmarkFavicon(forUrl: Uri) = bookmarkFavicon(forUrl.toString()).check( matches( withEffectiveVisibility( ViewMatchers.Visibility.VISIBLE @@ -168,7 +187,7 @@ private fun assertBookmarkFavicon() = bookmarkFavicon().check( ) ) -private fun assertBookmarkURL(expectedURL: Uri) = bookmarkURL() +private fun assertBookmarkURL(expectedURL: Uri) = bookmarkURL(expectedURL.toString()) .check(matches(ViewMatchers.isCompletelyDisplayed())) .check(matches(withText(containsString(expectedURL.toString())))) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/LibrarySubMenusMultipleSelectionToolbarRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/LibrarySubMenusMultipleSelectionToolbarRobot.kt index 5f63ee4e3..480047094 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/LibrarySubMenusMultipleSelectionToolbarRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/LibrarySubMenusMultipleSelectionToolbarRobot.kt @@ -4,18 +4,22 @@ package org.mozilla.fenix.ui.robots +import android.net.Uri import androidx.test.espresso.Espresso.onView import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.espresso.matcher.ViewMatchers.withParent +import androidx.test.espresso.matcher.ViewMatchers.withChild import androidx.test.uiautomator.By import androidx.test.uiautomator.Until import org.mozilla.fenix.R import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime import org.mozilla.fenix.helpers.click import org.mozilla.fenix.helpers.ext.waitNotNull +import org.hamcrest.Matchers.allOf /* * Implementation of Robot Pattern for the multiple selection toolbar of History and Bookmarks menus. @@ -24,6 +28,8 @@ class LibrarySubMenusMultipleSelectionToolbarRobot { fun verifyMultiSelectionCheckmark() = assertMultiSelectionCheckmark() + fun verifyMultiSelectionCheckmark(url: Uri) = assertMultiSelectionCheckmark(url) + fun verifyMultiSelectionCounter() = assertMultiSelectionCounter() fun verifyShareHistoryButton() = assertShareHistoryButton() @@ -122,6 +128,21 @@ private fun assertMultiSelectionCheckmark() = onView(withId(R.id.checkmark)) .check(matches(isDisplayed())) +private fun assertMultiSelectionCheckmark(url: Uri) = + onView( + allOf( + withId(R.id.checkmark), + withParent(withParent(withChild(allOf(withId(R.id.url), withText(url.toString()))))), + + // This is used as part of the `multiSelectionToolbarItemsTest` test. Somehow, in the view hierarchy, + // the match above is finding two checkmark views - one visible, one hidden, which is throwing off + // the matcher. This 'isDisplayed' check is a hacky workaround for this, we're explicitly ignoring + // the hidden one. Why are there two to begin with, though? + isDisplayed() + ) + ) + .check(matches(isDisplayed())) + private fun assertMultiSelectionCounter() = onView(withText("1 selected")).check(matches(isDisplayed())) diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 7160241d2..33e7d33fa 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -20,11 +20,6 @@ object FeatureFlags { */ const val asFeatureSyncDisabled = false - /** - * Disables FxA Application Services Pairing feature - */ - const val asFeatureFxAPairingDisabled = false - /** * Enables dynamic bottom toolbar */ diff --git a/app/src/main/java/org/mozilla/fenix/components/Services.kt b/app/src/main/java/org/mozilla/fenix/components/Services.kt index 075bb2c61..65b11d6d4 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Services.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Services.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.components import android.content.Context -import androidx.navigation.NavController import androidx.preference.PreferenceManager import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -13,12 +12,7 @@ import kotlinx.coroutines.launch import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature import mozilla.components.feature.app.links.AppLinksInterceptor import mozilla.components.service.fxa.manager.FxaAccountManager -import mozilla.components.support.ktx.android.content.hasCamera -import org.mozilla.fenix.FeatureFlags -import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.utils.Mockable @@ -31,14 +25,8 @@ class Services( private val context: Context, private val accountManager: FxaAccountManager ) { - - val fxaRedirectUrl = FxaServer.redirectUrl(context) - val accountsAuthFeature by lazy { - FirefoxAccountsAuthFeature( - accountManager, - redirectUrl = fxaRedirectUrl - ) { context, authUrl -> + FirefoxAccountsAuthFeature(accountManager, FxaServer.redirectUrl(context)) { context, authUrl -> CoroutineScope(Dispatchers.Main).launch { val intent = SupportUtils.createAuthCustomTabIntent(context, authUrl) context.startActivity(intent) @@ -56,25 +44,4 @@ class Services( } ) } - - /** - * Launches the sign in and pairing custom tab from any screen in the app. - * @param context the current Context - * @param navController the navController to use for navigation - */ - fun launchPairingSignIn(context: Context, navController: NavController) { - // Do not navigate to pairing UI if camera not available or pairing is disabled - if (context.hasCamera() && !FeatureFlags.asFeatureFxAPairingDisabled) { - val directions = NavGraphDirections.actionGlobalTurnOnSync() - navController.navigate(directions) - } else { - context.components.services.accountsAuthFeature.beginAuthentication(context) - // TODO The sign-in web content populates session history, - // so pressing "back" after signing in won't take us back into the settings screen, but rather up the - // session history stack. - // We could auto-close this tab once we get to the end of the authentication process? - // Via an interceptor, perhaps. - context.components.analytics.metrics.track(Event.SyncAuthSignIn) - } - } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt index 8c4d191db..b0cadb66d 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt @@ -18,9 +18,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.Services import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.nav /** @@ -39,7 +37,6 @@ interface BookmarkController { fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleBookmarkDeletion(nodes: Set, eventType: Event) fun handleBackPressed() - fun handleSigningIn() } @SuppressWarnings("TooManyFunctions") @@ -53,7 +50,6 @@ class DefaultBookmarkController( private val activity: HomeActivity = context as HomeActivity private val resources: Resources = context.resources - private val services: Services = activity.components.services override fun handleBookmarkTapped(item: BookmarkNode) { openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, activity.browsingModeManager.mode) @@ -104,11 +100,6 @@ class DefaultBookmarkController( navController.popBackStack() } - override fun handleSigningIn() { - invokePendingDeletion.invoke() - services.launchPairingSignIn(context, navController) - } - private fun openInNewTab( searchTermOrURL: String, newTab: Boolean, 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 054394561..d87cf902b 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 @@ -31,9 +31,6 @@ import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType -import mozilla.components.concept.sync.AccountObserver -import mozilla.components.concept.sync.AuthType -import mozilla.components.concept.sync.OAuthAccount import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler import org.mozilla.fenix.HomeActivity @@ -46,6 +43,7 @@ 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.library.LibraryPageFragment import org.mozilla.fenix.utils.allowUndo @@ -64,11 +62,6 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan lateinit var initialJob: Job private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null private var pendingBookmarksToDelete: MutableSet = mutableSetOf() - private val refreshOnSignInListener = object : AccountObserver { - override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - lifecycleScope.launch { refreshBookmarks() } - } - } private val metrics get() = context?.components?.analytics?.metrics @@ -101,9 +94,6 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor) - val signInView = SignInView(view.bookmarkLayout, findNavController()) - sharedViewModel.signedIn.observe(viewLifecycleOwner, signInView) - lifecycle.addObserver( BookmarkDeselectNavigationListener( findNavController(), @@ -132,13 +122,18 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan super.onResume() (activity as HomeActivity).getSupportActionBarAndInflateIfNecessary().show() - context?.components?.backgroundServices?.accountManager?.let { accountManager -> - sharedViewModel.observeAccountManager(accountManager, owner = this) - accountManager.register(refreshOnSignInListener, owner = this) - } - val currentGuid = BookmarkFragmentArgs.fromBundle(arguments!!).currentRoot.ifEmpty { BookmarkRoot.Mobile.id } + // Only display the sign-in prompt if we're inside of the virtual "Desktop Bookmarks" node. + // Don't want to pester user too much with it, and if there are lots of bookmarks present, + // it'll just get visually lost. Inside of the "Desktop Bookmarks" node, it'll nicely stand-out, + // since there are always only three other items in there. It's also the right place contextually. + if (currentGuid == BookmarkRoot.Root.id && + requireComponents.backgroundServices.accountManager.authenticatedAccount() == null + ) { + view?.let { SignInView(it.bookmarkLayout, findNavController()) } + } + initialJob = loadInitialBookmarkFolder(currentGuid) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModel.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModel.kt index 9eeaafcab..a3b712839 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModel.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModel.kt @@ -4,53 +4,15 @@ package org.mozilla.fenix.library.bookmarks -import androidx.lifecycle.LifecycleOwner -import androidx.lifecycle.LiveData -import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.sync.AccountObserver -import mozilla.components.concept.sync.AuthType -import mozilla.components.concept.sync.OAuthAccount -import mozilla.components.service.fxa.manager.FxaAccountManager /** * [ViewModel] that shares data between various bookmarks fragments. */ -class BookmarksSharedViewModel : ViewModel(), AccountObserver { - - private val signedInMutable = MutableLiveData(true) - - /** - * Whether or not the user is signed in. - */ - val signedIn: LiveData get() = signedInMutable - +class BookmarksSharedViewModel : ViewModel() { /** * The currently selected bookmark root. */ var selectedFolder: BookmarkNode? = null - - /** - * Updates the [signedIn] boolean once the account observer sees that the user logged in. - */ - override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - signedInMutable.postValue(true) - } - - /** - * Updates the [signedIn] boolean once the account observer sees that the user logged out. - */ - override fun onLoggedOut() { - signedInMutable.postValue(false) - } - - fun observeAccountManager(accountManager: FxaAccountManager, owner: LifecycleOwner) { - accountManager.register(this, owner = owner) - if (accountManager.authenticatedAccount() != null) { - signedInMutable.postValue(true) - } else { - signedInMutable.postValue(false) - } - } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/DesktopFolders.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/DesktopFolders.kt index 7438d5fa2..f53ce4b91 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/DesktopFolders.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/DesktopFolders.kt @@ -16,7 +16,6 @@ class DesktopFolders( ) { private val bookmarksStorage = context.components.core.bookmarksStorage - private val accountManager = context.components.backgroundServices.accountManager private val bookmarksTitle = context.getString(R.string.library_bookmarks) @@ -44,18 +43,14 @@ class DesktopFolders( if (rootTitles.containsKey(node.title)) node.copy(title = rootTitles[node.title]) else node suspend fun withOptionalDesktopFolders(node: BookmarkNode): BookmarkNode { - val loggedIn = accountManager.authenticatedAccount() != null - return when (node.guid) { - BookmarkRoot.Mobile.id -> if (loggedIn) { + BookmarkRoot.Mobile.id -> { // We're going to make a copy of the mobile node, and add-in a synthetic child folder to the top of the // children's list that contains all of the desktop roots. val childrenWithVirtualFolder = listOfNotNull(virtualDesktopFolder()) + node.children.orEmpty() node.copy(children = childrenWithVirtualFolder) - } else { - node } BookmarkRoot.Root.id -> node.copy( @@ -97,14 +92,9 @@ class DesktopFolders( private fun restructureMobileRoots(roots: List?): List? { roots ?: return null - val loggedIn = accountManager.authenticatedAccount() != null - - val others = if (loggedIn) { - roots.filter { it.guid != BookmarkRoot.Mobile.id } - .map { it.copy(title = rootTitles[it.title]) } - } else { - emptyList() - } + val others = roots + .filter { it.guid != BookmarkRoot.Mobile.id } + .map { it.copy(title = rootTitles[it.title]) } val mobileRoot = roots.find { it.guid == BookmarkRoot.Mobile.id } ?: return roots val mobileChildren = others + mobileRoot.children.orEmpty() diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt index f135e174b..c4c8bc5fa 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/SignInView.kt @@ -7,36 +7,25 @@ package org.mozilla.fenix.library.bookmarks import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import androidx.core.view.isGone -import androidx.lifecycle.Observer import androidx.navigation.NavController import com.google.android.material.button.MaterialButton import kotlinx.android.extensions.LayoutContainer +import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R -import org.mozilla.fenix.ext.components class SignInView( private val container: ViewGroup, private val navController: NavController -) : LayoutContainer, Observer { +) : LayoutContainer { override val containerView: View? get() = container - val view: MaterialButton = LayoutInflater.from(container.context) + private val view: MaterialButton = LayoutInflater.from(container.context) .inflate(R.layout.component_sign_in, container, true) .findViewById(R.id.bookmark_folders_sign_in) init { - view.setOnClickListener { - view.context.components.services.launchPairingSignIn(view.context, navController) - } - } - - /** - * Hides or shows the sign-in button. Should be called whenever the sign-in state changes. - */ - override fun onChanged(signedIn: Boolean) { - view.isGone = signedIn + view.setOnClickListener { navController.navigate(NavGraphDirections.actionGlobalTurnOnSync()) } } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt index 1fd035a7b..ca2969f74 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderFragment.kt @@ -15,10 +15,8 @@ import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.lifecycleScope -import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import kotlinx.android.synthetic.main.fragment_select_bookmark_folder.* -import kotlinx.android.synthetic.main.fragment_select_bookmark_folder.view.* import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.launch @@ -28,11 +26,9 @@ import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.R 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.library.bookmarks.BookmarksSharedViewModel import org.mozilla.fenix.library.bookmarks.DesktopFolders -import org.mozilla.fenix.library.bookmarks.SignInView class SelectBookmarkFolderFragment : Fragment() { @@ -47,21 +43,13 @@ class SelectBookmarkFolderFragment : Fragment() { } override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { - val view = inflater.inflate(R.layout.fragment_select_bookmark_folder, container, false) - - val signInView = SignInView(view.selectBookmarkLayout, findNavController()) - sharedViewModel.signedIn.observe(viewLifecycleOwner, signInView) - - return view + return inflater.inflate(R.layout.fragment_select_bookmark_folder, container, false) } override fun onResume() { super.onResume() showToolbar(getString(R.string.bookmark_select_folder_fragment_label)) - val accountManager = requireComponents.backgroundServices.accountManager - sharedViewModel.observeAccountManager(accountManager, owner = this) - lifecycleScope.launch(Main) { bookmarkNode = withContext(IO) { val context = requireContext() diff --git a/app/src/main/res/layout/component_sign_in.xml b/app/src/main/res/layout/component_sign_in.xml index 65458fdfa..82cadc03b 100644 --- a/app/src/main/res/layout/component_sign_in.xml +++ b/app/src/main/res/layout/component_sign_in.xml @@ -8,7 +8,6 @@ android:id="@+id/bookmark_folders_sign_in" style="@style/NeutralButton" android:text="@string/bookmark_sign_in_button" - android:visibility="gone" android:layout_marginHorizontal="16dp" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt index ccedd1973..03ffca3a9 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -257,14 +257,4 @@ class BookmarkControllerTest { navController.popBackStack() } } - - @Test - fun `handleSigningIn should trigger 'PairingSignIn`() { - controller.handleSigningIn() - - verify { - invokePendingDeletion.invoke() - services.launchPairingSignIn(homeActivity, navController) - } - } } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModelTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModelTest.kt deleted file mode 100644 index 4406f8071..000000000 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarksSharedViewModelTest.kt +++ /dev/null @@ -1,58 +0,0 @@ -/* 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.lifecycle.LifecycleOwner -import io.mockk.every -import io.mockk.mockk -import io.mockk.verify -import mozilla.components.concept.sync.AuthType -import mozilla.components.service.fxa.manager.FxaAccountManager -import mozilla.components.support.test.mock -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@RunWith(FenixRobolectricTestRunner::class) -internal class BookmarksSharedViewModelTest { - - private lateinit var viewModel: BookmarksSharedViewModel - - @Before - fun setup() { - viewModel = BookmarksSharedViewModel() - viewModel.signedIn.observeForever { } - } - - @Test - fun `onAuthenticated and onLoggedOut modify signedIn value`() { - viewModel.onLoggedOut() - assertFalse(viewModel.signedIn.value!!) - - viewModel.onAuthenticated(mock(), AuthType.Existing) - assertTrue(viewModel.signedIn.value!!) - } - - @Test - fun `observeAccountManager registers observer`() { - val accountManager: FxaAccountManager = mockk(relaxed = true) - val lifecycleOwner: LifecycleOwner = mockk() - - every { accountManager.authenticatedAccount() } returns null - viewModel.observeAccountManager(accountManager, lifecycleOwner) - - verify { accountManager.register(viewModel, lifecycleOwner) } - assertFalse(viewModel.signedIn.value!!) - - every { accountManager.authenticatedAccount() } returns mockk() - viewModel.observeAccountManager(accountManager, lifecycleOwner) - - verify { accountManager.register(viewModel, lifecycleOwner) } - assertTrue(viewModel.signedIn.value!!) - } -} diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/DesktopFoldersTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/DesktopFoldersTest.kt index f680fef9d..603048a28 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/DesktopFoldersTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/DesktopFoldersTest.kt @@ -10,7 +10,6 @@ import io.mockk.mockk import io.mockk.spyk import kotlinx.coroutines.runBlocking import mozilla.appservices.places.BookmarkRoot -import mozilla.components.browser.storage.sync.PlacesBookmarksStorage import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.test.robolectric.testContext @@ -27,7 +26,6 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner class DesktopFoldersTest { private lateinit var context: Context - private lateinit var bookmarksStorage: PlacesBookmarksStorage private val basicNode = BookmarkNode( type = BookmarkNodeType.FOLDER, @@ -42,9 +40,7 @@ class DesktopFoldersTest { @Before fun setup() { context = spyk(testContext) - bookmarksStorage = mockk() - every { context.components.core.bookmarksStorage } returns bookmarksStorage - every { context.components.backgroundServices.accountManager.authenticatedAccount() } returns null + every { context.components.core.bookmarksStorage } returns mockk() } @Test @@ -69,15 +65,6 @@ class DesktopFoldersTest { assertEquals(testContext.getString(R.string.library_desktop_bookmarks_unfiled), desktopFolders.withRootTitle(mockNodeWithTitle("unfiled")).title) } - @Test - fun `withOptionalDesktopFolders mobile node and logged out`() = runBlocking { - every { context.components.backgroundServices.accountManager.authenticatedAccount() } returns null - val node = basicNode.copy(guid = BookmarkRoot.Mobile.id, title = BookmarkRoot.Mobile.name) - val desktopFolders = DesktopFolders(context, showMobileRoot = true) - - assertSame(node, desktopFolders.withOptionalDesktopFolders(node)) - } - @Test fun `withOptionalDesktopFolders other node`() = runBlocking { val node = basicNode.copy(guid = "12345")