diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index 0d057d508..4cb7f99e3 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -116,7 +116,7 @@ class DefaultToolbarMenu( highlight = if (hasAccountProblem) { BrowserMenuHighlightableItem.Highlight( imageResource = R.drawable.ic_alert, - backgroundResource = R.color.sync_error_color + backgroundResource = R.color.sync_error_background_color ) } else null ) { diff --git a/app/src/main/java/org/mozilla/fenix/settings/AccountAuthErrorPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/AccountAuthErrorPreference.kt new file mode 100644 index 000000000..dca3e3df0 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/AccountAuthErrorPreference.kt @@ -0,0 +1,35 @@ +/* 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.settings + +import android.content.Context +import android.util.AttributeSet +import android.view.View +import android.widget.TextView +import androidx.preference.Preference +import androidx.preference.PreferenceViewHolder +import org.mozilla.fenix.R + +class AccountAuthErrorPreference : Preference { + var email: String? = null + + constructor(context: Context) : super(context) + constructor(context: Context, attrs: AttributeSet?) : super(context, attrs) + constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super(context, attrs, attributeSetId) + + init { + layoutResource = R.layout.account_auth_error_preference + } + + override fun onBindViewHolder(holder: PreferenceViewHolder) { + super.onBindViewHolder(holder) + val emailView = holder.findViewById(R.id.email) as TextView + emailView.text = email.orEmpty() + emailView.visibility = when (email.isNullOrEmpty()) { + true -> View.GONE + false -> View.VISIBLE + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/AccountPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/AccountPreference.kt index 88bacf8c5..34e77f549 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/AccountPreference.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/AccountPreference.kt @@ -7,26 +7,18 @@ package org.mozilla.fenix.settings import android.content.Context import android.util.AttributeSet import android.view.View -import android.widget.ImageView import android.widget.TextView import androidx.preference.Preference import androidx.preference.PreferenceViewHolder import org.mozilla.fenix.R class AccountPreference : Preference { - - var title: TextView? = null - var summary: TextView? = null - var errorIcon: ImageView? = null - var background: View? = null + var displayName: String? = null + var email: String? = null constructor(context: Context) : super(context) constructor(context: Context, attrs: AttributeSet?) : super(context, attrs) - constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super( - context, - attrs, - attributeSetId - ) + constructor(context: Context, attrs: AttributeSet?, attributeSetId: Int) : super(context, attrs, attributeSetId) init { layoutResource = R.layout.account_preference @@ -34,9 +26,18 @@ class AccountPreference : Preference { override fun onBindViewHolder(holder: PreferenceViewHolder) { super.onBindViewHolder(holder) - title = holder.findViewById(R.id.title) as TextView - summary = holder.findViewById(R.id.summary) as TextView - errorIcon = holder.findViewById(R.id.error_icon) as ImageView - background = holder.itemView + val displayNameView = holder.findViewById(R.id.displayName) as TextView + val emailView = holder.findViewById(R.id.email) as TextView + + displayNameView.text = displayName.orEmpty() + displayNameView.visibility = when (displayName.isNullOrEmpty()) { + true -> View.GONE + false -> View.VISIBLE + } + // There is a potential for a race condition here. We might not have the user profile by the time we display + // this field, in which case we won't have the email address (or the display name, but that we may just not have + // at all even after fetching the profile). We don't hide the email field or change its text if email is missing + // because in the layout a default value ("Firefox Account") is specified, which will be displayed instead. + email?.let { emailView.text = it } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt index 6bf346cb9..81f2832bb 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt @@ -5,15 +5,14 @@ package org.mozilla.fenix.settings import android.content.ActivityNotFoundException +import android.content.Context import android.content.Intent import android.content.SharedPreferences import android.net.Uri import android.os.Bundle import android.provider.Settings -import android.view.View import android.widget.Toast import androidx.appcompat.app.AppCompatActivity -import androidx.core.content.ContextCompat import androidx.navigation.Navigation import androidx.preference.Preference import androidx.preference.Preference.OnPreferenceClickListener @@ -21,12 +20,10 @@ import androidx.preference.PreferenceCategory import androidx.preference.PreferenceFragmentCompat import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job import kotlinx.coroutines.launch import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile -import mozilla.components.service.fxa.FxaUnauthorizedException import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.Config import org.mozilla.fenix.FenixApplication @@ -35,6 +32,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.R.string.pref_key_about import org.mozilla.fenix.R.string.pref_key_accessibility import org.mozilla.fenix.R.string.pref_key_account +import org.mozilla.fenix.R.string.pref_key_account_auth_error import org.mozilla.fenix.R.string.pref_key_account_category import org.mozilla.fenix.R.string.pref_key_data_choices import org.mozilla.fenix.R.string.pref_key_delete_browsing_data @@ -53,18 +51,12 @@ import org.mozilla.fenix.R.string.pref_key_tracking_protection_settings import org.mozilla.fenix.R.string.pref_key_your_rights import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.getColorFromAttr import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.utils.ItsNotBrokenSnack -import kotlin.coroutines.CoroutineContext @SuppressWarnings("TooManyFunctions", "LargeClass") -class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObserver { - private lateinit var job: Job - override val coroutineContext: CoroutineContext - get() = Dispatchers.Main + job - +class SettingsFragment : PreferenceFragmentCompat(), AccountObserver { private val preferenceChangeListener = SharedPreferences.OnSharedPreferenceChangeListener { sharedPreferences, key -> try { @@ -83,10 +75,15 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - job = Job() - setupAccountUI() - updateSignInVisibility() - displayAccountErrorIfNecessary() + + // Observe account changes to keep the UI up-to-date. + requireComponents.backgroundServices.accountManager.register(this, owner = this, autoPause = true) + + // It's important to update the account UI state in onCreate, even though we also call it in onResume, since + // that ensures we'll never display an incorrect state in the UI. For example, if user is signed-in, and we + // don't perform this call in onCreate, we'll briefly display a "Sign In" preference, which will then get + // replaced by the correct account information once this call is ran in onResume shortly after. + updateAccountUIState(context!!, requireComponents.backgroundServices.accountManager.accountProfile()) preferenceManager.sharedPreferences.registerOnSharedPreferenceChangeListener(preferenceChangeListener) } @@ -131,9 +128,8 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse aboutPreference?.title = getString(R.string.preferences_about, appName) setupPreferences() - setupAccountUI() - updateSignInVisibility() - displayAccountErrorIfNecessary() + + updateAccountUIState(context!!, requireComponents.backgroundServices.accountManager.accountProfile()) } @Suppress("ComplexMethod") @@ -182,11 +178,10 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse navigateToAbout() } resources.getString(pref_key_account) -> { - if (requireComponents.backgroundServices.accountManager.accountNeedsReauth()) { - navigateToAccountProblem() - } else { - navigateToAccountSettings() - } + navigateToAccountSettings() + } + resources.getString(pref_key_account_auth_error) -> { + navigateToAccountProblem() } resources.getString(pref_key_delete_browsing_data) -> { navigateToDeleteBrowsingData() @@ -215,19 +210,9 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse override fun onDestroy() { super.onDestroy() - job.cancel() preferenceManager.sharedPreferences.unregisterOnSharedPreferenceChangeListener(preferenceChangeListener) } - private fun setupAccountUI() { - val accountManager = requireComponents.backgroundServices.accountManager - // Observe account changes to keep the UI up-to-date. - accountManager.register(this, owner = this) - - updateAuthState(accountManager.authenticatedAccount()) - accountManager.accountProfile()?.let { updateAccountProfile(it) } - } - private fun getClickListenerForSignIn(): OnPreferenceClickListener { return OnPreferenceClickListener { val directions = SettingsFragmentDirections.actionSettingsFragmentToTurnOnSyncFragment() @@ -335,116 +320,85 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse } override fun onAuthenticated(account: OAuthAccount) { - updateAuthState(account) - updateSignInVisibility() - } - - override fun onError(error: Exception) { - // TODO we could display some error states in this UI. - when (error) { - is FxaUnauthorizedException -> { + CoroutineScope(Dispatchers.Main).launch { + context?.let { + updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile()) } } } + override fun onError(error: Exception) {} + override fun onLoggedOut() { - updateAuthState() - updateSignInVisibility() + CoroutineScope(Dispatchers.Main).launch { + context?.let { + updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile()) + } + } } override fun onProfileUpdated(profile: Profile) { - updateAccountProfile(profile) + CoroutineScope(Dispatchers.Main).launch { + context?.let { + updateAccountUIState(it, profile) + } + } } override fun onAuthenticationProblems() { - displayAccountErrorIfNecessary() + CoroutineScope(Dispatchers.Main).launch { + context?.let { + updateAccountUIState(it, it.components.backgroundServices.accountManager.accountProfile()) + } + } } - // --- Account UI helpers --- - private fun updateAuthState(account: OAuthAccount? = null) { - // Cache the user's auth state to improve performance of sign in visibility - org.mozilla.fenix.utils.Settings.getInstance(context!!).setHasCachedAccount(account != null) - } - - private fun updateSignInVisibility() { - val hasCachedAccount = org.mozilla.fenix.utils.Settings.getInstance(context!!).hasCachedAccount + /** + * Updates the UI to reflect current account state. + * Possible conditions are logged-in without problems, logged-out, and logged-in but needs to re-authenticate. + */ + private fun updateAccountUIState(context: Context, profile: Profile?) { val preferenceSignIn = - findPreference(context!!.getPreferenceKey(pref_key_sign_in)) + findPreference(context.getPreferenceKey(pref_key_sign_in)) val preferenceFirefoxAccount = - findPreference(context!!.getPreferenceKey(pref_key_account)) + findPreference(context.getPreferenceKey(pref_key_account)) + val preferenceFirefoxAccountAuthError = + findPreference(context.getPreferenceKey(pref_key_account_auth_error)) val accountPreferenceCategory = - findPreference(context!!.getPreferenceKey(pref_key_account_category)) + findPreference(context.getPreferenceKey(pref_key_account_category)) - if (hasCachedAccount) { + val accountManager = requireComponents.backgroundServices.accountManager + val account = accountManager.authenticatedAccount() + + // Signed-in, no problems. + if (account != null && !accountManager.accountNeedsReauth()) { preferenceSignIn?.isVisible = false preferenceSignIn?.onPreferenceClickListener = null + preferenceFirefoxAccountAuthError?.isVisible = false preferenceFirefoxAccount?.isVisible = true accountPreferenceCategory?.isVisible = true + + preferenceFirefoxAccount?.displayName = profile?.displayName + preferenceFirefoxAccount?.email = profile?.email + + // Signed-in, need to re-authenticate. + } else if (account != null && accountManager.accountNeedsReauth()) { + preferenceFirefoxAccount?.isVisible = false + preferenceFirefoxAccountAuthError?.isVisible = true + accountPreferenceCategory?.isVisible = true + + preferenceSignIn?.isVisible = false + preferenceSignIn?.onPreferenceClickListener = null + + preferenceFirefoxAccountAuthError?.email = profile?.email + + // Signed-out. } else { preferenceSignIn?.isVisible = true preferenceSignIn?.onPreferenceClickListener = getClickListenerForSignIn() preferenceFirefoxAccount?.isVisible = false + preferenceFirefoxAccountAuthError?.isVisible = false accountPreferenceCategory?.isVisible = false } } - - private fun updateAccountProfile(profile: Profile) { - launch { - context?.let { context -> - val preferenceFirefoxAccount = - findPreference(context.getPreferenceKey(pref_key_account)) - - preferenceFirefoxAccount?.title?.setTextColor( - R.attr.primaryText.getColorFromAttr(context) - ) - preferenceFirefoxAccount?.title?.text = profile.displayName.orEmpty() - preferenceFirefoxAccount?.title?.visibility = - if (preferenceFirefoxAccount?.title?.text.isNullOrEmpty()) View.GONE else View.VISIBLE - - preferenceFirefoxAccount?.summary?.setTextColor( - R.attr.primaryText.getColorFromAttr(context) - ) - preferenceFirefoxAccount?.summary?.text = profile.email.orEmpty() - - preferenceFirefoxAccount?.icon = ContextCompat.getDrawable(context, R.drawable.ic_shortcuts) - preferenceFirefoxAccount?.errorIcon?.visibility = View.GONE - preferenceFirefoxAccount?.background?.background = null - } - } - } - - private fun displayAccountErrorIfNecessary() { - launch { - context?.let { context -> - if (!context.components.backgroundServices.accountManager.accountNeedsReauth()) { return@launch } - - val preferenceFirefoxAccount = - findPreference(context.getPreferenceKey(pref_key_account)) - - preferenceFirefoxAccount?.title?.setTextColor( - ContextCompat.getColor( - context, - R.color.sync_error_text_color - ) - ) - preferenceFirefoxAccount?.title?.text = context.getString(R.string.preferences_account_sync_error) - preferenceFirefoxAccount?.title?.visibility = - if (preferenceFirefoxAccount?.title?.text.isNullOrEmpty()) View.GONE else View.VISIBLE - - preferenceFirefoxAccount?.summary?.setTextColor( - ContextCompat.getColor( - context, - R.color.sync_error_text_color - ) - ) - preferenceFirefoxAccount?.summary?.text = - context.components.backgroundServices.accountManager.accountProfile()?.email.orEmpty() - - preferenceFirefoxAccount?.icon = ContextCompat.getDrawable(context, R.drawable.ic_account_warning) - preferenceFirefoxAccount?.errorIcon?.visibility = View.VISIBLE - preferenceFirefoxAccount?.background?.background = - ContextCompat.getDrawable(context, R.color.sync_error_color) - } - } - } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 1d134e821..7572d615b 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -152,9 +152,6 @@ class Settings private constructor(context: Context) { else -> appContext.getString(R.string.preference_light_theme) } - val hasCachedAccount: Boolean - get() = preferences.getBoolean(appContext.getPreferenceKey(R.string.pref_key_cached_account), false) - private val autoBounceQuickActionSheetCount: Int get() = (preferences.getInt(appContext.getPreferenceKey(R.string.pref_key_bounce_quick_action), 0)) @@ -229,12 +226,6 @@ class Settings private constructor(context: Context) { ) } - fun setHasCachedAccount(isCached: Boolean) { - preferences.edit() - .putBoolean(appContext.getPreferenceKey(R.string.pref_key_cached_account), isCached) - .apply() - } - private val SitePermissionsRules.Action.id: Int get() { return when (this) { diff --git a/app/src/main/res/layout/account_auth_error_preference.xml b/app/src/main/res/layout/account_auth_error_preference.xml new file mode 100644 index 000000000..faeeb3333 --- /dev/null +++ b/app/src/main/res/layout/account_auth_error_preference.xml @@ -0,0 +1,67 @@ + + + + + + + + + + + + + + + + + + + diff --git a/app/src/main/res/layout/account_preference.xml b/app/src/main/res/layout/account_preference.xml index 45d72aef8..5cddc705f 100644 --- a/app/src/main/res/layout/account_preference.xml +++ b/app/src/main/res/layout/account_preference.xml @@ -35,21 +35,20 @@ android:paddingBottom="16dp" android:layout_weight="1"> - - @@ -62,17 +61,4 @@ android:gravity="center_vertical" android:orientation="vertical"/> - - diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 63d4e73d7..d1a2865e5 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -139,7 +139,7 @@ #cccccc #E7DFFF #232749 - #FFF36E + #FFF36E #960E18 diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 536c35ef9..08d835413 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -22,6 +22,7 @@ pref_key_your_rights pref_key_account pref_key_sign_in + pref_key_account_auth_error pref_key_private_mode pref_key_theme pref_key_leakcanary diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 021a4d585..5fa23c715 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -14,14 +14,18 @@ android:title="@string/preferences_sign_in" /> + android:key="@string/pref_key_account" /> + +