diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/DeepLinkTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/DeepLinkTest.kt index 7bca26ebe..ff9159210 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/DeepLinkTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/DeepLinkTest.kt @@ -108,7 +108,6 @@ class DeepLinkTest { fun openSettingsLogins() { robot.openSettingsLogins { verifyDefaultView() - verifyDefaultValueSyncLogins() verifyDefaultValueAutofillLogins() } } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsPrivacyTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsPrivacyTest.kt index 81a1a4459..283420d89 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsPrivacyTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsPrivacyTest.kt @@ -170,7 +170,6 @@ class SettingsPrivacyTest { TestHelper.scrollToElementByText("Logins and passwords") }.openLoginsAndPasswordSubMenu { verifyDefaultView() - verifyDefaultValueSyncLogins() verifyDefaultValueAutofillLogins() verifyDefaultValueExceptions() }.openSavedLogins { @@ -203,7 +202,6 @@ class SettingsPrivacyTest { TestHelper.scrollToElementByText("Logins and passwords") }.openLoginsAndPasswordSubMenu { verifyDefaultView() - verifyDefaultValueSyncLogins() }.openSavedLogins { verifySecurityPromptForLogins() tapSetupLater() @@ -228,7 +226,6 @@ class SettingsPrivacyTest { }.openSettings { }.openLoginsAndPasswordSubMenu { verifyDefaultView() - verifyDefaultValueSyncLogins() }.openSavedLogins { verifySecurityPromptForLogins() tapSetupLater() diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuLoginsAndPasswordRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuLoginsAndPasswordRobot.kt index 46b8656ce..74a6805de 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuLoginsAndPasswordRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuLoginsAndPasswordRobot.kt @@ -26,7 +26,7 @@ import org.mozilla.fenix.helpers.ext.waitNotNull class SettingsSubMenuLoginsAndPasswordRobot { fun verifyDefaultView() { - mDevice.waitNotNull(Until.findObjects(By.text("Sync logins")), TestAssetHelper.waitingTime) + mDevice.waitNotNull(Until.findObjects(By.text("Sync logins across devices")), TestAssetHelper.waitingTime) assertDefaultView() } @@ -42,8 +42,6 @@ class SettingsSubMenuLoginsAndPasswordRobot { fun verifyDefaultValueAutofillLogins() = assertDefaultValueAutofillLogins() - fun verifyDefaultValueSyncLogins() = assertDefaultValueSyncLogins() - class Transition { val mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) @@ -71,7 +69,7 @@ class SettingsSubMenuLoginsAndPasswordRobot { } fun openSyncLogins(interact: SettingsTurnOnSyncRobot.() -> Unit): SettingsTurnOnSyncRobot.Transition { - fun syncLoginsButton() = onView(ViewMatchers.withText("Sync logins")) + fun syncLoginsButton() = onView(ViewMatchers.withText("Sync logins across devices")) syncLoginsButton().click() SettingsTurnOnSyncRobot().interact() @@ -96,7 +94,7 @@ fun settingsSubMenuLoginsAndPassword(interact: SettingsSubMenuLoginsAndPasswordR private fun goBackButton() = onView(CoreMatchers.allOf(ViewMatchers.withContentDescription("Navigate up"))) -private fun assertDefaultView() = onView(ViewMatchers.withText("Sync logins")) +private fun assertDefaultView() = onView(ViewMatchers.withText("Sync logins across devices")) .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE))) private fun assertDefaultValueAutofillLogins() = onView(ViewMatchers.withText("Autofill")) diff --git a/app/src/main/java/org/mozilla/fenix/settings/SyncPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/SyncPreference.kt new file mode 100644 index 000000000..376829002 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/settings/SyncPreference.kt @@ -0,0 +1,43 @@ +/* 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 androidx.appcompat.widget.SwitchCompat +import androidx.preference.PreferenceViewHolder +import androidx.preference.SwitchPreferenceCompat +import org.mozilla.fenix.R + +/** + * Variation of [SwitchPreferenceCompat] that uses a custom widgetLayoutResource in order to implement + * visibility changes to it. + * */ +class SyncPreference @JvmOverloads constructor( + context: Context, + attrs: AttributeSet? = null +) : SwitchPreferenceCompat(context, attrs) { + + private var switchView: SwitchCompat? = null + + /** + * Whether or not switch's toggle widget is visible. + * */ + var isSwitchWidgetVisible: Boolean = false + + init { + widgetLayoutResource = R.layout.preference_sync + } + + override fun onBindViewHolder(holder: PreferenceViewHolder) { + super.onBindViewHolder(holder) + + switchView = holder.findViewById(R.id.switch_widget) as SwitchCompat? + + switchView?.isChecked = isChecked + switchView?.visibility = if (isSwitchWidgetVisible) View.VISIBLE else View.INVISIBLE + } +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/SyncPreferenceView.kt b/app/src/main/java/org/mozilla/fenix/settings/SyncPreferenceView.kt index 578d1eea3..b6e95d75c 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/SyncPreferenceView.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/SyncPreferenceView.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.settings import androidx.lifecycle.LifecycleOwner -import androidx.preference.Preference import kotlinx.coroutines.MainScope import kotlinx.coroutines.launch import mozilla.components.concept.sync.AccountObserver @@ -14,33 +13,33 @@ import mozilla.components.concept.sync.OAuthAccount import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.fxa.manager.SyncEnginesStorage -import org.mozilla.fenix.R /** * A view to help manage the sync preference in the "Logins and passwords" and "Credit cards" * settings. The provided [syncPreference] is used to navigate to the different fragments - * that manages the sync account authentication. A summary status will be also added + * that manages the sync account authentication. A toggle will be also added * depending on the sync account status. * - * @param syncPreference The sync [Preference] to update and handle navigation. + * @param syncPreference The sync [SyncPreference] to update and handle navigation. * @param lifecycleOwner View lifecycle owner used to determine when to cancel UI jobs. * @param accountManager An instance of [FxaAccountManager]. * @param syncEngine The sync engine that will be used for the sync status lookup. + * @param loggedOffTitle Text label for the setting when user is not logged in. + * @param loggedInTitle Text label for the setting when user is logged in. * @param onSignInToSyncClicked A callback executed when the [syncPreference] is clicked with a * preference status of "Sign in to Sync". - * @param onSyncStatusClicked A callback executed when the [syncPreference] is clicked with a - * preference status of "On" or "Off". * @param onReconnectClicked A callback executed when the [syncPreference] is clicked with a * preference status of "Reconnect". */ @Suppress("LongParameterList") class SyncPreferenceView( - private val syncPreference: Preference, + private val syncPreference: SyncPreference, lifecycleOwner: LifecycleOwner, accountManager: FxaAccountManager, private val syncEngine: SyncEngine, + private val loggedOffTitle: String, + private val loggedInTitle: String, private val onSignInToSyncClicked: () -> Unit = {}, - private val onSyncStatusClicked: () -> Unit = {}, private val onReconnectClicked: () -> Unit = {} ) { @@ -70,49 +69,53 @@ class SyncPreferenceView( } /** - * Shows the current status of the sync preference ("On"/"Off") for the logged in user. + * Shows a switch toggle for the sync preference when the user is logged in. */ private fun updateSyncPreferenceStatus() { syncPreference.apply { + isSwitchWidgetVisible = true + val syncEnginesStatus = SyncEnginesStorage(context).getStatus() - val loginsSyncStatus = syncEnginesStatus.getOrElse(syncEngine) { false } + val syncStatus = syncEnginesStatus.getOrElse(syncEngine) { false } - summary = context.getString( - if (loginsSyncStatus) R.string.preferences_passwords_sync_logins_on - else R.string.preferences_passwords_sync_logins_off - ) + title = loggedInTitle + isChecked = syncStatus - setOnPreferenceClickListener { - onSyncStatusClicked() + setOnPreferenceChangeListener { _, newValue -> + SyncEnginesStorage(context).setStatus(syncEngine, newValue as Boolean) true } } } /** - * Display that the user can "Sign in to Sync" when the user is logged off. + * Display that the user can sync across devices when the user is logged off. */ private fun updateSyncPreferenceNeedsLogin() { syncPreference.apply { - summary = context.getString(R.string.preferences_passwords_sync_logins_sign_in) + isSwitchWidgetVisible = false + + title = loggedOffTitle - setOnPreferenceClickListener { + setOnPreferenceChangeListener { _, _ -> onSignInToSyncClicked() - true + false } } } /** - * Displays that the user needs to "Reconnect" to fix their account problems with sync. + * Displays the logged off title to prompt the user to to re-authenticate their sync account. */ private fun updateSyncPreferenceNeedsReauth() { syncPreference.apply { - summary = context.getString(R.string.preferences_passwords_sync_logins_reconnect) + isSwitchWidgetVisible = false - setOnPreferenceClickListener { + title = loggedOffTitle + + setOnPreferenceChangeListener { _, _ -> onReconnectClicked() - true + false } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt index d8896b0b9..42c3364f0 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsSettingFragment.kt @@ -80,17 +80,16 @@ class CreditCardsSettingFragment : PreferenceFragmentCompat() { syncPreference = requirePreference(R.string.pref_key_credit_cards_sync_cards_across_devices), lifecycleOwner = viewLifecycleOwner, accountManager = requireComponents.backgroundServices.accountManager, - syncEngine = SyncEngine.Passwords, + syncEngine = SyncEngine.CreditCards, + loggedOffTitle = requireContext() + .getString(R.string.preferences_credit_cards_sync_cards_across_devices), + loggedInTitle = requireContext() + .getString(R.string.preferences_credit_cards_sync_cards), onSignInToSyncClicked = { val directions = CreditCardsSettingFragmentDirections.actionCreditCardsSettingFragmentToTurnOnSyncFragment() findNavController().navigateBlockingForAsyncNavGraph(directions) }, - onSyncStatusClicked = { - val directions = - CreditCardsSettingFragmentDirections.actionGlobalAccountSettingsFragment() - findNavController().navigateBlockingForAsyncNavGraph(directions) - }, onReconnectClicked = { val directions = CreditCardsSettingFragmentDirections.actionGlobalAccountProblemFragment() diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt index f92d070d4..dba760b5d 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt @@ -53,7 +53,7 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat() { * https://github.com/mozilla-mobile/fenix/issues/12312 */ private fun togglePrefsEnabledWhileAuthenticating(enabled: Boolean) { - requirePreference(R.string.pref_key_password_sync_logins).isEnabled = enabled + requirePreference(R.string.pref_key_sync_logins).isEnabled = enabled requirePreference(R.string.pref_key_save_logins_settings).isEnabled = enabled requirePreference(R.string.pref_key_saved_logins).isEnabled = enabled } @@ -124,20 +124,19 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat() { } SyncPreferenceView( - syncPreference = requirePreference(R.string.pref_key_password_sync_logins), + syncPreference = requirePreference(R.string.pref_key_sync_logins), lifecycleOwner = viewLifecycleOwner, accountManager = requireComponents.backgroundServices.accountManager, syncEngine = SyncEngine.Passwords, + loggedOffTitle = requireContext() + .getString(R.string.preferences_passwords_sync_logins_across_devices), + loggedInTitle = requireContext() + .getString(R.string.preferences_passwords_sync_logins), onSignInToSyncClicked = { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment() findNavController().navigateBlockingForAsyncNavGraph(directions) }, - onSyncStatusClicked = { - val directions = - SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment() - findNavController().navigateBlockingForAsyncNavGraph(directions) - }, onReconnectClicked = { val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() diff --git a/app/src/main/res/layout/preference_sync.xml b/app/src/main/res/layout/preference_sync.xml new file mode 100644 index 000000000..bd13e71ce --- /dev/null +++ b/app/src/main/res/layout/preference_sync.xml @@ -0,0 +1,14 @@ + + + + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 354dc6794..c95b89d90 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1419,10 +1419,8 @@ Autofill Sync logins - - On - - Off + + Sync logins across devices Reconnect @@ -1521,6 +1519,8 @@ Data is encrypted Sync cards across devices + + Sync cards Add credit card diff --git a/app/src/main/res/xml/credit_cards_preferences.xml b/app/src/main/res/xml/credit_cards_preferences.xml index 64efeb6f3..3a36fb7c8 100644 --- a/app/src/main/res/xml/credit_cards_preferences.xml +++ b/app/src/main/res/xml/credit_cards_preferences.xml @@ -9,7 +9,7 @@ android:key="@string/pref_key_credit_cards_save_and_autofill_cards" android:summary="@string/preferences_credit_cards_save_and_autofill_cards_summary" android:title="@string/preferences_credit_cards_save_and_autofill_cards" /> - diff --git a/app/src/main/res/xml/logins_preferences.xml b/app/src/main/res/xml/logins_preferences.xml index 146a83de2..608a23303 100644 --- a/app/src/main/res/xml/logins_preferences.xml +++ b/app/src/main/res/xml/logins_preferences.xml @@ -12,9 +12,8 @@ android:defaultValue="true" android:key="@string/pref_key_autofill_logins" android:title="@string/preferences_passwords_autofill" /> - - private lateinit var clickListener: CapturingSlot + private lateinit var preferenceChangeListener: CapturingSlot + private lateinit var widgetVisibilitySlot: CapturingSlot @Before fun setup() { @@ -42,17 +46,29 @@ class LoginsSyncPreferenceViewTest { mockkConstructor(SyncEnginesStorage::class) accountObserver = slot() - clickListener = slot() + preferenceChangeListener = slot() + widgetVisibilitySlot = slot() + val context = mockk { - every { getString(R.string.preferences_passwords_sync_logins_reconnect) } returns "Reconnect" - every { getString(R.string.preferences_passwords_sync_logins_sign_in) } returns "Sign in to Sync" - every { getString(R.string.preferences_passwords_sync_logins_on) } returns "On" - every { getString(R.string.preferences_passwords_sync_logins_off) } returns "Off" + every { getString(R.string.pref_key_credit_cards_sync_cards_across_devices) } returns "pref_key_credit_cards_sync_cards_across_devices" + every { getString(R.string.preferences_credit_cards_sync_cards_across_devices) } returns "Sync cards across devices" + every { getString(R.string.preferences_credit_cards_sync_cards) } returns "Sync cards" + + every { getString(R.string.pref_key_sync_logins) } returns "pref_key_sync_logins" + every { getString(R.string.preferences_passwords_sync_logins) } returns "Sync logins" + every { getString(R.string.preferences_passwords_sync_logins_across_devices) } returns "Sync logins across devices" } - every { syncLoginsPreference.summary = any() } just Runs - every { syncLoginsPreference.onPreferenceClickListener = capture(clickListener) } just Runs - every { syncLoginsPreference.context } returns context + syncPreference = mockk { + every { isSwitchWidgetVisible = any() } just Runs + every { key } returns "pref_key_sync_logins" + every { isChecked = any() } just Runs + every { title = any() } just Runs + } + + every { syncPreference.title = any() } just Runs + every { syncPreference.onPreferenceChangeListener = capture(preferenceChangeListener) } just Runs + every { syncPreference.context } returns context every { accountManager.register(capture(accountObserver), owner = lifecycleOwner) } just Runs every { anyConstructed().getStatus() } returns emptyMap() } @@ -66,11 +82,12 @@ class LoginsSyncPreferenceViewTest { fun `needs reauth ui on init`() { every { accountManager.authenticatedAccount() } returns mockk() every { accountManager.accountNeedsReauth() } returns true - createView() - verify { syncLoginsPreference.summary = "Reconnect" } - assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference)) + createView() + verify { syncPreference.isSwitchWidgetVisible = false } + verify { syncPreference.title = notLoggedInTitle } + assertFalse(preferenceChangeListener.captured.onPreferenceChange(syncPreference, any())) verify { navController.navigate( SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() @@ -82,20 +99,29 @@ class LoginsSyncPreferenceViewTest { fun `needs reauth ui on init even if null account`() { every { accountManager.authenticatedAccount() } returns null every { accountManager.accountNeedsReauth() } returns true + createView() - verify { syncLoginsPreference.summary = "Reconnect" } + verify { syncPreference.isSwitchWidgetVisible = false } + verify { syncPreference.title = notLoggedInTitle } + assertFalse(preferenceChangeListener.captured.onPreferenceChange(syncPreference, any())) + verify { + navController.navigate( + SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() + ) + } } @Test fun `needs login if account does not exist`() { every { accountManager.authenticatedAccount() } returns null every { accountManager.accountNeedsReauth() } returns false - createView() - verify { syncLoginsPreference.summary = "Sign in to Sync" } - assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference)) + createView() + verify { syncPreference.isSwitchWidgetVisible = false } + verify { syncPreference.title = notLoggedInTitle } + assertFalse(preferenceChangeListener.captured.onPreferenceChange(syncPreference, any())) verify { navController.navigate( SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment() @@ -104,59 +130,62 @@ class LoginsSyncPreferenceViewTest { } @Test - fun `show status for existing account`() { + fun `GIVEN LoginScreen and syncLogins true WHEN updateSyncPreferenceStatus THEN setStatus false`() { every { accountManager.authenticatedAccount() } returns mockk() every { accountManager.accountNeedsReauth() } returns false - createView() + every { anyConstructed().getStatus() } returns mapOf( + SyncEngine.Passwords to true + ) + every { anyConstructed().setStatus(any(), any()) } just Runs - verify { syncLoginsPreference.summary = "Off" } - assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference)) + createView() - verify { - navController.navigate( - SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment() - ) - } + verify { syncPreference.isSwitchWidgetVisible = true } + verify { syncPreference.isChecked = true } + verify { syncPreference.title = loggedInTitle } + assertTrue(preferenceChangeListener.captured.onPreferenceChange(syncPreference, false)) + verify { anyConstructed().setStatus(any(), false) } } @Test - fun `show status for existing account with passwords`() { - every { anyConstructed().getStatus() } returns mapOf( - SyncEngine.Passwords to true - ) + fun `GIVEN LoginScreen and syncLogins false WHEN updateSyncPreferenceStatus THEN setStatus true`() { every { accountManager.authenticatedAccount() } returns mockk() every { accountManager.accountNeedsReauth() } returns false - createView() + every { anyConstructed().getStatus() } returns mapOf( + SyncEngine.Passwords to false + ) + every { anyConstructed().setStatus(any(), any()) } just Runs - verify { syncLoginsPreference.summary = "On" } - assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference)) + createView() - verify { - navController.navigate( - SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment() - ) - } + verify { syncPreference.isSwitchWidgetVisible = true } + verify { syncPreference.isChecked = false } + verify { syncPreference.title = loggedInTitle } + assertTrue(preferenceChangeListener.captured.onPreferenceChange(syncPreference, true)) + verify { anyConstructed().setStatus(any(), true) } } private fun createView() = SyncPreferenceView( - syncPreference = syncLoginsPreference, + syncPreference = syncPreference, lifecycleOwner = lifecycleOwner, accountManager = accountManager, syncEngine = SyncEngine.Passwords, + loggedOffTitle = notLoggedInTitle, + loggedInTitle = loggedInTitle, onSignInToSyncClicked = { val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment() navController.navigate(directions) }, - onSyncStatusClicked = { - val directions = - SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment() - navController.navigate(directions) - }, onReconnectClicked = { val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment() navController.navigate(directions) } ) + + companion object { + const val notLoggedInTitle: String = "Sync logins across devices" + const val loggedInTitle: String = "Sync logins" + } }