Code review fixes:

Redundant Box in SkipButton
Unused string resources
remember { fadeIn() } pattern
This commit is contained in:
davotoula
2026-02-26 18:22:11 +00:00
parent ebbc7b5526
commit ce76f8fb27
4 changed files with 31 additions and 32 deletions
@@ -29,13 +29,15 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.height
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.State import androidx.compose.runtime.State
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Brush import androidx.compose.ui.graphics.Brush
import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
private val FadeIn = fadeIn()
private val FadeOut = fadeOut()
private val TopGradientColors = private val TopGradientColors =
listOf( listOf(
Color.Black.copy(alpha = 0.6f), Color.Black.copy(alpha = 0.6f),
@@ -59,8 +61,8 @@ fun TopGradientOverlay(
AnimatedVisibility( AnimatedVisibility(
visible = controllerVisible.value, visible = controllerVisible.value,
modifier = modifier, modifier = modifier,
enter = remember { fadeIn() }, enter = FadeIn,
exit = remember { fadeOut() }, exit = FadeOut,
) { ) {
Box( Box(
modifier = modifier =
@@ -83,8 +85,8 @@ fun BottomGradientOverlay(
AnimatedVisibility( AnimatedVisibility(
visible = controllerVisible.value, visible = controllerVisible.value,
modifier = modifier, modifier = modifier,
enter = remember { fadeIn() }, enter = FadeIn,
exit = remember { fadeOut() }, exit = FadeOut,
) { ) {
Box( Box(
modifier = modifier =
@@ -55,6 +55,9 @@ import com.vitorpamplona.amethyst.ui.theme.Size50Modifier
import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn
import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize import com.vitorpamplona.amethyst.ui.theme.VolumeBottomIconSize
private val FadeIn = fadeIn()
private val FadeOut = fadeOut()
@Preview @Preview
@Composable @Composable
fun OverflowMenuButtonPreview() { fun OverflowMenuButtonPreview() {
@@ -86,8 +89,8 @@ fun AnimatedOverflowMenuButton(
AnimatedVisibility( AnimatedVisibility(
visible = controllerVisible.value, visible = controllerVisible.value,
modifier = modifier, modifier = modifier,
enter = remember { fadeIn() }, enter = FadeIn,
exit = remember { fadeOut() }, exit = FadeOut,
) { ) {
OverflowMenuButton( OverflowMenuButton(
showShare = showShare, showShare = showShare,
@@ -33,8 +33,6 @@ import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton import androidx.compose.material3.IconButton
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.State import androidx.compose.runtime.State
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Color
import androidx.compose.ui.tooling.preview.Preview 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.BitcoinOrange
import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonColumn
private val FadeIn = fadeIn()
private val FadeOut = fadeOut()
@Preview @Preview
@Composable @Composable
fun SkipBackButtonPreview() { fun SkipBackButtonPreview() {
@@ -83,8 +84,8 @@ fun AnimatedSkipButton(
AnimatedVisibility( AnimatedVisibility(
visible = controllerVisible.value, visible = controllerVisible.value,
modifier = modifier, modifier = modifier,
enter = remember { fadeIn() }, enter = FadeIn,
exit = remember { fadeOut() }, exit = FadeOut,
) { ) {
SkipButton( SkipButton(
isForward = isForward, isForward = isForward,
@@ -100,25 +101,20 @@ fun SkipButton(
skipSeconds: Int = 10, skipSeconds: Int = 10,
onClick: () -> Unit, onClick: () -> Unit,
) { ) {
Box( IconButton(
onClick = onClick,
modifier = Modifier.size(48.dp), modifier = Modifier.size(48.dp),
contentAlignment = Alignment.Center,
) { ) {
IconButton( Icon(
onClick = onClick, imageVector = if (isForward) Icons.Default.Forward10 else Icons.Default.Replay10,
modifier = Modifier.size(48.dp), contentDescription =
) { if (isForward) {
Icon( stringRes(R.string.skip_forward, skipSeconds)
imageVector = if (isForward) Icons.Default.Forward10 else Icons.Default.Replay10, } else {
contentDescription = stringRes(R.string.skip_back, skipSeconds)
if (isForward) { },
stringRes(R.string.skip_forward, skipSeconds) tint = Color.White,
} else { modifier = Modifier.size(32.dp),
stringRes(R.string.skip_back, skipSeconds) )
},
tint = Color.White,
modifier = Modifier.size(32.dp),
)
}
} }
} }
+1 -3
View File
@@ -904,8 +904,6 @@
<string name="skip_back">Skip back %d seconds</string> <string name="skip_back">Skip back %d seconds</string>
<string name="skip_forward">Skip forward %d seconds</string> <string name="skip_forward">Skip forward %d seconds</string>
<string name="picture_in_picture">Picture-in-Picture</string> <string name="picture_in_picture">Picture-in-Picture</string>
<string name="live">LIVE</string>
<string name="tap_to_retry">Tap to retry</string>
<string name="search_button">Search local and remote records</string> <string name="search_button">Search local and remote records</string>
<string name="nip05_verified">Nostr address was verified</string> <string name="nip05_verified">Nostr address was verified</string>
@@ -1505,7 +1503,7 @@
<string name="share_of">%1$d/%2$d</string> <string name="share_of">%1$d/%2$d</string>
<string name="broadcasting">Broadcasting</string> <string name="broadcasting">Broadcasting</string>
<string name="broadcasting_name">Broadcasting %1$s</string> <string name="broadcasting_name">Broadcasting %1$s</string>
<string name="broadcasting_number_events">Broadcasting %1$d events...</string> <string name="broadcasting_number_events">Broadcasting %1$d events</string>
<string name="sent_number_events">%1$d events sent</string> <string name="sent_number_events">%1$d events sent</string>
<string name="bradcasting_result_partial">Some events failed</string> <string name="bradcasting_result_partial">Some events failed</string>