From 08cea484a5897a7fc6bee118d272eb09e3707759 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Tue, 25 Jul 2023 22:35:05 +0100 Subject: [PATCH] Lots of Paging tidy-ups - Removed our hacky items() overload for LazyGridScope - Removed workaround for https://issuetracker.google.com/issues/177245496 --- .../app/tivi/common/compose/EntryGrid.kt | 9 +- .../app/tivi/common/compose/LazyList.kt | 102 ------------------ .../common/compose/LazyPagingExtensions.kt | 42 ++------ .../kotlin/app/tivi/home/library/Library.kt | 11 +- .../app/tivi/home/library/LibraryPresenter.kt | 5 +- .../home/popular/PopularShowsPresenter.kt | 5 +- .../recommended/RecommendedShowsPresenter.kt | 5 +- .../home/trending/TrendingShowsPresenter.kt | 5 +- .../kotlin/app/tivi/home/upnext/UpNext.kt | 11 +- .../app/tivi/home/upnext/UpNextPresenter.kt | 5 +- 10 files changed, 44 insertions(+), 156 deletions(-) diff --git a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/EntryGrid.kt b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/EntryGrid.kt index dcb3770437..3a4e844ffc 100644 --- a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/EntryGrid.kt +++ b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/EntryGrid.kt @@ -47,6 +47,7 @@ import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.unit.dp import app.cash.paging.LoadStateLoading import app.cash.paging.compose.LazyPagingItems +import app.cash.paging.compose.itemKey import app.tivi.common.compose.ui.PlaceholderPosterCard import app.tivi.common.compose.ui.PosterCard import app.tivi.common.compose.ui.RefreshButton @@ -130,7 +131,6 @@ fun EntryGrid( val gutter = Layout.gutter LazyVerticalGrid( - state = rememberLazyGridState(lazyPagingItems.itemCount == 0), columns = GridCells.Fixed((columns / 1.5).roundToInt()), contentPadding = paddingValues + PaddingValues(horizontal = bodyMargin, vertical = gutter), @@ -143,9 +143,10 @@ fun EntryGrid( .fillMaxHeight(), ) { items( - items = lazyPagingItems, - key = { it.show.id }, - ) { entry -> + count = lazyPagingItems.itemCount, + key = lazyPagingItems.itemKey { it.show.id }, + ) { index -> + val entry = lazyPagingItems[index] val mod = Modifier .animateItemPlacement() .aspectRatio(2 / 3f) diff --git a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyList.kt b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyList.kt index 85f0f71a55..edd114de76 100644 --- a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyList.kt +++ b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyList.kt @@ -5,28 +5,16 @@ package app.tivi.common.compose -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.PaddingValues -import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.calculateEndPadding -import androidx.compose.foundation.layout.calculateStartPadding import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.width -import androidx.compose.foundation.lazy.LazyItemScope import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyGridItemScope -import androidx.compose.foundation.lazy.grid.LazyGridItemSpanScope import androidx.compose.foundation.lazy.grid.LazyGridScope import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.unit.Dp -import androidx.compose.ui.unit.dp -import app.cash.paging.compose.LazyPagingItems inline fun LazyListScope.itemSpacer(height: Dp) { item { @@ -68,44 +56,6 @@ inline fun LazyGridScope.itemSpacer(height: Dp) { } } -fun LazyGridScope.items( - items: LazyPagingItems, - key: ((item: T) -> Any)? = null, - span: (LazyGridItemSpanScope.(item: T) -> GridItemSpan)? = null, - contentType: (item: T) -> Any? = { null }, - itemContent: @Composable LazyGridItemScope.(item: T?) -> Unit, -) { - items( - count = items.itemCount, - span = { index -> - val item = items.peek(index) - when { - item != null && span != null -> span(item) - else -> GridItemSpan(1) - } - }, - contentType = { index -> - items.peek(index)?.let { contentType(it) } - }, - key = if (key == null) { - null - } else { - { index -> - val item = items.peek(index) - if (item == null) { - PagingPlaceholderKey(index) - } else { - key(item) - } - } - }, - ) { index -> - itemContent(items[index]) - } -} - -internal data class PagingPlaceholderKey(private val index: Int) - inline fun LazyGridScope.fullSpanItem( key: Any? = null, contentType: Any? = null, @@ -118,55 +68,3 @@ inline fun LazyGridScope.fullSpanItem( content = content, ) } - -/** - * Displays a 'fake' grid using [LazyColumn]'s DSL. It's fake in that we just we add individual - * column items, with a inner fake row. - */ -fun LazyListScope.itemsInGrid( - items: List, - columns: Int, - contentPadding: PaddingValues = PaddingValues(), - horizontalItemPadding: Dp = 0.dp, - verticalItemPadding: Dp = 0.dp, - itemContent: @Composable LazyItemScope.(T) -> Unit, -) { - val rows = when { - items.size % columns == 0 -> items.size / columns - else -> (items.size / columns) + 1 - } - - for (row in 0 until rows) { - if (row == 0) itemSpacer(contentPadding.calculateTopPadding()) - - item { - val layoutDirection = LocalLayoutDirection.current - Row( - modifier = Modifier - .fillMaxWidth() - .padding( - start = contentPadding.calculateStartPadding(layoutDirection), - end = contentPadding.calculateEndPadding(layoutDirection), - ), - ) { - for (column in 0 until columns) { - Box(modifier = Modifier.weight(1f)) { - val index = (row * columns) + column - if (index < items.size) { - itemContent(items[index]) - } - } - if (column < columns - 1) { - Spacer(modifier = Modifier.width(horizontalItemPadding)) - } - } - } - } - - if (row < rows - 1) { - itemSpacer(verticalItemPadding) - } else { - itemSpacer(contentPadding.calculateBottomPadding()) - } - } -} diff --git a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyPagingExtensions.kt b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyPagingExtensions.kt index 415040a278..4871994c57 100644 --- a/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyPagingExtensions.kt +++ b/common/ui/compose/src/commonMain/kotlin/app/tivi/common/compose/LazyPagingExtensions.kt @@ -1,14 +1,18 @@ // Copyright 2021, Google LLC, Christopher Banes and the Tivi project contributors // SPDX-License-Identifier: Apache-2.0 +@file:Suppress("USELESS_IS_CHECK", "CAST_NEVER_SUCCEEDS") + package app.tivi.common.compose -import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.foundation.lazy.grid.LazyGridState import androidx.compose.runtime.Composable import androidx.compose.runtime.remember import app.cash.paging.CombinedLoadStates import app.cash.paging.LoadStateError +import app.cash.paging.PagingData +import app.cash.paging.cachedIn +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.Flow fun CombinedLoadStates.appendErrorOrNull(): UiMessage? { return (append.takeIf { it is LoadStateError } as? LoadStateError) @@ -25,35 +29,7 @@ fun CombinedLoadStates.refreshErrorOrNull(): UiMessage? { ?.let { UiMessage(it.error) } } -/* - * The following are workarounds for https://issuetracker.google.com/issues/177245496. - * - * Due to the way which paging loads data, it will always result in an initial empty state, - * and then the loaded items. That conflicts with the way which rememberLazyListState() and - * friends store their state, as they rely on the items being present at the time of - * (Android) state restoration. To workaround this, we use a different LazyListState while the - * lazy paging items are empty, and then switcheroo to the standard state once we know we have the - * items available. - */ - -@Composable -fun rememberLazyListState(empty: Boolean = false): LazyListState { - return if (empty) { - // Return a different state instance - remember { LazyListState(0, 0) } - } else { - // Return restored state - androidx.compose.foundation.lazy.rememberLazyListState() - } -} - @Composable -fun rememberLazyGridState(empty: Boolean = false): LazyGridState { - return if (empty) { - // Return a different state instance - remember { LazyGridState(0, 0) } - } else { - // Return restored state - androidx.compose.foundation.lazy.grid.rememberLazyGridState() - } -} +inline fun Flow>.rememberCachedPagingFlow( + scope: CoroutineScope = rememberCoroutineScope(), +): Flow> = remember(this, scope) { cachedIn(scope) } diff --git a/ui/library/src/commonMain/kotlin/app/tivi/home/library/Library.kt b/ui/library/src/commonMain/kotlin/app/tivi/home/library/Library.kt index 0fc5c62bcd..3141fec46a 100644 --- a/ui/library/src/commonMain/kotlin/app/tivi/home/library/Library.kt +++ b/ui/library/src/commonMain/kotlin/app/tivi/home/library/Library.kt @@ -62,15 +62,14 @@ import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.unit.dp import app.cash.paging.LoadStateLoading import app.cash.paging.compose.LazyPagingItems +import app.cash.paging.compose.itemKey import app.tivi.common.compose.Layout import app.tivi.common.compose.LocalStrings import app.tivi.common.compose.LocalTiviDateFormatter import app.tivi.common.compose.LocalTiviTextCreator import app.tivi.common.compose.bodyWidth import app.tivi.common.compose.fullSpanItem -import app.tivi.common.compose.items import app.tivi.common.compose.rememberCoroutineScope -import app.tivi.common.compose.rememberLazyGridState import app.tivi.common.compose.rememberTiviFlingBehavior import app.tivi.common.compose.ui.EmptyContent import app.tivi.common.compose.ui.PosterCard @@ -254,7 +253,6 @@ private fun LibraryGrid( var filterExpanded by remember { mutableStateOf(false) } LazyVerticalGrid( - state = rememberLazyGridState(lazyPagingItems.itemCount == 0), columns = GridCells.Fixed(columns / 4), flingBehavior = rememberTiviFlingBehavior(), contentPadding = paddingValues + PaddingValues( @@ -355,9 +353,10 @@ private fun LibraryGrid( } items( - items = lazyPagingItems, - key = { it.show.id }, - ) { entry -> + count = state.items.itemCount, + key = state.items.itemKey { it.show.id }, + ) { index -> + val entry = state.items[index] if (entry != null) { LibraryItem( show = entry.show, diff --git a/ui/library/src/commonMain/kotlin/app/tivi/home/library/LibraryPresenter.kt b/ui/library/src/commonMain/kotlin/app/tivi/home/library/LibraryPresenter.kt index 6af43fa0eb..edd64587e6 100644 --- a/ui/library/src/commonMain/kotlin/app/tivi/home/library/LibraryPresenter.kt +++ b/ui/library/src/commonMain/kotlin/app/tivi/home/library/LibraryPresenter.kt @@ -15,6 +15,7 @@ import app.cash.paging.PagingConfig import app.cash.paging.compose.collectAsLazyPagingItems import app.tivi.common.compose.UiMessage import app.tivi.common.compose.UiMessageManager +import app.tivi.common.compose.rememberCachedPagingFlow import app.tivi.common.compose.rememberCoroutineScope import app.tivi.data.models.SortOption import app.tivi.data.traktauth.TraktAuthState @@ -70,7 +71,9 @@ class LibraryPresenter( val scope = rememberCoroutineScope() val uiMessageManager = remember { UiMessageManager() } - val items = observePagedLibraryShows.flow.collectAsLazyPagingItems() + val items = observePagedLibraryShows.flow + .rememberCachedPagingFlow(scope) + .collectAsLazyPagingItems() var filter by remember { mutableStateOf(null) } var sort by remember { mutableStateOf(SortOption.LAST_WATCHED) } diff --git a/ui/popular/src/commonMain/kotlin/app/tivi/home/popular/PopularShowsPresenter.kt b/ui/popular/src/commonMain/kotlin/app/tivi/home/popular/PopularShowsPresenter.kt index 0eb22fddf2..606116ebf9 100644 --- a/ui/popular/src/commonMain/kotlin/app/tivi/home/popular/PopularShowsPresenter.kt +++ b/ui/popular/src/commonMain/kotlin/app/tivi/home/popular/PopularShowsPresenter.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import app.cash.paging.PagingConfig import app.cash.paging.compose.collectAsLazyPagingItems +import app.tivi.common.compose.rememberCachedPagingFlow import app.tivi.domain.observers.ObservePagedPopularShows import app.tivi.screens.PopularShowsScreen import app.tivi.screens.ShowDetailsScreen @@ -39,7 +40,9 @@ class PopularShowsPresenter( @Composable override fun present(): PopularShowsUiState { - val items = pagingInteractor.flow.collectAsLazyPagingItems() + val items = pagingInteractor.flow + .rememberCachedPagingFlow() + .collectAsLazyPagingItems() LaunchedEffect(Unit) { pagingInteractor(ObservePagedPopularShows.Params(PAGING_CONFIG)) diff --git a/ui/recommended/src/commonMain/kotlin/app/tivi/home/recommended/RecommendedShowsPresenter.kt b/ui/recommended/src/commonMain/kotlin/app/tivi/home/recommended/RecommendedShowsPresenter.kt index f4ea65b25a..dd87d9d5d3 100644 --- a/ui/recommended/src/commonMain/kotlin/app/tivi/home/recommended/RecommendedShowsPresenter.kt +++ b/ui/recommended/src/commonMain/kotlin/app/tivi/home/recommended/RecommendedShowsPresenter.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import app.cash.paging.PagingConfig import app.cash.paging.compose.collectAsLazyPagingItems +import app.tivi.common.compose.rememberCachedPagingFlow import app.tivi.domain.observers.ObservePagedRecommendedShows import app.tivi.screens.RecommendedShowsScreen import app.tivi.screens.ShowDetailsScreen @@ -39,7 +40,9 @@ class RecommendedShowsPresenter( @Composable override fun present(): RecommendedShowsUiState { - val items = pagingInteractor.flow.collectAsLazyPagingItems() + val items = pagingInteractor.flow + .rememberCachedPagingFlow() + .collectAsLazyPagingItems() LaunchedEffect(Unit) { pagingInteractor(ObservePagedRecommendedShows.Params(PAGING_CONFIG)) diff --git a/ui/trending/src/commonMain/kotlin/app/tivi/home/trending/TrendingShowsPresenter.kt b/ui/trending/src/commonMain/kotlin/app/tivi/home/trending/TrendingShowsPresenter.kt index 9a831a833e..6643b776eb 100644 --- a/ui/trending/src/commonMain/kotlin/app/tivi/home/trending/TrendingShowsPresenter.kt +++ b/ui/trending/src/commonMain/kotlin/app/tivi/home/trending/TrendingShowsPresenter.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import app.cash.paging.PagingConfig import app.cash.paging.compose.collectAsLazyPagingItems +import app.tivi.common.compose.rememberCachedPagingFlow import app.tivi.domain.observers.ObservePagedTrendingShows import app.tivi.screens.ShowDetailsScreen import app.tivi.screens.TrendingShowsScreen @@ -39,7 +40,9 @@ class TrendingShowsPresenter( @Composable override fun present(): TrendingShowsUiState { - val items = pagingInteractor.flow.collectAsLazyPagingItems() + val items = pagingInteractor.flow + .rememberCachedPagingFlow() + .collectAsLazyPagingItems() LaunchedEffect(Unit) { pagingInteractor(ObservePagedTrendingShows.Params(PAGING_CONFIG)) diff --git a/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNext.kt b/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNext.kt index 0faf3377bb..34fe5ca1b9 100644 --- a/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNext.kt +++ b/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNext.kt @@ -58,14 +58,13 @@ import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.unit.dp import app.cash.paging.LoadStateLoading +import app.cash.paging.compose.itemKey import app.tivi.common.compose.Layout import app.tivi.common.compose.LocalStrings import app.tivi.common.compose.LocalTiviTextCreator import app.tivi.common.compose.bodyWidth import app.tivi.common.compose.fullSpanItem -import app.tivi.common.compose.items import app.tivi.common.compose.rememberCoroutineScope -import app.tivi.common.compose.rememberLazyGridState import app.tivi.common.compose.rememberTiviFlingBehavior import app.tivi.common.compose.ui.AsyncImage import app.tivi.common.compose.ui.EmptyContent @@ -217,7 +216,6 @@ internal fun UpNext( val gutter = Layout.gutter LazyVerticalGrid( - state = rememberLazyGridState(state.items.itemCount == 0), columns = GridCells.Fixed(columns / 4), contentPadding = paddingValues + PaddingValues( horizontal = (bodyMargin - 8.dp).coerceAtLeast(0.dp), @@ -258,9 +256,10 @@ internal fun UpNext( } items( - items = state.items, - key = { it.show.id }, - ) { entry -> + count = state.items.itemCount, + key = state.items.itemKey { it.show.id }, + ) { index -> + val entry = state.items[index] if (entry != null) { SwipeUpNextItem( onSwipe = { openTrackEpisode(entry.episode.id) }, diff --git a/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNextPresenter.kt b/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNextPresenter.kt index adb5b4fcf9..145a924843 100644 --- a/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNextPresenter.kt +++ b/ui/upnext/src/commonMain/kotlin/app/tivi/home/upnext/UpNextPresenter.kt @@ -15,6 +15,7 @@ import app.cash.paging.PagingConfig import app.cash.paging.compose.collectAsLazyPagingItems import app.tivi.common.compose.UiMessage import app.tivi.common.compose.UiMessageManager +import app.tivi.common.compose.rememberCachedPagingFlow import app.tivi.common.compose.rememberCoroutineScope import app.tivi.data.models.SortOption import app.tivi.data.traktauth.TraktAuthState @@ -73,7 +74,9 @@ class UpNextPresenter( val uiMessageManager = remember { UiMessageManager() } - val items = observePagedUpNextShows.flow.collectAsLazyPagingItems() + val items = observePagedUpNextShows.flow + .rememberCachedPagingFlow(scope) + .collectAsLazyPagingItems() var sort by remember { mutableStateOf(SortOption.LAST_WATCHED) }