From e56f6c10177949c88e61d13e95b23545bef06641 Mon Sep 17 00:00:00 2001 From: Ivan Iskandar <12537387+ivaniskandar@users.noreply.github.com> Date: Sat, 9 Jul 2022 23:38:33 +0700 Subject: [PATCH] ChapterDownloadIndicator: Fixes and improvements (#7485) * Increased touch target * Fix downloaded icon smaller than other states * Deferred state reads to minimize recompose works * Move things around to eliminate unnecessary elements --- .../components/ChapterDownloadIndicator.kt | 213 +++++++++--------- .../kanade/presentation/manga/MangaScreen.kt | 4 +- .../manga/components/MangaChapterListItem.kt | 8 +- .../ui/manga/chapter/ChapterDownloadView.kt | 4 +- 4 files changed, 120 insertions(+), 109 deletions(-) diff --git a/app/src/main/java/eu/kanade/presentation/components/ChapterDownloadIndicator.kt b/app/src/main/java/eu/kanade/presentation/components/ChapterDownloadIndicator.kt index 11eb812e0e..375bb2565b 100644 --- a/app/src/main/java/eu/kanade/presentation/components/ChapterDownloadIndicator.kt +++ b/app/src/main/java/eu/kanade/presentation/components/ChapterDownloadIndicator.kt @@ -1,21 +1,22 @@ package eu.kanade.presentation.components import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.foundation.combinedClickable +import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowDownward import androidx.compose.material.icons.filled.CheckCircle +import androidx.compose.material.ripple.rememberRipple import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon -import androidx.compose.material3.LocalMinimumTouchTargetEnforcement import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ProgressIndicatorDefaults import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -24,6 +25,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.unit.dp import eu.kanade.presentation.manga.ChapterDownloadAction import eu.kanade.presentation.util.secondaryItemAlpha @@ -33,23 +35,18 @@ import eu.kanade.tachiyomi.data.download.model.Download @Composable fun ChapterDownloadIndicator( modifier: Modifier = Modifier, - downloadState: Download.State, - downloadProgress: Int, + downloadStateProvider: () -> Download.State, + downloadProgressProvider: () -> Int, onClick: (ChapterDownloadAction) -> Unit, ) { - Box(modifier = modifier, contentAlignment = Alignment.Center) { - CompositionLocalProvider(LocalMinimumTouchTargetEnforcement provides false) { - val isDownloaded = downloadState == Download.State.DOWNLOADED - val isDownloading = downloadState != Download.State.NOT_DOWNLOADED - var isMenuExpanded by remember(downloadState) { mutableStateOf(false) } - IconButton( - onClick = { - if (isDownloaded || isDownloading) { - isMenuExpanded = true - } else { - onClick(ChapterDownloadAction.START) - } - }, + val downloadState = downloadStateProvider() + val isDownloaded = downloadState == Download.State.DOWNLOADED + val isDownloading = downloadState != Download.State.NOT_DOWNLOADED + var isMenuExpanded by remember(downloadState) { mutableStateOf(false) } + Box( + modifier = modifier + .size(IconButtonTokens.StateLayerSize) + .combinedClickable( onLongClick = { val chapterDownloadAction = when { isDownloaded -> ChapterDownloadAction.DELETE @@ -58,93 +55,101 @@ fun ChapterDownloadIndicator( } onClick(chapterDownloadAction) }, - ) { - val indicatorModifier = Modifier - .size(IndicatorSize) - .padding(IndicatorPadding) - if (isDownloaded) { - Icon( - imageVector = Icons.Default.CheckCircle, - contentDescription = null, - modifier = indicatorModifier, - tint = MaterialTheme.colorScheme.onSurfaceVariant, - ) - DropdownMenu(expanded = isMenuExpanded, onDismissRequest = { isMenuExpanded = false }) { - DropdownMenuItem( - text = { Text(text = stringResource(R.string.action_delete)) }, - onClick = { - onClick(ChapterDownloadAction.DELETE) - isMenuExpanded = false - }, - ) - } - } else { - val inactiveAlphaModifier = if (!isDownloading) Modifier.secondaryItemAlpha() else Modifier - val arrowModifier = Modifier - .size(IndicatorSize - 7.dp) - .then(inactiveAlphaModifier) - val arrowColor: Color - val strokeColor = MaterialTheme.colorScheme.onSurfaceVariant - if (isDownloading) { - val indeterminate = downloadState == Download.State.QUEUE || - (downloadState == Download.State.DOWNLOADING && downloadProgress == 0) - if (indeterminate) { - arrowColor = strokeColor - CircularProgressIndicator( - modifier = indicatorModifier, - color = strokeColor, - strokeWidth = IndicatorStrokeWidth, - ) - } else { - val animatedProgress by animateFloatAsState( - targetValue = downloadProgress / 100f, - animationSpec = ProgressIndicatorDefaults.ProgressAnimationSpec, - ) - arrowColor = if (animatedProgress < 0.5f) { - strokeColor - } else { - MaterialTheme.colorScheme.background - } - CircularProgressIndicator( - progress = animatedProgress, - modifier = indicatorModifier, - color = strokeColor, - strokeWidth = IndicatorSize / 2, - ) - } + onClick = { + if (isDownloaded || isDownloading) { + isMenuExpanded = true } else { - arrowColor = strokeColor - CircularProgressIndicator( - progress = 1f, - modifier = indicatorModifier.then(inactiveAlphaModifier), - color = strokeColor, - strokeWidth = IndicatorStrokeWidth, - ) + onClick(ChapterDownloadAction.START) } - Icon( - imageVector = Icons.Default.ArrowDownward, - contentDescription = null, - modifier = arrowModifier, - tint = arrowColor, - ) - DropdownMenu(expanded = isMenuExpanded, onDismissRequest = { isMenuExpanded = false }) { - DropdownMenuItem( - text = { Text(text = stringResource(R.string.action_start_downloading_now)) }, - onClick = { - onClick(ChapterDownloadAction.START_NOW) - isMenuExpanded = false - }, - ) - DropdownMenuItem( - text = { Text(text = stringResource(R.string.action_cancel)) }, - onClick = { - onClick(ChapterDownloadAction.CANCEL) - isMenuExpanded = false - }, - ) - } - } + }, + role = Role.Button, + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple( + bounded = false, + radius = IconButtonTokens.StateLayerSize / 2, + ), + ), + contentAlignment = Alignment.Center, + ) { + if (isDownloaded) { + Icon( + imageVector = Icons.Default.CheckCircle, + contentDescription = null, + modifier = Modifier.size(IndicatorSize), + tint = MaterialTheme.colorScheme.onSurfaceVariant, + ) + DropdownMenu(expanded = isMenuExpanded, onDismissRequest = { isMenuExpanded = false }) { + DropdownMenuItem( + text = { Text(text = stringResource(R.string.action_delete)) }, + onClick = { + onClick(ChapterDownloadAction.DELETE) + isMenuExpanded = false + }, + ) } + } else { + val inactiveAlphaModifier = if (!isDownloading) Modifier.secondaryItemAlpha() else Modifier + val arrowColor: Color + val strokeColor = MaterialTheme.colorScheme.onSurfaceVariant + if (isDownloading) { + val downloadProgress = downloadProgressProvider() + val indeterminate = downloadState == Download.State.QUEUE || + (downloadState == Download.State.DOWNLOADING && downloadProgress == 0) + if (indeterminate) { + arrowColor = strokeColor + CircularProgressIndicator( + modifier = IndicatorModifier, + color = strokeColor, + strokeWidth = IndicatorStrokeWidth, + ) + } else { + val animatedProgress by animateFloatAsState( + targetValue = downloadProgress / 100f, + animationSpec = ProgressIndicatorDefaults.ProgressAnimationSpec, + ) + arrowColor = if (animatedProgress < 0.5f) { + strokeColor + } else { + MaterialTheme.colorScheme.background + } + CircularProgressIndicator( + progress = animatedProgress, + modifier = IndicatorModifier, + color = strokeColor, + strokeWidth = IndicatorSize / 2, + ) + } + DropdownMenu(expanded = isMenuExpanded, onDismissRequest = { isMenuExpanded = false }) { + DropdownMenuItem( + text = { Text(text = stringResource(R.string.action_start_downloading_now)) }, + onClick = { + onClick(ChapterDownloadAction.START_NOW) + isMenuExpanded = false + }, + ) + DropdownMenuItem( + text = { Text(text = stringResource(R.string.action_cancel)) }, + onClick = { + onClick(ChapterDownloadAction.CANCEL) + isMenuExpanded = false + }, + ) + } + } else { + arrowColor = strokeColor + CircularProgressIndicator( + progress = 1f, + modifier = IndicatorModifier.then(inactiveAlphaModifier), + color = strokeColor, + strokeWidth = IndicatorStrokeWidth, + ) + } + Icon( + imageVector = Icons.Default.ArrowDownward, + contentDescription = null, + modifier = ArrowModifier.then(inactiveAlphaModifier), + tint = arrowColor, + ) } } } @@ -154,3 +159,9 @@ private val IndicatorPadding = 2.dp // To match composable parameter name when used later private val IndicatorStrokeWidth = IndicatorPadding + +private val IndicatorModifier = Modifier + .size(IndicatorSize) + .padding(IndicatorPadding) +private val ArrowModifier = Modifier + .size(IndicatorSize - 7.dp) diff --git a/app/src/main/java/eu/kanade/presentation/manga/MangaScreen.kt b/app/src/main/java/eu/kanade/presentation/manga/MangaScreen.kt index 95beb41cca..189df5a8fd 100644 --- a/app/src/main/java/eu/kanade/presentation/manga/MangaScreen.kt +++ b/app/src/main/java/eu/kanade/presentation/manga/MangaScreen.kt @@ -685,8 +685,8 @@ private fun LazyListScope.sharedChapterItems( read = chapter.read, bookmark = chapter.bookmark, selected = selected.contains(chapterItem), - downloadState = downloadState, - downloadProgress = downloadProgress, + downloadStateProvider = { downloadState }, + downloadProgressProvider = { downloadProgress }, onLongClick = { val dispatched = onChapterItemLongClick( chapterItem = chapterItem, diff --git a/app/src/main/java/eu/kanade/presentation/manga/components/MangaChapterListItem.kt b/app/src/main/java/eu/kanade/presentation/manga/components/MangaChapterListItem.kt index f170c795ed..583a0e59b0 100644 --- a/app/src/main/java/eu/kanade/presentation/manga/components/MangaChapterListItem.kt +++ b/app/src/main/java/eu/kanade/presentation/manga/components/MangaChapterListItem.kt @@ -44,8 +44,8 @@ fun MangaChapterListItem( read: Boolean, bookmark: Boolean, selected: Boolean, - downloadState: Download.State, - downloadProgress: Int, + downloadStateProvider: () -> Download.State, + downloadProgressProvider: () -> Int, onLongClick: () -> Unit, onClick: () -> Unit, onDownloadClick: ((ChapterDownloadAction) -> Unit)?, @@ -127,8 +127,8 @@ fun MangaChapterListItem( if (onDownloadClick != null) { ChapterDownloadIndicator( modifier = Modifier.padding(start = 4.dp), - downloadState = downloadState, - downloadProgress = downloadProgress, + downloadStateProvider = downloadStateProvider, + downloadProgressProvider = downloadProgressProvider, onClick = onDownloadClick, ) } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/manga/chapter/ChapterDownloadView.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/manga/chapter/ChapterDownloadView.kt index 593195d35b..049102931e 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/manga/chapter/ChapterDownloadView.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/manga/chapter/ChapterDownloadView.kt @@ -27,8 +27,8 @@ class ChapterDownloadView @JvmOverloads constructor( override fun Content() { TachiyomiTheme { ChapterDownloadIndicator( - downloadState = state, - downloadProgress = progress, + downloadStateProvider = { state }, + downloadProgressProvider = { progress }, onClick = listener, ) }