Skip to content

Commit

Permalink
Fix crash in CircularProgressIndicator when switching themes
Browse files Browse the repository at this point in the history
Fixes #359 by making sure we only have one source of truth when it comes
to the number of frames in the indicator.

Having the frame count as a separate state might lead to unfortunate
race conditions, where the frames array has changed but the count hasn't
yet, causing index out of bounds error. Now, the frames count is only
computed when needed, synchronously, in the composition.

I have been furiously switching themes for over a minute and can't make
the component crash anymore.
  • Loading branch information
rock3r committed Apr 20, 2024
1 parent c3cb6b8 commit 84f8f56
Showing 1 changed file with 2 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ 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
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.painter.Painter
Expand Down Expand Up @@ -76,7 +74,6 @@ private fun CircularProgressIndicatorImpl(
) {
val defaultColor = if (JewelTheme.isDark) Color(0xFF6F737A) else Color(0xFFA8ADBD)
val frames = remember { mutableStateListOf<Painter>() }
var framesCount by remember { mutableStateOf(0) }

val density = LocalDensity.current
LaunchedEffect(density, style.color, defaultColor) {
Expand All @@ -87,13 +84,13 @@ private fun CircularProgressIndicatorImpl(
loadSvgPainter(it.byteInputStream(), density)
},
)
framesCount = frames.size
}
}

if (framesCount == 0) {
if (frames.isEmpty()) {
Box(modifier.size(iconSize))
} else {
val framesCount = frames.size
val transition = rememberInfiniteTransition("CircularProgressIndicator")
val currentIndex by
transition.animateValue(
Expand Down

0 comments on commit 84f8f56

Please sign in to comment.