From ce76f8fb27536e2656fff6bc08afe6a543c28888 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 26 Feb 2026 18:22:11 +0000 Subject: [PATCH] Code review fixes: Redundant Box in SkipButton Unused string resources remember { fadeIn() } pattern --- .../composable/controls/GradientOverlay.kt | 12 +++--- .../composable/controls/OverflowMenu.kt | 7 +++- .../composable/controls/SkipButton.kt | 40 +++++++++---------- amethyst/src/main/res/values/strings.xml | 4 +- 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt index 18f26df4c..9b54413e2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/GradientOverlay.kt @@ -29,13 +29,15 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.runtime.Composable import androidx.compose.runtime.State -import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Color import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +private val FadeIn = fadeIn() +private val FadeOut = fadeOut() + private val TopGradientColors = listOf( Color.Black.copy(alpha = 0.6f), @@ -59,8 +61,8 @@ fun TopGradientOverlay( AnimatedVisibility( visible = controllerVisible.value, modifier = modifier, - enter = remember { fadeIn() }, - exit = remember { fadeOut() }, + enter = FadeIn, + exit = FadeOut, ) { Box( modifier = @@ -83,8 +85,8 @@ fun BottomGradientOverlay( AnimatedVisibility( visible = controllerVisible.value, modifier = modifier, - enter = remember { fadeIn() }, - exit = remember { fadeOut() }, + enter = FadeIn, + exit = FadeOut, ) { Box( modifier = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt index f3d18eec0..fe9b09abc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/OverflowMenu.kt @@ -55,6 +55,9 @@ import com.vitorpamplona.amethyst.ui.theme.Size50Modifier import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize +private val FadeIn = fadeIn() +private val FadeOut = fadeOut() + @Preview @Composable fun OverflowMenuButtonPreview() { @@ -86,8 +89,8 @@ fun AnimatedOverflowMenuButton( AnimatedVisibility( visible = controllerVisible.value, modifier = modifier, - enter = remember { fadeIn() }, - exit = remember { fadeOut() }, + enter = FadeIn, + exit = FadeOut, ) { OverflowMenuButton( showShare = showShare, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/SkipButton.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/SkipButton.kt index 65bc39ead..2d37909e5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/SkipButton.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/SkipButton.kt @@ -33,8 +33,6 @@ import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.runtime.Composable import androidx.compose.runtime.State -import androidx.compose.runtime.remember -import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.tooling.preview.Preview @@ -44,6 +42,9 @@ import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.BitcoinOrange import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn +private val FadeIn = fadeIn() +private val FadeOut = fadeOut() + @Preview @Composable fun SkipBackButtonPreview() { @@ -83,8 +84,8 @@ fun AnimatedSkipButton( AnimatedVisibility( visible = controllerVisible.value, modifier = modifier, - enter = remember { fadeIn() }, - exit = remember { fadeOut() }, + enter = FadeIn, + exit = FadeOut, ) { SkipButton( isForward = isForward, @@ -100,25 +101,20 @@ fun SkipButton( skipSeconds: Int = 10, onClick: () -> Unit, ) { - Box( + IconButton( + onClick = onClick, modifier = Modifier.size(48.dp), - contentAlignment = Alignment.Center, ) { - IconButton( - onClick = onClick, - modifier = Modifier.size(48.dp), - ) { - Icon( - imageVector = if (isForward) Icons.Default.Forward10 else Icons.Default.Replay10, - contentDescription = - if (isForward) { - stringRes(R.string.skip_forward, skipSeconds) - } else { - stringRes(R.string.skip_back, skipSeconds) - }, - tint = Color.White, - modifier = Modifier.size(32.dp), - ) - } + Icon( + imageVector = if (isForward) Icons.Default.Forward10 else Icons.Default.Replay10, + contentDescription = + if (isForward) { + stringRes(R.string.skip_forward, skipSeconds) + } else { + stringRes(R.string.skip_back, skipSeconds) + }, + tint = Color.White, + modifier = Modifier.size(32.dp), + ) } } diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index e98d1c14e..b6134b087 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -904,8 +904,6 @@ Skip back %d seconds Skip forward %d seconds Picture-in-Picture - LIVE - Tap to retry Search local and remote records Nostr address was verified @@ -1505,7 +1503,7 @@ %1$d/%2$d Broadcasting Broadcasting %1$s - Broadcasting %1$d events... + Broadcasting %1$d events… %1$d events sent Some events failed