From 90171468ccc6db9e81cb9134f137e0b104d3b5f1 Mon Sep 17 00:00:00 2001
From: austinried <4966622+austinried@users.noreply.github.com>
Date: Sat, 26 Sep 2026 01:53:35 +0000
Subject: [PATCH] address review: keep one unbounded cover axis undefined
 instead of forcing a square

---
 .../com/subtracks/ui/components/Components.kt | 38 +++++++++++--------
 .../ui/components/CoverArtRequestTest.kt      | 14 ++++++-
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/app/src/main/kotlin/com/subtracks/ui/components/Components.kt b/app/src/main/kotlin/com/subtracks/ui/components/Components.kt
index 82e70775..2eaa92b7 100644
--- a/app/src/main/kotlin/com/subtracks/ui/components/Components.kt
+++ b/app/src/main/kotlin/com/subtracks/ui/components/Components.kt
@@ -36,13 +36,13 @@ import androidx.compose.ui.layout.ContentScale
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.unit.Dp
-import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.dp
 import coil3.compose.AsyncImage
 import coil3.compose.LocalPlatformContext
 import coil3.compose.rememberAsyncImagePainter
 import coil3.request.ImageRequest
 import coil3.request.crossfade
+import coil3.size.Dimension
 import com.subtracks.data.model.CoverArtRef
 
 private const val MAX_CACHED_RATIOS = 256
@@ -112,22 +112,25 @@ fun CoverArt(
     } else {
         BoxWithConstraints(modifier, contentAlignment = Alignment.Center) {
             val density = LocalDensity.current
-            val targetSize =
+            val target =
                 remember(maxWidth, maxHeight, density) {
-                    val width = with(density) { if (maxWidth.value.isFinite()) maxWidth.roundToPx() else 0 }
-                    val height = with(density) { if (maxHeight.value.isFinite()) maxHeight.roundToPx() else 0 }
-                    when {
-                        width > 0 && height > 0 -> IntSize(width, height)
-                        width > 0 -> IntSize(width, width)
-                        height > 0 -> IntSize(height, height)
-                        else -> null
-                    }
+                    val width = with(density) { if (maxWidth.value.isFinite()) maxWidth.roundToPx() else 0 }.takeIf { it > 0 }
+                    val height = with(density) { if (maxHeight.value.isFinite()) maxHeight.roundToPx() else 0 }.takeIf { it > 0 }
+                    width to height
                 }
             val painter =
                 rememberAsyncImagePainter(
                     model =
-                        remember(ref, thumbnailRef, targetSize) {
-                            ref?.let { imageRequest(context, it, crossfade = thumbnailRef != null, size = targetSize) }
+                        remember(ref, thumbnailRef, target) {
+                            ref?.let {
+                                imageRequest(
+                                    context,
+                                    it,
+                                    crossfade = thumbnailRef != null,
+                                    width = target.first,
+                                    height = target.second,
+                                )
+                            }
                         },
                     onError = { failed = true },
                     contentScale = ContentScale.Fit,
@@ -211,7 +214,8 @@ internal fun imageRequest(
     context: Context,
     ref: CoverArtRef,
     crossfade: Boolean,
-    size: IntSize? = null,
+    width: Int? = null,
+    height: Int? = null,
 ): ImageRequest {
     val builder =
         ImageRequest
@@ -219,12 +223,14 @@ internal fun imageRequest(
             .data(ref.url)
             .diskCacheKey(ref.cacheKey)
             .crossfade(crossfade)
-    if (size == null) {
+    if (width == null && height == null) {
         builder.memoryCacheKey(ref.cacheKey)
     } else {
         builder
-            .size(size.width, size.height)
-            .memoryCacheKey("${ref.cacheKey}:${size.width}x${size.height}")
+            .size(
+                width?.let { Dimension.Pixels(it) } ?: Dimension.Undefined,
+                height?.let { Dimension.Pixels(it) } ?: Dimension.Undefined,
+            ).memoryCacheKey("${ref.cacheKey}:${width ?: "u"}x${height ?: "u"}")
     }
     return builder.build()
 }
diff --git a/app/src/test/kotlin/com/subtracks/ui/components/CoverArtRequestTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/CoverArtRequestTest.kt
index 749f83c2..2dec34b2 100644
--- a/app/src/test/kotlin/com/subtracks/ui/components/CoverArtRequestTest.kt
+++ b/app/src/test/kotlin/com/subtracks/ui/components/CoverArtRequestTest.kt
@@ -1,7 +1,6 @@
 package com.subtracks.ui.components
 
 import android.content.Context
-import androidx.compose.ui.unit.IntSize
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.subtracks.data.model.CoverArtRef
@@ -16,12 +15,23 @@ class CoverArtRequestTest {
         val context = ApplicationProvider.getApplicationContext<Context>()
         val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false")
 
-        val request = imageRequest(context, ref, crossfade = false, size = IntSize(600, 800))
+        val request = imageRequest(context, ref, crossfade = false, width = 600, height = 800)
 
         assertEquals("1:art:false:600x800", request.memoryCacheKey)
         assertEquals(ref.cacheKey, request.diskCacheKey)
     }
 
+    @Test
+    fun oneUnboundedAxisIsNotForcedSquare() {
+        val context = ApplicationProvider.getApplicationContext<Context>()
+        val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false")
+
+        val request = imageRequest(context, ref, crossfade = false, width = 600)
+
+        assertEquals("1:art:false:600xu", request.memoryCacheKey)
+        assertEquals(ref.cacheKey, request.diskCacheKey)
+    }
+
     @Test
     fun unsizedRequestKeepsTheIdentityMemoryKey() {
         val context = ApplicationProvider.getApplicationContext<Context>()