From 523312512d294c44c4fe91d976009c57473f5a48 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Fri, 12 Apr 2024 18:10:22 +0200 Subject: [PATCH 1/3] Improve CircularProgressIndicator code (#352) We only use the launched effect to load the new frames, and instead use an infinitely repeating animation for the actual frame selection. This is done so that UI tests don't hang on the infinite work done in the launched effect. We also now allow providing a dispatcher on which we should load the frames, which defaults to Default, but can be overridden in tests with a TestDispatcher. --- ui/api/ui.api | 4 +- .../ui/component/CircularProgressIndicator.kt | 74 ++++++++++++------- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/ui/api/ui.api b/ui/api/ui.api index beca9a392..745955d6e 100644 --- a/ui/api/ui.api +++ b/ui/api/ui.api @@ -221,8 +221,8 @@ public final class org/jetbrains/jewel/ui/component/ChipState$Companion { } public final class org/jetbrains/jewel/ui/component/CircularProgressIndicatorKt { - public static final fun CircularProgressIndicator (Landroidx/compose/ui/Modifier;Lorg/jetbrains/jewel/ui/component/styling/CircularProgressStyle;Landroidx/compose/runtime/Composer;II)V - public static final fun CircularProgressIndicatorBig (Landroidx/compose/ui/Modifier;Lorg/jetbrains/jewel/ui/component/styling/CircularProgressStyle;Landroidx/compose/runtime/Composer;II)V + public static final fun CircularProgressIndicator (Landroidx/compose/ui/Modifier;Lorg/jetbrains/jewel/ui/component/styling/CircularProgressStyle;Lkotlinx/coroutines/CoroutineDispatcher;Landroidx/compose/runtime/Composer;II)V + public static final fun CircularProgressIndicatorBig (Landroidx/compose/ui/Modifier;Lorg/jetbrains/jewel/ui/component/styling/CircularProgressStyle;Lkotlinx/coroutines/CoroutineDispatcher;Landroidx/compose/runtime/Composer;II)V } public final class org/jetbrains/jewel/ui/component/ComposableSingletons$MenuKt { diff --git a/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt b/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt index 563c40166..6b0af39b6 100644 --- a/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt +++ b/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt @@ -1,10 +1,18 @@ package org.jetbrains.jewel.ui.component +import androidx.compose.animation.core.InfiniteRepeatableSpec +import androidx.compose.animation.core.LinearEasing +import androidx.compose.animation.core.RepeatMode +import androidx.compose.animation.core.VectorConverter +import androidx.compose.animation.core.animateValue +import androidx.compose.animation.core.rememberInfiniteTransition +import androidx.compose.animation.core.tween import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.size import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -16,7 +24,9 @@ import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.loadSvgPainter import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp -import kotlinx.coroutines.delay +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import org.jetbrains.jewel.foundation.theme.JewelTheme import org.jetbrains.jewel.ui.component.styling.CircularProgressStyle import org.jetbrains.jewel.ui.theme.circularProgressStyle @@ -26,11 +36,13 @@ import org.jetbrains.jewel.ui.util.toRgbaHexString public fun CircularProgressIndicator( modifier: Modifier = Modifier, style: CircularProgressStyle = JewelTheme.circularProgressStyle, + loadingDispatcher: CoroutineDispatcher = Dispatchers.Default, ) { CircularProgressIndicatorImpl( modifier = modifier, iconSize = DpSize(16.dp, 16.dp), style = style, + dispatcher = loadingDispatcher, frameRetriever = { color -> SpinnerProgressIconGenerator.Small.generateSvgFrames(color.toRgbaHexString()) }, @@ -41,11 +53,13 @@ public fun CircularProgressIndicator( public fun CircularProgressIndicatorBig( modifier: Modifier = Modifier, style: CircularProgressStyle = JewelTheme.circularProgressStyle, + loadingDispatcher: CoroutineDispatcher = Dispatchers.Default, ) { CircularProgressIndicatorImpl( modifier = modifier, iconSize = DpSize(32.dp, 32.dp), style = style, + dispatcher = loadingDispatcher, frameRetriever = { color -> SpinnerProgressIconGenerator.Big.generateSvgFrames(color.toRgbaHexString()) }, @@ -57,38 +71,48 @@ private fun CircularProgressIndicatorImpl( modifier: Modifier = Modifier, iconSize: DpSize, style: CircularProgressStyle, + dispatcher: CoroutineDispatcher, frameRetriever: (Color) -> List, ) { val defaultColor = if (JewelTheme.isDark) Color(0xFF6F737A) else Color(0xFFA8ADBD) - var isFrameReady by remember { mutableStateOf(false) } - var currentFrame: Painter? by remember { mutableStateOf(null) } - val currentPainter = currentFrame - - if (currentPainter == null) { - Box(modifier.size(iconSize)) - } else { - Icon( - modifier = modifier.size(iconSize), - painter = currentPainter, - contentDescription = null, - ) - } + val frames = remember { mutableStateListOf() } + var framesCount by remember { mutableStateOf(0) } val density = LocalDensity.current LaunchedEffect(density, style.color) { - val frames = frameRetriever(style.color.takeOrElse { defaultColor }) - .map { - loadSvgPainter(it.byteInputStream(), density) - } - - while (true) { - for (i in frames.indices) { - currentFrame = frames[i] - isFrameReady = true - delay(style.frameTime.inWholeMilliseconds) - } + launch(dispatcher) { + frames.clear() + frames.addAll( + frameRetriever(style.color.takeOrElse { defaultColor }).map { + loadSvgPainter(it.byteInputStream(), density) + }, + ) + framesCount = frames.size } } + + if (framesCount == 0) { + Box(modifier.size(iconSize)) + } else { + val transition = rememberInfiniteTransition("CircularProgressIndicator") + val currentIndex by + transition.animateValue( + initialValue = 0, + targetValue = framesCount, + typeConverter = Int.VectorConverter, + animationSpec = + InfiniteRepeatableSpec( + tween( + easing = LinearEasing, + durationMillis = (style.frameTime.inWholeMilliseconds * framesCount).toInt(), + ), + repeatMode = RepeatMode.Restart, + ), + ) + + val currentPainter = frames[currentIndex] + Icon(modifier = modifier.size(iconSize), painter = currentPainter, contentDescription = null) + } } private object SpinnerProgressIconGenerator { From 955ac6b9e8175308125af75866c36e82672593aa Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Fri, 12 Apr 2024 18:14:21 +0200 Subject: [PATCH 2/3] Bump version to 0.17.2 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 2d7c90b60..dc83fd79b 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,4 +7,4 @@ kotlin.stdlib.default.dependency=false kotlin.incremental.useClasspathSnapshot=false ijp.target=241 -jewel.release.version=0.17.1 +jewel.release.version=0.17.2 From a51ae1f4a3a66b2d554e35658053698c4347f5ab Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Fri, 12 Apr 2024 18:15:32 +0200 Subject: [PATCH 3/3] Add defaultColor as effect key in circular indicator That could also change based on the isDark value, so while style.color will likely be a good proxy for that in 99.9% of cases, this is a better safe than sorry tweak. --- .../jetbrains/jewel/ui/component/CircularProgressIndicator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt b/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt index 6b0af39b6..a92731eeb 100644 --- a/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt +++ b/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt @@ -79,7 +79,7 @@ private fun CircularProgressIndicatorImpl( var framesCount by remember { mutableStateOf(0) } val density = LocalDensity.current - LaunchedEffect(density, style.color) { + LaunchedEffect(density, style.color, defaultColor) { launch(dispatcher) { frames.clear() frames.addAll(