From 795683c6ef390342bb294b35fe063f975c089861 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 21 Oct 2021 11:55:47 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/21854 - Split the big ComposeView in 3 smaller ones This would shorten the time needed to layout all Pocket recommended stories content in one go, though it may lead to shorten hiccups over a bigger period of time. --- .../home/pocket/PocketCategoriesViewHolder.kt | 111 +++++++++++++ .../PocketRecommendationsHeaderViewHolder.kt | 65 ++++++++ .../home/pocket/PocketStoriesComposables.kt | 18 +-- .../home/pocket/PocketStoriesController.kt | 12 +- .../home/pocket/PocketStoriesViewHolder.kt | 147 +++++++----------- .../sessioncontrol/SessionControlAdapter.kt | 22 ++- .../home/sessioncontrol/SessionControlView.kt | 2 + .../sessioncontrol/SessionControlViewTest.kt | 4 +- 8 files changed, 277 insertions(+), 104 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt create mode 100644 app/src/main/java/org/mozilla/fenix/home/pocket/PocketRecommendationsHeaderViewHolder.kt diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt new file mode 100644 index 000000000..4fef8f2fc --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt @@ -0,0 +1,111 @@ +/* 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.home.pocket + +import android.view.View +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.wrapContentHeight +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import androidx.lifecycle.LifecycleOwner +import androidx.recyclerview.widget.RecyclerView +import mozilla.components.lib.state.ext.observeAsComposableState +import org.mozilla.fenix.R +import org.mozilla.fenix.compose.ComposeViewHolder +import org.mozilla.fenix.compose.SectionHeader +import org.mozilla.fenix.home.HomeFragmentStore +import org.mozilla.fenix.theme.FirefoxTheme + +internal const val POCKET_CATEGORIES_SELECTED_AT_A_TIME_COUNT = 8 + +/** + * [RecyclerView.ViewHolder] for displaying the list of [PocketRecommendedStoriesCategory]s from [HomeFragmentStore]. + * + * @param composeView [ComposeView] which will be populated with Jetpack Compose UI content. + * @param viewLifecycleOwner [LifecycleOwner] to which this Composable will be tied to. + * @param store [HomeFragmentStore] containing the list of Pocket stories categories to be displayed. + * @param interactor [PocketStoriesInteractor] callback for user interaction. + */ +class PocketCategoriesViewHolder( + composeView: ComposeView, + viewLifecycleOwner: LifecycleOwner, + private val store: HomeFragmentStore, + private val interactor: PocketStoriesInteractor +) : ComposeViewHolder(composeView, viewLifecycleOwner) { + + @Composable + override fun Content() { + val horizontalPadding = + composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) + composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) + + val categories = store + .observeAsComposableState { state -> state.pocketStoriesCategories }.value + val categoriesSelections = store + .observeAsComposableState { state -> state.pocketStoriesCategoriesSelections }.value + + Column { + Spacer(Modifier.height(24.dp)) + + PocketTopics( + categories = categories ?: emptyList(), + categoriesSelections = categoriesSelections ?: emptyList(), + onCategoryClick = interactor::onCategoryClicked + ) + } + } + + companion object { + val LAYOUT_ID = View.generateViewId() + } +} + +@Composable +private fun PocketTopics( + categories: List = emptyList(), + categoriesSelections: List = emptyList(), + onCategoryClick: (PocketRecommendedStoriesCategory) -> Unit +) { + Column { + SectionHeader( + text = stringResource(R.string.pocket_stories_categories_header), + modifier = Modifier + .fillMaxWidth() + .wrapContentHeight(align = Alignment.Top) + ) + + Spacer(Modifier.height(17.dp)) + + PocketStoriesCategories( + categories = categories, + selections = categoriesSelections, + onCategoryClick = onCategoryClick, + modifier = Modifier + .fillMaxWidth() + ) + } +} + +@Composable +@Preview +private fun PocketCategoriesViewHolderPreview() { + FirefoxTheme { + PocketTopics( + categories = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor" + .split(" ") + .map { PocketRecommendedStoriesCategory(it) }, + categoriesSelections = emptyList(), + onCategoryClick = {} + ) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketRecommendationsHeaderViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketRecommendationsHeaderViewHolder.kt new file mode 100644 index 000000000..0a9faf5af --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketRecommendationsHeaderViewHolder.kt @@ -0,0 +1,65 @@ +/* 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/. */ + +@file:Suppress("MagicNumber") + +package org.mozilla.fenix.home.pocket + +import android.view.View +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import androidx.lifecycle.LifecycleOwner +import androidx.recyclerview.widget.RecyclerView +import org.mozilla.fenix.R +import org.mozilla.fenix.compose.ComposeViewHolder +import org.mozilla.fenix.theme.FirefoxTheme + +/** + * [RecyclerView.ViewHolder] for displaying the Pocket feature header. + * + * @param composeView [ComposeView] which will be populated with Jetpack Compose UI content. + * @param viewLifecycleOwner [LifecycleOwner] to which this Composable will be tied to. + * @param interactor [PocketStoriesInteractor] callback for user interaction. + */ +class PocketRecommendationsHeaderViewHolder( + composeView: ComposeView, + viewLifecycleOwner: LifecycleOwner, + private val interactor: PocketStoriesInteractor +) : ComposeViewHolder(composeView, viewLifecycleOwner) { + + @Composable + override fun Content() { + val horizontalPadding = + composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) + composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) + + Column { + Spacer(Modifier.height(24.dp)) + + PoweredByPocketHeader( + interactor::onLearnMoreClicked, + modifier = Modifier.fillMaxWidth() + ) + } + } + + companion object { + val LAYOUT_ID = View.generateViewId() + } +} + +@Composable +@Preview +fun PocketRecommendationsFooterViewHolderPreview() { + FirefoxTheme { + PoweredByPocketHeader({}) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt index c4e6853ed..ff930a51d 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt @@ -211,7 +211,7 @@ fun PoweredByPocketHeader( Row( Modifier .fillMaxWidth() - .semantics(mergeDescendants = true) { }, + .semantics(mergeDescendants = true) {}, verticalAlignment = Alignment.CenterVertically ) { Icon( @@ -249,20 +249,20 @@ private fun PocketStoriesComposablesPreview() { stories = getFakePocketStories(8), contentPadding = 0.dp, onStoryClicked = { _, _ -> }, - onDiscoverMoreClicked = { } + onDiscoverMoreClicked = {} ) Spacer(Modifier.height(10.dp)) PocketStoriesCategories( - "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor".split(" ").map { - PocketRecommendedStoriesCategory(it) - }, - emptyList(), - { } + categories = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor" + .split(" ") + .map { PocketRecommendedStoriesCategory(it) }, + selections = emptyList(), + onCategoryClick = {} ) Spacer(Modifier.height(10.dp)) - PoweredByPocketHeader({ }) + PoweredByPocketHeader({}) } } } @@ -273,7 +273,7 @@ private class PocketStoryProvider : PreviewParameterProvider { +internal fun getFakePocketStories(limit: Int = 1): List { return mutableListOf().apply { for (index in 0 until limit) { val randomNumber = Random.nextInt(0, 10) diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt index 065ae1b21..803b22adc 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesController.kt @@ -115,10 +115,18 @@ internal class DefaultPocketStoriesController( ) } - override fun handleStoryClicked(storyClicked: PocketRecommendedStory, storyPosition: Pair) { + override fun handleStoryClicked( + storyClicked: PocketRecommendedStory, + storyPosition: Pair + ) { dismissSearchDialogIfDisplayed() homeActivity.openToBrowserAndLoad(storyClicked.url, true, BrowserDirection.FromHome) - metrics.track(Event.PocketHomeRecsStoryClicked(storyClicked.timesShown.inc(), storyPosition)) + metrics.track( + Event.PocketHomeRecsStoryClicked( + storyClicked.timesShown.inc(), + storyPosition + ) + ) } override fun handleLearnMoreClicked(link: String) { diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt index 1587f3849..d4d7a72a3 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.home.pocket import android.view.View -import androidx.annotation.Dimension import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth @@ -17,33 +16,36 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.res.dimensionResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.lifecycle.LifecycleOwner import androidx.recyclerview.widget.RecyclerView import mozilla.components.lib.state.ext.observeAsComposableState import mozilla.components.service.pocket.PocketRecommendedStory import org.mozilla.fenix.R +import org.mozilla.fenix.R.string import org.mozilla.fenix.compose.ComposeViewHolder import org.mozilla.fenix.compose.SectionHeader import org.mozilla.fenix.home.HomeFragmentStore +import org.mozilla.fenix.theme.FirefoxTheme internal const val POCKET_STORIES_TO_SHOW_COUNT = 8 -internal const val POCKET_CATEGORIES_SELECTED_AT_A_TIME_COUNT = 8 /** - * [RecyclerView.ViewHolder] that will display a list of [PocketRecommendedStory]es - * which is to be provided in the [bind] method. + * [RecyclerView.ViewHolder] for displaying the list of [PocketRecommendedStory]s from [HomeFragmentStore]. * * @param composeView [ComposeView] which will be populated with Jetpack Compose UI content. + * @param viewLifecycleOwner [LifecycleOwner] to which this Composable will be tied to. * @param store [HomeFragmentStore] containing the list of Pocket stories to be displayed. * @param interactor [PocketStoriesInteractor] callback for user interaction. */ class PocketStoriesViewHolder( composeView: ComposeView, viewLifecycleOwner: LifecycleOwner, - val store: HomeFragmentStore, - val interactor: PocketStoriesInteractor + private val store: HomeFragmentStore, + private val interactor: PocketStoriesInteractor ) : ComposeViewHolder(composeView, viewLifecycleOwner) { companion object { @@ -52,94 +54,61 @@ class PocketStoriesViewHolder( @Composable override fun Content() { - PocketStories( - store, - interactor::onStoriesShown, - interactor::onStoryClicked, - interactor::onCategoryClicked, - interactor::onDiscoverMoreClicked, - interactor::onLearnMoreClicked, - with(composeView.resources) { - getDimensionPixelSize(R.dimen.home_item_horizontal_margin) / displayMetrics.density - } - ) - } -} - -@Composable -@Suppress("LongParameterList") -fun PocketStories( - store: HomeFragmentStore, - onStoriesShown: (List) -> Unit, - onStoryClicked: (PocketRecommendedStory, Pair) -> Unit, - onCategoryClicked: (PocketRecommendedStoriesCategory) -> Unit, - onDiscoverMoreClicked: (String) -> Unit, - onLearnMoreClicked: (String) -> Unit, - @Dimension horizontalPadding: Float = 0f -) { - val stories = store - .observeAsComposableState { state -> state.pocketStories }.value + val horizontalPadding = dimensionResource(R.dimen.home_item_horizontal_margin) - val categories = store - .observeAsComposableState { state -> state.pocketStoriesCategories }.value + val stories = store + .observeAsComposableState { state -> state.pocketStories }.value - val categoriesSelections = store - .observeAsComposableState { state -> state.pocketStoriesCategoriesSelections }.value + LaunchedEffect(stories) { + // We should report back when a certain story is actually being displayed. + // Cannot do it reliably so for now we'll just mass report everything as being displayed. + stories?.let { + interactor::onStoriesShown + } + } - LaunchedEffect(stories) { - // We should report back when a certain story is actually being displayed. - // Cannot do it reliably so for now we'll just mass report everything as being displayed. - stories?.let { - onStoriesShown(it) + Column(modifier = Modifier.padding(top = 72.dp)) { + SectionHeader( + text = stringResource(R.string.pocket_stories_header_1), + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = horizontalPadding) + .wrapContentHeight(align = Alignment.Top) + ) + + Spacer(Modifier.height(17.dp)) + + PocketStories( + stories ?: emptyList(), + horizontalPadding, + interactor::onStoryClicked, + interactor::onDiscoverMoreClicked + ) } } +} - Column(modifier = Modifier.padding(top = 72.dp)) { - SectionHeader( - text = stringResource(R.string.pocket_stories_header_1), - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = horizontalPadding.dp) - .wrapContentHeight(align = Alignment.Top) - ) - - Spacer(Modifier.height(17.dp)) - - PocketStories( - stories ?: emptyList(), - horizontalPadding.dp, - onStoryClicked, - onDiscoverMoreClicked - ) - - Spacer(Modifier.height(24.dp)) - - SectionHeader( - text = stringResource(R.string.pocket_stories_categories_header), - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = horizontalPadding.dp) - .wrapContentHeight(align = Alignment.Top) - ) - - Spacer(Modifier.height(17.dp)) - - PocketStoriesCategories( - categories = categories ?: emptyList(), - selections = categoriesSelections ?: emptyList(), - onCategoryClick = onCategoryClicked, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = horizontalPadding.dp) - ) - - Spacer(Modifier.height(24.dp)) - - PoweredByPocketHeader( - onLearnMoreClicked, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = horizontalPadding.dp) - ) +@Composable +@Preview +fun PocketStoriesViewHolderPreview() { + FirefoxTheme { + Column { + SectionHeader( + text = stringResource(string.pocket_stories_header_1), + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp) + .wrapContentHeight(align = Alignment.Top) + ) + + Spacer(Modifier.height(17.dp)) + + PocketStories( + stories = getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT), + contentPadding = 0.dp, + onStoryClicked = { _, _ -> }, + onDiscoverMoreClicked = {} + ) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt index 7445945f0..46f81221d 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt @@ -23,6 +23,8 @@ import org.mozilla.fenix.home.HomeFragmentStore import org.mozilla.fenix.home.TopPlaceholderViewHolder import org.mozilla.fenix.home.pocket.PocketStoriesViewHolder import org.mozilla.fenix.home.recentbookmarks.view.RecentBookmarksHeaderViewHolder +import org.mozilla.fenix.home.pocket.PocketCategoriesViewHolder +import org.mozilla.fenix.home.pocket.PocketRecommendationsHeaderViewHolder import org.mozilla.fenix.home.recentbookmarks.view.RecentBookmarksViewHolder import org.mozilla.fenix.home.recenttabs.view.RecentTabViewHolder import org.mozilla.fenix.home.recenttabs.view.RecentTabsHeaderViewHolder @@ -165,8 +167,9 @@ sealed class AdapterItem(@LayoutRes val viewType: Int) { object RecentBookmarksHeader : AdapterItem(RecentBookmarksHeaderViewHolder.LAYOUT_ID) object RecentBookmarks : AdapterItem(RecentBookmarksViewHolder.LAYOUT_ID) - object PocketStoriesItem : - AdapterItem(PocketStoriesViewHolder.LAYOUT_ID) + object PocketStoriesItem : AdapterItem(PocketStoriesViewHolder.LAYOUT_ID) + object PocketCategoriesItem : AdapterItem(PocketCategoriesViewHolder.LAYOUT_ID) + object PocketRecommendationsFooterItem : AdapterItem(PocketRecommendationsHeaderViewHolder.LAYOUT_ID) object BottomSpacer : AdapterItem(BottomSpacerViewHolder.LAYOUT_ID) @@ -215,10 +218,21 @@ class SessionControlAdapter( ) PocketStoriesViewHolder.LAYOUT_ID -> return PocketStoriesViewHolder( composeView = ComposeView(parent.context), - viewLifecycleOwner, + viewLifecycleOwner = viewLifecycleOwner, + store = store, + interactor = interactor + ) + PocketCategoriesViewHolder.LAYOUT_ID -> return PocketCategoriesViewHolder( + composeView = ComposeView(parent.context), + viewLifecycleOwner = viewLifecycleOwner, store = store, interactor = interactor ) + PocketRecommendationsHeaderViewHolder.LAYOUT_ID -> return PocketRecommendationsHeaderViewHolder( + composeView = ComposeView(parent.context), + viewLifecycleOwner = viewLifecycleOwner, + interactor = interactor + ) RecentBookmarksViewHolder.LAYOUT_ID -> return RecentBookmarksViewHolder( composeView = ComposeView(parent.context), viewLifecycleOwner, @@ -296,6 +310,8 @@ class SessionControlAdapter( is RecentlyVisitedViewHolder, is RecentBookmarksViewHolder, is RecentTabViewHolder, + is PocketCategoriesViewHolder, + is PocketRecommendationsHeaderViewHolder, is PocketStoriesViewHolder -> { // no op // This previously called "composeView.disposeComposition" which would have the 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 59020b097..7517d0d29 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 @@ -87,6 +87,8 @@ internal fun normalModeAdapterItems( if (pocketStories.isNotEmpty()) { shouldShowCustomizeHome = true items.add(AdapterItem.PocketStoriesItem) + items.add(AdapterItem.PocketCategoriesItem) + items.add(AdapterItem.PocketRecommendationsFooterItem) } if (shouldShowCustomizeHome) { diff --git a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt index a9f214f43..a671644d5 100644 --- a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt @@ -243,7 +243,9 @@ class SessionControlViewTest { assertTrue(results[0] is AdapterItem.TopPlaceholderItem) assertTrue(results[1] is AdapterItem.PocketStoriesItem) - assertTrue(results[2] is AdapterItem.CustomizeHomeButton) + assertTrue(results[2] is AdapterItem.PocketCategoriesItem) + assertTrue(results[3] is AdapterItem.PocketRecommendationsFooterItem) + assertTrue(results[4] is AdapterItem.CustomizeHomeButton) } @Test