From 02541b08bec5bbc5e56118625082c488801add31 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Tue, 2 Feb 2021 13:41:54 +0000 Subject: [PATCH 1/9] Fix method name suppression --- .../tivi/showdetails/details/ShowDetails.kt | 90 +++++++++---------- 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index 3cbedc5a56..3cad4b9f83 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -367,7 +367,7 @@ private fun ShowDetailsScrollingContent( } seasons.forEach { - SeasonWithEpisodesRow( + seasonWithEpisodesRow( season = it.season, episodes = it.episodes, expanded = it.season.id in expandedSeasonIds, @@ -818,66 +818,62 @@ private fun WatchStats( } } -@Suppress("FunctionName") @OptIn(ExperimentalAnimationApi::class) -private fun LazyListScope.SeasonWithEpisodesRow( +private fun LazyListScope.seasonWithEpisodesRow( season: Season, episodes: List, expanded: Boolean, actioner: (ShowDetailsAction) -> Unit, -) { - item { - val elevation by animateAsState(if (expanded) 2.dp else 0.dp) - Surface( - elevation = elevation, - modifier = Modifier.fillMaxWidth() - ) { - Column(Modifier.fillMaxWidth()) { - SeasonRow( - season = season, - episodesAired = episodes.numberAired, - episodesWatched = episodes.numberWatched, - episodesToWatch = episodes.numberAiredToWatch, - episodesToAir = episodes.numberToAir, - nextToAirDate = episodes.nextToAir?.firstAired, - actioner = actioner, - modifier = Modifier - .fillMaxWidth() - .clickable(enabled = !season.ignored) { - actioner(ChangeSeasonExpandedAction(season.id, !expanded)) - } - ) - - // Ideally each EpisodeWithWatchesRow would be in a different item {}, but there - // are currently 2 issues for that: - // #1: AnimatedVisibility currently crashes in Lazy*: b/170287733 - // #2: Can't use a Surface across different items: b/170472398 - // So instead we bundle the items in an inner Column, within a single item. - episodes.forEach { episodeEntry -> - AnimatedVisibility(visible = expanded) { - EpisodeWithWatchesRow( - episode = episodeEntry.episode, - isWatched = episodeEntry.isWatched, - hasPending = episodeEntry.hasPending, - onlyPendingDeletes = episodeEntry.onlyPendingDeletes, - modifier = Modifier - .fillMaxWidth() - .clickable { - actioner(OpenEpisodeDetails(episodeEntry.episode.id)) - } - ) +) = item { + val elevation by animateAsState(if (expanded) 2.dp else 0.dp) + Surface( + elevation = elevation, + modifier = Modifier.fillParentMaxWidth() + ) { + Column(Modifier.fillMaxWidth()) { + SeasonRow( + season = season, + episodesAired = episodes.numberAired, + episodesWatched = episodes.numberWatched, + episodesToWatch = episodes.numberAiredToWatch, + episodesToAir = episodes.numberToAir, + nextToAirDate = episodes.nextToAir?.firstAired, + actioner = actioner, + modifier = Modifier + .fillMaxWidth() + .clickable(enabled = !season.ignored) { + actioner(ChangeSeasonExpandedAction(season.id, !expanded)) } - } + ) + // Ideally each EpisodeWithWatchesRow would be in a different item {}, but there + // are currently 2 issues for that: + // #1: AnimatedVisibility currently crashes in Lazy*: b/170287733 + // #2: Can't use a Surface across different items: b/170472398 + // So instead we bundle the items in an inner Column, within a single item. + episodes.forEach { episodeEntry -> AnimatedVisibility(visible = expanded) { - Divider() + EpisodeWithWatchesRow( + episode = episodeEntry.episode, + isWatched = episodeEntry.isWatched, + hasPending = episodeEntry.hasPending, + onlyPendingDeletes = episodeEntry.onlyPendingDeletes, + modifier = Modifier + .fillMaxWidth() + .clickable { + actioner(OpenEpisodeDetails(episodeEntry.episode.id)) + } + ) } } + + AnimatedVisibility(visible = expanded) { + Divider() + } } } } -@Suppress("UNUSED_PARAMETER") @Composable private fun SeasonRow( season: Season, From 9db4566edee7e59e37ef0d4b9c2791e492ac0463 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Tue, 2 Feb 2021 14:19:02 +0000 Subject: [PATCH 2/9] Convert show details to transition v2 APIs --- .../tivi/showdetails/details/ShowDetails.kt | 89 ++++++++----------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index 3cad4b9f83..9610bd4e75 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -14,20 +14,17 @@ * limitations under the License. */ -@file:Suppress("DEPRECATION") // Compose transition v1 APIs - package app.tivi.showdetails.details import androidx.compose.animation.AnimatedVisibility -import androidx.compose.animation.DpPropKey import androidx.compose.animation.ExperimentalAnimationApi -import androidx.compose.animation.core.FloatPropKey -import androidx.compose.animation.core.animateAsState +import androidx.compose.animation.core.animateDp +import androidx.compose.animation.core.animateDpAsState +import androidx.compose.animation.core.animateFloat import androidx.compose.animation.core.snap import androidx.compose.animation.core.spring -import androidx.compose.animation.core.transitionDefinition import androidx.compose.animation.core.tween -import androidx.compose.animation.transition +import androidx.compose.animation.core.updateTransition import androidx.compose.foundation.border import androidx.compose.foundation.clickable import androidx.compose.foundation.isSystemInDarkTheme @@ -79,12 +76,11 @@ import androidx.compose.material.icons.filled.MoreVert import androidx.compose.material.icons.filled.Refresh import androidx.compose.material.icons.filled.Star import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Providers import androidx.compose.runtime.emptyContent import androidx.compose.runtime.getValue -import androidx.compose.runtime.onCommit import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.runtime.staticAmbientOf import androidx.compose.ui.Alignment @@ -137,7 +133,6 @@ import dev.chrisbanes.accompanist.coil.CoilImage import dev.chrisbanes.accompanist.insets.AmbientWindowInsets import dev.chrisbanes.accompanist.insets.navigationBarsPadding import dev.chrisbanes.accompanist.insets.statusBarsHeight -import kotlinx.coroutines.launch import org.threeten.bp.OffsetDateTime val AmbientShowDetailsTextCreator = staticAmbientOf() @@ -193,7 +188,6 @@ fun ShowDetails( } val snackbarHostState = remember { SnackbarHostState() } - val snackbarScope = rememberCoroutineScope() Column( Modifier @@ -224,11 +218,9 @@ fun ShowDetails( ) } - onCommit(viewState.refreshError) { + LaunchedEffect(viewState.refreshError) { viewState.refreshError?.let { error -> - snackbarScope.launch { - snackbarHostState.showSnackbar(error.message) - } + snackbarHostState.showSnackbar(error.message) } } } @@ -441,13 +433,36 @@ private fun OverlaidStatusBarAppBar( LogCompositions("OverlaidStatusBarAppBar") Column(modifier) { - val props = transition( - definition = statusBarTransitionDef, - initState = false, - toState = showAppBar + val transition = updateTransition(showAppBar) + + val elevation by transition.animateDp { value -> + if (value) 2.dp else 0.dp + } + + val barAlpha by transition.animateFloat( + transitionSpec = { + when { + false isTransitioningTo true -> snap() + else -> tween(durationMillis = 300) + } + }, + targetValueByState = { value -> if (value) 1f else 0f } ) - val elevation by animateAsState(if (showAppBar) 2.dp else 0.dp) + val barOffset by transition.animateFloat( + transitionSpec = { + when { + false isTransitioningTo true -> spring() + // This is a bit of a hack. We don't actually want an offset transition + // on exit, so we just run a snap AFTER the alpha animation + // has finished (with some buffer) + else -> snap(delayMillis = 320) + } + }, + targetValueByState = { value -> + if (value) 0f else AmbientWindowInsets.current.statusBars.top.toFloat() + } + ) Surface( elevation = elevation, @@ -455,9 +470,9 @@ private fun OverlaidStatusBarAppBar( .fillMaxWidth() .statusBarsHeight() .graphicsLayer { - alpha = props[AlphaKey] - } - .offset(y = props[OffsetYKey]), + this.alpha = barAlpha + translationY = barOffset + }, content = emptyContent() ) @@ -471,32 +486,6 @@ private fun OverlaidStatusBarAppBar( } } -private val OffsetYKey = DpPropKey() -private val AlphaKey = FloatPropKey() - -private val statusBarTransitionDef = transitionDefinition { - state(false) { - this[OffsetYKey] = 24.dp - this[AlphaKey] = 0f - } - state(true) { - this[OffsetYKey] = 0.dp - this[AlphaKey] = 1f - } - - transition(toState = true) { - OffsetYKey using spring() - AlphaKey using snap() - } - - transition(toState = false) { - AlphaKey using tween(durationMillis = 300) - // This is a bit of a hack. We don't actually want an offset transition on exit, - // so we just run a snap AFTER the alpha animation has finished (with some buffer) - OffsetYKey using snap(delayMillis = 320) - } -} - @Composable private fun NetworkInfoPanel( networkName: String, @@ -825,7 +814,7 @@ private fun LazyListScope.seasonWithEpisodesRow( expanded: Boolean, actioner: (ShowDetailsAction) -> Unit, ) = item { - val elevation by animateAsState(if (expanded) 2.dp else 0.dp) + val elevation by animateDpAsState(if (expanded) 2.dp else 0.dp) Surface( elevation = elevation, modifier = Modifier.fillParentMaxWidth() From 6497e112693ed193d8537e2588b9ae30cc0e3aac Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Tue, 2 Feb 2021 14:50:55 +0000 Subject: [PATCH 3/9] Update EpisodeDetails to transition v2 APIs --- .../app/tivi/episodedetails/EpisodeDetails.kt | 67 ++++++------------- 1 file changed, 20 insertions(+), 47 deletions(-) diff --git a/ui-episodedetails/src/main/java/app/tivi/episodedetails/EpisodeDetails.kt b/ui-episodedetails/src/main/java/app/tivi/episodedetails/EpisodeDetails.kt index 077d0cfd62..99f7cce40d 100644 --- a/ui-episodedetails/src/main/java/app/tivi/episodedetails/EpisodeDetails.kt +++ b/ui-episodedetails/src/main/java/app/tivi/episodedetails/EpisodeDetails.kt @@ -14,16 +14,10 @@ * limitations under the License. */ -@file:Suppress("DEPRECATION") // Compose transition v1 APIs - package app.tivi.episodedetails -import androidx.compose.animation.ColorPropKey +import androidx.compose.animation.animateColorAsState import androidx.compose.animation.core.FastOutLinearInEasing -import androidx.compose.animation.core.transitionDefinition -import androidx.compose.animation.core.tween -import androidx.compose.animation.transition -import androidx.compose.foundation.ScrollableColumn import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -36,7 +30,9 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.preferredHeight import androidx.compose.foundation.layout.preferredSizeIn +import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll import androidx.compose.material.AmbientContentAlpha import androidx.compose.material.AmbientContentColor import androidx.compose.material.Button @@ -61,12 +57,11 @@ import androidx.compose.material.icons.filled.Refresh import androidx.compose.material.icons.filled.Star import androidx.compose.material.rememberDismissState import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Providers import androidx.compose.runtime.getValue import androidx.compose.runtime.key -import androidx.compose.runtime.onCommit import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -102,7 +97,6 @@ import dev.chrisbanes.accompanist.coil.CoilImage import dev.chrisbanes.accompanist.insets.navigationBarsHeight import dev.chrisbanes.accompanist.insets.navigationBarsPadding import dev.chrisbanes.accompanist.insets.statusBarsPadding -import kotlinx.coroutines.launch import org.threeten.bp.OffsetDateTime import kotlin.math.absoluteValue import kotlin.math.hypot @@ -115,7 +109,7 @@ fun EpisodeDetails( ) { Box { Column { - Box { + Surface { if (viewState.episode != null && viewState.season != null) { Backdrop( season = viewState.season, @@ -133,7 +127,7 @@ fun EpisodeDetails( .statusBarsPadding() ) } - ScrollableColumn { + Column(Modifier.verticalScroll(rememberScrollState())) { Surface(elevation = 2.dp) { Column { val episode = viewState.episode @@ -214,7 +208,6 @@ fun EpisodeDetails( } val snackbarHostState = remember { SnackbarHostState() } - val snackbarScope = rememberCoroutineScope() SnackbarHost( hostState = snackbarHostState, @@ -231,11 +224,9 @@ fun EpisodeDetails( .padding(start = 16.dp, end = 16.dp, bottom = 16.dp) ) - onCommit(viewState.error) { + LaunchedEffect(viewState.error) { viewState.error?.let { error -> - snackbarScope.launch { - snackbarHostState.showSnackbar(error.message) - } + snackbarHostState.showSnackbar(error.message) } } } @@ -424,44 +415,17 @@ private fun EpisodeWatch(episodeWatchEntry: EpisodeWatchEntry) { } } -private val color = ColorPropKey() - @Composable private fun EpisodeWatchSwipeBackground( swipeProgress: Float, wouldCompleteOnRelease: Boolean = false ) { var iconCenter by rememberMutableState { Offset(0f, 0f) } + val maxRadius = hypot(iconCenter.x.toDouble(), iconCenter.y.toDouble()) - val maxRadius = remember(iconCenter) { - hypot(iconCenter.x.toDouble(), iconCenter.y.toDouble()) - } - - // Note: can't reference these directly in transitionDefinition {} as - // it's not @Composable val secondary = MaterialTheme.colors.error.copy(alpha = 0.5f) val default = MaterialTheme.colors.onSurface.copy(alpha = 0.2f) - val transition = remember(secondary, default) { - transitionDefinition { - state(true) { - this[color] = secondary - } - state(false) { - this[color] = default - } - - transition { - color using tween(durationMillis = 200) - } - } - } - - val transitionState = transition( - definition = transition, - toState = wouldCompleteOnRelease - ) - Box( Modifier .fillMaxSize() @@ -472,9 +436,18 @@ private fun EpisodeWatchSwipeBackground( modifier = Modifier .fillMaxSize() .drawGrowingCircle( - color = transitionState[color], + color = animateColorAsState( + targetValue = when (wouldCompleteOnRelease) { + true -> secondary + false -> default + } + ).value, center = iconCenter, - radius = lerp(0f, maxRadius.toFloat(), FastOutLinearInEasing.transform(swipeProgress)) + radius = lerp( + startValue = 0f, + endValue = maxRadius.toFloat(), + fraction = FastOutLinearInEasing.transform(swipeProgress) + ) ) ) From 65af773a3acf2c29bf696c84f97716c83e06b021 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Tue, 2 Feb 2021 14:52:46 +0000 Subject: [PATCH 4/9] Tweak suppression on AccountUi --- ui-account/src/main/java/app/tivi/account/AccountUi.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ui-account/src/main/java/app/tivi/account/AccountUi.kt b/ui-account/src/main/java/app/tivi/account/AccountUi.kt index fcdf7dc7b5..79ec0957a1 100644 --- a/ui-account/src/main/java/app/tivi/account/AccountUi.kt +++ b/ui-account/src/main/java/app/tivi/account/AccountUi.kt @@ -14,15 +14,12 @@ * limitations under the License. */ -@file:Suppress("DEPRECATION") - package app.tivi.account import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ExperimentalLayout -import androidx.compose.foundation.layout.FlowMainAxisAlignment import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -86,7 +83,7 @@ fun AccountUi( ) { @Suppress("DEPRECATION") // TODO: Migrate away from FlowRow FlowRow( - mainAxisAlignment = FlowMainAxisAlignment.End, + mainAxisAlignment = androidx.compose.foundation.layout.FlowMainAxisAlignment.End, mainAxisSpacing = 8.dp, crossAxisSpacing = 4.dp, ) { From 986d3a2997f71c801f34e4830125d6852f6f3e7a Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Wed, 3 Feb 2021 12:42:14 +0000 Subject: [PATCH 5/9] Refactor show details transition --- .../tivi/showdetails/details/ShowDetails.kt | 94 ++++++++++++------- 1 file changed, 60 insertions(+), 34 deletions(-) diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index 9610bd4e75..b9cb2ab092 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -78,8 +78,10 @@ import androidx.compose.material.icons.filled.Star import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Providers +import androidx.compose.runtime.Stable import androidx.compose.runtime.emptyContent import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.runtime.staticAmbientOf @@ -433,52 +435,23 @@ private fun OverlaidStatusBarAppBar( LogCompositions("OverlaidStatusBarAppBar") Column(modifier) { - val transition = updateTransition(showAppBar) - - val elevation by transition.animateDp { value -> - if (value) 2.dp else 0.dp - } - - val barAlpha by transition.animateFloat( - transitionSpec = { - when { - false isTransitioningTo true -> snap() - else -> tween(durationMillis = 300) - } - }, - targetValueByState = { value -> if (value) 1f else 0f } - ) - - val barOffset by transition.animateFloat( - transitionSpec = { - when { - false isTransitioningTo true -> spring() - // This is a bit of a hack. We don't actually want an offset transition - // on exit, so we just run a snap AFTER the alpha animation - // has finished (with some buffer) - else -> snap(delayMillis = 320) - } - }, - targetValueByState = { value -> - if (value) 0f else AmbientWindowInsets.current.statusBars.top.toFloat() - } - ) + val transition = updateOverlaidStatusBarAppBarTransition(showAppBar) Surface( - elevation = elevation, + elevation = transition.elevation, modifier = Modifier .fillMaxWidth() .statusBarsHeight() .graphicsLayer { - this.alpha = barAlpha - translationY = barOffset + alpha = transition.alpha + translationY = transition.offset }, content = emptyContent() ) if (showAppBar) { Surface( - elevation = elevation, + elevation = transition.elevation, modifier = Modifier.fillMaxWidth(), content = content, ) @@ -486,6 +459,59 @@ private fun OverlaidStatusBarAppBar( } } +@Composable +private fun updateOverlaidStatusBarAppBarTransition( + showAppBar: Boolean +): OverlaidStatusBarAppBarTransition { + LogCompositions("updateOverlaidStatusBarAppBarTransition") + + val transition = updateTransition(showAppBar) + + val elevation by transition.animateDp { show -> if (show) 2.dp else 0.dp } + + val alpha by transition.animateFloat( + transitionSpec = { + when { + false isTransitioningTo true -> snap() + else -> tween(durationMillis = 300) + } + } + ) { show -> + if (show) 1f else 0f + } + + val offset by transition.animateFloat( + transitionSpec = { + when { + false isTransitioningTo true -> spring() + // This is a bit of a hack. We don't actually want an offset transition + // on exit, so we just run a snap AFTER the alpha animation + // has finished (with some buffer) + else -> snap(delayMillis = 320) + } + } + ) { show -> + if (show) 0f else AmbientWindowInsets.current.statusBars.top.toFloat() + } + + val props = remember { OverlaidStatusBarAppBarTransition() } + + props.apply { + this.alpha = alpha + this.elevation = elevation + this.offset = offset + } + + return props +} + +@Stable +class OverlaidStatusBarAppBarTransition { + var elevation: Dp by mutableStateOf(0.dp) + var alpha: Float by mutableStateOf(0f) + var offset: Float by mutableStateOf(0f) +} + @Composable private fun NetworkInfoPanel( networkName: String, From c5ef4b7395e687cdfd8f683dc318dcaecec0678f Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Wed, 3 Feb 2021 12:48:19 +0000 Subject: [PATCH 6/9] Refactor SeasonWithEpisodesRow to be a composable --- .../java/app/tivi/common/compose/LazyList.kt | 14 ++++----- .../java/app/tivi/home/discover/Discover.kt | 8 ++--- .../java/app/tivi/home/followed/Followed.kt | 4 +-- .../java/app/tivi/home/popular/Popular.kt | 4 +-- .../app/tivi/home/recommended/Recommended.kt | 4 +-- .../tivi/showdetails/details/ShowDetails.kt | 29 ++++++++++--------- .../java/app/tivi/home/trending/Trending.kt | 4 +-- .../java/app/tivi/home/watched/Watched.kt | 4 +-- 8 files changed, 37 insertions(+), 34 deletions(-) diff --git a/common-ui-compose/src/main/java/app/tivi/common/compose/LazyList.kt b/common-ui-compose/src/main/java/app/tivi/common/compose/LazyList.kt index 09c234bdfa..ef526f2d23 100644 --- a/common-ui-compose/src/main/java/app/tivi/common/compose/LazyList.kt +++ b/common-ui-compose/src/main/java/app/tivi/common/compose/LazyList.kt @@ -32,7 +32,7 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import app.tivi.common.compose.paging.LazyPagingItems -fun LazyListScope.spacerItem(height: Dp) { +fun LazyListScope.itemSpacer(height: Dp) { item { Spacer(Modifier.preferredHeight(height).fillParentMaxWidth()) } @@ -56,7 +56,7 @@ fun LazyListScope.itemsInGrid( } for (row in 0 until rows) { - if (row == 0) spacerItem(contentPadding.top) + if (row == 0) itemSpacer(contentPadding.top) item { Row( @@ -78,9 +78,9 @@ fun LazyListScope.itemsInGrid( } if (row < rows - 1) { - spacerItem(verticalItemPadding) + itemSpacer(verticalItemPadding) } else { - spacerItem(contentPadding.bottom) + itemSpacer(contentPadding.bottom) } } } @@ -103,7 +103,7 @@ fun LazyListScope.itemsInGrid( } for (row in 0 until rows) { - if (row == 0) spacerItem(contentPadding.top) + if (row == 0) itemSpacer(contentPadding.top) item { Row( @@ -125,9 +125,9 @@ fun LazyListScope.itemsInGrid( } if (row < rows - 1) { - spacerItem(verticalItemPadding) + itemSpacer(verticalItemPadding) } else { - spacerItem(contentPadding.bottom) + itemSpacer(contentPadding.bottom) } } } diff --git a/ui-discover/src/main/java/app/tivi/home/discover/Discover.kt b/ui-discover/src/main/java/app/tivi/home/discover/Discover.kt index f1836fdebe..29473e8684 100644 --- a/ui-discover/src/main/java/app/tivi/home/discover/Discover.kt +++ b/ui-discover/src/main/java/app/tivi/home/discover/Discover.kt @@ -57,8 +57,8 @@ import app.tivi.common.compose.Carousel import app.tivi.common.compose.PosterCard import app.tivi.common.compose.RefreshButton import app.tivi.common.compose.UserProfileButton +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.entities.Episode import app.tivi.data.entities.Season import app.tivi.data.entities.TiviShow @@ -83,7 +83,7 @@ fun Discover( Spacer(Modifier.preferredHeight(height)) } - spacerItem(16.dp) + itemSpacer(16.dp) state.nextEpisodeWithShowToWatched?.let { nextEpisodeToWatch -> item { @@ -106,7 +106,7 @@ fun Discover( ) } - spacerItem(16.dp) + itemSpacer(16.dp) } item { @@ -139,7 +139,7 @@ fun Discover( ) } - spacerItem(16.dp) + itemSpacer(16.dp) } DiscoverAppBar( diff --git a/ui-followed/src/main/java/app/tivi/home/followed/Followed.kt b/ui-followed/src/main/java/app/tivi/home/followed/Followed.kt index 987642155a..26ef762dc6 100644 --- a/ui-followed/src/main/java/app/tivi/home/followed/Followed.kt +++ b/ui-followed/src/main/java/app/tivi/home/followed/Followed.kt @@ -61,8 +61,8 @@ import app.tivi.common.compose.RefreshButton import app.tivi.common.compose.SearchTextField import app.tivi.common.compose.SortMenuPopup import app.tivi.common.compose.UserProfileButton +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.entities.ShowTmdbImage import app.tivi.data.entities.SortOption import app.tivi.data.entities.TiviShow @@ -116,7 +116,7 @@ fun Followed( } } - spacerItem(16.dp) + itemSpacer(16.dp) } FollowedAppBar( diff --git a/ui-popular/src/main/java/app/tivi/home/popular/Popular.kt b/ui-popular/src/main/java/app/tivi/home/popular/Popular.kt index 4ef8ee1490..edca17ab81 100644 --- a/ui-popular/src/main/java/app/tivi/home/popular/Popular.kt +++ b/ui-popular/src/main/java/app/tivi/home/popular/Popular.kt @@ -51,10 +51,10 @@ import androidx.paging.LoadState import app.tivi.common.compose.AutoSizedCircularProgressIndicator import app.tivi.common.compose.PlaceholderPosterCard import app.tivi.common.compose.PosterCard +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.itemsInGrid import app.tivi.common.compose.paging.LazyPagingItems import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.resultentities.PopularEntryWithShow import dev.chrisbanes.accompanist.insets.statusBarsPadding @@ -101,7 +101,7 @@ fun Popular( } } - spacerItem(16.dp) + itemSpacer(16.dp) } PopularAppBar( diff --git a/ui-recommended/src/main/java/app/tivi/home/recommended/Recommended.kt b/ui-recommended/src/main/java/app/tivi/home/recommended/Recommended.kt index 4399945c0b..0fae6843a5 100644 --- a/ui-recommended/src/main/java/app/tivi/home/recommended/Recommended.kt +++ b/ui-recommended/src/main/java/app/tivi/home/recommended/Recommended.kt @@ -51,10 +51,10 @@ import androidx.paging.LoadState import app.tivi.common.compose.AutoSizedCircularProgressIndicator import app.tivi.common.compose.PlaceholderPosterCard import app.tivi.common.compose.PosterCard +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.itemsInGrid import app.tivi.common.compose.paging.LazyPagingItems import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.resultentities.RecommendedEntryWithShow import dev.chrisbanes.accompanist.insets.statusBarsPadding @@ -101,7 +101,7 @@ fun Recommended( } } - spacerItem(16.dp) + itemSpacer(16.dp) } RecommendedAppBar( diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index b9cb2ab092..a7c91d22fb 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -49,8 +49,8 @@ import androidx.compose.foundation.layout.preferredSizeIn import androidx.compose.foundation.layout.preferredWidth import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.LazyListState +import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.AmbientContentAlpha @@ -110,8 +110,8 @@ import app.tivi.common.compose.PosterCard import app.tivi.common.compose.SwipeDismissSnackbar import app.tivi.common.compose.VectorImage import app.tivi.common.compose.foregroundColor +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.common.imageloading.TrimTransparentEdgesTransformation import app.tivi.data.entities.Episode import app.tivi.data.entities.Genre @@ -287,7 +287,7 @@ private fun ShowDetailsScrollingContent( ) } - spacerItem(16.dp) + itemSpacer(16.dp) item { Header(stringResource(R.string.details_about)) @@ -311,7 +311,7 @@ private fun ShowDetailsScrollingContent( } if (nextEpisodeToWatch?.episode != null && nextEpisodeToWatch.season != null) { - spacerItem(8.dp) + itemSpacer(8.dp) item { Header(stringResource(id = R.string.details_next_episode_to_watch)) @@ -326,7 +326,7 @@ private fun ShowDetailsScrollingContent( } if (relatedShows.isNotEmpty()) { - spacerItem(8.dp) + itemSpacer(8.dp) item { Header(stringResource(R.string.details_related)) @@ -343,7 +343,7 @@ private fun ShowDetailsScrollingContent( } if (watchStats != null) { - spacerItem(8.dp) + itemSpacer(8.dp) item { Header(stringResource(R.string.details_view_stats)) @@ -354,24 +354,25 @@ private fun ShowDetailsScrollingContent( } if (seasons.isNotEmpty()) { - spacerItem(8.dp) + itemSpacer(8.dp) item { Header(stringResource(R.string.show_details_seasons)) } - seasons.forEach { - seasonWithEpisodesRow( + items(seasons) { + SeasonWithEpisodesRow( season = it.season, episodes = it.episodes, expanded = it.season.id in expandedSeasonIds, actioner = actioner, + modifier = Modifier.fillParentMaxWidth(), ) } } // Spacer to push up content from under the FloatingActionButton - spacerItem(56.dp + 16.dp + 16.dp) + itemSpacer(56.dp + 16.dp + 16.dp) } } @@ -834,16 +835,18 @@ private fun WatchStats( } @OptIn(ExperimentalAnimationApi::class) -private fun LazyListScope.seasonWithEpisodesRow( +@Composable +private fun SeasonWithEpisodesRow( season: Season, episodes: List, expanded: Boolean, actioner: (ShowDetailsAction) -> Unit, -) = item { + modifier: Modifier = Modifier +) { val elevation by animateDpAsState(if (expanded) 2.dp else 0.dp) Surface( elevation = elevation, - modifier = Modifier.fillParentMaxWidth() + modifier = modifier ) { Column(Modifier.fillMaxWidth()) { SeasonRow( diff --git a/ui-trending/src/main/java/app/tivi/home/trending/Trending.kt b/ui-trending/src/main/java/app/tivi/home/trending/Trending.kt index 6f8dce5d71..ee707ceeb4 100644 --- a/ui-trending/src/main/java/app/tivi/home/trending/Trending.kt +++ b/ui-trending/src/main/java/app/tivi/home/trending/Trending.kt @@ -51,10 +51,10 @@ import androidx.paging.LoadState import app.tivi.common.compose.AutoSizedCircularProgressIndicator import app.tivi.common.compose.PlaceholderPosterCard import app.tivi.common.compose.PosterCard +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.itemsInGrid import app.tivi.common.compose.paging.LazyPagingItems import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.resultentities.TrendingEntryWithShow import dev.chrisbanes.accompanist.insets.statusBarsPadding @@ -107,7 +107,7 @@ fun Trending( } } - spacerItem(16.dp) + itemSpacer(16.dp) } TrendingAppBar( diff --git a/ui-watched/src/main/java/app/tivi/home/watched/Watched.kt b/ui-watched/src/main/java/app/tivi/home/watched/Watched.kt index 034ed48b59..2efbdde683 100644 --- a/ui-watched/src/main/java/app/tivi/home/watched/Watched.kt +++ b/ui-watched/src/main/java/app/tivi/home/watched/Watched.kt @@ -61,8 +61,8 @@ import app.tivi.common.compose.RefreshButton import app.tivi.common.compose.SearchTextField import app.tivi.common.compose.SortMenuPopup import app.tivi.common.compose.UserProfileButton +import app.tivi.common.compose.itemSpacer import app.tivi.common.compose.rememberMutableState -import app.tivi.common.compose.spacerItem import app.tivi.data.entities.ShowTmdbImage import app.tivi.data.entities.SortOption import app.tivi.data.entities.TiviShow @@ -116,7 +116,7 @@ fun Watched( } } - spacerItem(16.dp) + itemSpacer(16.dp) } WatchedAppBar( From 9ee69c803b7e445ff9765b6c3b7e05f1ac07f8c7 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Wed, 3 Feb 2021 16:43:20 +0000 Subject: [PATCH 7/9] Try using lambdas for state reads --- .../java/app/tivi/common/compose/Debug.kt | 2 +- .../tivi/showdetails/details/ShowDetails.kt | 37 +++++++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt b/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt index 917948443f..2b3888e150 100644 --- a/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt +++ b/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt @@ -25,7 +25,7 @@ import androidx.compose.runtime.remember class Ref(var value: Int) -const val EnableDebugCompositionLogs = false +const val EnableDebugCompositionLogs = true /** * An effect which longs the number compositions at the invoked point of the slot table. diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index a7c91d22fb..99b294cee1 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -14,6 +14,8 @@ * limitations under the License. */ +@file:Suppress("UNUSED_VARIABLE") + package app.tivi.showdetails.details import androidx.compose.animation.AnimatedVisibility @@ -79,6 +81,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Providers import androidx.compose.runtime.Stable +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.emptyContent import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -169,12 +172,11 @@ fun ShowDetails( } val trigger = backdropHeight - AmbientWindowInsets.current.statusBars.top + OverlaidStatusBarAppBar( - showAppBar = when (listState.firstVisibleItemIndex) { - // We only show the app bar when the first item is shown, and it's offset off screen - // more than the trigger value - 0 -> listState.firstVisibleItemScrollOffset >= trigger - else -> true + showAppBar = { + listState.firstVisibleItemIndex > 0 || + listState.firstVisibleItemScrollOffset >= trigger }, modifier = Modifier .fillMaxWidth() @@ -209,9 +211,13 @@ fun ShowDetails( .fillMaxWidth() ) + val expanded by remember { + derivedStateOf { listState.firstVisibleItemIndex > 0 } + } + ToggleShowFollowFloatingActionButton( isFollowed = viewState.isFollowed, - expanded = listState.firstVisibleItemIndex >= 1, + expanded = { expanded }, onClick = { actioner(FollowShowToggleAction) }, modifier = Modifier .align(Alignment.End) @@ -260,10 +266,9 @@ private fun ShowDetailsScrollingContent( .offset { IntOffset( x = 0, - y = when (listState.firstVisibleItemIndex) { - 0 -> listState.firstVisibleItemScrollOffset / 2 - else -> 0 - } + y = if (listState.firstVisibleItemIndex == 0) { + listState.firstVisibleItemScrollOffset / 2 + } else 0 ) } ) @@ -384,7 +389,7 @@ private fun PosterInfoRow( ) { Row(modifier) { if (posterImage != null) { - Spacer(modifier = Modifier.preferredWidth(16.dp)) + Spacer(Modifier.preferredWidth(16.dp)) CoilImage( data = posterImage, @@ -429,14 +434,14 @@ private fun BackdropImage( @Composable private fun OverlaidStatusBarAppBar( - showAppBar: Boolean, + showAppBar: () -> Boolean, modifier: Modifier = Modifier, content: @Composable () -> Unit, ) { LogCompositions("OverlaidStatusBarAppBar") Column(modifier) { - val transition = updateOverlaidStatusBarAppBarTransition(showAppBar) + val transition = updateOverlaidStatusBarAppBarTransition(showAppBar()) Surface( elevation = transition.elevation, @@ -450,7 +455,7 @@ private fun OverlaidStatusBarAppBar( content = emptyContent() ) - if (showAppBar) { + if (showAppBar()) { Surface( elevation = transition.elevation, modifier = Modifier.fillMaxWidth(), @@ -1124,8 +1129,8 @@ private fun ShowDetailsAppBar( private fun ToggleShowFollowFloatingActionButton( isFollowed: Boolean, onClick: () -> Unit, + expanded: () -> Boolean, modifier: Modifier = Modifier, - expanded: Boolean = true, ) { LogCompositions("ToggleShowFollowFloatingActionButton") @@ -1155,7 +1160,7 @@ private fun ToggleShowFollowFloatingActionButton( isFollowed -> MaterialTheme.colors.surface else -> MaterialTheme.colors.primary }, - expanded = expanded, + expanded = expanded(), modifier = modifier ) } From 0abc329644e8e7367868ed3001e6d5eb5b666d22 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Mon, 8 Feb 2021 11:07:25 +0000 Subject: [PATCH 8/9] Turn debug back off --- .../src/main/java/app/tivi/common/compose/Debug.kt | 2 +- .../src/main/java/app/tivi/showdetails/details/ShowDetails.kt | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt b/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt index 2b3888e150..917948443f 100644 --- a/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt +++ b/common-ui-compose/src/main/java/app/tivi/common/compose/Debug.kt @@ -25,7 +25,7 @@ import androidx.compose.runtime.remember class Ref(var value: Int) -const val EnableDebugCompositionLogs = true +const val EnableDebugCompositionLogs = false /** * An effect which longs the number compositions at the invoked point of the slot table. diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index 99b294cee1..fbf13bd630 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -14,8 +14,6 @@ * limitations under the License. */ -@file:Suppress("UNUSED_VARIABLE") - package app.tivi.showdetails.details import androidx.compose.animation.AnimatedVisibility From a04980e821514a3eed73c08b6673b3dabb58b0d3 Mon Sep 17 00:00:00 2001 From: Chris Banes Date: Mon, 8 Feb 2021 11:12:09 +0000 Subject: [PATCH 9/9] Address naming feedback --- .../main/java/app/tivi/showdetails/details/ShowDetails.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt index fbf13bd630..13c8a7fec6 100644 --- a/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt +++ b/ui-showdetails/src/main/java/app/tivi/showdetails/details/ShowDetails.kt @@ -363,11 +363,11 @@ private fun ShowDetailsScrollingContent( Header(stringResource(R.string.show_details_seasons)) } - items(seasons) { + items(seasons) { season -> SeasonWithEpisodesRow( - season = it.season, - episodes = it.episodes, - expanded = it.season.id in expandedSeasonIds, + season = season.season, + episodes = season.episodes, + expanded = season.season.id in expandedSeasonIds, actioner = actioner, modifier = Modifier.fillParentMaxWidth(), )