Kamel-Media / Kamel

Kotlin asynchronous media loading and caching library for Compose.
Apache License 2.0
636 stars 24 forks source link

KamelImage fills up max size #94

Open zeroeightysix opened 7 months ago

zeroeightysix commented 7 months ago

Rather counterintuitively, and annoyingly, KamelImage without a Modifier to restrict its size will consume all space it can get due to this default modifier:

https://github.com/Kamel-Media/Kamel/blob/291f157dd43ad073ad280331567f6305c4867159/kamel-image/src/commonMain/kotlin/io/kamel/image/KamelImage.kt#L45-L54

This leads to it being the case that if you include any image in an unbounded box that the entire UI just becomes that image. For our usecase, we have a very wide banner image that is supposed to fill up the parent's entire width but not the height. This default prevents that from happening :slightly_frowning_face:

Can't we make the inner Image have no modifier?

luca992 commented 7 months ago

@zeroeightysix So that modifier will actually cause it to fill the size of KamelImageBox. and the modifier here lets you apply whatever modifiers you want to the box. Is that not working for you?

Also you can always make your own version of KamelImage using KamelImageBox if you need some non-standard behavior

zeroeightysix commented 7 months ago

@zeroeightysix So that modifier will actually cause it to fill the size of KamelImageBox. and the modifier here lets you apply whatever modifiers you want to the box. Is that not working for you?

Also you can always make your own version of KamelImage using KamelImageBox if you need some non-standard behavior

As the box sizes to fit its contents, it will grow larger than the image dimensions themselves.

You'd expect, without any modifiers, for the box to be the same size as the actual image, tightly wrapping around it. Instead, both the box and inner Image take up more space than is required to display the image; is that intentional?

luca992 commented 7 months ago

Yeah I suppose it makes more sense to display an image in its actual size by default. I think it should just work that way if I removed that modifier... The only thing I'm worried about is that it may cause issues with contentScale. I'd have to check I think

wuujcik commented 3 months ago

I have the same issue. I'm loading image that can have basically any dimentions and I want the contentScale to adjust the size to hardcoded height of 50.dp, but the below code does not wrap the width of the image, but uses the whole width of the Row in which it is placed. In the same Row I need Spacer with weight (to take any space left) and some other items on the right, so I really need the width to be taken from scaling the content, which does not happen right now

KamelImage(
    resource = asyncPainterResource(logoUrl),
    modifier = Modifier
        .height(50.dp)
        .wrapContentWidth(),
    contentDescription = null,
    contentScale = ContentScale.FillHeight,
    alignment = Alignment.CenterStart

I was able to resove my issue by using custom KamelImageBox, but I find this default behaviour to be counter-intuitive.

luca992 commented 3 months ago

If you want to make a proposal as a pr with your custom kamel image I can take a look and we can go from there @wuujcik