From c9f822d0d4033697aa8fd1980e11e7667921440f Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Thu, 18 Mar 2021 23:25:20 -0400 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/15372 Optimize the order and messages of onboarding cards --- .../java/org/mozilla/fenix/ui/SmokeTest.kt | 16 +--- .../fenix/ui/robots/HomeScreenRobot.kt | 75 ++++++--------- ...sSubMenuEnhancedTrackingProtectionRobot.kt | 14 +-- .../java/org/mozilla/fenix/ui/util/Strings.kt | 9 ++ .../home/sessioncontrol/SessionControlView.kt | 16 ++-- .../OnboardingManualSignInViewHolder.kt | 12 +-- .../OnboardingTrackingProtectionViewHolder.kt | 27 +----- .../fenix/onboarding/OnboardingController.kt | 22 ----- .../fenix/onboarding/OnboardingInteractor.kt | 14 --- .../ic_onboarding_avatar_anonymous_large.xml | 4 +- .../res/layout/onboarding_manual_signin.xml | 36 +++---- .../res/layout/onboarding_theme_picker.xml | 2 +- .../onboarding_toolbar_position_picker.xml | 93 +++++++++---------- .../layout/onboarding_tracking_protection.xml | 20 +--- app/src/main/res/values/strings.xml | 38 ++++---- .../xml/tracking_protection_preferences.xml | 4 +- .../OnboardingManualSignInViewHolderTest.kt | 11 +-- ...oardingTrackingProtectionViewHolderTest.kt | 6 +- 18 files changed, 148 insertions(+), 271 deletions(-) create mode 100644 app/src/androidTest/java/org/mozilla/fenix/ui/util/Strings.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/onboarding/OnboardingController.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/onboarding/OnboardingInteractor.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt index 63b5882862..82c9fa4f00 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt @@ -35,6 +35,7 @@ import org.mozilla.fenix.ui.robots.enhancedTrackingProtection import org.mozilla.fenix.ui.robots.homeScreen import org.mozilla.fenix.ui.robots.navigationToolbar import org.mozilla.fenix.ui.robots.tabDrawer +import org.mozilla.fenix.ui.util.STRING_ONBOARDING_TRACKING_PROTECTION_HEADER /** * Test Suite that contains tests defined as part of the Smoke and Sanity check defined in Test rail. @@ -125,13 +126,9 @@ class SmokeTest { verifyStartSyncHeader() verifyAccountsSignInButton() - // Intro to other sections - verifyGetToKnowHeader() - - // Automatic privacy - scrollToElementByText("Automatic privacy") + // Always-on privacy + scrollToElementByText(STRING_ONBOARDING_TRACKING_PROTECTION_HEADER) verifyAutomaticPrivacyHeader() - verifyTrackingProtectionToggle() verifyAutomaticPrivacyText() // Choose your theme @@ -142,11 +139,7 @@ class SmokeTest { verifyLightThemeDescription() verifyLightThemeToggle() - // Browse privately - verifyBrowsePrivatelyHeader() - verifyBrowsePrivatelyText() - - // Take a position + // Pick your toolbar placement verifyTakePositionHeader() verifyTakePositionElements() @@ -161,6 +154,7 @@ class SmokeTest { } @Test + @Ignore("https://github.com/mozilla-mobile/fenix/issues/18603") // Verifies the functionality of the onboarding Start Browsing button fun startBrowsingButtonTest() { homeScreen { diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt index 2db81496c3..cc649577d5 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt @@ -54,6 +54,9 @@ import org.mozilla.fenix.helpers.click import org.mozilla.fenix.helpers.ext.waitNotNull import org.mozilla.fenix.helpers.matchers.hasItem import org.mozilla.fenix.helpers.withBitmapDrawable +import org.mozilla.fenix.ui.util.STRING_ONBOARDING_ACCOUNT_SIGN_IN_HEADER +import org.mozilla.fenix.ui.util.STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER +import org.mozilla.fenix.ui.util.STRING_ONBOARDING_TRACKING_PROTECTION_HEADER /** * Implementation of Robot Pattern for the home screen menu. @@ -85,7 +88,6 @@ class HomeScreenRobot { fun verifyStartSyncHeader() = assertStartSyncHeader() fun verifyAccountsSignInButton() = assertAccountsSignInButton() - fun verifyGetToKnowHeader() = assertGetToKnowHeader() fun verifyChooseThemeHeader() = assertChooseThemeHeader() fun verifyChooseThemeText() = assertChooseThemeText() fun verifyLightThemeToggle() = assertLightThemeToggle() @@ -95,18 +97,13 @@ class HomeScreenRobot { fun verifyAutomaticThemeToggle() = assertAutomaticThemeToggle() fun verifyAutomaticThemeDescription() = assertAutomaticThemeDescription() fun verifyAutomaticPrivacyHeader() = assertAutomaticPrivacyHeader() - fun verifyTrackingProtectionToggle() = assertTrackingProtectionToggle() - fun verifyAutomaticPrivacyText() = assertAutomaticPrivacyText() + fun verifyAutomaticPrivacyText() = assertAlwaysPrivacyText() - // Browse privately - fun verifyBrowsePrivatelyHeader() = assertBrowsePrivatelyHeader() - fun verifyBrowsePrivatelyText() = assertBrowsePrivatelyText() - - // Take a position - fun verifyTakePositionHeader() = assertTakePositionheader() + // Pick your toolbar placement + fun verifyTakePositionHeader() = assertTakePlacementHeader() fun verifyTakePositionElements() { - assertTakePositionBottomRadioButton() - assertTakePositionTopRadioButton() + assertTakePlacementBottomRadioButton() + assertTakePacementTopRadioButton() } // Your privacy @@ -549,18 +546,15 @@ private fun assertWelcomeHeader() = onView(allOf(withText("Welcome to ${appContext.appName}!"))) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -private fun assertStartSyncHeader() = - onView(allOf(withText("Start syncing bookmarks, passwords, and more with your Firefox account."))) +private fun assertStartSyncHeader() { + scrollToElementByText(STRING_ONBOARDING_ACCOUNT_SIGN_IN_HEADER) + onView(allOf(withText(R.string.onboarding_account_sign_in_header_1))) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - +} private fun assertAccountsSignInButton() = onView(ViewMatchers.withResourceName("fxa_sign_in_button")) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -private fun assertGetToKnowHeader() = - onView(allOf(withText("Get to know ${appContext.appName}"))) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - private fun assertChooseThemeHeader() { scrollToElementByText("Choose your theme") onView(withText("Choose your theme")) @@ -568,7 +562,7 @@ private fun assertChooseThemeHeader() { } private fun assertChooseThemeText() { scrollToElementByText("Choose your theme") - onView(allOf(withText("Save some battery and your eyesight by enabling dark mode."))) + onView(allOf(withText("Save some battery and your eyesight with dark mode."))) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } @@ -608,40 +602,23 @@ private fun assertAutomaticThemeDescription() { } private fun assertAutomaticPrivacyHeader() { - scrollToElementByText("Automatic privacy") - onView(allOf(withText("Automatic privacy"))) + scrollToElementByText(STRING_ONBOARDING_TRACKING_PROTECTION_HEADER) + onView(allOf(withText(STRING_ONBOARDING_TRACKING_PROTECTION_HEADER))) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } -private fun assertTrackingProtectionToggle() { - scrollToElementByText("Automatic privacy") - onView(withId(R.id.tracking_protection_toggle)) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -} - -private fun assertAutomaticPrivacyText() { - scrollToElementByText("Automatic privacy") +private fun assertAlwaysPrivacyText() { + scrollToElementByText(STRING_ONBOARDING_TRACKING_PROTECTION_HEADER) onView( allOf( withText( - "Privacy and security settings block trackers, malware, and companies that follow you." + R.string.onboarding_tracking_protection_description_3 ) ) ) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } -private fun assertBrowsePrivatelyHeader() { - scrollToElementByText("Browse privately") - onView(allOf(withText("Browse privately"))) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -} - -private fun assertBrowsePrivatelyText() { - scrollToElementByText("Browse privately") - onView(allOf(withText(containsString("Update your private browsing settings.")))) - .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) -} private fun assertYourPrivacyHeader() { scrollToElementByText("Your privacy") onView(allOf(withText("Your privacy"))) @@ -672,21 +649,21 @@ private fun assertStartBrowsingButton() { .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } -// Take a position -private fun assertTakePositionheader() { - scrollToElementByText("Take a position") - onView(allOf(withText("Take a position"))) +// Pick your toolbar placement +private fun assertTakePlacementHeader() { + scrollToElementByText(STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER) + onView(allOf(withText(STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER))) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } -private fun assertTakePositionTopRadioButton() { - scrollToElementByText("Take a position") +private fun assertTakePacementTopRadioButton() { + scrollToElementByText(STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER) onView(ViewMatchers.withResourceName("toolbar_top_radio_button")) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } -private fun assertTakePositionBottomRadioButton() { - scrollToElementByText("Take a position") +private fun assertTakePlacementBottomRadioButton() { + scrollToElementByText(STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER) onView(ViewMatchers.withResourceName("toolbar_bottom_radio_button")) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuEnhancedTrackingProtectionRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuEnhancedTrackingProtectionRobot.kt index 14e7b0a8e7..e3fe7d6fad 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuEnhancedTrackingProtectionRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuEnhancedTrackingProtectionRobot.kt @@ -143,16 +143,13 @@ private fun assertEnhancedTrackingProtectionOptions() { onView(withText("Standard (default)")) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - val stdText = "Blocks fewer trackers. Pages will load normally." - onView(withText(stdText)) + onView(withText(org.mozilla.fenix.R.string.preference_enhanced_tracking_protection_standard_description_4)) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) onView(withText("Strict")) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) - val strictText = - "Blocks more trackers, ads, and popups. Pages load faster, but some functionality might not work." - onView(withText(strictText)) + onView(withText(org.mozilla.fenix.R.string.preference_enhanced_tracking_protection_strict_description_3)) .check(matches(withEffectiveVisibility(Visibility.VISIBLE))) onView(withText("Custom")) @@ -168,16 +165,13 @@ private fun assertEnhancedTrackingProtectionOptionsGrayedOut() { onView(withText("Standard (default)")) .check(matches(not(isEnabled(true)))) - val stdText = "Blocks fewer trackers. Pages will load normally." - onView(withText(stdText)) + onView(withText(org.mozilla.fenix.R.string.preference_enhanced_tracking_protection_standard_description_4)) .check(matches(not(isEnabled(true)))) onView(withText("Strict")) .check(matches(not(isEnabled(true)))) - val strictText = - "Blocks more trackers, ads, and popups. Pages load faster, but some functionality might not work." - onView(withText(strictText)) + onView(withText(org.mozilla.fenix.R.string.preference_enhanced_tracking_protection_strict_description_3)) .check(matches(not(isEnabled(true)))) onView(withText("Custom")) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/util/Strings.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/util/Strings.kt new file mode 100644 index 0000000000..3550cf1bc5 --- /dev/null +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/util/Strings.kt @@ -0,0 +1,9 @@ +/* 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.ui.util + +const val STRING_ONBOARDING_ACCOUNT_SIGN_IN_HEADER = "Sync Firefox between devices" +const val STRING_ONBOARDING_TRACKING_PROTECTION_HEADER = "Always-on privacy" +const val STRING_ONBOARDING_TOOLBAR_PLACEMENT_HEADER = "Pick your toolbar placement" diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt index 3258fb31de..1f22ca0640 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt @@ -12,7 +12,6 @@ import androidx.recyclerview.widget.RecyclerView import kotlinx.android.extensions.LayoutContainer import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSite -import org.mozilla.fenix.R import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.components import org.mozilla.fenix.home.HomeFragmentState @@ -71,6 +70,13 @@ private fun privateModeAdapterItems() = listOf(AdapterItem.PrivateBrowsingDescri private fun onboardingAdapterItems(onboardingState: OnboardingState): List { val items: MutableList = mutableListOf(AdapterItem.OnboardingHeader) + items.addAll( + listOf( + AdapterItem.OnboardingThemePicker, + AdapterItem.OnboardingToolbarPositionPicker, + AdapterItem.OnboardingTrackingProtection + ) + ) // Customize FxA items based on where we are with the account state: items.addAll( when (onboardingState) { @@ -90,14 +96,6 @@ private fun onboardingAdapterItems(onboardingState: OnboardingState): List - updateTrackingProtectionSetting(isChecked) - updateRadioGroupState(isChecked) - } - } - - setupRadioGroup(trackingProtectionToggle.isChecked) - updateRadioGroupState(trackingProtectionToggle.isChecked) + val isTrackingProtectionEnabled = view.context.settings().shouldUseTrackingProtection + setupRadioGroup(isTrackingProtectionEnabled) + updateRadioGroupState(isTrackingProtectionEnabled) } private fun setupRadioGroup(isChecked: Boolean) { @@ -74,15 +64,6 @@ class OnboardingTrackingProtectionViewHolder(view: View) : RecyclerView.ViewHold strictTrackingProtection.isEnabled = isChecked } - private fun updateTrackingProtectionSetting(enabled: Boolean) { - itemView.context.settings().shouldUseTrackingProtection = enabled - with(itemView.context.components) { - val policy = core.trackingProtectionPolicyFactory.createTrackingProtectionPolicy() - useCases.settingsUseCases.updateTrackingProtection.invoke(policy) - useCases.sessionUseCases.reload.invoke() - } - } - private fun updateTrackingProtectionPolicy() { itemView.context?.components?.let { val policy = it.core.trackingProtectionPolicyFactory diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingController.kt b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingController.kt deleted file mode 100644 index 3e19451037..0000000000 --- a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingController.kt +++ /dev/null @@ -1,22 +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.onboarding - -import android.content.Context -import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.settings.SupportUtils - -class OnboardingController( - private val context: Context -) { - fun handleLearnMoreClicked() { - (context as HomeActivity).openToBrowserAndLoad( - searchTermOrURL = SupportUtils.getFirefoxAccountSumoUrl(), - newTab = true, - from = BrowserDirection.FromHome - ) - } -} diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingInteractor.kt b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingInteractor.kt deleted file mode 100644 index ad11d459c1..0000000000 --- a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingInteractor.kt +++ /dev/null @@ -1,14 +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.onboarding - -class OnboardingInteractor(private val onboardingController: OnboardingController) { - - /** - * Called when the user clicks the learn more link - * @param url the url the suggestion was providing - */ - fun onLearnMoreClicked() = onboardingController.handleLearnMoreClicked() -} diff --git a/app/src/main/res/drawable/ic_onboarding_avatar_anonymous_large.xml b/app/src/main/res/drawable/ic_onboarding_avatar_anonymous_large.xml index 422897fd9e..078d9b0ecd 100644 --- a/app/src/main/res/drawable/ic_onboarding_avatar_anonymous_large.xml +++ b/app/src/main/res/drawable/ic_onboarding_avatar_anonymous_large.xml @@ -8,9 +8,9 @@ android:viewportWidth="24" android:viewportHeight="24"> diff --git a/app/src/main/res/layout/onboarding_manual_signin.xml b/app/src/main/res/layout/onboarding_manual_signin.xml index c1503c97b4..dbbcadd165 100644 --- a/app/src/main/res/layout/onboarding_manual_signin.xml +++ b/app/src/main/res/layout/onboarding_manual_signin.xml @@ -7,7 +7,7 @@ xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools" android:id="@+id/onboarding_card" - style="@style/OnboardingCardDark" + style="@style/OnboardingCardLightWithPadding" android:layout_width="match_parent" android:layout_height="wrap_content" android:orientation="vertical"> @@ -26,37 +26,31 @@ android:id="@+id/header_text" android:layout_width="match_parent" android:layout_height="wrap_content" - android:textAppearance="@style/Header16TextStyle" + android:textAppearance="@style/HeaderTextStyle" android:lineSpacingExtra="8sp" - android:textColor="@color/neutral_text" - app:drawableTint="@color/white_color" + android:layout_marginTop="10dp" app:layout_constraintTop_toTopOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="@id/avatar_icon" android:layout_marginStart="52dp" - tools:text="@string/onboarding_account_sign_in_header" - /> + tools:text="@string/onboarding_account_sign_in_header_1" /> - + android:layout_marginTop="14dp" + android:text="@string/onboarding_manual_sign_in_description" + android:textAppearance="@style/Body14TextStyle" + app:layout_constraintStart_toStartOf="@id/avatar_icon" + app:layout_constraintTop_toBottomOf="@id/avatar_icon" />