Skip to content

Commit d5fc241

Browse files
committedOct 25, 2022
Add loading and failure parameters to GlideImage
These parameters will override equivalent values set via requestBuilderTransform if the parameters are non-null. If the parameters are null, then we'll apply whatever is specified by requestBuilderTransform. I plan to add similar explicit parameters for other values settable via requestBuilderTransform over time with similar override behavior unless someone objects. The `fallback` on `RequestBuilder` is not replicated. Callers can use `failure` or if/else to provide custom behavior for null models. Similarly `loading` is only shown while the request is loading. Unlike `RequestBuilder.placeholder`, `loading` will never be shown after the load fail. `failure` should instead be used to explicitly duplicate the `loading` behavior if that functionality is desired.
1 parent 4298bb7 commit d5fc241

File tree

13 files changed

+686
-100
lines changed

13 files changed

+686
-100
lines changed
 

‎instrumentation/src/androidTest/java/com/bumptech/glide/CachingTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
import com.bumptech.glide.test.GlideApp;
3232
import com.bumptech.glide.test.ResourceIds;
3333
import com.bumptech.glide.test.ResourceIds.raw;
34-
import com.bumptech.glide.test.WaitModelLoader;
35-
import com.bumptech.glide.test.WaitModelLoader.WaitModel;
34+
import com.bumptech.glide.testutil.WaitModelLoader;
35+
import com.bumptech.glide.testutil.WaitModelLoader.WaitModel;
3636
import com.bumptech.glide.testutil.ConcurrencyHelper;
3737
import com.bumptech.glide.testutil.TearDownGlide;
3838
import com.google.common.truth.Truth;
@@ -308,7 +308,7 @@ public void clearDiskCache_doesNotPreventFutureLoads() {
308308
// Tests #2428.
309309
@Test
310310
public void onlyRetrieveFromCache_withPreviousRequestLoadingFromSource_doesNotBlock() {
311-
final WaitModel<Integer> waitModel = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
311+
final WaitModel<Integer> waitModel = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
312312

313313
FutureTarget<Drawable> loadFromSourceFuture = GlideApp.with(context).load(waitModel).submit();
314314

‎instrumentation/src/androidTest/java/com/bumptech/glide/ErrorHandlingTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import com.bumptech.glide.request.RequestListener;
2626
import com.bumptech.glide.request.target.Target;
2727
import com.bumptech.glide.test.ResourceIds;
28-
import com.bumptech.glide.test.WaitModelLoader;
29-
import com.bumptech.glide.test.WaitModelLoader.WaitModel;
28+
import com.bumptech.glide.testutil.WaitModelLoader;
29+
import com.bumptech.glide.testutil.WaitModelLoader.WaitModel;
3030
import com.bumptech.glide.testutil.ConcurrencyHelper;
3131
import com.bumptech.glide.testutil.TearDownGlide;
3232
import java.io.File;
@@ -114,7 +114,7 @@ public void load_whenLoadSucceeds_butEncoderFails_doesNotCallOnLoadFailed() {
114114

115115
@Test
116116
public void clearRequest_withError_afterPrimaryFails_clearsErrorRequest() {
117-
WaitModel<Integer> errorModel = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
117+
WaitModel<Integer> errorModel = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
118118

119119
FutureTarget<Drawable> target =
120120
Glide.with(context)

‎instrumentation/src/androidTest/java/com/bumptech/glide/RequestTest.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
import com.bumptech.glide.request.target.Target;
1919
import com.bumptech.glide.test.GlideApp;
2020
import com.bumptech.glide.test.ResourceIds;
21-
import com.bumptech.glide.test.WaitModelLoader;
22-
import com.bumptech.glide.test.WaitModelLoader.WaitModel;
21+
import com.bumptech.glide.testutil.WaitModelLoader;
22+
import com.bumptech.glide.testutil.WaitModelLoader.WaitModel;
2323
import com.bumptech.glide.testutil.ConcurrencyHelper;
2424
import com.bumptech.glide.testutil.TearDownGlide;
2525
import org.junit.Before;
@@ -111,7 +111,7 @@ public void run() {
111111

112112
@Test
113113
public void onStop_withSingleRequestInProgress_nullsOutDrawableInView() {
114-
final WaitModel<Integer> model = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
114+
final WaitModel<Integer> model = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
115115
concurrency.runOnMainThread(
116116
new Runnable() {
117117
@Override
@@ -132,7 +132,7 @@ public void run() {
132132

133133
@Test
134134
public void onStop_withRequestWithThumbnailBothInProgress_nullsOutDrawableInView() {
135-
final WaitModel<Integer> model = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
135+
final WaitModel<Integer> model = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
136136
concurrency.runOnMainThread(
137137
new Runnable() {
138138
@Override
@@ -158,7 +158,7 @@ public void run() {
158158
/** Tests #2555. */
159159
@Test
160160
public void clear_withRequestWithOnlyFullInProgress_nullsOutDrawableInView() {
161-
final WaitModel<Integer> mainModel = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
161+
final WaitModel<Integer> mainModel = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
162162
concurrency.loadUntilFirstFinish(
163163
GlideApp.with(context)
164164
.load(mainModel)
@@ -198,7 +198,7 @@ public void run() {
198198

199199
@Test
200200
public void clear_withRequestWithOnlyFullInProgress_doesNotNullOutDrawableInView() {
201-
final WaitModel<Integer> mainModel = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
201+
final WaitModel<Integer> mainModel = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
202202
concurrency.loadUntilFirstFinish(
203203
GlideApp.with(context)
204204
.load(mainModel)
@@ -238,7 +238,7 @@ public void run() {
238238

239239
@Test
240240
public void onStop_withRequestWithOnlyThumbnailInProgress_doesNotNullOutDrawableInView() {
241-
final WaitModel<Integer> thumbModel = WaitModelLoader.Factory.waitOn(ResourceIds.raw.canonical);
241+
final WaitModel<Integer> thumbModel = WaitModelLoader.waitOn(ResourceIds.raw.canonical);
242242
concurrency.loadUntilFirstFinish(
243243
GlideApp.with(context)
244244
.load(ResourceIds.raw.canonical)

‎integration/compose/api/compose.api

+8-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,14 @@ public abstract interface annotation class com/bumptech/glide/integration/compos
22
}
33

44
public final class com/bumptech/glide/integration/compose/GlideImageKt {
5-
public static final fun GlideImage (Ljava/lang/Object;Ljava/lang/String;Landroidx/compose/ui/Modifier;Landroidx/compose/ui/Alignment;Landroidx/compose/ui/layout/ContentScale;FLandroidx/compose/ui/graphics/ColorFilter;Lkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
5+
public static final fun GlideImage (Ljava/lang/Object;Ljava/lang/String;Landroidx/compose/ui/Modifier;Landroidx/compose/ui/Alignment;Landroidx/compose/ui/layout/ContentScale;FLandroidx/compose/ui/graphics/ColorFilter;Lcom/bumptech/glide/integration/compose/Placeholder;Lcom/bumptech/glide/integration/compose/Placeholder;Lkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
6+
public static final fun placeholder (I)Lcom/bumptech/glide/integration/compose/Placeholder;
7+
public static final fun placeholder (Landroid/graphics/drawable/Drawable;)Lcom/bumptech/glide/integration/compose/Placeholder;
8+
public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder;
9+
}
10+
11+
public abstract class com/bumptech/glide/integration/compose/Placeholder {
12+
public static final field $stable I
613
}
714

815
public final class com/bumptech/glide/integration/compose/PreloadKt {

‎integration/compose/build.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ dependencies {
6161
androidTestImplementation "androidx.test.espresso.idling:idling-concurrent:$ANDROID_X_TEST_ESPRESSO_VERSION"
6262
androidTestImplementation "androidx.test.ext:junit:$ANDROID_X_TEST_JUNIT_VERSION"
6363
androidTestImplementation "androidx.compose.material:material:$ANDROID_X_COMPOSE_VERSION"
64+
androidTestImplementation project(':testutil')
6465
}
6566

6667
apply from: "${rootProject.projectDir}/scripts/upload.gradle"

‎integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideComposeTest.kt

+8-44
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,15 @@
33
package com.bumptech.glide.integration.compose
44

55
import android.content.Context
6-
import android.graphics.drawable.BitmapDrawable
76
import android.graphics.drawable.Drawable
87
import androidx.compose.foundation.layout.Column
98
import androidx.compose.foundation.layout.size
10-
import androidx.compose.runtime.MutableState
119
import androidx.compose.runtime.mutableStateOf
1210
import androidx.compose.material.Text
1311
import androidx.compose.material.TextButton
1412
import androidx.compose.runtime.remember
1513
import androidx.compose.ui.Modifier
1614
import androidx.compose.ui.platform.LocalDensity
17-
import androidx.compose.ui.semantics.SemanticsPropertyKey
18-
import androidx.compose.ui.test.SemanticsMatcher
1915
import androidx.compose.ui.test.assert
2016
import androidx.compose.ui.test.junit4.createComposeRule
2117
import androidx.compose.ui.test.onNodeWithContentDescription
@@ -24,29 +20,28 @@ import androidx.compose.ui.test.performClick
2420
import androidx.compose.ui.unit.dp
2521
import androidx.test.core.app.ApplicationProvider
2622
import com.bumptech.glide.Glide
23+
import com.bumptech.glide.integration.compose.test.bitmapSize
24+
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
25+
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawableSize
2726
import com.bumptech.glide.integration.ktx.InternalGlideApi
2827
import com.bumptech.glide.integration.ktx.Size
2928
import com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit
29+
import com.bumptech.glide.testutil.TearDownGlide
3030
import java.util.concurrent.atomic.AtomicReference
31-
import org.junit.After
3231
import org.junit.Before
3332
import org.junit.Rule
3433
import org.junit.Test
3534

3635
class GlideComposeTest {
3736
private val context: Context = ApplicationProvider.getApplicationContext()
38-
@get:Rule val composeRule = createComposeRule()
37+
@get:Rule(order = 1) val composeRule = createComposeRule()
38+
@get:Rule(order = 2) val tearDownGlide = TearDownGlide()
3939

4040
@Before
4141
fun setUp() {
4242
GlideIdlingResourceInit.initGlide(composeRule)
4343
}
4444

45-
@After
46-
fun tearDown() {
47-
Glide.tearDown()
48-
}
49-
5045
@Test
5146
fun glideImage_noModifierSize_resourceDrawable_displaysDrawable() {
5247
val description = "test"
@@ -109,10 +104,9 @@ class GlideComposeTest {
109104
composeRule.onNodeWithText("Swap").performClick()
110105
composeRule.waitForIdle()
111106

112-
val fullsizeBitmap = (secondDrawable as BitmapDrawable).bitmap
113107
composeRule
114108
.onNodeWithContentDescription(description)
115-
.assert(expectDisplayedDrawable(fullsizeBitmap) { (it as BitmapDrawable).bitmap })
109+
.assert(expectDisplayedDrawable(secondDrawable))
116110
}
117111

118112
@Test
@@ -148,8 +142,6 @@ class GlideComposeTest {
148142
val thumbnailDrawable = context.getDrawable(android.R.drawable.star_big_off)
149143
val fullsizeDrawable = context.getDrawable(android.R.drawable.star_big_on)
150144

151-
val fullsizeBitmap = (fullsizeDrawable as BitmapDrawable).bitmap
152-
153145
composeRule.setContent {
154146
GlideImage(
155147
model = fullsizeDrawable,
@@ -162,34 +154,6 @@ class GlideComposeTest {
162154

163155
composeRule
164156
.onNodeWithContentDescription(description)
165-
.assert(expectDisplayedDrawable(fullsizeBitmap) { (it as BitmapDrawable).bitmap })
157+
.assert(expectDisplayedDrawable(fullsizeDrawable))
166158
}
167-
168-
private fun Int.bitmapSize() = context.resources.getDrawable(this, context.theme).size()
169159
}
170-
171-
private fun Drawable.size() = (this as BitmapDrawable).bitmap.let { Size(it.width, it.height) }
172-
173-
private fun expectDisplayedDrawableSize(widthPixels: Int, heightPixels: Int): SemanticsMatcher =
174-
expectDisplayedDrawable(Size(widthPixels, heightPixels)) { it?.size() }
175-
176-
private fun expectDisplayedDrawableSize(expectedSize: Size): SemanticsMatcher =
177-
expectDisplayedDrawable(expectedSize) { it?.size() }
178-
179-
private fun <ValueT> expectDisplayedDrawable(
180-
expectedValue: ValueT,
181-
transform: (Drawable?) -> ValueT
182-
): SemanticsMatcher = expectStateValue(DisplayedDrawableKey, expectedValue) { transform(it) }
183-
184-
private fun <ValueT, TransformedValueT> expectStateValue(
185-
key: SemanticsPropertyKey<MutableState<ValueT?>>,
186-
expectedValue: TransformedValueT,
187-
transform: (ValueT?) -> TransformedValueT?
188-
): SemanticsMatcher =
189-
SemanticsMatcher("${key.name} = '$expectedValue'") {
190-
val value = transform(it.config.getOrElseNullable(key) { null }?.value)
191-
if (value != expectedValue) {
192-
throw AssertionError("Expected: $expectedValue, but was: $value")
193-
}
194-
true
195-
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
@file:OptIn(ExperimentalGlideComposeApi::class)
2+
3+
package com.bumptech.glide.integration.compose
4+
5+
import android.content.Context
6+
import android.graphics.drawable.Drawable
7+
import androidx.compose.ui.test.assert
8+
import androidx.compose.ui.test.junit4.createComposeRule
9+
import androidx.compose.ui.test.onNodeWithContentDescription
10+
import androidx.test.core.app.ApplicationProvider
11+
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
12+
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
13+
import com.bumptech.glide.integration.compose.test.expectNoDrawable
14+
import com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit
15+
import com.bumptech.glide.testutil.TearDownGlide
16+
import org.junit.Before
17+
import org.junit.Rule
18+
import org.junit.Test
19+
20+
/**
21+
* Avoids [com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit] because we want to make
22+
* assertions about loads that have not yet completed.
23+
*/
24+
class GlideImageErrorTest {
25+
private val context: Context = ApplicationProvider.getApplicationContext()
26+
@get:Rule(order = 1) val composeRule = createComposeRule()
27+
@get:Rule(order = 2) val tearDownGlide = TearDownGlide()
28+
29+
@Before
30+
public fun before() {
31+
GlideIdlingResourceInit.initGlide(composeRule)
32+
}
33+
34+
@Test
35+
fun requestBuilderTransform_withErrorResourceId_displaysError() {
36+
val description = "test"
37+
val errorResourceId = android.R.drawable.star_big_off
38+
composeRule.setContent {
39+
GlideImage(model = null, contentDescription = description) {
40+
it.error(errorResourceId)
41+
}
42+
}
43+
44+
composeRule.onNodeWithContentDescription(description)
45+
.assert(expectDisplayedResource(errorResourceId))
46+
}
47+
48+
@Test
49+
fun requestBuilderTransform_withErrorDrawable_displaysError() {
50+
val description = "test"
51+
val errorDrawable = context.getDrawable(android.R.drawable.star_big_off)
52+
composeRule.setContent {
53+
GlideImage(model = null, contentDescription = description) {
54+
it.error(errorDrawable)
55+
}
56+
}
57+
58+
composeRule.onNodeWithContentDescription(description)
59+
.assert(expectDisplayedDrawable(errorDrawable))
60+
}
61+
62+
@Test
63+
fun failureParameter_withErrorResourceId_displaysError() {
64+
val description = "test"
65+
val failureResourceId = android.R.drawable.star_big_off
66+
composeRule.setContent {
67+
GlideImage(
68+
model = null,
69+
contentDescription = description,
70+
failure = placeholder(failureResourceId),
71+
)
72+
}
73+
74+
composeRule.onNodeWithContentDescription(description)
75+
.assert(expectDisplayedResource(failureResourceId))
76+
}
77+
78+
@Test
79+
fun failureParameter_withDrawable_displaysDrawable() {
80+
val description = "test"
81+
val failureDrawable = context.getDrawable(android.R.drawable.star_big_off)
82+
composeRule.setContent {
83+
GlideImage(
84+
model = null,
85+
contentDescription = description,
86+
failure = placeholder(failureDrawable),
87+
)
88+
}
89+
90+
composeRule.onNodeWithContentDescription(description)
91+
.assert(expectDisplayedDrawable(failureDrawable))
92+
}
93+
94+
@Test
95+
fun failureParameter_withNullDrawable_displaysNothing() {
96+
val description = "test"
97+
composeRule.setContent {
98+
GlideImage(
99+
model = null,
100+
contentDescription = description,
101+
failure = placeholder(null as Drawable?)
102+
)
103+
}
104+
105+
composeRule.onNodeWithContentDescription(description).assert(expectNoDrawable())
106+
}
107+
108+
@Test
109+
fun failureParameter_withComposable_displaysComposable() {
110+
val failureResourceId = android.R.drawable.star_big_off
111+
val description = "test"
112+
composeRule.setContent {
113+
GlideImage(
114+
model = null,
115+
contentDescription = "none",
116+
failure = placeholder {
117+
// Nesting GlideImage is not really a good idea, but it's convenient for this test because
118+
// we can use our helpers to assert on its contents.
119+
GlideImage(
120+
model = null,
121+
contentDescription = description,
122+
failure = placeholder(failureResourceId),
123+
)
124+
}
125+
)
126+
}
127+
128+
composeRule.onNodeWithContentDescription(description)
129+
.assert(expectDisplayedResource(failureResourceId))
130+
}
131+
132+
@Test
133+
fun failure_setViaFailureParameterWithResourceId_andRequestBuilderTransform_prefersFailureParameter() {
134+
val description = "test"
135+
val failureResourceId = android.R.drawable.star_big_off
136+
composeRule.setContent {
137+
GlideImage(
138+
model = null,
139+
contentDescription = description,
140+
failure = placeholder(failureResourceId),
141+
) {
142+
it.error(android.R.drawable.btn_star)
143+
}
144+
}
145+
146+
composeRule.onNodeWithContentDescription(description)
147+
.assert(expectDisplayedResource(failureResourceId))
148+
}
149+
150+
@Test
151+
fun failure_setViaFailureParameterWithDrawable_andRequestBuilderTransform_prefersFailureParameter() {
152+
val description = "test"
153+
val failureDrawable = context.getDrawable(android.R.drawable.star_big_off)
154+
composeRule.setContent {
155+
GlideImage(
156+
model = null,
157+
contentDescription = description,
158+
failure = placeholder(failureDrawable),
159+
) {
160+
it.error(android.R.drawable.btn_star)
161+
}
162+
}
163+
164+
composeRule.onNodeWithContentDescription(description)
165+
.assert(expectDisplayedDrawable(failureDrawable))
166+
}
167+
168+
@Test
169+
fun failure_setViaFailureParameterWithNullDrawable_andRequestBuilderTransformWithNonNullDrawable_showsNoPlaceholder() {
170+
val description = "test"
171+
composeRule.setContent {
172+
GlideImage(
173+
model = null,
174+
contentDescription = description,
175+
failure = placeholder(null as Drawable?),
176+
) {
177+
it.error(android.R.drawable.btn_star)
178+
}
179+
}
180+
181+
composeRule.onNodeWithContentDescription(description)
182+
.assert(expectNoDrawable())
183+
}
184+
185+
@Test
186+
fun failure_setViaFailureParameterWithComposable_andRequestBuilderTransform_showsComposable() {
187+
val description = "test"
188+
val failureResourceId = android.R.drawable.star_big_off
189+
composeRule.setContent {
190+
GlideImage(
191+
model = null,
192+
contentDescription = "other",
193+
failure = placeholder {
194+
GlideImage(
195+
model = null,
196+
contentDescription = description,
197+
failure = placeholder(failureResourceId),
198+
)
199+
},
200+
) {
201+
it.error(android.R.drawable.btn_star)
202+
}
203+
}
204+
205+
composeRule.onNodeWithContentDescription(description)
206+
.assert(expectDisplayedResource(failureResourceId))
207+
}
208+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
@file:OptIn(ExperimentalGlideComposeApi::class)
2+
3+
package com.bumptech.glide.integration.compose
4+
5+
import android.content.Context
6+
import android.graphics.drawable.Drawable
7+
import androidx.compose.ui.test.assert
8+
import androidx.compose.ui.test.junit4.createComposeRule
9+
import androidx.compose.ui.test.onNodeWithContentDescription
10+
import androidx.test.core.app.ApplicationProvider
11+
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
12+
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
13+
import com.bumptech.glide.integration.compose.test.expectNoDrawable
14+
import com.bumptech.glide.testutil.TearDownGlide
15+
import com.bumptech.glide.testutil.WaitModelLoaderRule
16+
import org.junit.Rule
17+
import org.junit.Test
18+
19+
/**
20+
* Avoids [com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit] because we want to make
21+
* assertions about loads that have not yet completed.
22+
*/
23+
class GlideImagePlaceholderTest {
24+
private val context: Context = ApplicationProvider.getApplicationContext()
25+
@get:Rule(order = 1) val composeRule = createComposeRule()
26+
@get:Rule(order = 2) val waitModelLoaderRule = WaitModelLoaderRule()
27+
@get:Rule(order = 3) val tearDownGlide = TearDownGlide()
28+
29+
@Test
30+
fun requestBuilderTransform_withPlaceholderResourceId_displaysPlaceholder() {
31+
val description = "test"
32+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
33+
val placeholderResourceId = android.R.drawable.star_big_off
34+
composeRule.setContent {
35+
GlideImage(model = waitModel, contentDescription = description) {
36+
it.placeholder(placeholderResourceId)
37+
}
38+
}
39+
40+
composeRule.onNodeWithContentDescription(description)
41+
.assert(expectDisplayedResource(placeholderResourceId))
42+
}
43+
44+
@Test
45+
fun requestBuilderTransform_withPlaceholderDrawable_displaysPlaceholder() {
46+
val description = "test"
47+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
48+
val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
49+
composeRule.setContent {
50+
GlideImage(model = waitModel, contentDescription = description) {
51+
it.placeholder(placeholderDrawable)
52+
}
53+
}
54+
55+
composeRule.onNodeWithContentDescription(description)
56+
.assert(expectDisplayedDrawable(placeholderDrawable))
57+
}
58+
59+
@Test
60+
fun loadingParameter_withResourceId_displaysResource() {
61+
val description = "test"
62+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
63+
val placeholderResourceId = android.R.drawable.star_big_off
64+
composeRule.setContent {
65+
GlideImage(
66+
model = waitModel,
67+
contentDescription = description,
68+
loading = placeholder(placeholderResourceId),
69+
)
70+
}
71+
72+
composeRule.onNodeWithContentDescription(description)
73+
.assert(expectDisplayedResource(placeholderResourceId))
74+
}
75+
76+
@Test
77+
fun loadingParameter_withDrawable_displaysResource() {
78+
val description = "test"
79+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
80+
val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
81+
composeRule.setContent {
82+
GlideImage(
83+
model = waitModel,
84+
contentDescription = description,
85+
loading = placeholder(placeholderDrawable),
86+
)
87+
}
88+
89+
composeRule.onNodeWithContentDescription(description)
90+
.assert(expectDisplayedDrawable(placeholderDrawable))
91+
}
92+
93+
@Test
94+
fun loadingParameter_withNullDrawable_displaysNothing() {
95+
val description = "test"
96+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
97+
composeRule.setContent {
98+
GlideImage(
99+
model = waitModel,
100+
contentDescription = description,
101+
loading = placeholder(null as Drawable?)
102+
)
103+
}
104+
105+
composeRule.onNodeWithContentDescription(description).assert(expectNoDrawable())
106+
}
107+
108+
@Test
109+
fun loadingParameter_withComposable_displaysComposable() {
110+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
111+
val placeholderResourceId = android.R.drawable.star_big_off
112+
val description = "test"
113+
composeRule.setContent {
114+
GlideImage(
115+
model = waitModel,
116+
contentDescription = "none",
117+
loading = placeholder {
118+
// Nesting GlideImage is not really a good idea, but it's convenient for this test because
119+
// we can use our helpers to assert on its contents.
120+
GlideImage(
121+
model = waitModel,
122+
contentDescription = description,
123+
loading = placeholder(placeholderResourceId),
124+
)
125+
}
126+
)
127+
}
128+
129+
composeRule.onNodeWithContentDescription(description)
130+
.assert(expectDisplayedResource(placeholderResourceId))
131+
}
132+
133+
@Test
134+
fun loading_setViaLoadingParameterWithResourceId_andRequestBuilderTransform_prefersLoadingParameter() {
135+
val description = "test"
136+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
137+
val placeholderResourceId = android.R.drawable.star_big_off
138+
composeRule.setContent {
139+
GlideImage(
140+
model = waitModel,
141+
contentDescription = description,
142+
loading = placeholder(placeholderResourceId),
143+
) {
144+
it.placeholder(android.R.drawable.btn_star)
145+
}
146+
}
147+
148+
composeRule.onNodeWithContentDescription(description)
149+
.assert(expectDisplayedResource(placeholderResourceId))
150+
}
151+
152+
@Test
153+
fun loading_setViaLoadingParameterWithDrawable_andRequestBuilderTransform_prefersLoadingParameter() {
154+
val description = "test"
155+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
156+
val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
157+
composeRule.setContent {
158+
GlideImage(
159+
model = waitModel,
160+
contentDescription = description,
161+
loading = placeholder(placeholderDrawable),
162+
) {
163+
it.placeholder(android.R.drawable.btn_star)
164+
}
165+
}
166+
167+
composeRule.onNodeWithContentDescription(description)
168+
.assert(expectDisplayedDrawable(placeholderDrawable))
169+
}
170+
171+
@Test
172+
fun loading_setViaLoadingParameterWithNullDrawable_andRequestBuilderTransform_showsNoResource() {
173+
val description = "test"
174+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
175+
composeRule.setContent {
176+
GlideImage(
177+
model = waitModel,
178+
contentDescription = description,
179+
loading = placeholder(null as Drawable?),
180+
) {
181+
it.placeholder(android.R.drawable.btn_star)
182+
}
183+
}
184+
185+
composeRule.onNodeWithContentDescription(description)
186+
.assert(expectNoDrawable())
187+
}
188+
189+
190+
@Test
191+
fun loading_setViaLoadingParameterWithComposable_andRequestBuilderTransform_showsComposable() {
192+
val description = "test"
193+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
194+
val placeholderResourceId = android.R.drawable.star_big_off
195+
composeRule.setContent {
196+
GlideImage(
197+
model = waitModel,
198+
contentDescription = "other",
199+
loading = placeholder {
200+
GlideImage(
201+
model = waitModel,
202+
contentDescription = description,
203+
loading = placeholder(placeholderResourceId),
204+
)
205+
},
206+
) {
207+
it.placeholder(android.R.drawable.btn_star)
208+
}
209+
}
210+
211+
composeRule.onNodeWithContentDescription(description)
212+
.assert(expectDisplayedResource(placeholderResourceId))
213+
}
214+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
@file:OptIn(InternalGlideApi::class)
2+
3+
package com.bumptech.glide.integration.compose.test
4+
5+
import android.content.Context
6+
import android.graphics.Bitmap
7+
import android.graphics.drawable.BitmapDrawable
8+
import android.graphics.drawable.Drawable
9+
import androidx.compose.runtime.MutableState
10+
import androidx.compose.ui.semantics.SemanticsPropertyKey
11+
import androidx.compose.ui.test.SemanticsMatcher
12+
import androidx.test.core.app.ApplicationProvider
13+
import com.bumptech.glide.integration.compose.DisplayedDrawableKey
14+
import com.bumptech.glide.integration.ktx.InternalGlideApi
15+
import com.bumptech.glide.integration.ktx.Size
16+
17+
private val context = ApplicationProvider.getApplicationContext<Context>()
18+
19+
fun Int.bitmapSize() = context.resources.getDrawable(this, context.theme).size()
20+
21+
fun Drawable.size() = (this as BitmapDrawable).bitmap.let { Size(it.width, it.height) }
22+
23+
fun expectDisplayedResource(resourceId: Int) =
24+
expectDisplayedDrawable(context.getDrawable(resourceId))
25+
26+
fun Drawable?.bitmapOrThrow(): Bitmap? = if (this == null) null else (this as BitmapDrawable).bitmap
27+
28+
fun expectDisplayedDrawableSize(expectedSize: Size): SemanticsMatcher =
29+
expectDisplayedDrawable(expectedSize) { it?.size() }
30+
31+
fun expectDisplayedDrawable(
32+
expectedValue: Drawable?
33+
): SemanticsMatcher =
34+
expectDisplayedDrawable(expectedValue.bitmapOrThrow()) { it.bitmapOrThrow() }
35+
36+
fun expectNoDrawable(): SemanticsMatcher = expectDisplayedDrawable(null)
37+
38+
private fun <ValueT> expectDisplayedDrawable(
39+
expectedValue: ValueT,
40+
transform: (Drawable?) -> ValueT
41+
): SemanticsMatcher = expectStateValue(DisplayedDrawableKey, expectedValue) { transform(it) }
42+
43+
private fun <ValueT, TransformedValueT> expectStateValue(
44+
key: SemanticsPropertyKey<MutableState<ValueT?>>,
45+
expectedValue: TransformedValueT,
46+
transform: (ValueT?) -> TransformedValueT?
47+
): SemanticsMatcher =
48+
SemanticsMatcher("${key.name} = '$expectedValue'") {
49+
val value = transform(it.config.getOrElseNullable(key) { null }?.value)
50+
if (value != expectedValue) {
51+
throw AssertionError("Expected: $expectedValue, but was: $value")
52+
}
53+
true
54+
}

‎integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt

+114-11
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.bumptech.glide.integration.compose
22

33
import android.graphics.drawable.Drawable
44
import androidx.compose.foundation.Image
5+
import androidx.compose.foundation.layout.Box
56
import androidx.compose.runtime.Composable
67
import androidx.compose.runtime.MutableState
78
import androidx.compose.runtime.remember
@@ -20,10 +21,12 @@ import com.bumptech.glide.Glide
2021
import com.bumptech.glide.RequestBuilder
2122
import com.bumptech.glide.RequestManager
2223
import com.bumptech.glide.integration.ktx.AsyncGlideSize
24+
import com.bumptech.glide.integration.ktx.ExperimentGlideFlows
2325
import com.bumptech.glide.integration.ktx.ImmediateGlideSize
2426
import com.bumptech.glide.integration.ktx.InternalGlideApi
2527
import com.bumptech.glide.integration.ktx.ResolvableGlideSize
2628
import com.bumptech.glide.integration.ktx.Size
29+
import com.bumptech.glide.integration.ktx.Status
2730

2831
/** Mutates and returns the given [RequestBuilder] to apply relevant options. */
2932
public typealias RequestBuilderTransform<T> = (RequestBuilder<T>) -> RequestBuilder<T>
@@ -58,6 +61,24 @@ public typealias RequestBuilderTransform<T> = (RequestBuilder<T>) -> RequestBuil
5861
* Note - this method is likely to change while we work on improving the API. Transitions are one
5962
* significant unexplored area. It's also possible we'll try and remove the [RequestBuilder] from
6063
* the direct API and instead allow all options to be set directly in the method.
64+
*
65+
* [requestBuilderTransform] is overridden by any overlapping parameter defined in this method if
66+
* that parameter is non-null. For example, [loading] and [failure], if non-null will be used in
67+
* place of any placeholder set by [requestBuilderTransform] using [RequestBuilder.placeholder] or
68+
* [RequestBuilder.error].
69+
*
70+
* @param loading A [Placeholder] that will be displayed while the request is loading. Specifically
71+
* it's used if the request is cleared ([com.bumptech.glide.request.target.Target.onLoadCleared]) or
72+
* loading ([com.bumptech.glide.request.target.Target.onLoadStarted]. There's a subtle difference
73+
* in behavior depending on which type of [Placeholder] you use. The resource and `Drawable`
74+
* variants will be displayed if the request fails and no other failure handling is specified, but
75+
* the `Composable` will not.
76+
* @param failure A [Placeholder] that will be displayed if the request fails. Specifically it's
77+
* used when [com.bumptech.glide.request.target.Target.onLoadFailed] is called. If
78+
* [RequestBuilder.error] is called in [requestBuilderTransform] with a valid [RequestBuilder] (as
79+
* opposed to resource id or [Drawable]), this [Placeholder] will not be used unless the `error`
80+
* [RequestBuilder] also fails. This parameter does not override error [RequestBuilder]s, only
81+
* error resource ids and/or [Drawable]s.
6182
*/
6283
// TODO(judds): the API here is not particularly composeesque, we should consider alternatives
6384
// to RequestBuilder (though thumbnail() may make that a challenge).
@@ -73,12 +94,21 @@ public fun GlideImage(
7394
contentScale: ContentScale = ContentScale.Fit,
7495
alpha: Float = DefaultAlpha,
7596
colorFilter: ColorFilter? = null,
97+
loading: Placeholder? = null,
98+
failure: Placeholder? = null,
7699
// TODO(judds): Consider defaulting to load the model here instead of always doing so below.
77100
requestBuilderTransform: RequestBuilderTransform<Drawable> = { it },
78101
) {
79102
val requestManager: RequestManager = LocalContext.current.let { remember(it) { Glide.with(it) } }
80103
val requestBuilder =
81-
rememberRequestBuilderWithDefaults(model, requestManager, requestBuilderTransform, contentScale)
104+
rememberRequestBuilderWithDefaults(
105+
model, requestManager, requestBuilderTransform, contentScale
106+
).let {
107+
loading?.apply(it::placeholder, it::placeholder) ?: it
108+
}.let {
109+
failure?.apply(it::error, it::error) ?: it
110+
}
111+
82112
val overrideSize: Size? = requestBuilder.overrideSize()
83113
val (size, finalModifier) = rememberSizeAndModifier(overrideSize, modifier)
84114

@@ -91,9 +121,60 @@ public fun GlideImage(
91121
contentScale = contentScale,
92122
alpha = alpha,
93123
colorFilter = colorFilter,
124+
placeholder = loading?.maybeComposable(),
125+
failure = failure?.maybeComposable(),
94126
)
95127
}
96128

129+
/**
130+
* Ideally [drawable] is non-null, but because [android.content.Context.getDrawable] can return
131+
* null, we allow it here. `placeholder(null)` has the same override behavior as if a non-null
132+
* `Drawable` were provided.
133+
*/
134+
@ExperimentalGlideComposeApi
135+
public fun placeholder(drawable: Drawable?): Placeholder = Placeholder.OfDrawable(drawable)
136+
@ExperimentalGlideComposeApi
137+
public fun placeholder(resourceId: Int): Placeholder = Placeholder.OfResourceId(resourceId)
138+
@ExperimentalGlideComposeApi
139+
public fun placeholder(composable: @Composable () -> Unit): Placeholder =
140+
Placeholder.OfComposable(composable)
141+
142+
/**
143+
* Content to display during a particular state of a Glide Request, for example while the request is
144+
* loading or if the request fails.
145+
*
146+
* `of(Drawable)` and `of(resourceId)` trigger fewer recompositions than
147+
* `of(@Composable () -> Unit)` so you should only use the Composable variant if you require
148+
* something more complex than a simple color or a static image.
149+
*
150+
* `of(@Composable () -> Unit)` will display the [Composable] inside a [Box] whose modifier
151+
* is the one provided to [GlideImage]. Doing so allows Glide to infer the requested size if one
152+
* is not explicitly specified on the request itself.
153+
*/
154+
@ExperimentalGlideComposeApi
155+
public sealed class Placeholder {
156+
internal class OfDrawable(internal val drawable: Drawable?) : Placeholder()
157+
internal class OfResourceId(internal val resourceId: Int) : Placeholder()
158+
internal class OfComposable(internal val composable: @Composable () -> Unit) : Placeholder()
159+
160+
internal fun maybeComposable(): (@Composable () -> Unit)? =
161+
when(this) {
162+
is OfComposable -> this.composable
163+
else -> null
164+
}
165+
166+
internal fun <T> apply(
167+
resource: (Int) -> RequestBuilder<T>,
168+
drawable: (Drawable?) -> RequestBuilder<T>
169+
): RequestBuilder<T> =
170+
when(this) {
171+
is OfDrawable -> drawable(this.drawable)
172+
is OfResourceId -> resource(this.resourceId)
173+
// Clear out any previously set placeholder.
174+
else -> drawable(null)
175+
}
176+
}
177+
97178
@OptIn(InternalGlideApi::class)
98179
private data class SizeAndModifier(val size: ResolvableGlideSize, val modifier: Modifier)
99180

@@ -148,7 +229,7 @@ private fun RequestBuilder<Drawable>.contentScaleTransform(
148229
// TODO(judds): Think about how to handle the various fills
149230
}
150231

151-
@OptIn(InternalGlideApi::class)
232+
@OptIn(InternalGlideApi::class, ExperimentGlideFlows::class)
152233
@Composable
153234
private fun SizedGlideImage(
154235
requestBuilder: RequestBuilder<Drawable>,
@@ -159,23 +240,45 @@ private fun SizedGlideImage(
159240
contentScale: ContentScale,
160241
alpha: Float,
161242
colorFilter: ColorFilter?,
243+
placeholder: @Composable (() -> Unit)?,
244+
failure: @Composable (() -> Unit)?,
162245
) {
246+
// Use a Box so we can infer the size if the request doesn't have an explicit size.
247+
@Composable fun @Composable () -> Unit.boxed() =
248+
Box(modifier = modifier) {
249+
this@boxed()
250+
}
251+
163252
val painter =
164253
rememberGlidePainter(
165254
requestBuilder = requestBuilder,
166255
size = size,
167256
)
168-
Image(
169-
painter = painter,
170-
contentDescription = contentDescription,
171-
alignment = alignment,
172-
contentScale = contentScale,
173-
alpha = alpha,
174-
colorFilter = colorFilter,
175-
modifier = modifier.then(Modifier.semantics { displayedDrawable = painter.currentDrawable }),
176-
)
257+
if (placeholder != null && painter.status.showPlaceholder()) {
258+
placeholder.boxed()
259+
} else if (failure != null && painter.status == Status.FAILED) {
260+
failure.boxed()
261+
} else {
262+
Image(
263+
painter = painter,
264+
contentDescription = contentDescription,
265+
alignment = alignment,
266+
contentScale = contentScale,
267+
alpha = alpha,
268+
colorFilter = colorFilter,
269+
modifier = modifier.then(Modifier.semantics { displayedDrawable = painter.currentDrawable })
270+
)
271+
}
177272
}
178273

274+
@OptIn(ExperimentGlideFlows::class)
275+
private fun Status.showPlaceholder(): Boolean =
276+
when (this) {
277+
Status.RUNNING -> true
278+
Status.CLEARED -> true
279+
else -> false
280+
}
281+
179282
@OptIn(InternalGlideApi::class)
180283
@Composable
181284
private fun rememberGlidePainter(

‎integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt

+5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.bumptech.glide.integration.compose
33
import android.graphics.drawable.BitmapDrawable
44
import android.graphics.drawable.ColorDrawable
55
import android.graphics.drawable.Drawable
6+
import android.util.Log
67
import androidx.compose.runtime.MutableState
78
import androidx.compose.runtime.RememberObserver
89
import androidx.compose.runtime.Stable
@@ -24,6 +25,7 @@ import com.bumptech.glide.integration.ktx.InternalGlideApi
2425
import com.bumptech.glide.integration.ktx.Placeholder
2526
import com.bumptech.glide.integration.ktx.ResolvableGlideSize
2627
import com.bumptech.glide.integration.ktx.Resource
28+
import com.bumptech.glide.integration.ktx.Status
2729
import com.bumptech.glide.integration.ktx.flowResolvable
2830
import com.google.accompanist.drawablepainter.DrawablePainter
2931
import kotlinx.coroutines.CoroutineScope
@@ -44,6 +46,8 @@ constructor(
4446
private val size: ResolvableGlideSize,
4547
scope: CoroutineScope,
4648
) : Painter(), RememberObserver {
49+
@OptIn(ExperimentGlideFlows::class)
50+
internal var status: Status by mutableStateOf(Status.CLEARED)
4751
internal val currentDrawable: MutableState<Drawable?> = mutableStateOf(null)
4852
private var alpha: Float by mutableStateOf(DefaultAlpha)
4953
private var colorFilter: ColorFilter? by mutableStateOf(null)
@@ -81,6 +85,7 @@ constructor(
8185
is Placeholder -> it.placeholder
8286
}
8387
)
88+
status = it.status
8489
}
8590
}
8691
}

‎instrumentation/src/androidTest/java/com/bumptech/glide/test/WaitModelLoader.java ‎testutil/src/main/java/com/bumptech/glide/testutil/WaitModelLoader.java

+34-31
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package com.bumptech.glide.test;
1+
package com.bumptech.glide.testutil;
22

33
import androidx.annotation.NonNull;
44
import androidx.annotation.Nullable;
@@ -11,8 +11,7 @@
1111
import com.bumptech.glide.load.model.ModelLoader;
1212
import com.bumptech.glide.load.model.ModelLoaderFactory;
1313
import com.bumptech.glide.load.model.MultiModelLoaderFactory;
14-
import com.bumptech.glide.test.WaitModelLoader.WaitModel;
15-
import com.bumptech.glide.testutil.ConcurrencyHelper;
14+
import com.bumptech.glide.testutil.WaitModelLoader.WaitModel;
1615
import java.io.InputStream;
1716
import java.util.concurrent.CountDownLatch;
1817

@@ -22,6 +21,37 @@
2221
*/
2322
public final class WaitModelLoader<Model, Data> implements ModelLoader<WaitModel<Model>, Data> {
2423

24+
public static final class WaitModel<T> {
25+
private final CountDownLatch latch = new CountDownLatch(1);
26+
private final T wrapped;
27+
28+
WaitModel(T wrapped) {
29+
this.wrapped = wrapped;
30+
}
31+
32+
public void countDown() {
33+
if (latch.getCount() != 1) {
34+
throw new IllegalStateException();
35+
}
36+
latch.countDown();
37+
}
38+
}
39+
40+
/**
41+
* Use {@link WaitModelLoaderRule#waitOn(Object)} instead
42+
*/
43+
@Deprecated
44+
public static synchronized <T> WaitModel<T> waitOn(T model) {
45+
@SuppressWarnings("unchecked")
46+
ModelLoaderFactory<WaitModel<T>, InputStream> streamFactory =
47+
new Factory<>((Class<T>) model.getClass(), InputStream.class);
48+
Glide.get(ApplicationProvider.getApplicationContext())
49+
.getRegistry()
50+
.replace(WaitModel.class, InputStream.class, streamFactory);
51+
52+
return new WaitModel<>(model);
53+
}
54+
2555
private final ModelLoader<Model, Data> wrapped;
2656

2757
private WaitModelLoader(ModelLoader<Model, Data> wrapped) {
@@ -46,23 +76,7 @@ public boolean handles(@NonNull WaitModel<Model> waitModel) {
4676
return wrapped.handles(waitModel.wrapped);
4777
}
4878

49-
public static final class WaitModel<T> {
50-
private final CountDownLatch latch = new CountDownLatch(1);
51-
private final T wrapped;
52-
53-
WaitModel(T wrapped) {
54-
this.wrapped = wrapped;
55-
}
56-
57-
public void countDown() {
58-
if (latch.getCount() != 1) {
59-
throw new IllegalStateException();
60-
}
61-
latch.countDown();
62-
}
63-
}
64-
65-
public static final class Factory<Model, Data>
79+
private static final class Factory<Model, Data>
6680
implements ModelLoaderFactory<WaitModel<Model>, Data> {
6781

6882
private final Class<Model> modelClass;
@@ -73,17 +87,6 @@ public static final class Factory<Model, Data>
7387
this.dataClass = dataClass;
7488
}
7589

76-
public static synchronized <T> WaitModel<T> waitOn(T model) {
77-
@SuppressWarnings("unchecked")
78-
ModelLoaderFactory<WaitModel<T>, InputStream> streamFactory =
79-
new Factory<>((Class<T>) model.getClass(), InputStream.class);
80-
Glide.get(ApplicationProvider.getApplicationContext())
81-
.getRegistry()
82-
.replace(WaitModel.class, InputStream.class, streamFactory);
83-
84-
return new WaitModel<>(model);
85-
}
86-
8790
@NonNull
8891
@Override
8992
public ModelLoader<WaitModel<Model>, Data> build(MultiModelLoaderFactory multiFactory) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.bumptech.glide.testutil;
2+
3+
import com.bumptech.glide.testutil.WaitModelLoader.WaitModel;
4+
import java.util.ArrayList;
5+
import java.util.List;
6+
import org.junit.rules.ExternalResource;
7+
8+
/**
9+
* Makes sure that all {@link WaitModel}s created by it are unblocked before the test ends.
10+
*/
11+
public final class WaitModelLoaderRule extends ExternalResource {
12+
private final List<WaitModel<?>> waitModels = new ArrayList<>();
13+
14+
public <T>WaitModel<T> waitOn(T model) {
15+
WaitModel<T> waitModel = WaitModelLoader.waitOn(model);
16+
waitModels.add(waitModel);
17+
return waitModel;
18+
}
19+
20+
@Override
21+
protected void after() {
22+
super.after();
23+
for (WaitModel<?> waitModel : waitModels) {
24+
waitModel.countDown();
25+
}
26+
}
27+
}

0 commit comments

Comments
 (0)
Please sign in to comment.