Merge pull request #61 from chrisbanes/cb/coil-implicit

Fix using CoilImage with an implicit size

coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt

@@ -31,8 +31,12 @@ import androidx.compose.ui.unit.dp
 import androidx.core.net.toUri
 import androidx.test.filters.LargeTest
 import androidx.test.filters.SdkSuppress
+import androidx.ui.test.assertHeightIsAtLeast
+import androidx.ui.test.assertHeightIsEqualTo
 import androidx.ui.test.assertIsDisplayed
 import androidx.ui.test.assertPixels
+import androidx.ui.test.assertWidthIsAtLeast
+import androidx.ui.test.assertWidthIsEqualTo
 import androidx.ui.test.captureToBitmap
 import androidx.ui.test.createComposeRule
 import androidx.ui.test.onNodeWithTag
@@ -105,7 +109,8 @@ class CoilTest {
 
         onNodeWithTag(CoilTestTags.Image)
             .assertIsDisplayed()
-            .assertSize(composeTestRule.density, 128.dp, 128.dp)
+            .assertWidthIsEqualTo(128.dp)
+            .assertHeightIsEqualTo(128.dp)
     }
 
     @Test
@@ -125,7 +130,8 @@ class CoilTest {
         latch.await(5, TimeUnit.SECONDS)
 
         onNodeWithTag(CoilTestTags.Image)
-            .assertSize(composeTestRule.density, 128.dp, 128.dp)
+            .assertWidthIsEqualTo(128.dp)
+            .assertHeightIsEqualTo(128.dp)
             .assertIsDisplayed()
             .captureToBitmap()
             .assertPixels { Color.Red }
@@ -154,7 +160,8 @@ class CoilTest {
 
         // Assert that the content is completely Red
         onNodeWithTag(CoilTestTags.Image)
-            .assertSize(composeTestRule.density, 128.dp, 128.dp)
+            .assertWidthIsEqualTo(128.dp)
+            .assertHeightIsEqualTo(128.dp)
             .assertIsDisplayed()
             .captureToBitmap()
             .assertPixels { Color.Red }
@@ -167,7 +174,8 @@ class CoilTest {
 
         // Assert that the content is completely Blue
         onNodeWithTag(CoilTestTags.Image)
-            .assertSize(composeTestRule.density, 128.dp, 128.dp)
+            .assertWidthIsEqualTo(128.dp)
+            .assertHeightIsEqualTo(128.dp)
             .assertIsDisplayed()
             .captureToBitmap()
             .assertPixels { Color.Blue }
@@ -209,6 +217,27 @@ class CoilTest {
         loadCompleteSignal.close()
     }
 
+    @Test
+    fun basicLoad_nosize() {
+        val latch = CountDownLatch(1)
+
+        composeTestRule.setContent {
+            CoilImage(
+                data = resourceUri(R.raw.sample),
+                modifier = Modifier.testTag(CoilTestTags.Image),
+                onRequestCompleted = { latch.countDown() }
+            )
+        }
+
+        // Wait for the onRequestCompleted to release the latch
+        latch.await(5, TimeUnit.SECONDS)
+
+        onNodeWithTag(CoilTestTags.Image)
+            .assertWidthIsAtLeast(1.dp)
+            .assertHeightIsAtLeast(1.dp)
+            .assertIsDisplayed()
+    }
+
     @Test
     @SdkSuppress(minSdkVersion = 26) // captureToBitmap is SDK 26+
     fun customGetPainter() {
@@ -254,7 +283,8 @@ class CoilTest {
         // Assert that the layout is in the tree and has the correct size
         onNodeWithTag(CoilTestTags.Image)
             .assertIsDisplayed()
-            .assertSize(composeTestRule.density, 128.dp, 128.dp)
+            .assertWidthIsEqualTo(128.dp)
+            .assertHeightIsEqualTo(128.dp)
     }
 
     @OptIn(ExperimentalCoroutinesApi::class)

coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt

@@ -20,6 +20,7 @@ import android.graphics.drawable.Drawable
 import androidx.compose.foundation.Box
 import androidx.compose.foundation.Image
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.Stable
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.launchInComposition
 import androidx.compose.runtime.remember
@@ -28,6 +29,7 @@ import androidx.compose.runtime.state
 import androidx.compose.runtime.stateFor
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.WithConstraints
 import androidx.compose.ui.graphics.ColorFilter
 import androidx.compose.ui.graphics.ImageAsset
 import androidx.compose.ui.graphics.asImageAsset
@@ -186,36 +188,50 @@ fun CoilImage(
         else -> null
     }
 
-    // Update the modifier, so that we listen for size changes to invoke the actor
-    val mod = modifier.onSizeChanged { size ->
+    WithConstraints(modifier) {
+        val lastRequestedSize = remember(requestActor) { MutableRef(IntSize.Zero) }
+
+        val requestSize = IntSize(
+            width = if (constraints.hasBoundedWidth) constraints.maxWidth else UNSPECIFIED,
+            height = if (constraints.hasBoundedHeight) constraints.maxHeight else UNSPECIFIED
+        )
+
         val r = result
-        if (r == null || shouldRefetchOnSizeChange(r, size)) {
-            // If we don't have a result yet, or the size has changed and we should re-fetch,
-            // send it to the request actor
-            requestActor.send(size)
+        if (lastRequestedSize.value != requestSize &&
+            (r == null || shouldRefetchOnSizeChange(r, requestSize))
+        ) {
+            requestActor.send(requestSize)
+            lastRequestedSize.value = requestSize
         }
-    }
 
-    if (painter == null) {
-        // If we don't have a result painter, we add a Box with our modifier
-        Box(mod) {
-            // If we don't have a result yet, we can show the loading content
-            // (if not null)
-            if (result == null && loading != null) {
-                loading()
+        if (painter == null) {
+            // If we don't have a result painter, we add a Box with our modifier
+            Box {
+                // If we don't have a result yet, we can show the loading content
+                // (if not null)
+                if (result == null && loading != null) {
+                    loading()
+                }
             }
+        } else {
+            Image(
+                painter = painter,
+                contentScale = contentScale,
+                alignment = alignment,
+                colorFilter = colorFilter,
+            )
         }
-    } else {
-        Image(
-            painter = painter,
-            contentScale = contentScale,
-            alignment = alignment,
-            colorFilter = colorFilter,
-            modifier = mod
-        )
     }
 }
 
+/**
+ * Value for a [IntSize] dimension, where the dimension is not specified or is unknown.
+ */
+private const val UNSPECIFIED = -1
+
+@Stable
+private data class MutableRef<T>(var value: T)
+
 private fun CoilRequestActor(
     request: GetRequest
 ) = RequestActor<IntSize, RequestResult?> { size ->
@@ -224,6 +240,11 @@ private fun CoilRequestActor(
             // If the request has a sizeResolver set, we just execute the request as-is
             request
         }
+        size.width == UNSPECIFIED || size.height == UNSPECIFIED -> {
+            // If the size contains an unspecified dimension, we don't specify a size
+            // in the Coil request
+            request
+        }
         size != IntSize.Zero -> {
             // If we have a non-zero size, we can modify the request to include the size
             request.newBuilder()

sample/src/main/java/dev/chrisbanes/accompanist/sample/coil/CoilBasicSample.kt

@@ -126,6 +126,16 @@ private fun Sample() {
                     },
                     modifier = Modifier.preferredSize(128.dp)
                 )
+
+                // CoilImage with an implicit size
+                CoilImage(
+                    data = randomSampleImageUrl(),
+                    loading = {
+                        Stack(Modifier.fillMaxSize()) {
+                            CircularProgressIndicator(Modifier.gravity(Alignment.Center))
+                        }
+                    }
+                )
             }
         }
     }