Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Painter variant of the placeholder APIs #5296

Merged
merged 1 commit into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions integration/compose/api/compose.api
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public final class com/bumptech/glide/integration/compose/GlideImageKt {
public static final fun GlideSubcomposition (Ljava/lang/Object;Landroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V
public static final fun placeholder (I)Lcom/bumptech/glide/integration/compose/Placeholder;
public static final fun placeholder (Landroid/graphics/drawable/Drawable;)Lcom/bumptech/glide/integration/compose/Placeholder;
public static final fun placeholder (Landroidx/compose/ui/graphics/painter/Painter;)Lcom/bumptech/glide/integration/compose/Placeholder;
public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.test.core.app.ApplicationProvider
import com.bumptech.glide.integration.compose.test.GlideComposeRule
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
import com.bumptech.glide.integration.compose.test.expectNoDrawable
import org.junit.Rule
Expand Down Expand Up @@ -143,6 +144,25 @@ class GlideImageErrorTest {
.assert(expectDisplayedResource(failureResourceId))
}

@Test
fun failure_setViaFailureParameterWithPainter_andRequestBuilderTransform_prefersFailurePainter() {
val description = "test"
val failurePainter = context.getDrawable(android.R.drawable.star_big_off).toPainter()
glideComposeRule.setContent {
GlideImage(
model = null,
contentDescription = description,
failure = placeholder(failurePainter),
) {
it.error(android.R.drawable.btn_star)
}
}

glideComposeRule
.onNodeWithContentDescription(description)
.assert(expectDisplayedPainter(failurePainter))
}

@Test
fun failure_setViaFailureParameterWithDrawable_andRequestBuilderTransform_prefersFailureParameter() {
val description = "test"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.test.core.app.ApplicationProvider
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
import com.bumptech.glide.integration.compose.test.expectNoDrawable
import com.bumptech.glide.testutil.TearDownGlide
Expand Down Expand Up @@ -181,6 +182,27 @@ class GlideImagePlaceholderTest {
.assert(expectDisplayedDrawable(placeholderDrawable))
}

@Test
fun loading_setViaLoadingParameterWithPainter_andRequestBuilderTransform_prefersLoadingParameter() {
val description = "test"
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
val placeholderPainter = placeholderDrawable.toPainter()
composeRule.setContent {
GlideImage(
model = waitModel,
contentDescription = description,
loading = placeholder(placeholderPainter),
) {
it.placeholder(android.R.drawable.btn_star)
}
}

composeRule
.onNodeWithContentDescription(description)
.assert(expectDisplayedPainter(placeholderPainter))
}

@Test
fun loading_setViaLoadingParameterWithNullDrawable_andRequestBuilderTransform_showsNoResource() {
val description = "test"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import android.graphics.drawable.BitmapDrawable
import android.graphics.drawable.Drawable
import android.util.TypedValue
import androidx.compose.runtime.MutableState
import androidx.compose.ui.graphics.painter.Painter
import androidx.compose.ui.semantics.SemanticsPropertyKey
import androidx.compose.ui.test.SemanticsMatcher
import androidx.test.core.app.ApplicationProvider
import com.bumptech.glide.integration.compose.DisplayedDrawableKey
import com.bumptech.glide.integration.compose.DisplayedPainterKey
import com.bumptech.glide.integration.ktx.InternalGlideApi
import com.bumptech.glide.integration.ktx.Size
import kotlin.math.roundToInt
Expand Down Expand Up @@ -43,10 +45,10 @@ fun expectDisplayedDrawableSize(expectedSize: Size): SemanticsMatcher =
fun expectDisplayedDrawable(expectedValue: Drawable?): SemanticsMatcher =
expectDisplayedDrawable(expectedValue.bitmapOrThrow(), ::compareBitmaps) { it.bitmapOrThrow() }

fun expectAnimatingDrawable(): SemanticsMatcher =
expectDisplayedDrawable(true) {
(it as Animatable).isRunning
}
fun expectDisplayedPainter(expectedValue: Painter?): SemanticsMatcher =
expectStateValue(
DisplayedPainterKey, expectedValue, {first, second -> first == second}, {value -> value}
)

fun expectNoDrawable(): SemanticsMatcher = expectDisplayedDrawable(null)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ public fun GlideImage(
alpha,
colorFilter,
transition,
loadingPlaceholder = loading?.maybePainter(),
errorPlaceholder = failure?.maybePainter(),
)
)
}
Expand All @@ -167,13 +169,10 @@ public interface GlideSubcompositionScope {

@ExperimentalGlideComposeApi
internal class GlideSubcompositionScopeImpl(
private val drawable: Drawable?,
maybePainter: Painter?,
override val state: RequestState
) : GlideSubcompositionScope {

override val painter: Painter
get() = drawable?.toPainter() ?: ColorPainter(Color.Transparent)

override val painter: Painter = maybePainter ?: ColorPainter(Color.Transparent)
}

/**
Expand Down Expand Up @@ -266,19 +265,19 @@ public fun GlideSubcomposition(
remember(model, requestManager, requestBuilderTransform) {
mutableStateOf(RequestState.Loading)
}
val drawable: MutableState<Drawable?> = remember(model, requestManager, requestBuilderTransform) {
val painter: MutableState<Painter?> = remember(model, requestManager, requestBuilderTransform) {
mutableStateOf(null)
}

val requestListener: StateTrackingListener =
remember(model, requestManager, requestBuilderTransform) {
StateTrackingListener(
requestState,
drawable
painter
)
}

val scope = GlideSubcompositionScopeImpl(drawable.value, requestState.value)
val scope = GlideSubcompositionScopeImpl(painter.value, requestState.value)

Box(
modifier
Expand All @@ -295,12 +294,12 @@ public fun GlideSubcomposition(
@ExperimentalGlideComposeApi
private class StateTrackingListener(
val state: MutableState<RequestState>,
val drawable: MutableState<Drawable?>
val painter: MutableState<Painter?>
) : RequestListener {

override fun onStateChanged(model: Any?, drawable: Drawable?, requestState: RequestState) {
override fun onStateChanged(model: Any?, painter: Painter?, requestState: RequestState) {
state.value = requestState
this.drawable.value = drawable
this.painter.value = painter
}
}

Expand All @@ -311,15 +310,18 @@ private fun PreviewResourceOrDrawable(
contentDescription: String?,
modifier: Modifier,
) {
val drawable =
val painter =
when (loading) {
is Placeholder.OfDrawable -> loading.drawable
is Placeholder.OfResourceId -> LocalContext.current.getDrawable(loading.resourceId)
is Placeholder.OfDrawable -> loading.drawable.toPainter()
is Placeholder.OfResourceId ->
LocalContext.current.getDrawable(loading.resourceId).toPainter()

is Placeholder.OfPainter -> loading.painter
is Placeholder.OfComposable ->
throw IllegalArgumentException("Composables should go through the production codepath")
}
Image(
painter = remember(drawable) { drawable.toPainter() },
painter = painter,
modifier = modifier,
contentDescription = contentDescription,
)
Expand Down Expand Up @@ -348,6 +350,14 @@ public fun placeholder(drawable: Drawable?): Placeholder = Placeholder.OfDrawabl
public fun placeholder(@DrawableRes resourceId: Int): Placeholder =
Placeholder.OfResourceId(resourceId)

/**
* Used to specify a [Painter] to use in conjunction with [GlideImage]'s `loading` or `failure`
* parameters.
*/
@ExperimentalGlideComposeApi
public fun placeholder(painter: Painter?): Placeholder =
Placeholder.OfPainter(painter ?: ColorPainter(Color.Transparent))

/**
* Used to specify a [Composable] function to use in conjunction with [GlideImage]'s `loading` or
* `failure` parameter.
Expand Down Expand Up @@ -380,13 +390,16 @@ public fun placeholder(composable: @Composable () -> Unit): Placeholder =
public sealed class Placeholder {
internal class OfDrawable(internal val drawable: Drawable?) : Placeholder()
internal class OfResourceId(@DrawableRes internal val resourceId: Int) : Placeholder()

internal class OfPainter(internal val painter: Painter) : Placeholder()
internal class OfComposable(internal val composable: @Composable () -> Unit) : Placeholder()

internal fun isResourceOrDrawable() =
when (this) {
is OfDrawable -> true
is OfResourceId -> true
is OfComposable -> false
is OfPainter -> false
}

internal fun maybeComposable(): (@Composable () -> Unit)? =
Expand All @@ -395,6 +408,12 @@ public sealed class Placeholder {
else -> null
}

internal fun maybePainter() =
when (this) {
is OfPainter -> this.painter
else -> null
}

internal fun <T> apply(
resource: (Int) -> RequestBuilder<T>,
drawable: (Drawable?) -> RequestBuilder<T>
Expand Down
Loading
Loading