Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Closes #26750: add Wallpapers Onboarding dialog #26758

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class FeatureSettingsHelper {
private var isRecentlyVisitedFeatureEnabled: Boolean = settings.historyMetadataUIFeature
private var isUserKnowsAboutPwasTrue: Boolean = settings.userKnowsAboutPwas
private var isTCPCFREnabled: Boolean = settings.shouldShowTotalCookieProtectionCFR
private var isWallpaperOnboardingEnabled: Boolean = settings.showWallpaperOnboarding

fun setPocketEnabled(enabled: Boolean) {
settings.showPocketRecommendationsFeature = enabled
Expand All @@ -28,6 +29,10 @@ class FeatureSettingsHelper {
settings.shouldShowJumpBackInCFR = enabled
}

fun setShowWallpaperOnboarding(enabled: Boolean) {
settings.showWallpaperOnboarding = enabled
}

fun setRecentTabsFeatureEnabled(enabled: Boolean) {
settings.showRecentTabsFeature = enabled
}
Expand Down Expand Up @@ -65,5 +70,6 @@ class FeatureSettingsHelper {
settings.historyMetadataUIFeature = isRecentlyVisitedFeatureEnabled
settings.userKnowsAboutPwas = isUserKnowsAboutPwasTrue
settings.shouldShowTotalCookieProtectionCFR = isTCPCFREnabled
settings.showWallpaperOnboarding = isWallpaperOnboardingEnabled
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class CrashReportingTest {
fun setUp() {
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setPocketEnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)

mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
mockWebServer = MockWebServer().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class HomeScreenTest {
@Test
fun dismissOnboardingUsingHelpTest() {
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)

homeScreen {
verifyWelcomeHeader()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class NoNetworkAccessStartupTests {
@Before
fun setUp() {
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class SearchTest {
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setPocketEnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class SettingsBasicsTest {

featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class SettingsHomepageTest {
}
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class SettingsPrivacyTest {

featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
featureSettingsHelper.disablePwaCFR(true)

if (Build.VERSION.SDK_INT == Build.VERSION_CODES.R) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class SettingsSearchTest {
start()
}
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class SmokeTest {
// disabling the new homepage pop-up that interferes with the tests.
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)

mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
mockWebServer = MockWebServer().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class StrictEnhancedTrackingProtectionTest {
featureSettingsHelper.setStrictETPEnabled()
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class TabbedBrowsingTest {
// disabling the new homepage pop-up that interferes with the tests.
featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)

mDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
mockWebServer = MockWebServer().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class TopSitesTest {

featureSettingsHelper.setJumpBackCFREnabled(false)
featureSettingsHelper.setTCPCFREnabled(false)
featureSettingsHelper.setShowWallpaperOnboarding(false)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ import org.mozilla.fenix.gleanplumb.MessageController
import org.mozilla.fenix.home.HomeFragment
import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.home.Mode
import org.mozilla.fenix.onboarding.WallpaperOnboardingDialogFragment
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.settings.SupportUtils.SumoTopic.PRIVATE_BROWSING_MYTHS
import org.mozilla.fenix.utils.Settings
import org.mozilla.fenix.wallpapers.Wallpaper
import org.mozilla.fenix.wallpapers.WallpaperState
import mozilla.components.feature.tab.collections.Tab as ComponentTab

/**
Expand Down Expand Up @@ -197,6 +200,11 @@ interface SessionControlController {
*/
fun handleCustomizeHomeTapped()

/**
* @see [OnboardingInteractor.showWallpapersOnboardingDialog]
*/
fun handleShowWallpapersOnboardingDialog(state: WallpaperState): Boolean

/**
* @see [SessionControlInteractor.reportSessionMetrics]
*/
Expand Down Expand Up @@ -501,6 +509,19 @@ class DefaultSessionControlController(
HomeScreen.customizeHomeClicked.record(NoExtras())
}

override fun handleShowWallpapersOnboardingDialog(state: WallpaperState): Boolean {
if (state.availableWallpapers.all { it.thumbnailFileState == Wallpaper.ImageFileState.Downloaded } &&
state.availableWallpapers.size >= WallpaperOnboardingDialogFragment.THUMBNAILS_COUNT
) {
navController.nav(
R.id.homeFragment,
HomeFragmentDirections.actionGlobalWallpaperOnboardingDialog(),
)
return true
}
return false
}

override fun handleReadPrivacyNoticeClicked() {
activity.openToBrowserAndLoad(
searchTermOrURL = SupportUtils.getMozillaPageUrl(SupportUtils.MozillaPage.PRIVATE_NOTICE),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGrou
import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight
import org.mozilla.fenix.home.recentvisits.controller.RecentVisitsController
import org.mozilla.fenix.home.recentvisits.interactor.RecentVisitsInteractor
import org.mozilla.fenix.wallpapers.WallpaperState

/**
* Interface for tab related actions in the [SessionControlInteractor].
Expand Down Expand Up @@ -162,6 +163,15 @@ interface OnboardingInteractor {
* Opens a custom tab to privacy notice url. Called when a user clicks on the "read our privacy notice" button.
*/
fun onReadPrivacyNoticeClicked()

/**
* Show Wallpapers onboarding dialog to onboard users about the feature if conditions are met.
* Returns true if the call has been passed down to the controller.
*
* @param state The wallpaper state.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add a new line above to separate the description and @param

* @return Whether the onboarding dialog is currently shown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: a period at the end of this comment for consistency

*/
fun showWallpapersOnboardingDialog(state: WallpaperState): Boolean
}

interface CustomizeHomeIteractor {
Expand Down Expand Up @@ -322,6 +332,10 @@ class SessionControlInteractor(
controller.handleReadPrivacyNoticeClicked()
}

override fun showWallpapersOnboardingDialog(state: WallpaperState): Boolean {
return controller.handleShowWallpapersOnboardingDialog(state)
}

override fun onToggleCollectionExpanded(collection: TabCollection, expand: Boolean) {
controller.handleToggleCollectionExpanded(collection, expand)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,28 +199,43 @@ class SessionControlView(

val view: RecyclerView = containerView as RecyclerView

// We want to limit feature recommendations to one per HomePage visit.
var featureRecommended = false

private val sessionControlAdapter = SessionControlAdapter(
interactor,
viewLifecycleOwner,
containerView.context.components,
)

init {
@Suppress("NestedBlockDepth")
view.apply {
adapter = sessionControlAdapter
layoutManager = object : LinearLayoutManager(containerView.context) {
override fun onLayoutCompleted(state: RecyclerView.State?) {
super.onLayoutCompleted(state)

if (!context.settings().showHomeOnboardingDialog && (
context.settings().showSyncCFR ||
context.settings().shouldShowJumpBackInCFR
)
) {
HomeCFRPresenter(
context = context,
recyclerView = view,
).show()
if (!featureRecommended && !context.settings().showHomeOnboardingDialog) {
Copy link
Member

@gabrielluong gabrielluong Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't featureRecommended always be false anyways when this class is created? I think we can avoid this check since it's always true and simply declare featureRecommended inside the if block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it will be. but onLayoutCompleted is being called multiple times during one lifecycle, in cases like adding items, keyboard, orientation change, and it would launch multiple CFR during one homescreen lifecycle if we don't keep track of CFRs being actually displayed during a single fragment lifecycle

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in general there is a future opportunity to solidify our work around when different onboarding items are shown. That said, I think this solution would work for now given the details @mavduevskiy has listed and our time constraints.

if (!context.settings().showHomeOnboardingDialog && (
context.settings().showSyncCFR ||
context.settings().shouldShowJumpBackInCFR
)
) {
featureRecommended = HomeCFRPresenter(
context = context,
recyclerView = view,
).show()
}

if (!context.settings().shouldShowJumpBackInCFR &&
context.settings().showWallpaperOnboarding &&
!featureRecommended
) {
featureRecommended = interactor.showWallpapersOnboardingDialog(
context.components.appStore.state.wallpaperState,
)
}
}

// We want some parts of the home screen UI to be rendered first if they are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,17 @@ class HomeCFRPresenter(
* Determine the CFR to be shown on the Home screen and show a CFR for the resultant view
* if any.
*/
fun show() {
when (val result = getCFRToShow()) {
is Result.SyncedTab -> showSyncedTabCFR(view = result.view)
is Result.JumpBackIn -> showJumpBackInCFR(view = result.view)
else -> {
// no-op
fun show(): Boolean {
return when (val result = getCFRToShow()) {
is Result.SyncedTab -> {
showSyncedTabCFR(view = result.view)
true
}
is Result.JumpBackIn -> {
showJumpBackInCFR(view = result.view)
true
}
else -> false
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* 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.annotation.SuppressLint
import android.content.pm.ActivityInfo
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.compose.material.ExperimentalMaterialApi
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.ViewCompositionStrategy
import androidx.navigation.fragment.findNavController
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import kotlinx.coroutines.launch
import mozilla.components.lib.state.ext.observeAsComposableState
import org.mozilla.fenix.NavGraphDirections
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.theme.FirefoxTheme
import org.mozilla.fenix.wallpapers.Wallpaper
import org.mozilla.fenix.wallpapers.WallpaperOnboarding

/**
* Dialog displaying the wallpapers onboarding.
*/
@OptIn(ExperimentalMaterialApi::class)
class WallpaperOnboardingDialogFragment : BottomSheetDialogFragment() {
private val appStore by lazy {
requireComponents.appStore
}

private val wallpaperUseCases by lazy {
requireComponents.useCases.wallpaperUseCases
}

@SuppressLint("SourceLockedOrientationActivity")
MatthewTighe marked this conversation as resolved.
Show resolved Hide resolved
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setStyle(STYLE_NO_TITLE, R.style.WallpaperOnboardingDialogStyle)
activity?.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_PORTRAIT
}

override fun onDestroy() {
super.onDestroy()
activity?.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED
MatthewTighe marked this conversation as resolved.
Show resolved Hide resolved
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
requireContext().settings().showWallpaperOnboarding = false
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
): View = ComposeView(requireContext()).apply {
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)

setContent {
FirefoxTheme {
val wallpapers = appStore.observeAsComposableState { state ->
state.wallpaperState.availableWallpapers.subList(0, THUMBNAILS_COUNT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: .take() feels slightly more idiomatic than .subList() to me, but I would also be okay with this for now

}.value ?: listOf()
val currentWallpaper = appStore.observeAsComposableState { state ->
state.wallpaperState.currentWallpaper
}.value ?: Wallpaper.Default

val coroutineScope = rememberCoroutineScope()

WallpaperOnboarding(
wallpapers = wallpapers,
currentWallpaper = currentWallpaper,
onCloseClicked = { dismiss() },
onBottomButtonClicked = {
val directions = NavGraphDirections.actionGlobalWallpaperSettingsFragment()
findNavController().navigate(directions)
},
loadWallpaperResource = { wallpaperUseCases.loadThumbnail(it) },
onSelectWallpaper = {
coroutineScope.launch { wallpaperUseCases.selectWallpaper(it) }
},
)
}
}
}

companion object {
const val THUMBNAILS_COUNT = 6
}
}
Loading