For #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.
2024-11-03 23:15:31 +00:00 · 2021-10-21 11:55:47 +03:00 · 2021-10-21 11:55:47 +03:00 · 5bd6f1cb8c
commit 5bd6f1cb8c
parent 9b7e07ab0e
8 changed files with 275 additions and 102 deletions
--- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketCategoriesViewHolder.kt
+++ 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<PocketRecommendedStoriesCategory> = emptyList(),
+    categoriesSelections: List<PocketRecommendedStoriesSelectedCategory> = 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 = {}
+        )
+    }
+}
--- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketRecommendationsHeaderViewHolder.kt
+++ 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({})
+    }
+}
--- 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<PocketRecommendedSt
    override val count = 8
 }

-private fun getFakePocketStories(limit: Int = 1): List<PocketRecommendedStory> {
+internal fun getFakePocketStories(limit: Int = 1): List<PocketRecommendedStory> {
    return mutableListOf<PocketRecommendedStory>().apply {
        for (index in 0 until limit) {
            val randomNumber = Random.nextInt(0, 10)
--- 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<Int, Int>) {
+    override fun handleStoryClicked(
+        storyClicked: PocketRecommendedStory,
+        storyPosition: Pair<Int, Int>
+    ) {
        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) {
--- 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
+        val horizontalPadding = dimensionResource(R.dimen.home_item_horizontal_margin)
+
+        val stories = store
+            .observeAsComposableState { state -> state.pocketStories }.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
            }
-        )
+        }
+
+        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
+            )
+        }
    }
 }

@Composable
-@Suppress("LongParameterList")
-fun PocketStories(
-    store: HomeFragmentStore,
-    onStoriesShown: (List<PocketRecommendedStory>) -> Unit,
-    onStoryClicked: (PocketRecommendedStory, Pair<Int, Int>) -> Unit,
-    onCategoryClicked: (PocketRecommendedStoriesCategory) -> Unit,
-    onDiscoverMoreClicked: (String) -> Unit,
-    onLearnMoreClicked: (String) -> Unit,
-    @Dimension horizontalPadding: Float = 0f
-) {
-    val stories = store
-        .observeAsComposableState { state -> state.pocketStories }.value
+@Preview
+fun PocketStoriesViewHolderPreview() {
+    FirefoxTheme {
+        Column {
+            SectionHeader(
+                text = stringResource(string.pocket_stories_header_1),
+                modifier = Modifier
+                    .fillMaxWidth()
+                    .padding(horizontal = 16.dp)
+                    .wrapContentHeight(align = Alignment.Top)
+            )

-    val categories = store
-        .observeAsComposableState { state -> state.pocketStoriesCategories }.value
+            Spacer(Modifier.height(17.dp))

-    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 {
-            onStoriesShown(it)
+            PocketStories(
+                stories = getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT),
+                contentPadding = 0.dp,
+                onStoryClicked = { _, _ -> },
+                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)
-        )
-    }
 }
--- 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
--- 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) {
--- 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