Adds Paintable class to represent generic image #1868
Conversation
...bles/src/main/java/com/google/android/horologist/auth/composables/chips/CreateAccountChip.kt
Outdated
Show resolved
Hide resolved
| @@ -87,8 +87,8 @@ public fun ToggleButton( | |||
| */ | |||
| @Composable | |||
| public fun ToggleButton( | |||
| checkedIcon: Any, | |||
| notCheckedIcon: Any, | |||
| checkedIcon: PaintableIcon, | |||
There was a problem hiding this comment.
Should this be just Paintable, why this specific PaintableIcon?
There was a problem hiding this comment.
this might be surfacing the lack of implementation in the current code of ToggleButton and Button to also support Image
There was a problem hiding this comment.
PaintableIcon is a subtype used just to indicate "this could be used in an Icon instead of an Image". This is to keep compatibility with existing code that in some places uses Icon or Image and in some only Icon (like here).
compose-material/src/main/java/com/google/android/horologist/compose/material/Paintable.kt
Show resolved
Hide resolved
| contentScale = ContentScale.Crop, | ||
| alpha = LocalContentAlpha.current, | ||
| ) | ||
| if (it is PaintableIcon) { |
There was a problem hiding this comment.
Should we avoid repeating this? pull it out somewhere?
...omposables/src/main/java/com/google/android/horologist/auth/composables/chips/AccountChip.kt
Outdated
Show resolved
Hide resolved
compose-material/src/main/java/com/google/android/horologist/compose/material/Paintable.kt
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Stable | ||
| public class CoilPaintable(private val model: Any?, private val placeholder: Painter? = null) : Paintable { |
There was a problem hiding this comment.
do we want to have "Coil" in the name, exposing the implementation details? ImageLoadingPaintable would be my suggestion but doesn't read great..
There was a problem hiding this comment.
If this get extracted to a separate module as previously suggested, then it should be fine to keep the name, and having "-coil" in the name of the lib.
Worth doing that extraction before next release (not necessarily in this PR) to avoid having 2 separated "big" API changes in the same area?
There was a problem hiding this comment.
Agreed, moving this to a separate -coil library will be a quick follow-up. I only added it here to stay backwards-compatible.
...ug/java/com/google/android/horologist/media/ui/components/actions/ShowPlaylistChipPreview.kt
Show resolved
Hide resolved
...n/java/com/google/android/horologist/media/ui/screens/browse/PlaylistDownloadBrowseScreen.kt
Show resolved
Hide resolved
...in/java/com/google/android/horologist/auth/composables/dialogs/SignedInConfirmationDialog.kt
Show resolved
Hide resolved
...aterial/src/main/java/com/google/android/horologist/compose/material/ChipIconWithProgress.kt
Show resolved
Hide resolved
fstanis
force-pushed
the
main
branch
6 times, most recently
from
ca8e0db to
6daba44
Compare
WHAT
Adds a new
Paintableinterface that can be used to represent "any kind of image".WHY
This decouples most parts of the library from
coilwhile making it trivial to still use it (seeCoilPaintable) and removes the need to useAnyacross the codebase.HOW
By replacing uses of
AnywithPaintablein the codebase, which itself has only one method, which returns aPainterthat can be used inside bothIconandImageelements. This logic previously lived in individual components (seeChipdelta) and is now encapsulated in the various implementations of this interface.Checklist 📋