Closes #4046: Always display 'Desktop' bookmarks folder

See https://github.com/mozilla-mobile/fenix/issues/4046 for a detailed discussion of this.

In short, this patch removes code that would conditionally hide desktop bookmarks depending
on the signed-in state of the browser.
fennec/production
Grisha Kruglov 5 years ago committed by Grisha Kruglov
parent c16283a923
commit 853a0acab4

@ -91,7 +91,7 @@ class BookmarksTest {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
verifyBookmarkedURL(defaultWebPage.url) verifyBookmarkedURL(defaultWebPage.url)
verifyBookmarkFavicon() verifyBookmarkFavicon(defaultWebPage.url)
} }
} }
@ -132,7 +132,7 @@ class BookmarksTest {
}.openThreeDotMenu { }.openThreeDotMenu {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
}.openThreeDotMenu { }.openThreeDotMenu(defaultWebPage.url) {
}.clickEdit { }.clickEdit {
verifyEditBookmarksView() verifyEditBookmarksView()
verifyBookmarkNameEditBox() verifyBookmarkNameEditBox()
@ -152,7 +152,7 @@ class BookmarksTest {
}.openThreeDotMenu { }.openThreeDotMenu {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
}.openThreeDotMenu { }.openThreeDotMenu(defaultWebPage.url) {
}.clickCopy { }.clickCopy {
verifyCopySnackBarText() verifyCopySnackBarText()
} }
@ -167,7 +167,7 @@ class BookmarksTest {
}.openThreeDotMenu { }.openThreeDotMenu {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
}.openThreeDotMenu { }.openThreeDotMenu(defaultWebPage.url) {
}.clickOpenInNewTab { }.clickOpenInNewTab {
verifyPageContent(defaultWebPage.content) verifyPageContent(defaultWebPage.content)
}.openHomeScreen { }.openHomeScreen {
@ -184,7 +184,7 @@ class BookmarksTest {
}.openThreeDotMenu { }.openThreeDotMenu {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
}.openThreeDotMenu { }.openThreeDotMenu(defaultWebPage.url) {
}.clickOpenInPrivateTab { }.clickOpenInPrivateTab {
verifyPageContent(defaultWebPage.content) verifyPageContent(defaultWebPage.content)
}.openHomeScreen { }.openHomeScreen {
@ -201,7 +201,7 @@ class BookmarksTest {
}.openThreeDotMenu { }.openThreeDotMenu {
}.openLibrary { }.openLibrary {
}.openBookmarks { }.openBookmarks {
}.openThreeDotMenu { }.openThreeDotMenu(defaultWebPage.url) {
}.clickDelete { }.clickDelete {
verifyDeleteSnackBarText() verifyDeleteSnackBarText()
} }
@ -220,7 +220,7 @@ class BookmarksTest {
} }
multipleSelectionToolbar { multipleSelectionToolbar {
verifyMultiSelectionCheckmark() verifyMultiSelectionCheckmark(defaultWebPage.url)
verifyMultiSelectionCounter() verifyMultiSelectionCounter()
verifyShareBookmarksButton() verifyShareBookmarksButton()
verifyCloseToolbarButton() verifyCloseToolbarButton()

@ -38,7 +38,7 @@ class BookmarksRobot {
fun verifyEmptyBookmarksList() = assertEmptyBookmarksList() fun verifyEmptyBookmarksList() = assertEmptyBookmarksList()
fun verifyBookmarkFavicon() = assertBookmarkFavicon() fun verifyBookmarkFavicon(forUrl: Uri) = assertBookmarkFavicon(forUrl)
fun verifyBookmarkedURL(url: Uri) = assertBookmarkURL(url) fun verifyBookmarkedURL(url: Uri) = assertBookmarkURL(url)
@ -114,6 +114,13 @@ class BookmarksRobot {
ThreeDotMenuBookmarksRobot().interact() ThreeDotMenuBookmarksRobot().interact()
return ThreeDotMenuBookmarksRobot.Transition() 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 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)) 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 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.url), withText(bookmarkUrl.toString()))))
)
)
private fun threeDotMenu(bookmarkTitle: String) = onView(
allOf( allOf(
withId(R.id.overflow_menu), withId(R.id.overflow_menu),
withParent(withChild(allOf(withId(R.id.title), withText(folderName)))) withParent(withChild(allOf(withId(R.id.title), withText(bookmarkTitle))))
) )
) )
private fun threeDotMenu() = onView(withId(R.id.overflow_menu)) private fun threeDotMenu() = onView(withId(R.id.overflow_menu)).check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
private fun snackBarText() = onView(withId(R.id.snackbar_text)) private fun snackBarText() = onView(withId(R.id.snackbar_text))
@ -160,7 +179,7 @@ private fun assertBookmarksView() {
private fun assertEmptyBookmarksList() = private fun assertEmptyBookmarksList() =
onView(withId(R.id.bookmarks_empty_view)).check(matches(withText("No bookmarks here"))) 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( matches(
withEffectiveVisibility( withEffectiveVisibility(
ViewMatchers.Visibility.VISIBLE 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(ViewMatchers.isCompletelyDisplayed()))
.check(matches(withText(containsString(expectedURL.toString())))) .check(matches(withText(containsString(expectedURL.toString()))))

@ -4,18 +4,22 @@
package org.mozilla.fenix.ui.robots package org.mozilla.fenix.ui.robots
import android.net.Uri
import androidx.test.espresso.Espresso.onView import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withContentDescription
import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText 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.By
import androidx.test.uiautomator.Until import androidx.test.uiautomator.Until
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime
import org.mozilla.fenix.helpers.click import org.mozilla.fenix.helpers.click
import org.mozilla.fenix.helpers.ext.waitNotNull 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. * 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() = assertMultiSelectionCheckmark()
fun verifyMultiSelectionCheckmark(url: Uri) = assertMultiSelectionCheckmark(url)
fun verifyMultiSelectionCounter() = assertMultiSelectionCounter() fun verifyMultiSelectionCounter() = assertMultiSelectionCounter()
fun verifyShareHistoryButton() = assertShareHistoryButton() fun verifyShareHistoryButton() = assertShareHistoryButton()
@ -122,6 +128,21 @@ private fun assertMultiSelectionCheckmark() =
onView(withId(R.id.checkmark)) onView(withId(R.id.checkmark))
.check(matches(isDisplayed())) .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() = private fun assertMultiSelectionCounter() =
onView(withText("1 selected")).check(matches(isDisplayed())) onView(withText("1 selected")).check(matches(isDisplayed()))

@ -20,11 +20,6 @@ object FeatureFlags {
*/ */
const val asFeatureSyncDisabled = false const val asFeatureSyncDisabled = false
/**
* Disables FxA Application Services Pairing feature
*/
const val asFeatureFxAPairingDisabled = false
/** /**
* Enables dynamic bottom toolbar * Enables dynamic bottom toolbar
*/ */

@ -5,7 +5,6 @@
package org.mozilla.fenix.components package org.mozilla.fenix.components
import android.content.Context import android.content.Context
import androidx.navigation.NavController
import androidx.preference.PreferenceManager import androidx.preference.PreferenceManager
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
@ -13,12 +12,7 @@ import kotlinx.coroutines.launch
import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature import mozilla.components.feature.accounts.FirefoxAccountsAuthFeature
import mozilla.components.feature.app.links.AppLinksInterceptor import mozilla.components.feature.app.links.AppLinksInterceptor
import mozilla.components.service.fxa.manager.FxaAccountManager 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.R
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.getPreferenceKey
import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.utils.Mockable import org.mozilla.fenix.utils.Mockable
@ -31,14 +25,8 @@ class Services(
private val context: Context, private val context: Context,
private val accountManager: FxaAccountManager private val accountManager: FxaAccountManager
) { ) {
val fxaRedirectUrl = FxaServer.redirectUrl(context)
val accountsAuthFeature by lazy { val accountsAuthFeature by lazy {
FirefoxAccountsAuthFeature( FirefoxAccountsAuthFeature(accountManager, FxaServer.redirectUrl(context)) { context, authUrl ->
accountManager,
redirectUrl = fxaRedirectUrl
) { context, authUrl ->
CoroutineScope(Dispatchers.Main).launch { CoroutineScope(Dispatchers.Main).launch {
val intent = SupportUtils.createAuthCustomTabIntent(context, authUrl) val intent = SupportUtils.createAuthCustomTabIntent(context, authUrl)
context.startActivity(intent) 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)
}
}
} }

@ -18,9 +18,7 @@ import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.Services
import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.nav
/** /**
@ -39,7 +37,6 @@ interface BookmarkController {
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event) fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
fun handleBackPressed() fun handleBackPressed()
fun handleSigningIn()
} }
@SuppressWarnings("TooManyFunctions") @SuppressWarnings("TooManyFunctions")
@ -53,7 +50,6 @@ class DefaultBookmarkController(
private val activity: HomeActivity = context as HomeActivity private val activity: HomeActivity = context as HomeActivity
private val resources: Resources = context.resources private val resources: Resources = context.resources
private val services: Services = activity.components.services
override fun handleBookmarkTapped(item: BookmarkNode) { override fun handleBookmarkTapped(item: BookmarkNode) {
openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, activity.browsingModeManager.mode) openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, activity.browsingModeManager.mode)
@ -104,11 +100,6 @@ class DefaultBookmarkController(
navController.popBackStack() navController.popBackStack()
} }
override fun handleSigningIn() {
invokePendingDeletion.invoke()
services.launchPairingSignIn(context, navController)
}
private fun openInNewTab( private fun openInNewTab(
searchTermOrURL: String, searchTermOrURL: String,
newTab: Boolean, newTab: Boolean,

@ -31,9 +31,6 @@ import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType 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.lib.state.ext.consumeFrom
import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.UserInteractionHandler
import org.mozilla.fenix.HomeActivity 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.minus
import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.toShortUrl import org.mozilla.fenix.ext.toShortUrl
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.library.LibraryPageFragment
import org.mozilla.fenix.utils.allowUndo import org.mozilla.fenix.utils.allowUndo
@ -64,11 +62,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
lateinit var initialJob: Job lateinit var initialJob: Job
private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null
private var pendingBookmarksToDelete: MutableSet<BookmarkNode> = mutableSetOf() private var pendingBookmarksToDelete: MutableSet<BookmarkNode> = mutableSetOf()
private val refreshOnSignInListener = object : AccountObserver {
override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
lifecycleScope.launch { refreshBookmarks() }
}
}
private val metrics private val metrics
get() = context?.components?.analytics?.metrics get() = context?.components?.analytics?.metrics
@ -101,9 +94,6 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor) bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor)
val signInView = SignInView(view.bookmarkLayout, findNavController())
sharedViewModel.signedIn.observe(viewLifecycleOwner, signInView)
lifecycle.addObserver( lifecycle.addObserver(
BookmarkDeselectNavigationListener( BookmarkDeselectNavigationListener(
findNavController(), findNavController(),
@ -132,13 +122,18 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
super.onResume() super.onResume()
(activity as HomeActivity).getSupportActionBarAndInflateIfNecessary().show() (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 } 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) initialJob = loadInitialBookmarkFolder(currentGuid)
} }

@ -4,53 +4,15 @@
package org.mozilla.fenix.library.bookmarks package org.mozilla.fenix.library.bookmarks
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModel
import mozilla.components.concept.storage.BookmarkNode 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. * [ViewModel] that shares data between various bookmarks fragments.
*/ */
class BookmarksSharedViewModel : ViewModel(), AccountObserver { class BookmarksSharedViewModel : ViewModel() {
private val signedInMutable = MutableLiveData(true)
/**
* Whether or not the user is signed in.
*/
val signedIn: LiveData<Boolean> get() = signedInMutable
/** /**
* The currently selected bookmark root. * The currently selected bookmark root.
*/ */
var selectedFolder: BookmarkNode? = null 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)
}
}
} }

@ -16,7 +16,6 @@ class DesktopFolders(
) { ) {
private val bookmarksStorage = context.components.core.bookmarksStorage private val bookmarksStorage = context.components.core.bookmarksStorage
private val accountManager = context.components.backgroundServices.accountManager
private val bookmarksTitle = context.getString(R.string.library_bookmarks) 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 if (rootTitles.containsKey(node.title)) node.copy(title = rootTitles[node.title]) else node
suspend fun withOptionalDesktopFolders(node: BookmarkNode): BookmarkNode { suspend fun withOptionalDesktopFolders(node: BookmarkNode): BookmarkNode {
val loggedIn = accountManager.authenticatedAccount() != null
return when (node.guid) { 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 // 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. // children's list that contains all of the desktop roots.
val childrenWithVirtualFolder = val childrenWithVirtualFolder =
listOfNotNull(virtualDesktopFolder()) + node.children.orEmpty() listOfNotNull(virtualDesktopFolder()) + node.children.orEmpty()
node.copy(children = childrenWithVirtualFolder) node.copy(children = childrenWithVirtualFolder)
} else {
node
} }
BookmarkRoot.Root.id -> BookmarkRoot.Root.id ->
node.copy( node.copy(
@ -97,14 +92,9 @@ class DesktopFolders(
private fun restructureMobileRoots(roots: List<BookmarkNode>?): List<BookmarkNode>? { private fun restructureMobileRoots(roots: List<BookmarkNode>?): List<BookmarkNode>? {
roots ?: return null roots ?: return null
val loggedIn = accountManager.authenticatedAccount() != null val others = roots
.filter { it.guid != BookmarkRoot.Mobile.id }
val others = if (loggedIn) {
roots.filter { it.guid != BookmarkRoot.Mobile.id }
.map { it.copy(title = rootTitles[it.title]) } .map { it.copy(title = rootTitles[it.title]) }
} else {
emptyList()
}
val mobileRoot = roots.find { it.guid == BookmarkRoot.Mobile.id } ?: return roots val mobileRoot = roots.find { it.guid == BookmarkRoot.Mobile.id } ?: return roots
val mobileChildren = others + mobileRoot.children.orEmpty() val mobileChildren = others + mobileRoot.children.orEmpty()

@ -7,36 +7,25 @@ package org.mozilla.fenix.library.bookmarks
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import androidx.core.view.isGone
import androidx.lifecycle.Observer
import androidx.navigation.NavController import androidx.navigation.NavController
import com.google.android.material.button.MaterialButton import com.google.android.material.button.MaterialButton
import kotlinx.android.extensions.LayoutContainer import kotlinx.android.extensions.LayoutContainer
import org.mozilla.fenix.NavGraphDirections
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components
class SignInView( class SignInView(
private val container: ViewGroup, private val container: ViewGroup,
private val navController: NavController private val navController: NavController
) : LayoutContainer, Observer<Boolean> { ) : LayoutContainer {
override val containerView: View? override val containerView: View?
get() = container 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) .inflate(R.layout.component_sign_in, container, true)
.findViewById(R.id.bookmark_folders_sign_in) .findViewById(R.id.bookmark_folders_sign_in)
init { init {
view.setOnClickListener { view.setOnClickListener { navController.navigate(NavGraphDirections.actionGlobalTurnOnSync()) }
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
} }
} }

@ -15,10 +15,8 @@ import androidx.fragment.app.Fragment
import androidx.fragment.app.activityViewModels import androidx.fragment.app.activityViewModels
import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import androidx.navigation.fragment.navArgs import androidx.navigation.fragment.navArgs
import kotlinx.android.synthetic.main.fragment_select_bookmark_folder.* 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.IO
import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.Dispatchers.Main
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
@ -28,11 +26,9 @@ import mozilla.components.concept.storage.BookmarkNode
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.ext.showToolbar
import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
import org.mozilla.fenix.library.bookmarks.DesktopFolders import org.mozilla.fenix.library.bookmarks.DesktopFolders
import org.mozilla.fenix.library.bookmarks.SignInView
class SelectBookmarkFolderFragment : Fragment() { class SelectBookmarkFolderFragment : Fragment() {
@ -47,21 +43,13 @@ class SelectBookmarkFolderFragment : Fragment() {
} }
override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
val view = inflater.inflate(R.layout.fragment_select_bookmark_folder, container, false) return inflater.inflate(R.layout.fragment_select_bookmark_folder, container, false)
val signInView = SignInView(view.selectBookmarkLayout, findNavController())
sharedViewModel.signedIn.observe(viewLifecycleOwner, signInView)
return view
} }
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
showToolbar(getString(R.string.bookmark_select_folder_fragment_label)) showToolbar(getString(R.string.bookmark_select_folder_fragment_label))
val accountManager = requireComponents.backgroundServices.accountManager
sharedViewModel.observeAccountManager(accountManager, owner = this)
lifecycleScope.launch(Main) { lifecycleScope.launch(Main) {
bookmarkNode = withContext(IO) { bookmarkNode = withContext(IO) {
val context = requireContext() val context = requireContext()

@ -8,7 +8,6 @@
android:id="@+id/bookmark_folders_sign_in" android:id="@+id/bookmark_folders_sign_in"
style="@style/NeutralButton" style="@style/NeutralButton"
android:text="@string/bookmark_sign_in_button" android:text="@string/bookmark_sign_in_button"
android:visibility="gone"
android:layout_marginHorizontal="16dp" android:layout_marginHorizontal="16dp"
app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent" app:layout_constraintEnd_toEndOf="parent"

@ -257,14 +257,4 @@ class BookmarkControllerTest {
navController.popBackStack() navController.popBackStack()
} }
} }
@Test
fun `handleSigningIn should trigger 'PairingSignIn`() {
controller.handleSigningIn()
verify {
invokePendingDeletion.invoke()
services.launchPairingSignIn(homeActivity, navController)
}
}
} }

@ -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!!)
}
}

@ -10,7 +10,6 @@ import io.mockk.mockk
import io.mockk.spyk import io.mockk.spyk
import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runBlocking
import mozilla.appservices.places.BookmarkRoot import mozilla.appservices.places.BookmarkRoot
import mozilla.components.browser.storage.sync.PlacesBookmarksStorage
import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.robolectric.testContext
@ -27,7 +26,6 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
class DesktopFoldersTest { class DesktopFoldersTest {
private lateinit var context: Context private lateinit var context: Context
private lateinit var bookmarksStorage: PlacesBookmarksStorage
private val basicNode = BookmarkNode( private val basicNode = BookmarkNode(
type = BookmarkNodeType.FOLDER, type = BookmarkNodeType.FOLDER,
@ -42,9 +40,7 @@ class DesktopFoldersTest {
@Before @Before
fun setup() { fun setup() {
context = spyk(testContext) context = spyk(testContext)
bookmarksStorage = mockk() every { context.components.core.bookmarksStorage } returns mockk()
every { context.components.core.bookmarksStorage } returns bookmarksStorage
every { context.components.backgroundServices.accountManager.authenticatedAccount() } returns null
} }
@Test @Test
@ -69,15 +65,6 @@ class DesktopFoldersTest {
assertEquals(testContext.getString(R.string.library_desktop_bookmarks_unfiled), desktopFolders.withRootTitle(mockNodeWithTitle("unfiled")).title) 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 @Test
fun `withOptionalDesktopFolders other node`() = runBlocking { fun `withOptionalDesktopFolders other node`() = runBlocking {
val node = basicNode.copy(guid = "12345") val node = basicNode.copy(guid = "12345")

Loading…
Cancel
Save