From 931c43127fd0702350a37cdbf3700a9ca6a8c9f3 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Mon, 15 Jan 2024 16:28:12 -0500 Subject: [PATCH] Bug 1874768 - Hide default browser onboarding card if user already set us as default browser --- app/onboarding.fml.yaml | 10 +- .../onboarding/view/OnboardingMapperTest.kt | 152 +++++++++++++++--- .../fenix/ui/robots/HomeScreenRobot.kt | 2 +- .../fenix/onboarding/OnboardingFragment.kt | 70 +++++--- .../fenix/onboarding/view/OnboardingMapper.kt | 33 ++-- .../fenix/onboarding/view/OnboardingPage.kt | 39 ++--- .../onboarding/view/OnboardingPageState.kt | 12 +- .../onboarding/view/OnboardingPageUiData.kt | 2 +- .../fenix/onboarding/view/OnboardingScreen.kt | 36 +++-- app/src/main/res/values/strings.xml | 9 +- .../onboarding/view/OnboardingMapperTest.kt | 25 +-- .../view/OnboardingPageUiDataTest.kt | 4 +- 12 files changed, 261 insertions(+), 133 deletions(-) diff --git a/app/onboarding.fml.yaml b/app/onboarding.fml.yaml index dcd7be764a..f4c3f1e12f 100644 --- a/app/onboarding.fml.yaml +++ b/app/onboarding.fml.yaml @@ -25,8 +25,7 @@ features: card-type: default-browser title: juno_onboarding_default_browser_title_nimbus_2 ordering: 10 - body: juno_onboarding_default_browser_description_nimbus_2 - link-text: juno_onboarding_default_browser_description_link_text + body: juno_onboarding_default_browser_description_nimbus_3 image-res: ic_onboarding_welcome primary-button-label: juno_onboarding_default_browser_positive_button secondary-button-label: juno_onboarding_default_browser_negative_button @@ -83,13 +82,6 @@ objects: description: The message text displayed to the user. May contain linkable text. # This should never be defaulted. default: "" - link-text: - type: Option - description: > - The text to link from the body text. This should match the linkable text from the body text exactly. - e.g. body: This is a policy link - link-text: policy link - default: null image-res: type: Image description: The resource id of the image to be displayed. diff --git a/app/src/androidTest/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt b/app/src/androidTest/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt index ac48f485ee..1a9024ee67 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt @@ -45,10 +45,12 @@ class OnboardingMapperTest { @Test fun showNotificationTrue_showAddWidgetFalse_pagesToDisplay_returnsSortedListOfAllConvertedPages_withoutAddWidgetPage() { - val expected = listOf(defaultBrowserPageUiData, syncPageUiData, notificationPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption, syncPageUiData, notificationPageUiData) assertEquals( expected, unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = true, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -59,10 +61,12 @@ class OnboardingMapperTest { @Test fun showNotificationFalse_showAddWidgetFalse_pagesToDisplay_returnsSortedListOfConvertedPages_withoutNotificationPage_and_addWidgetPage() { - val expected = listOf(defaultBrowserPageUiData, syncPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption, syncPageUiData) assertEquals( expected, unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -71,12 +75,77 @@ class OnboardingMapperTest { ) } + @Test + fun pagesToDisplay_returnsSortedListOfConvertedPages_withPrivacyCaption_alwaysOnFirstPage() { + var result = unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = false, + showNotificationPage = false, + showAddWidgetPage = false, + jexlConditions = jexlConditions, + func = evalFunction, + ) + assertEquals(result[0].privacyCaption, privacyCaption) + + result = unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = false, + showNotificationPage = true, + showAddWidgetPage = false, + jexlConditions = jexlConditions, + func = evalFunction, + ) + assertEquals(result[0].privacyCaption, privacyCaption) + assertEquals(result[1].privacyCaption, null) + + result = unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, + showNotificationPage = true, + showAddWidgetPage = false, + jexlConditions = jexlConditions, + func = evalFunction, + ) + assertEquals(result[0].privacyCaption, privacyCaption) + assertEquals(result[1].privacyCaption, null) + assertEquals(result[2].privacyCaption, null) + + result = unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = false, + showNotificationPage = false, + showAddWidgetPage = true, + jexlConditions = jexlConditions, + func = evalFunction, + ) + assertEquals(result[0].privacyCaption, privacyCaption) + assertEquals(result[1].privacyCaption, null) + } + + @Test + fun showDefaultBrowserPageFalse_showNotificationFalse_showAddWidgetTrue_pagesToDisplay_returnsSortedListOfAllConvertedPages() { + val expected = listOf(addSearchWidgetPageUiDataWithPrivacyCaption, syncPageUiData) + assertEquals( + expected, + unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = false, + showNotificationPage = false, + showAddWidgetPage = true, + jexlConditions = jexlConditions, + func = evalFunction, + ), + ) + } + @Test fun showNotificationFalse_showAddWidgetTrue_pagesToDisplay_returnsSortedListOfAllConvertedPages_withoutNotificationPage() { - val expected = listOf(defaultBrowserPageUiData, addSearchWidgetPageUiData, syncPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption, addSearchWidgetPageUiData, syncPageUiData) assertEquals( expected, unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = true, jexlConditions = jexlConditions, @@ -88,7 +157,7 @@ class OnboardingMapperTest { @Test fun showNotificationTrue_and_showAddWidgetTrue_pagesToDisplay_returnsSortedListOfConvertedPages() { val expected = listOf( - defaultBrowserPageUiData, + defaultBrowserPageUiDataWithPrivacyCaption, addSearchWidgetPageUiData, syncPageUiData, notificationPageUiData, @@ -96,6 +165,8 @@ class OnboardingMapperTest { assertEquals( expected, unsortedAllKnownCardData.toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = true, showAddWidgetPage = true, jexlConditions = jexlConditions, @@ -107,11 +178,13 @@ class OnboardingMapperTest { @Test fun cardConditionsMatchJexlConditions_shouldDisplayCard_returnsConvertedPage() { val jexlConditions = mapOf("ALWAYS" to "true", "NEVER" to "false") - val expected = listOf(defaultBrowserPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption) assertEquals( expected, listOf(defaultBrowserCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -128,6 +201,8 @@ class OnboardingMapperTest { assertEquals( expected, listOf(addSearchWidgetCardDataNoConditions).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -144,6 +219,8 @@ class OnboardingMapperTest { assertEquals( expected, listOf(defaultBrowserCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -155,11 +232,13 @@ class OnboardingMapperTest { @Test fun prerequisitesMatchJexlConditions_shouldDisplayCard_returnsConvertedPage() { val jexlConditions = mapOf("ALWAYS" to "true") - val expected = listOf(defaultBrowserPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption) assertEquals( expected, listOf(defaultBrowserCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -176,6 +255,8 @@ class OnboardingMapperTest { assertEquals( expected, listOf(defaultBrowserCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -192,6 +273,8 @@ class OnboardingMapperTest { assertEquals( expected, listOf(addSearchWidgetCardDataNoConditions).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -203,11 +286,13 @@ class OnboardingMapperTest { @Test fun noDisqualifiers_shouldDisplayCard_returnsConvertedPage() { val jexlConditions = mapOf("ALWAYS" to "true", "NEVER" to "false") - val expected = listOf(defaultBrowserPageUiData) + val expected = listOf(defaultBrowserPageUiDataWithPrivacyCaption) assertEquals( expected, listOf(defaultBrowserCardDataNoDisqualifiers).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -219,11 +304,13 @@ class OnboardingMapperTest { @Test fun disqualifiersMatchJexlConditions_shouldDisplayCard_returnsConvertedPage() { val jexlConditions = mapOf("NEVER" to "false") - val expected = listOf(syncPageUiData) + val expected = listOf(syncPageUiDataWithPrivacyCaption) assertEquals( expected, listOf(syncCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -240,6 +327,8 @@ class OnboardingMapperTest { assertEquals( expected, listOf(notificationCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -251,11 +340,13 @@ class OnboardingMapperTest { @Test fun noPrerequisites_shouldDisplayCard_returnsConvertedPage() { val jexlConditions = mapOf("ALWAYS" to "true", "NEVER" to "false") - val expected = listOf(syncPageUiData) + val expected = listOf(syncPageUiDataWithPrivacyCaption) assertEquals( expected, listOf(syncCardData).toPageUiData( + privacyCaption = privacyCaption, + showDefaultBrowserPage = true, showNotificationPage = false, showAddWidgetPage = false, jexlConditions = jexlConditions, @@ -264,24 +355,34 @@ class OnboardingMapperTest { ) } } +val privacyCaption: Caption = mockk(relaxed = true) -private val defaultBrowserPageUiData = OnboardingPageUiData( +private val defaultBrowserPageUiDataWithPrivacyCaption = OnboardingPageUiData( type = OnboardingPageUiData.Type.DEFAULT_BROWSER, imageRes = R.drawable.ic_onboarding_welcome, title = "default browser title", - description = "default browser body with link text", - linkText = "link text", + description = "default browser body", primaryButtonLabel = "default browser primary button text", secondaryButtonLabel = "default browser secondary button text", + privacyCaption = privacyCaption, ) private val addSearchWidgetPageUiData = OnboardingPageUiData( type = OnboardingPageUiData.Type.ADD_SEARCH_WIDGET, imageRes = R.drawable.ic_onboarding_search_widget, title = "add search widget title", - description = "add search widget body with link text", - linkText = "link text", + description = "add search widget body", + primaryButtonLabel = "add search widget primary button text", + secondaryButtonLabel = "add search widget secondary button text", + privacyCaption = null, +) +private val addSearchWidgetPageUiDataWithPrivacyCaption = OnboardingPageUiData( + type = OnboardingPageUiData.Type.ADD_SEARCH_WIDGET, + imageRes = R.drawable.ic_onboarding_search_widget, + title = "add search widget title", + description = "add search widget body", primaryButtonLabel = "add search widget primary button text", secondaryButtonLabel = "add search widget secondary button text", + privacyCaption = privacyCaption, ) private val syncPageUiData = OnboardingPageUiData( type = OnboardingPageUiData.Type.SYNC_SIGN_IN, @@ -290,6 +391,16 @@ private val syncPageUiData = OnboardingPageUiData( description = "sync body", primaryButtonLabel = "sync primary button text", secondaryButtonLabel = "sync secondary button text", + privacyCaption = null, +) +private val syncPageUiDataWithPrivacyCaption = OnboardingPageUiData( + type = OnboardingPageUiData.Type.SYNC_SIGN_IN, + imageRes = R.drawable.ic_onboarding_sync, + title = "sync title", + description = "sync body", + primaryButtonLabel = "sync primary button text", + secondaryButtonLabel = "sync secondary button text", + privacyCaption = privacyCaption, ) private val notificationPageUiData = OnboardingPageUiData( type = OnboardingPageUiData.Type.NOTIFICATION_PERMISSION, @@ -298,14 +409,14 @@ private val notificationPageUiData = OnboardingPageUiData( description = "notification body", primaryButtonLabel = "notification primary button text", secondaryButtonLabel = "notification secondary button text", + privacyCaption = null, ) private val defaultBrowserCardData = OnboardingCardData( cardType = OnboardingCardType.DEFAULT_BROWSER, imageRes = R.drawable.ic_onboarding_welcome, title = StringHolder(null, "default browser title"), - body = StringHolder(null, "default browser body with link text"), - linkText = StringHolder(null, "link text"), + body = StringHolder(null, "default browser body"), primaryButtonLabel = StringHolder(null, "default browser primary button text"), secondaryButtonLabel = StringHolder(null, "default browser secondary button text"), ordering = 10, @@ -317,8 +428,7 @@ private val defaultBrowserCardDataNoDisqualifiers = OnboardingCardData( cardType = OnboardingCardType.DEFAULT_BROWSER, imageRes = R.drawable.ic_onboarding_welcome, title = StringHolder(null, "default browser title"), - body = StringHolder(null, "default browser body with link text"), - linkText = StringHolder(null, "link text"), + body = StringHolder(null, "default browser body"), primaryButtonLabel = StringHolder(null, "default browser primary button text"), secondaryButtonLabel = StringHolder(null, "default browser secondary button text"), ordering = 10, @@ -330,8 +440,7 @@ private val addSearchWidgetCardDataNoConditions = OnboardingCardData( cardType = OnboardingCardType.ADD_SEARCH_WIDGET, imageRes = R.drawable.ic_onboarding_search_widget, title = StringHolder(null, "add search widget title"), - body = StringHolder(null, "add search widget body with link text"), - linkText = StringHolder(null, "link text"), + body = StringHolder(null, "add search widget body"), primaryButtonLabel = StringHolder(null, "add search widget primary button text"), secondaryButtonLabel = StringHolder(null, "add search widget secondary button text"), ordering = 15, @@ -343,8 +452,7 @@ private val addSearchWidgetCardData = OnboardingCardData( cardType = OnboardingCardType.ADD_SEARCH_WIDGET, imageRes = R.drawable.ic_onboarding_search_widget, title = StringHolder(null, "add search widget title"), - body = StringHolder(null, "add search widget body with link text"), - linkText = StringHolder(null, "link text"), + body = StringHolder(null, "add search widget body"), primaryButtonLabel = StringHolder(null, "add search widget primary button text"), secondaryButtonLabel = StringHolder(null, "add search widget secondary button text"), ordering = 15, 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 11039e33c1..d3e78c462e 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 @@ -140,7 +140,7 @@ class HomeScreenRobot { ).assertExists() it.onNodeWithText( - getStringResource(R.string.juno_onboarding_default_browser_description_nimbus_2), + getStringResource(R.string.juno_onboarding_default_browser_description_nimbus_3), ).assertExists() it.onNodeWithText( diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt index 3791607f4a..d46adfac31 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/OnboardingFragment.kt @@ -18,21 +18,23 @@ import android.view.ViewGroup import androidx.annotation.RequiresApi import androidx.compose.runtime.Composable import androidx.compose.ui.platform.ComposeView -import androidx.compose.ui.platform.LocalContext import androidx.core.app.NotificationManagerCompat import androidx.fragment.app.Fragment import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.navigation.fragment.findNavController import mozilla.components.service.nimbus.evalJexlSafe import mozilla.components.support.base.ext.areNotificationsEnabledSafe +import mozilla.components.support.utils.BrowsersCache import org.mozilla.fenix.R import org.mozilla.fenix.components.accounts.FenixFxAEntryPoint +import org.mozilla.fenix.compose.LinkTextState import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.hideToolbar import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.openSetDefaultBrowserOption import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.nimbus.FxNimbus +import org.mozilla.fenix.onboarding.view.Caption import org.mozilla.fenix.onboarding.view.OnboardingPageUiData import org.mozilla.fenix.onboarding.view.OnboardingScreen import org.mozilla.fenix.onboarding.view.sequencePosition @@ -49,6 +51,7 @@ class OnboardingFragment : Fragment() { private val pagesToDisplay by lazy { pagesToDisplay( + shouldShowDefaultBrowserCard(requireContext()), canShowNotificationPage(requireContext()), canShowAddWidgetCard(), ) @@ -59,6 +62,11 @@ class OnboardingFragment : Fragment() { @SuppressLint("SourceLockedOrientationActivity") override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + if (pagesToDisplay.isEmpty()) { + /* do not continue if there's no onboarding pages to display */ + onFinish(null) + } + if (isNotATablet()) { activity?.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_PORTRAIT } @@ -97,7 +105,6 @@ class OnboardingFragment : Fragment() { @Composable @Suppress("LongMethod") private fun ScreenContent() { - val context = LocalContext.current OnboardingScreen( pagesToDisplay = pagesToDisplay, onMakeFirefoxDefaultClick = { @@ -113,18 +120,6 @@ class OnboardingFragment : Fragment() { pagesToDisplay.sequencePosition(OnboardingPageUiData.Type.DEFAULT_BROWSER), ) }, - onPrivacyPolicyClick = { url -> - startActivity( - SupportUtils.createSandboxCustomTabIntent( - context = context, - url = url, - ), - ) - telemetryRecorder.onPrivacyPolicyClick( - pagesToDisplay.telemetrySequenceId(), - pagesToDisplay.sequencePosition(OnboardingPageUiData.Type.DEFAULT_BROWSER), - ) - }, onSignInButtonClick = { findNavController().nav( id = R.id.onboardingFragment, @@ -172,10 +167,7 @@ class OnboardingFragment : Fragment() { ) }, onFinish = { - onFinish( - sequenceId = pagesToDisplay.telemetrySequenceId(), - sequencePosition = pagesToDisplay.sequencePosition(it.type), - ) + onFinish(it) }, onImpression = { telemetryRecorder.onImpression( @@ -200,18 +192,28 @@ class OnboardingFragment : Fragment() { } } - private fun onFinish(sequenceId: String, sequencePosition: String) { + private fun onFinish(onboardingPageUiData: OnboardingPageUiData?) { + /* onboarding page UI data can be null if there was no pages to display */ + if (onboardingPageUiData != null) { + val sequenceId = pagesToDisplay.telemetrySequenceId() + val sequencePosition = pagesToDisplay.sequencePosition(onboardingPageUiData.type) + + telemetryRecorder.onOnboardingComplete( + sequenceId = sequenceId, + sequencePosition = sequencePosition, + ) + } + requireComponents.fenixOnboarding.finish() findNavController().nav( id = R.id.onboardingFragment, directions = OnboardingFragmentDirections.actionHome(), ) - telemetryRecorder.onOnboardingComplete( - sequenceId = sequenceId, - sequencePosition = sequencePosition, - ) } + private fun shouldShowDefaultBrowserCard(context: Context) = + !BrowsersCache.all(context.applicationContext).isDefaultBrowser + private fun canShowNotificationPage(context: Context) = !NotificationManagerCompat.from(context.applicationContext) .areNotificationsEnabledSafe() && Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU @@ -221,13 +223,35 @@ class OnboardingFragment : Fragment() { private fun isNotATablet() = !resources.getBoolean(R.bool.tablet) private fun pagesToDisplay( + showDefaultBrowserPage: Boolean, showNotificationPage: Boolean, showAddWidgetPage: Boolean, ): List { val jexlConditions = FxNimbus.features.junoOnboarding.value().conditions val jexlHelper = requireContext().components.analytics.messagingStorage.helper + val privacyCaption = Caption( + text = getString(R.string.juno_onboarding_privacy_notice_text), + linkTextState = LinkTextState( + text = getString(R.string.juno_onboarding_privacy_notice_text), + url = SupportUtils.getMozillaPageUrl(SupportUtils.MozillaPage.PRIVATE_NOTICE), + onClick = { + startActivity( + SupportUtils.createSandboxCustomTabIntent( + context = requireContext(), + url = it, + ), + ) + telemetryRecorder.onPrivacyPolicyClick( + pagesToDisplay.telemetrySequenceId(), + pagesToDisplay.sequencePosition(OnboardingPageUiData.Type.DEFAULT_BROWSER), + ) + }, + ), + ) return FxNimbus.features.junoOnboarding.value().cards.values.toPageUiData( + privacyCaption, + showDefaultBrowserPage, showNotificationPage, showAddWidgetPage, jexlConditions, diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingMapper.kt b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingMapper.kt index 685587b27a..9e3853a1d4 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingMapper.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingMapper.kt @@ -4,15 +4,15 @@ package org.mozilla.fenix.onboarding.view -import org.mozilla.fenix.compose.LinkTextState import org.mozilla.fenix.nimbus.OnboardingCardData import org.mozilla.fenix.nimbus.OnboardingCardType -import org.mozilla.fenix.settings.SupportUtils /** * Returns a list of all the required Nimbus 'cards' that have been converted to [OnboardingPageUiData]. */ internal fun Collection.toPageUiData( + privacyCaption: Caption, + showDefaultBrowserPage: Boolean, showNotificationPage: Boolean, showAddWidgetPage: Boolean, jexlConditions: Map, @@ -21,16 +21,25 @@ internal fun Collection.toPageUiData( // we are first filtering the cards based on Nimbus configuration return filter { it.shouldDisplayCard(func, jexlConditions) } // we are then filtering again based on device capabilities - .filter { it.isCardEnabled(showNotificationPage, showAddWidgetPage) } + .filter { it.isCardEnabled(showDefaultBrowserPage, showNotificationPage, showAddWidgetPage) } .sortedBy { it.ordering } - .map { it.toPageUiData() } + .mapIndexed { + index, onboardingCardData -> + // only first onboarding card shows privacy caption + onboardingCardData.toPageUiData(if (index == 0) privacyCaption else null) + } } private fun OnboardingCardData.isCardEnabled( + showDefaultBrowserPage: Boolean, showNotificationPage: Boolean, showAddWidgetPage: Boolean, ): Boolean = when (cardType) { + OnboardingCardType.DEFAULT_BROWSER -> { + enabled && showDefaultBrowserPage + } + OnboardingCardType.NOTIFICATION_PERMISSION -> { enabled && showNotificationPage } @@ -91,14 +100,14 @@ private fun OnboardingCardData.shouldDisplayCard( return validPrerequisites && !hasDisqualifiers } -private fun OnboardingCardData.toPageUiData() = OnboardingPageUiData( +private fun OnboardingCardData.toPageUiData(privacyCaption: Caption?) = OnboardingPageUiData( type = cardType.toPageUiDataType(), imageRes = imageRes.resourceId, title = title, description = body, - linkText = linkText, primaryButtonLabel = primaryButtonLabel, secondaryButtonLabel = secondaryButtonLabel, + privacyCaption = privacyCaption, ) private fun OnboardingCardType.toPageUiDataType() = when (this) { @@ -117,7 +126,6 @@ internal fun mapToOnboardingPageState( onboardingPageUiData: OnboardingPageUiData, onMakeFirefoxDefaultClick: () -> Unit, onMakeFirefoxDefaultSkipClick: () -> Unit, - onPrivacyPolicyClick: (String) -> Unit, onSignInButtonClick: () -> Unit, onSignInSkipClick: () -> Unit, onNotificationPermissionButtonClick: () -> Unit, @@ -129,14 +137,12 @@ internal fun mapToOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onPositiveButtonClick = onMakeFirefoxDefaultClick, onNegativeButtonClick = onMakeFirefoxDefaultSkipClick, - onUrlClick = onPrivacyPolicyClick, ) OnboardingPageUiData.Type.ADD_SEARCH_WIDGET -> createOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onPositiveButtonClick = onAddFirefoxWidgetClick, onNegativeButtonClick = onAddFirefoxWidgetSkipClick, - onUrlClick = onPrivacyPolicyClick, ) OnboardingPageUiData.Type.SYNC_SIGN_IN -> createOnboardingPageState( @@ -156,18 +162,11 @@ private fun createOnboardingPageState( onboardingPageUiData: OnboardingPageUiData, onPositiveButtonClick: () -> Unit, onNegativeButtonClick: () -> Unit, - onUrlClick: (String) -> Unit = {}, ): OnboardingPageState = OnboardingPageState( imageRes = onboardingPageUiData.imageRes, title = onboardingPageUiData.title, description = onboardingPageUiData.description, - linkTextState = onboardingPageUiData.linkText?.let { - LinkTextState( - text = it, - url = SupportUtils.getMozillaPageUrl(SupportUtils.MozillaPage.PRIVATE_NOTICE), - onClick = onUrlClick, - ) - }, primaryButton = Action(onboardingPageUiData.primaryButtonLabel, onPositiveButtonClick), secondaryButton = Action(onboardingPageUiData.secondaryButtonLabel, onNegativeButtonClick), + privacyCaption = onboardingPageUiData.privacyCaption, ) diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPage.kt b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPage.kt index e590f1f352..743beec388 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPage.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPage.kt @@ -30,7 +30,6 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import org.mozilla.fenix.R import org.mozilla.fenix.compose.LinkText -import org.mozilla.fenix.compose.LinkTextState import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.compose.button.PrimaryButton import org.mozilla.fenix.compose.button.SecondaryButton @@ -61,6 +60,7 @@ private const val IMAGE_HEIGHT_RATIO_SMALL = 0.28f * it doesn't show the close button. */ @Composable +@Suppress("LongMethod") fun OnboardingPage( pageState: OnboardingPageState, modifier: Modifier = Modifier, @@ -116,10 +116,21 @@ fun OnboardingPage( Spacer(modifier = Modifier.height(16.dp)) - DescriptionText( - description = pageState.description, - linkTextState = pageState.linkTextState, + Text( + text = pageState.description, + color = FirefoxTheme.colors.textSecondary, + textAlign = TextAlign.Center, + style = FirefoxTheme.typography.body2, ) + + Spacer(modifier = Modifier.height(16.dp)) + + pageState.privacyCaption?.let { privacyCaption -> + LinkText( + text = privacyCaption.text, + linkTextStates = listOf(privacyCaption.linkTextState), + ) + } } Column( @@ -147,26 +158,6 @@ fun OnboardingPage( } } -@Composable -private fun DescriptionText( - description: String, - linkTextState: LinkTextState?, -) { - if (linkTextState != null && description.contains(linkTextState.text, ignoreCase = true)) { - LinkText( - text = description, - linkTextStates = listOf(linkTextState), - ) - } else { - Text( - text = description, - color = FirefoxTheme.colors.textSecondary, - textAlign = TextAlign.Center, - style = FirefoxTheme.typography.body2, - ) - } -} - /** * Calculates the image height to be set. The ratio is selected based on parent height. */ diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageState.kt b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageState.kt index befd3febce..11becda2e0 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageState.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageState.kt @@ -13,7 +13,7 @@ import org.mozilla.fenix.compose.LinkTextState * @property imageRes [DrawableRes] displayed on the page. * @property title [String] title of the page. * @property description [String] description of the page. - * @property linkTextState [LinkTextState] part of description text with a link. + * @property privacyCaption privacy caption to show and allow user to view on privacy policy. * @property primaryButton [Action] action for the primary button. * @property secondaryButton [Action] action for the secondary button. * @property onRecordImpressionEvent Callback for recording impression event. @@ -22,7 +22,7 @@ data class OnboardingPageState( @DrawableRes val imageRes: Int, val title: String, val description: String, - val linkTextState: LinkTextState? = null, + val privacyCaption: Caption? = null, val primaryButton: Action, val secondaryButton: Action? = null, val onRecordImpressionEvent: () -> Unit = {}, @@ -35,3 +35,11 @@ data class Action( val text: String, val onClick: () -> Unit, ) + +/** + * Model containing text and [LinkTextState] for a caption. + */ +data class Caption( + val text: String, + val linkTextState: LinkTextState, +) diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiData.kt b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiData.kt index f9713d1023..2781b01f35 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiData.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiData.kt @@ -15,9 +15,9 @@ data class OnboardingPageUiData( @DrawableRes val imageRes: Int, val title: String, val description: String, - val linkText: String? = null, val primaryButtonLabel: String, val secondaryButtonLabel: String, + val privacyCaption: Caption?, ) { /** * Model for different types of Onboarding Pages. diff --git a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingScreen.kt b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingScreen.kt index 6f3bb73def..9ae34da5bf 100644 --- a/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingScreen.kt +++ b/app/src/main/java/org/mozilla/fenix/onboarding/view/OnboardingScreen.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.launch import mozilla.components.lib.state.ext.observeAsComposableState import org.mozilla.fenix.R import org.mozilla.fenix.components.components +import org.mozilla.fenix.compose.LinkTextState import org.mozilla.fenix.compose.PagerIndicator import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.onboarding.WidgetPinnedReceiver.WidgetPinnedState @@ -48,7 +49,6 @@ import org.mozilla.fenix.theme.FirefoxTheme * @param pagesToDisplay List of pages to be displayed in onboarding pager ui. * @param onMakeFirefoxDefaultClick Invoked when positive button on default browser page is clicked. * @param onSkipDefaultClick Invoked when negative button on default browser page is clicked. - * @param onPrivacyPolicyClick Invoked when the privacy policy link text is clicked. * @param onSignInButtonClick Invoked when the positive button on the sign in page is clicked. * @param onSkipSignInClick Invoked when the negative button on the sign in page is clicked. * @param onNotificationPermissionButtonClick Invoked when positive button on notification page is @@ -65,7 +65,6 @@ fun OnboardingScreen( pagesToDisplay: List, onMakeFirefoxDefaultClick: () -> Unit, onSkipDefaultClick: () -> Unit, - onPrivacyPolicyClick: (url: String) -> Unit, onSignInButtonClick: () -> Unit, onSkipSignInClick: () -> Unit, onNotificationPermissionButtonClick: () -> Unit, @@ -127,9 +126,6 @@ fun OnboardingScreen( scrollToNextPageOrDismiss() onSkipDefaultClick() }, - onPrivacyPolicyClick = { - onPrivacyPolicyClick(it) - }, onSignInButtonClick = { onSignInButtonClick() scrollToNextPageOrDismiss() @@ -167,7 +163,6 @@ private fun OnboardingContent( pagerState: PagerState, onMakeFirefoxDefaultClick: () -> Unit, onMakeFirefoxDefaultSkipClick: () -> Unit, - onPrivacyPolicyClick: (String) -> Unit, onSignInButtonClick: () -> Unit, onSignInSkipClick: () -> Unit, onNotificationPermissionButtonClick: () -> Unit, @@ -195,7 +190,6 @@ private fun OnboardingContent( onboardingPageUiData = pageUiState, onMakeFirefoxDefaultClick = onMakeFirefoxDefaultClick, onMakeFirefoxDefaultSkipClick = onMakeFirefoxDefaultSkipClick, - onPrivacyPolicyClick = onPrivacyPolicyClick, onSignInButtonClick = onSignInButtonClick, onSignInSkipClick = onSignInSkipClick, onNotificationPermissionButtonClick = onNotificationPermissionButtonClick, @@ -251,7 +245,6 @@ private fun OnboardingScreenPreview() { }, onMakeFirefoxDefaultClick = {}, onMakeFirefoxDefaultSkipClick = {}, - onPrivacyPolicyClick = {}, onSignInButtonClick = {}, onSignInSkipClick = {}, onNotificationPermissionButtonClick = {}, @@ -268,10 +261,17 @@ private fun defaultPreviewPages() = listOf( type = OnboardingPageUiData.Type.DEFAULT_BROWSER, imageRes = R.drawable.ic_onboarding_welcome, title = stringResource(R.string.juno_onboarding_default_browser_title_nimbus_2), - description = stringResource(R.string.juno_onboarding_default_browser_description_nimbus_2), - linkText = stringResource(R.string.juno_onboarding_default_browser_description_link_text), + description = stringResource(R.string.juno_onboarding_default_browser_description_nimbus_3), primaryButtonLabel = stringResource(R.string.juno_onboarding_default_browser_positive_button), secondaryButtonLabel = stringResource(R.string.juno_onboarding_default_browser_negative_button), + privacyCaption = Caption( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + linkTextState = LinkTextState( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + url = "", + onClick = {}, + ), + ), ), OnboardingPageUiData( type = OnboardingPageUiData.Type.SYNC_SIGN_IN, @@ -280,6 +280,14 @@ private fun defaultPreviewPages() = listOf( description = stringResource(R.string.juno_onboarding_sign_in_description_2), primaryButtonLabel = stringResource(R.string.juno_onboarding_sign_in_positive_button), secondaryButtonLabel = stringResource(R.string.juno_onboarding_sign_in_negative_button), + privacyCaption = Caption( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + linkTextState = LinkTextState( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + url = "", + onClick = {}, + ), + ), ), OnboardingPageUiData( type = OnboardingPageUiData.Type.NOTIFICATION_PERMISSION, @@ -288,5 +296,13 @@ private fun defaultPreviewPages() = listOf( description = stringResource(R.string.juno_onboarding_enable_notifications_description_nimbus_2), primaryButtonLabel = stringResource(R.string.juno_onboarding_enable_notifications_positive_button), secondaryButtonLabel = stringResource(R.string.juno_onboarding_enable_notifications_negative_button), + privacyCaption = Caption( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + linkTextState = LinkTextState( + text = stringResource(R.string.juno_onboarding_privacy_notice_text), + url = "", + onClick = {}, + ), + ), ), ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d2e6842693..9564be03a8 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -310,13 +310,18 @@ Not now + + Firefox privacy notice + We love keeping you safe - Our non-profit backed browser helps stop companies from secretly following you around the web.\n\nLearn more in our privacy notice. + Our non-profit backed browser helps stop companies from secretly following you around the web. + + Our non-profit backed browser helps stop companies from secretly following you around the web.\n\nLearn more in our privacy notice. - privacy notice + privacy notice Set as default browser diff --git a/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt b/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt index cd9f92a485..4e903157b4 100644 --- a/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt +++ b/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingMapperTest.kt @@ -7,8 +7,6 @@ package org.mozilla.fenix.onboarding.view import org.junit.Assert.assertEquals import org.junit.Test import org.mozilla.fenix.R -import org.mozilla.fenix.compose.LinkTextState -import org.mozilla.fenix.settings.SupportUtils class OnboardingMapperTest { @@ -18,11 +16,6 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_onboarding_welcome, title = "default browser title", description = "default browser body with link text", - linkTextState = LinkTextState( - text = "link text", - url = SupportUtils.getMozillaPageUrl(SupportUtils.MozillaPage.PRIVATE_NOTICE), - onClick = stringLambda, - ), primaryButton = Action("default browser primary button text", unitLambda), secondaryButton = Action("default browser secondary button text", unitLambda), ) @@ -32,15 +25,14 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_onboarding_welcome, title = "default browser title", description = "default browser body with link text", - linkText = "link text", primaryButtonLabel = "default browser primary button text", secondaryButtonLabel = "default browser secondary button text", + privacyCaption = null, ) val actual = mapToOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onMakeFirefoxDefaultClick = unitLambda, onMakeFirefoxDefaultSkipClick = unitLambda, - onPrivacyPolicyClick = stringLambda, onSignInButtonClick = {}, onSignInSkipClick = {}, onNotificationPermissionButtonClick = {}, @@ -67,15 +59,14 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_onboarding_sync, title = "sync title", description = "sync body", - linkText = null, primaryButtonLabel = "sync primary button text", secondaryButtonLabel = "sync secondary button text", + privacyCaption = null, ) val actual = mapToOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onMakeFirefoxDefaultClick = {}, onMakeFirefoxDefaultSkipClick = {}, - onPrivacyPolicyClick = {}, onSignInButtonClick = unitLambda, onSignInSkipClick = unitLambda, onNotificationPermissionButtonClick = {}, @@ -102,15 +93,14 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_notification_permission, title = "notification title", description = "notification body", - linkText = null, primaryButtonLabel = "notification primary button text", secondaryButtonLabel = "notification secondary button text", + privacyCaption = null, ) val actual = mapToOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onMakeFirefoxDefaultClick = {}, onMakeFirefoxDefaultSkipClick = {}, - onPrivacyPolicyClick = {}, onSignInButtonClick = {}, onSignInSkipClick = {}, onNotificationPermissionButtonClick = unitLambda, @@ -128,11 +118,6 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_onboarding_search_widget, title = "add search widget title", description = "add search widget body with link text", - linkTextState = LinkTextState( - text = "link text", - url = SupportUtils.getMozillaPageUrl(SupportUtils.MozillaPage.PRIVATE_NOTICE), - onClick = stringLambda, - ), primaryButton = Action("add search widget primary button text", unitLambda), secondaryButton = Action("add search widget secondary button text", unitLambda), ) @@ -142,15 +127,14 @@ class OnboardingMapperTest { imageRes = R.drawable.ic_onboarding_search_widget, title = "add search widget title", description = "add search widget body with link text", - linkText = "link text", primaryButtonLabel = "add search widget primary button text", secondaryButtonLabel = "add search widget secondary button text", + privacyCaption = null, ) val actual = mapToOnboardingPageState( onboardingPageUiData = onboardingPageUiData, onMakeFirefoxDefaultClick = {}, onMakeFirefoxDefaultSkipClick = {}, - onPrivacyPolicyClick = stringLambda, onSignInButtonClick = {}, onSignInSkipClick = {}, onNotificationPermissionButtonClick = {}, @@ -164,7 +148,6 @@ class OnboardingMapperTest { } private val unitLambda = { dummyUnitFunc() } -private val stringLambda = { s: String -> dummyStringArgFunc(s) } private fun dummyUnitFunc() {} diff --git a/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiDataTest.kt b/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiDataTest.kt index e2d10e9711..ef06b4f3c7 100644 --- a/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiDataTest.kt +++ b/app/src/test/java/org/mozilla/fenix/onboarding/view/OnboardingPageUiDataTest.kt @@ -56,9 +56,9 @@ private val defaultBrowserPageUiData = OnboardingPageUiData( imageRes = R.drawable.ic_onboarding_welcome, title = "default browser title", description = "default browser body with link text", - linkText = "link text", primaryButtonLabel = "default browser primary button text", secondaryButtonLabel = "default browser secondary button text", + privacyCaption = null, ) private val syncPageUiData = OnboardingPageUiData( @@ -68,6 +68,7 @@ private val syncPageUiData = OnboardingPageUiData( description = "sync body", primaryButtonLabel = "sync primary button text", secondaryButtonLabel = "sync secondary button text", + privacyCaption = null, ) private val notificationPageUiData = OnboardingPageUiData( @@ -77,6 +78,7 @@ private val notificationPageUiData = OnboardingPageUiData( description = "notification body", primaryButtonLabel = "notification primary button text", secondaryButtonLabel = "notification secondary button text", + privacyCaption = null, ) private val allKnownPages = listOf(