Add crossfade between subsequent image results in AsyncImage#3141
Conversation
|
|
||
| internal const val DEFAULT_CROSSFADE_MILLIS = 200 | ||
|
|
||
| fun ImageRequest.Builder.crossfadeBetweenImages(enable: Boolean) = apply { |
There was a problem hiding this comment.
Can you add a short doc comment for this method?
Also we should add ImageLoader.Builder.crossfadeBetweenImages so this can be configured per-ImageLoader.
| extras[crossfadeBetweenImagesKey] = enable | ||
| } | ||
|
|
||
| val ImageRequest.crossfadeBetweenImages: Boolean |
There was a problem hiding this comment.
Please annotate this and crossfadeBetweenImages with @ExperimentalCoilApi.
| val ImageRequest.crossfadeBetweenImages: Boolean | ||
| get() = getExtra(crossfadeBetweenImagesKey) | ||
|
|
||
| private val crossfadeBetweenImagesKey = Extras.Key(default = false) |
There was a problem hiding this comment.
We should also add:
val Extras.Key.Companion.crossfadeBetweenImages: Extras.Key<Boolean>
get() = crossfadeBetweenImagesKey
colinrtwhite
left a comment
There was a problem hiding this comment.
Thanks for implementing this. I think this looks good for Compose, but do you know how we would also add support to Views?
Ideally, I'd like for this ImageRequest param to automatically support Views and Compose. Currently I could see someone using this param with an ImageView and being confused it doesn't work.
…in GenericViewTarget. Additionally, opt-in option is provided.
f2a7820 to
e887971
Compare
7780b60 to
3b6c58c
Compare
340a445 |
colinrtwhite
left a comment
There was a problem hiding this comment.
Thanks for working on this. Thinking more about it - I think we should make this solution more generic. Currently we're coupling this behaviour to crossfade, but really we want the ability to use the existing image as the placeholder for the next image request. If we do that then both the view and Compose-based crossfade implementations should automatically crossfade from what the previous image is. It would also allow users to skip setting the placeholder if they want to go from first loaded image -> second loaded image without setting a placeholder and without a crossfade.
Unfortunately we don't have knowledge of what the existing image is with views, but we do with Compose/AsyncImagePainter. As such, I think it's OK to only support this functionality in Compose. Here's what we should do:
- Revert the changes to
Targetto support views. - Rename the config option
crossfadeBetweenImagestouseExistingImageAsPlaceholder(enable: Boolean)(open to better names) and move it to animageRequest.ktfile incoil-compose-core(as it's only supported in Compose). We should note in the method description that this config option will only work in Compose. - Update
AsyncImagePainterto use the existingpainterinonStartifplaceholder == nullanduseExistingImageAsPlaceholderis enabled on theImageRequest.
Thanks again for working and iterating on this!
| /** | ||
| * Called when the request starts. | ||
| */ | ||
| fun onStart(placeholder: Image?) {} |
There was a problem hiding this comment.
We can't make this change as it breaks binary compatibility.
There was a problem hiding this comment.
There was a problem hiding this comment.
@colinrtwhite Could you please take a look?🙂
…rawable in GenericViewTarget. Additionally, opt-in option is provided." This reverts commit 340a445.
This reverts commit e887971.
* Rename `crossfadeBetweenImages` to `useExistingImageAsPlaceholder` for clarity * Move from `coil-core` to `coil-compose-core` to restrict to Compose usage only * Update documentation to explicitly state Compose-only support * Remove from core module to prevent incorrect usage in XML views
This reverts commit e17c262.
colinrtwhite
left a comment
There was a problem hiding this comment.
Thanks for implementing this!
Description
Currently,
AsyncImage's crossfade only applies from placeholder → result.When switching between different image models, the new image replaces the old one immediately ("jump cut") without a crossfade.
This PR introduces an optional parameter to support crossfading from the previous successful image result → the new result, skipping the placeholder.
Motivation
Notes