diff --git a/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/Assertions.kt b/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/Assertions.kt deleted file mode 100644 index 34e276d7e..000000000 --- a/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/Assertions.kt +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package dev.chrisbanes.accompanist.coil - -import androidx.compose.ui.unit.Density -import androidx.compose.ui.unit.Dp -import androidx.compose.ui.unit.IntSize -import androidx.ui.test.SemanticsNodeInteraction -import org.junit.Assert - -fun SemanticsNodeInteraction.assertSize( - density: Density, - width: Dp, - height: Dp -): SemanticsNodeInteraction { - return assertSize(with(density) { IntSize(width.toIntPx(), height.toIntPx()) }) -} - -fun SemanticsNodeInteraction.assertSize( - expected: IntSize -): SemanticsNodeInteraction { - val node = fetchSemanticsNode("Assert size") - Assert.assertEquals(expected, node.size) - return this -} diff --git a/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt b/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt index 240302be3..2a4660c6f 100644 --- a/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt +++ b/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) diff --git a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt b/coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt index 73a6c61ab..961f3245f 100644 --- a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt +++ b/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(var value: T) + private fun CoilRequestActor( request: GetRequest ) = RequestActor { 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() diff --git a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Modifier.kt b/coil/src/main/java/dev/chrisbanes/accompanist/coil/Modifier.kt deleted file mode 100644 index 7069edd8b..000000000 --- a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Modifier.kt +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package dev.chrisbanes.accompanist.coil - -import androidx.compose.runtime.getValue -import androidx.compose.runtime.setValue -import androidx.compose.runtime.state -import androidx.compose.ui.Modifier -import androidx.compose.ui.composed -import androidx.compose.ui.onPositioned -import androidx.compose.ui.unit.IntSize - -/** - * [Modifier] which will invoke [onSizeChanged] whenever the size of the element changes. This - * will be called after positioning, similar to `Modifier.onPositioned`. - */ -internal fun Modifier.onSizeChanged( - onSizeChanged: (IntSize) -> Unit -) = composed { - var lastSize by state { null } - onPositioned { coordinates -> - if (coordinates.size != lastSize) { - lastSize = coordinates.size - onSizeChanged(coordinates.size) - } - } -} diff --git a/sample/src/main/java/dev/chrisbanes/accompanist/sample/coil/CoilBasicSample.kt b/sample/src/main/java/dev/chrisbanes/accompanist/sample/coil/CoilBasicSample.kt index d073992e8..352d86db4 100644 --- a/sample/src/main/java/dev/chrisbanes/accompanist/sample/coil/CoilBasicSample.kt +++ b/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)) + } + } + ) } } }