From b8c04e02e9af0265778149704c6330c333823a0d Mon Sep 17 00:00:00 2001 From: Sawyer Blatz Date: Wed, 29 May 2019 14:31:06 -0700 Subject: [PATCH] For #2327: Fixes nits --- .../java/org/mozilla/fenix/HomeActivity.kt | 8 +- .../components/toolbar/DefaultToolbarMenu.kt | 8 +- .../fenix/components/toolbar/ToolbarUIView.kt | 3 +- .../org/mozilla/fenix/home/HomeFragment.kt | 5 +- .../library/bookmarks/BookmarkFragment.kt | 2 - .../SelectBookmarkFolderFragment.kt | 2 - ...mFragment.kt => AccountProblemFragment.kt} | 4 +- .../fenix/settings/SettingsFragment.kt | 97 +++++++++---------- .../java/org/mozilla/fenix/utils/Settings.kt | 9 -- .../main/res/drawable/ic_account_warning.xml | 4 + app/src/main/res/drawable/ic_alert.xml | 4 + app/src/main/res/navigation/nav_graph.xml | 11 ++- buildSrc/src/main/java/Dependencies.kt | 2 +- 13 files changed, 74 insertions(+), 85 deletions(-) rename app/src/main/java/org/mozilla/fenix/settings/{SyncProblemFragment.kt => AccountProblemFragment.kt} (96%) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 60ed7c2ce..03ed0358a 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -34,9 +34,9 @@ import org.mozilla.fenix.library.bookmarks.BookmarkFragmentDirections import org.mozilla.fenix.library.bookmarks.selectfolder.SelectBookmarkFolderFragmentDirections import org.mozilla.fenix.library.history.HistoryFragmentDirections import org.mozilla.fenix.search.SearchFragmentDirections +import org.mozilla.fenix.settings.AccountProblemFragmentDirections import org.mozilla.fenix.settings.PairFragmentDirections import org.mozilla.fenix.settings.SettingsFragmentDirections -import org.mozilla.fenix.settings.SyncProblemFragmentDirections import org.mozilla.fenix.settings.TurnOnSyncFragmentDirections import org.mozilla.fenix.utils.Settings @@ -201,8 +201,8 @@ open class HomeActivity : AppCompatActivity() { BrowserDirection.FromTurnOnSync -> TurnOnSyncFragmentDirections.actionTurnOnSyncFragmentToBrowserFragment( customTabSessionId ) - BrowserDirection.FromSyncProblem -> - SyncProblemFragmentDirections.actionSyncProblemFragmentToBrowserFragment(customTabSessionId) + BrowserDirection.FromAccountProblem -> + AccountProblemFragmentDirections.actionAccountProblemFragmentToBrowserFragment(customTabSessionId) } if (sessionObserver == null) sessionObserver = subscribeToSessions() @@ -292,5 +292,5 @@ open class HomeActivity : AppCompatActivity() { enum class BrowserDirection { FromGlobal, FromHome, FromSearch, FromSettings, FromBookmarks, FromBookmarksFolderSelect, FromHistory, FromPair, FromTurnOnSync, - FromSyncProblem + FromAccountProblem } 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 780cbe8c5..e18c4943b 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 @@ -19,7 +19,7 @@ import org.mozilla.fenix.ext.components class DefaultToolbarMenu( private val context: Context, - private val hasSyncError: Boolean = false, + private val hasAccountProblem: Boolean = false, private val requestDesktopStateProvider: () -> Boolean = { false }, private val onItemTapped: (ToolbarMenu.Item) -> Unit = {} ) : ToolbarMenu { @@ -107,13 +107,13 @@ class DefaultToolbarMenu( BrowserMenuHighlightableItem( label = context.getString(R.string.browser_menu_settings), imageResource = R.drawable.ic_settings, - iconTintColorResource = if (hasSyncError) + iconTintColorResource = if (hasAccountProblem) R.color.sync_error_text_color else DefaultThemeManager.resolveAttribute(R.attr.primaryText, context), - textColorResource = if (hasSyncError) + textColorResource = if (hasAccountProblem) R.color.sync_error_text_color else DefaultThemeManager.resolveAttribute(R.attr.primaryText, context), - highlight = if (hasSyncError) { + highlight = if (hasAccountProblem) { BrowserMenuHighlightableItem.Highlight( imageResource = R.drawable.ic_alert, backgroundResource = R.color.sync_error_color diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarUIView.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarUIView.kt index 935abf998..b12ecc3be 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/ToolbarUIView.kt @@ -20,7 +20,6 @@ import org.mozilla.fenix.R import org.mozilla.fenix.customtabs.CustomTabToolbarMenu import org.mozilla.fenix.ext.components import org.mozilla.fenix.mvi.UIView -import org.mozilla.fenix.utils.Settings class ToolbarUIView( sessionId: String?, @@ -105,7 +104,7 @@ class ToolbarUIView( ) } else { DefaultToolbarMenu(this, - hasSyncError = Settings.getInstance(this).hasSyncProblem, + hasAccountProblem = components.backgroundServices.accountManager.accountNeedsReauth(), requestDesktopStateProvider = { session?.desktopMode ?: false }, onItemTapped = { actionEmitter.onNext(SearchAction.ToolbarMenuItemTapped(it)) } ) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 593785d40..20d15add8 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -683,10 +683,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver { Mode.Normal } - override fun onAuthenticationProblems() { - Settings.getInstance(context!!).setHasAuthenticationProblem(true) - emitAccountChanges() - } + override fun onAuthenticationProblems() { emitAccountChanges() } override fun onAuthenticated(account: OAuthAccount) { emitAccountChanges() } override fun onError(error: Exception) { emitAccountChanges() } override fun onLoggedOut() { emitAccountChanges() } 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 dcea50b66..a4038887e 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 @@ -50,7 +50,6 @@ import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.getAutoDisposeObservable import org.mozilla.fenix.mvi.getManagedEmitter import org.mozilla.fenix.utils.allowUndo -import org.mozilla.fenix.utils.Settings import kotlin.coroutines.CoroutineContext @SuppressWarnings("TooManyFunctions", "LargeClass") @@ -365,7 +364,6 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve } override fun onAuthenticationProblems() { - Settings.getInstance(context!!).setHasAuthenticationProblem(true) } override fun onProfileUpdated(profile: Profile) { 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 921c5c2be..767e4e451 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 @@ -46,7 +46,6 @@ import org.mozilla.fenix.library.bookmarks.SignInViewModel import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.getAutoDisposeObservable import org.mozilla.fenix.mvi.getManagedEmitter -import org.mozilla.fenix.utils.Settings import kotlin.coroutines.CoroutineContext @SuppressWarnings("TooManyFunctions") @@ -158,7 +157,6 @@ class SelectBookmarkFolderFragment : Fragment(), CoroutineScope, AccountObserver } override fun onAuthenticationProblems() { - Settings.getInstance(context!!).setHasAuthenticationProblem(true) } override fun onAuthenticated(account: OAuthAccount) { diff --git a/app/src/main/java/org/mozilla/fenix/settings/SyncProblemFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt similarity index 96% rename from app/src/main/java/org/mozilla/fenix/settings/SyncProblemFragment.kt rename to app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt index d17db419b..6da67c86b 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SyncProblemFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/AccountProblemFragment.kt @@ -15,7 +15,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.requireComponents -class SyncProblemFragment : PreferenceFragmentCompat() { +class AccountProblemFragment : PreferenceFragmentCompat() { override fun onResume() { super.onResume() @@ -46,7 +46,7 @@ class SyncProblemFragment : PreferenceFragmentCompat() { // We could auto-close this tab once we get to the end of the authentication process? // Via an interceptor, perhaps. view?.let { - (activity as HomeActivity).openToBrowser(BrowserDirection.FromSyncProblem) + (activity as HomeActivity).openToBrowser(BrowserDirection.FromAccountProblem) } true } 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 02d5a3660..e3cfc06ee 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt @@ -178,9 +178,8 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse navigateToAbout() } resources.getString(pref_key_account) -> { - if (org.mozilla.fenix.utils.Settings.getInstance(preference.context).preferences - .getBoolean(context!!.getPreferenceKey(R.string.pref_key_sync_problem), false)) { - navigateToSyncProblem() + if (requireComponents.backgroundServices.accountManager.accountNeedsReauth()) { + navigateToAccountProblem() } else { navigateToAccountSettings() } @@ -314,8 +313,8 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse Navigation.findNavController(view!!).navigate(directions) } - private fun navigateToSyncProblem() { - val directions = SettingsFragmentDirections.actionSettingsFragmentToSyncProblemFragment() + private fun navigateToAccountProblem() { + val directions = SettingsFragmentDirections.actionSettingsFragmentToAccountProblemFragment() Navigation.findNavController(view!!).navigate(directions) } @@ -333,7 +332,6 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse override fun onAuthenticated(account: OAuthAccount) { updateAuthState(account) updateSignInVisibility() - displayAccountErrorIfNecessary() } override fun onError(error: Exception) { @@ -347,7 +345,6 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse override fun onLoggedOut() { updateAuthState() updateSignInVisibility() - displayAccountErrorIfNecessary() } override fun onProfileUpdated(profile: Profile) { @@ -355,7 +352,6 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse } override fun onAuthenticationProblems() { - org.mozilla.fenix.utils.Settings.getInstance(context!!).setHasAuthenticationProblem(true) displayAccountErrorIfNecessary() } @@ -363,9 +359,6 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse 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) - - // Unset sync problems - org.mozilla.fenix.utils.Settings.getInstance(context!!).setHasAuthenticationProblem(false) } private fun updateSignInVisibility() { @@ -390,59 +383,63 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse } } - private fun updateAccountProfile(profile: Profile, error: Boolean = false) { + private fun updateAccountProfile(profile: Profile) { launch { - val preferenceFirefoxAccount = - findPreference(context!!.getPreferenceKey(pref_key_account)) + 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?.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?.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 + preferenceFirefoxAccount?.icon = ContextCompat.getDrawable(context, R.drawable.ic_shortcuts) + preferenceFirefoxAccount?.errorIcon?.visibility = View.GONE + preferenceFirefoxAccount?.background?.background = null + } } } private fun displayAccountErrorIfNecessary() { - if (!org.mozilla.fenix.utils.Settings.getInstance(context!!).hasSyncProblem) { return } - launch { - val preferenceFirefoxAccount = - findPreference(context!!.getPreferenceKey(pref_key_account)) + context?.let { context -> + if (context.components.backgroundServices.accountManager.accountNeedsReauth()) { return@launch } - preferenceFirefoxAccount?.title?.setTextColor( - ContextCompat.getColor( - context!!, - R.color.sync_error_text_color + 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?.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?.setTextColor( + ContextCompat.getColor( + context, + R.color.sync_error_text_color + ) ) - ) - preferenceFirefoxAccount?.summary?.text = - requireComponents.backgroundServices.accountManager.accountProfile()?.email.orEmpty() + 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) + 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 466214477..1d134e821 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -155,9 +155,6 @@ class Settings private constructor(context: Context) { val hasCachedAccount: Boolean get() = preferences.getBoolean(appContext.getPreferenceKey(R.string.pref_key_cached_account), false) - val hasSyncProblem: Boolean - get() = preferences.getBoolean(appContext.getPreferenceKey(R.string.pref_key_sync_problem), false) - private val autoBounceQuickActionSheetCount: Int get() = (preferences.getInt(appContext.getPreferenceKey(R.string.pref_key_bounce_quick_action), 0)) @@ -238,12 +235,6 @@ class Settings private constructor(context: Context) { .apply() } - fun setHasAuthenticationProblem(hasProblem: Boolean) { - preferences.edit() - .putBoolean(appContext.getPreferenceKey(R.string.pref_key_sync_problem), hasProblem) - .apply() - } - private val SitePermissionsRules.Action.id: Int get() { return when (this) { diff --git a/app/src/main/res/drawable/ic_account_warning.xml b/app/src/main/res/drawable/ic_account_warning.xml index 5ecec3d7e..be0f92d29 100644 --- a/app/src/main/res/drawable/ic_account_warning.xml +++ b/app/src/main/res/drawable/ic_account_warning.xml @@ -1,3 +1,7 @@ + + + - + - - + + \ No newline at end of file diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 33574ebb7..19ac2756c 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -35,7 +35,7 @@ private object Versions { // The version number below tracks the application-services version // that we depend on directly for tests, and it's important that it // be kept in sync with the version used by android-components above. - const val mozilla_appservices = "0.28.1" + const val mozilla_appservices = "0.29.0" const val autodispose = "1.1.0" const val adjust = "4.11.4"