Skip to content

Commit

Permalink
Bug 1924978 - Map the Nimbus add-ons onboarding card type to `AddOn…
Browse files Browse the repository at this point in the history
…sOnboardingPage` r=android-reviewers,amejiamarmol

Differential Revision: https://phabricator.services.mozilla.com/D226295
  • Loading branch information
t-p-white committed Oct 23, 2024
1 parent 74008f2 commit b635460
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 50 deletions.
2 changes: 1 addition & 1 deletion mobile/android/fenix/app/onboarding.fml.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ objects:
secondary-button-label:
type: Text
description: The text to display on the secondary button.
# This should never be defaulted.
# This can be defaulted if the card type does not required it.
default: ""
prerequisites:
type: List<ConditionName>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ class OnboardingFragment : Fragment() {
pagesToDisplay.sequencePosition(OnboardingPageUiData.Type.ADD_SEARCH_WIDGET),
)
},
onAddOnsButtonClick = {
// Todo as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1926296
throw NotImplementedError()
},
onFinish = {
onFinish(it)
disableNavBarCFRForNewUser()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private const val MINIMUM_SCREEN_HEIGHT_FOR_IMAGE = 640
* @param pageState The page content that's displayed.
*/
@Composable
fun AddOnsOnboardingPage(pageState: OnboardingAddOnsPageState) {
fun AddOnsOnboardingPage(pageState: OnboardingPageState) {
// Base
Column(
modifier = Modifier
Expand All @@ -79,7 +79,7 @@ fun AddOnsOnboardingPage(pageState: OnboardingAddOnsPageState) {

Spacer(Modifier.height(16.dp))

AddOns(addOnsUiData)
addOns?.let { AddOns(it) }

Spacer(Modifier.height(5.dp))

Expand Down Expand Up @@ -127,7 +127,8 @@ private fun MoreExtensionsLink() {

@Composable
private fun Header(@DrawableRes imageRes: Int, title: String, description: String) {
val showHeaderImage = LocalConfiguration.current.screenHeightDp > MINIMUM_SCREEN_HEIGHT_FOR_IMAGE
val showHeaderImage =
LocalConfiguration.current.screenHeightDp > MINIMUM_SCREEN_HEIGHT_FOR_IMAGE

if (showHeaderImage) {
HeaderImage(imageRes)
Expand Down Expand Up @@ -186,7 +187,7 @@ private fun AddOnItem(addOnUiData: OnboardingAddOn) {
Column(
modifier = Modifier.weight(1f),
horizontalAlignment = Alignment.Start,
) { AddOnDetails(name, description, averageRating, numberOfReviews) }
) { AddOnDetails(name, description, averageRating, reviewCount) }
}

Spacer(Modifier.width(16.dp))
Expand All @@ -204,7 +205,7 @@ private fun AddOnDetails(
name: String,
description: String,
averageRating: String,
numberOfReviews: String,
reviewCount: String,
) {
Text(
color = FirefoxTheme.colors.textSecondary,
Expand All @@ -222,7 +223,7 @@ private fun AddOnDetails(

Spacer(Modifier.height(8.dp))

RatingAndReviewRow(averageRating, numberOfReviews)
RatingAndReviewRow(averageRating, reviewCount)
}

@Composable
Expand Down Expand Up @@ -256,19 +257,20 @@ private fun AddAddOnButton() {
}

@Composable
private fun RatingAndReviewRow(rating: String, numberOfReviews: String) {
private fun RatingAndReviewRow(rating: String, reviewCount: String) {
Row {
AverageRatingRow(rating)

Spacer(Modifier.width(8.dp))

ReviewCountRow(numberOfReviews)
ReviewCountRow(reviewCount)
}
}

@Composable
private fun AverageRatingRow(rating: String) {
val ratingContentDescription = stringResource(R.string.onboarding_add_on_star_rating_content_description)
val ratingContentDescription =
stringResource(R.string.onboarding_add_on_star_rating_content_description)
Row(
Modifier
.wrapContentWidth()
Expand Down Expand Up @@ -303,13 +305,13 @@ private fun AverageRatingRow(rating: String) {
}

@Composable
private fun ReviewCountRow(numberOfReviews: String) {
private fun ReviewCountRow(reviewCount: String) {
Row(Modifier.wrapContentWidth()) {
Text(
color = FirefoxTheme.colors.textSecondary,
style = FirefoxTheme.typography.caption,
maxLines = 1,
text = stringResource(R.string.onboarding_add_on_reviews_label, numberOfReviews),
text = stringResource(R.string.onboarding_add_on_reviews_label, reviewCount),
)
}
}
Expand All @@ -321,7 +323,7 @@ private fun ReviewCountRow(numberOfReviews: String) {
private fun OnboardingPagePreview() {
FirefoxTheme {
AddOnsOnboardingPage(
pageState = OnboardingAddOnsPageState(
pageState = OnboardingPageState(
imageRes = R.drawable.ic_onboarding_add_ons,
title = stringResource(id = R.string.onboarding_add_on_header),
description = stringResource(id = R.string.onboarding_add_on_sub_header),
Expand All @@ -331,7 +333,7 @@ private fun OnboardingPagePreview() {
),
onClick = {},
),
addOnsUiData = with(LocalContext.current) {
addOns = with(LocalContext.current) {
listOf(
addOnItemUblock(this),
addOnItemPrivacyBadger(this),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package org.mozilla.fenix.onboarding.view

import org.mozilla.fenix.nimbus.AddOnData
import org.mozilla.fenix.nimbus.OnboardingCardData
import org.mozilla.fenix.nimbus.OnboardingCardType

Expand Down Expand Up @@ -48,6 +49,8 @@ private fun OnboardingCardData.isCardEnabled(
enabled && showAddWidgetPage
}

OnboardingCardType.ADD_ONS -> addOnsData.isNotEmpty()

else -> {
enabled
}
Expand Down Expand Up @@ -108,6 +111,11 @@ private fun OnboardingCardData.toPageUiData(privacyCaption: Caption?) = Onboardi
primaryButtonLabel = primaryButtonLabel,
secondaryButtonLabel = secondaryButtonLabel,
privacyCaption = privacyCaption,
addOns = if (addOnsData.isEmpty()) {
null
} else {
addOnsData.toOnboardingAddOns()
},
)

private fun OnboardingCardType.toPageUiDataType() = when (this) {
Expand All @@ -118,6 +126,18 @@ private fun OnboardingCardType.toPageUiDataType() = when (this) {
OnboardingCardType.ADD_ONS -> OnboardingPageUiData.Type.ADD_ONS
}

private fun List<AddOnData>.toOnboardingAddOns() = map { it.toOnboardingAddOn() }

private fun AddOnData.toOnboardingAddOn() = with(this) {
OnboardingAddOn(
iconRes = iconRes.resourceId,
name = name,
description = description,
averageRating = averageRating,
reviewCount = reviewCount,
)
}

/**
* Mapper to convert [OnboardingPageUiData] to [OnboardingPageState] that is a param for
* [OnboardingPage] composable.
Expand All @@ -133,7 +153,7 @@ internal fun mapToOnboardingPageState(
onNotificationPermissionSkipClick: () -> Unit,
onAddFirefoxWidgetClick: () -> Unit,
onAddFirefoxWidgetSkipClick: () -> Unit,
onAddOnsButtonClick: () -> Unit = {},
onAddOnsButtonClick: () -> Unit,
): OnboardingPageState = when (onboardingPageUiData.type) {
OnboardingPageUiData.Type.DEFAULT_BROWSER -> createOnboardingPageState(
onboardingPageUiData = onboardingPageUiData,
Expand Down Expand Up @@ -162,7 +182,7 @@ internal fun mapToOnboardingPageState(
OnboardingPageUiData.Type.ADD_ONS -> createOnboardingPageState(
onboardingPageUiData = onboardingPageUiData,
onPositiveButtonClick = onAddOnsButtonClick,
onNegativeButtonClick = {},
onNegativeButtonClick = {}, // No negative button option for add-ons.
)
}

Expand All @@ -175,6 +195,9 @@ private fun createOnboardingPageState(
title = onboardingPageUiData.title,
description = onboardingPageUiData.description,
primaryButton = Action(onboardingPageUiData.primaryButtonLabel, onPositiveButtonClick),
secondaryButton = Action(onboardingPageUiData.secondaryButtonLabel, onNegativeButtonClick),
secondaryButton = onboardingPageUiData.secondaryButtonLabel?.let {
Action(it, onNegativeButtonClick)
},
privacyCaption = onboardingPageUiData.privacyCaption,
addOns = onboardingPageUiData.addOns,
)
Original file line number Diff line number Diff line change
Expand Up @@ -10,40 +10,23 @@ import org.mozilla.fenix.compose.LinkTextState
/**
* Model containing data for [OnboardingPage].
*
* @property imageRes [DrawableRes] displayed on the page.
* @property title [String] title of the page.
* @property description [String] description of the page.
* @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.
*/
data class OnboardingPageState(
@DrawableRes val imageRes: Int,
val title: String,
val description: String,
val privacyCaption: Caption? = null,
val primaryButton: Action,
val secondaryButton: Action? = null,
val onRecordImpressionEvent: () -> Unit = {},
)

/**
* Model containing data for [AddOnsOnboardingPage].
*
* @property imageRes Image resource for the main image to be displayed on the page.
* @property imageRes The main image to be displayed on the page.
* @property title Title of the page.
* @property description Description of the page.
* @property privacyCaption Optional privacy caption to show and allow user to view the privacy policy.
* @property primaryButton [Action] for the primary button.
* @property addOnsUiData List of add-ons to install during onboarding.
* @property secondaryButton Optional [Action] for the secondary button.
* @property addOns Optional list of add-ons to install during onboarding.
* @property onRecordImpressionEvent Callback for recording impression event.
*/
data class OnboardingAddOnsPageState(
data class OnboardingPageState(
@DrawableRes val imageRes: Int,
val title: String,
val description: String,
val privacyCaption: Caption? = null,
val primaryButton: Action,
val addOnsUiData: List<OnboardingAddOn>,
val secondaryButton: Action? = null,
val addOns: List<OnboardingAddOn>? = null,
val onRecordImpressionEvent: () -> Unit = {},
)

Expand Down Expand Up @@ -71,5 +54,5 @@ data class OnboardingAddOn(
val name: String,
val description: String,
val averageRating: String,
val numberOfReviews: String,
val reviewCount: String,
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ data class OnboardingPageUiData(
val title: String,
val description: String,
val primaryButtonLabel: String,
val secondaryButtonLabel: String,
val privacyCaption: Caption?,
val secondaryButtonLabel: String? = null,
val privacyCaption: Caption? = null,
val addOns: List<OnboardingAddOn>? = null,
) {
/**
* Model for different types of Onboarding Pages.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import org.mozilla.fenix.theme.FirefoxTheme
* @param onSkipNotificationClick Invoked when negative button on notification page is clicked.
* @param onAddFirefoxWidgetClick Invoked when positive button on add search widget page is clicked.
* @param onSkipFirefoxWidgetClick Invoked when negative button on add search widget page is clicked.
* @param onAddOnsButtonClick Invoked when the primary button on add-ons page is clicked.
* @param onFinish Invoked when the onboarding is completed.
* @param onImpression Invoked when a page in the pager is displayed.
*/
Expand All @@ -72,6 +73,7 @@ fun OnboardingScreen(
onSkipNotificationClick: () -> Unit,
onAddFirefoxWidgetClick: () -> Unit,
onSkipFirefoxWidgetClick: () -> Unit,
onAddOnsButtonClick: () -> Unit,
onFinish: (pageType: OnboardingPageUiData) -> Unit,
onImpression: (pageType: OnboardingPageUiData) -> Unit,
) {
Expand Down Expand Up @@ -151,6 +153,10 @@ fun OnboardingScreen(
scrollToNextPageOrDismiss()
onSkipFirefoxWidgetClick()
},
onAddOnsButtonClick = {
scrollToNextPageOrDismiss()
onAddOnsButtonClick()
},
)
}

Expand All @@ -167,6 +173,7 @@ private fun OnboardingContent(
onNotificationPermissionSkipClick: () -> Unit,
onAddFirefoxWidgetClick: () -> Unit,
onSkipFirefoxWidgetClick: () -> Unit,
onAddOnsButtonClick: () -> Unit,
) {
val nestedScrollConnection = remember { DisableForwardSwipeNestedScrollConnection(pagerState) }

Expand Down Expand Up @@ -194,8 +201,9 @@ private fun OnboardingContent(
onNotificationPermissionSkipClick = onNotificationPermissionSkipClick,
onAddFirefoxWidgetClick = onAddFirefoxWidgetClick,
onAddFirefoxWidgetSkipClick = onSkipFirefoxWidgetClick,
onAddOnsButtonClick = onAddOnsButtonClick,
)
OnboardingPage(pageState = onboardingPageState)
OnboardingPageForType(pageUiState.type, onboardingPageState)
}

PagerIndicator(
Expand All @@ -210,6 +218,19 @@ private fun OnboardingContent(
}
}

@Composable
private fun OnboardingPageForType(type: OnboardingPageUiData.Type, state: OnboardingPageState) {
when (type) {
OnboardingPageUiData.Type.DEFAULT_BROWSER,
OnboardingPageUiData.Type.SYNC_SIGN_IN,
OnboardingPageUiData.Type.ADD_SEARCH_WIDGET,
OnboardingPageUiData.Type.NOTIFICATION_PERMISSION,
-> OnboardingPage(state)

OnboardingPageUiData.Type.ADD_ONS -> AddOnsOnboardingPage(state)
}
}

private class DisableForwardSwipeNestedScrollConnection(
private val pagerState: PagerState,
) : NestedScrollConnection {
Expand Down Expand Up @@ -249,6 +270,7 @@ private fun OnboardingScreenPreview() {
onNotificationPermissionSkipClick = {},
onAddFirefoxWidgetClick = {},
onSkipFirefoxWidgetClick = {},
onAddOnsButtonClick = {},
)
}
}
Expand Down
Loading

0 comments on commit b635460

Please sign in to comment.