Update media title margin for large screens. (#2111)

* Update media title margin for large screens. * Update screenshot tests. --------- Co-authored-by: kpaikena <[email protected]>
google · Mar 12, 2024 · c25f407 · c25f407
1 parent 3fe51a9
commit c25f407
Show file tree

Hide file tree

Showing 34 changed files with 197 additions and 106 deletions.
diff --git a/...ava/com/google/android/horologist/media/ui/components/animated/MarqueeTextMediaDisplay.kt b/...ava/com/google/android/horologist/media/ui/components/animated/MarqueeTextMediaDisplay.kt
@@ -25,17 +25,15 @@ import androidx.compose.animation.fadeOut
 import androidx.compose.animation.slideInHorizontally
 import androidx.compose.animation.slideOutHorizontally
 import androidx.compose.animation.togetherWith
-import androidx.compose.foundation.Image
 import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.text.InlineTextContent
 import androidx.compose.foundation.text.appendInlineContent
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.layout.ContentScale
+import androidx.compose.ui.platform.LocalConfiguration
 import androidx.compose.ui.text.Placeholder
 import androidx.compose.ui.text.PlaceholderVerticalAlign
 import androidx.compose.ui.text.buildAnnotatedString
@@ -46,9 +44,9 @@ import androidx.wear.compose.material.MaterialTheme
 import androidx.wear.compose.material.Text
 import com.google.android.horologist.annotations.ExperimentalHorologistApi
 import com.google.android.horologist.composables.MarqueeText
-import com.google.android.horologist.compose.material.Icon
 import com.google.android.horologist.images.base.paintable.Paintable
-import com.google.android.horologist.images.base.paintable.PaintableIcon
+import com.google.android.horologist.media.ui.components.controls.MediaTitleIcon
+import com.google.android.horologist.media.ui.util.isLargeScreen
 import kotlin.math.roundToInt
 
 /**
@@ -65,6 +63,8 @@ public fun MarqueeTextMediaDisplay(
     subtextTransitionDelay: Int = 30,
     @FloatRange(from = 0.0, to = 1.0) transitionLength: Float = 0.125f,
 ) {
+    val isLargeScreen = LocalConfiguration.current.isLargeScreen
+
     fun getTransitionAnimation(delay: Int = 0): ContentTransform {
         return slideInHorizontally(animationSpec = tween(delayMillis = delay + enterTransitionDelay)) {
             (it * transitionLength).roundToInt()
@@ -105,7 +105,10 @@ public fun MarqueeTextMediaDisplay(
                 inlineContent = inlineContent,
                 modifier = Modifier
                     .fillMaxWidth(0.7f)
-                    .padding(top = 2.dp, bottom = .8.dp),
+                    .padding(
+                        top = if (isLargeScreen) 0.dp else 2.dp,
+                        bottom = if (isLargeScreen) 3.dp else 1.dp,
+                    ),
                 color = MaterialTheme.colors.onBackground,
                 style = textStyle,
                 textAlign = TextAlign.Center,
@@ -121,7 +124,7 @@ public fun MarqueeTextMediaDisplay(
                 text = currentArtist.orEmpty(),
                 modifier = Modifier
                     .fillMaxWidth(0.8f)
-                    .padding(top = 2.dp, bottom = .6.dp),
+                    .padding(top = 1.dp, bottom = .6.dp),
                 color = MaterialTheme.colors.onBackground,
                 textAlign = TextAlign.Center,
                 overflow = TextOverflow.Ellipsis,
@@ -131,22 +134,3 @@ public fun MarqueeTextMediaDisplay(
         }
     }
 }
-
-@Composable
-private fun MediaTitleIcon(icon: Paintable) {
-    if (icon is PaintableIcon) {
-        Icon(
-            modifier = Modifier.fillMaxSize(),
-            paintable = icon,
-            contentDescription = null,
-            tint = MaterialTheme.colors.onBackground,
-        )
-    } else {
-        Image(
-            modifier = Modifier.fillMaxSize(),
-            painter = icon.rememberPainter(),
-            contentDescription = null,
-            contentScale = ContentScale.FillHeight,
-        )
-    }
-}
diff --git a/...rc/main/java/com/google/android/horologist/media/ui/components/controls/MediaTitleIcon.kt b/...rc/main/java/com/google/android/horologist/media/ui/components/controls/MediaTitleIcon.kt
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2024 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 com.google.android.horologist.media.ui.components.controls
+
+import androidx.compose.foundation.Image
+import androidx.compose.foundation.layout.fillMaxSize
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.layout.ContentScale
+import androidx.wear.compose.material.MaterialTheme
+import com.google.android.horologist.compose.material.Icon
+import com.google.android.horologist.images.base.paintable.Paintable
+import com.google.android.horologist.images.base.paintable.PaintableIcon
+
+/**
+ * An icon to be shown in the media display before media title.
+ */
+@Composable
+internal fun MediaTitleIcon(icon: Paintable) {
+    if (icon is PaintableIcon) {
+        Icon(
+            modifier = Modifier.fillMaxSize(),
+            paintable = icon,
+            contentDescription = null,
+            tint = MaterialTheme.colors.onBackground,
+        )
+    } else {
+        Image(
+            modifier = Modifier.fillMaxSize(),
+            painter = icon.rememberPainter(),
+            contentDescription = null,
+            contentScale = ContentScale.FillHeight,
+        )
+    }
+}
diff --git a/...ain/java/com/google/android/horologist/media/ui/components/display/LoadingMediaDisplay.kt b/...ain/java/com/google/android/horologist/media/ui/components/display/LoadingMediaDisplay.kt
@@ -28,6 +28,7 @@ import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
+import androidx.compose.ui.platform.LocalConfiguration
 import androidx.compose.ui.unit.dp
 import androidx.wear.compose.material.ExperimentalWearMaterialApi
 import androidx.wear.compose.material.MaterialTheme
@@ -36,6 +37,7 @@ import androidx.wear.compose.material.placeholderShimmer
 import androidx.wear.compose.material.rememberPlaceholderState
 import com.google.android.horologist.annotations.ExperimentalHorologistApi
 import com.google.android.horologist.media.ui.components.animated.MarqueeTextMediaDisplay
+import com.google.android.horologist.media.ui.util.isLargeScreen
 
 /**
  * A loading state display. This style is matched to the Text of [TextMediaDisplay] as
@@ -49,9 +51,12 @@ public fun LoadingMediaDisplay(
 ) {
     // Always shimmer on the placeholder pills.
     val placeholderState = rememberPlaceholderState(isContentReady = { false })
+    val isLargeScreen = LocalConfiguration.current.isLargeScreen
 
     Column(modifier = modifier, horizontalAlignment = Alignment.CenterHorizontally) {
-        Spacer(modifier = Modifier.height(1.dp))
+        if (!isLargeScreen) {
+            Spacer(modifier = Modifier.height(1.dp))
+        }
         Box(
             modifier = Modifier
                 .clip(RoundedCornerShape(6.dp))
@@ -61,7 +66,11 @@ public fun LoadingMediaDisplay(
                 .width(120.dp)
                 .height(12.dp),
         )
-        Spacer(modifier = Modifier.height(8.dp))
+        Spacer(
+            modifier = Modifier.height(
+                if (isLargeScreen) 9.dp else 8.dp,
+            ),
+        )
         Box(
             modifier = Modifier
                 .clip(RoundedCornerShape(6.dp))

diff --git a/...c/main/java/com/google/android/horologist/media/ui/components/display/TextMediaDisplay.kt b/...c/main/java/com/google/android/horologist/media/ui/components/display/TextMediaDisplay.kt
@@ -16,25 +16,27 @@
 
 package com.google.android.horologist.media.ui.components.display
 
-import androidx.compose.foundation.Image
-import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Column
-import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.text.InlineTextContent
+import androidx.compose.foundation.text.appendInlineContent
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.graphics.ColorFilter
-import androidx.compose.ui.layout.ContentScale
-import androidx.compose.ui.semantics.semantics
+import androidx.compose.ui.platform.LocalConfiguration
+import androidx.compose.ui.text.Placeholder
+import androidx.compose.ui.text.PlaceholderVerticalAlign
+import androidx.compose.ui.text.buildAnnotatedString
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.unit.dp
 import androidx.wear.compose.material.MaterialTheme
 import androidx.wear.compose.material.Text
 import com.google.android.horologist.annotations.ExperimentalHorologistApi
 import com.google.android.horologist.images.base.paintable.Paintable
+import com.google.android.horologist.media.ui.components.controls.MediaTitleIcon
+import com.google.android.horologist.media.ui.util.isLargeScreen
 
 /**
  * A simple text only display showing artist and title in two separated rows.
@@ -47,35 +49,47 @@ public fun TextMediaDisplay(
     titleIcon: Paintable? = null,
     modifier: Modifier = Modifier,
 ) {
+    val isLargeScreen = LocalConfiguration.current.isLargeScreen
+
     Column(modifier = modifier, horizontalAlignment = Alignment.CenterHorizontally) {
-        Row(
-            modifier = Modifier
-                .fillMaxWidth(0.7f)
-                .padding(top = 2.dp, bottom = .8.dp)
-                .semantics(mergeDescendants = true) {},
-            horizontalArrangement = Arrangement.spacedBy(4.dp),
-        ) {
+        val textStyle = MaterialTheme.typography.button
+        val text = buildAnnotatedString {
             if (titleIcon != null) {
-                Image(
-                    painter = titleIcon.rememberPainter(),
-                    contentDescription = null,
-                    colorFilter = ColorFilter.tint(MaterialTheme.colors.onBackground),
-                    contentScale = ContentScale.FillHeight,
-                )
+                appendInlineContent(id = "iconSlot")
+                append(" ")
             }
-            Text(
-                text = title,
-                color = MaterialTheme.colors.onBackground,
-                textAlign = TextAlign.Center,
-                maxLines = 1,
-                style = MaterialTheme.typography.button,
+            append(title)
+        }
+        val inlineContent = if (titleIcon != null) {
+            mapOf(
+                "iconSlot" to InlineTextContent(
+                    Placeholder(textStyle.fontSize, textStyle.fontSize, PlaceholderVerticalAlign.TextCenter),
+                ) {
+                    MediaTitleIcon(titleIcon)
+                },
             )
+        } else {
+            emptyMap()
         }
+        Text(
+            text = text,
+            inlineContent = inlineContent,
+            modifier = Modifier
+                .fillMaxWidth(0.7f)
+                .padding(
+                    top = if (isLargeScreen) 0.dp else 2.dp,
+                    bottom = if (isLargeScreen) 3.dp else 1.dp,
+                ),
+            color = MaterialTheme.colors.onBackground,
+            textAlign = TextAlign.Center,
+            maxLines = 1,
+            style = textStyle,
+        )
         Text(
             text = subtitle,
             modifier = Modifier
                 .fillMaxWidth(0.8f)
-                .padding(top = 2.dp, bottom = .6.dp),
+                .padding(top = 1.dp, bottom = .6.dp),
             color = MaterialTheme.colors.onBackground,
             textAlign = TextAlign.Center,
             overflow = TextOverflow.Ellipsis,

diff --git a/...rc/test/java/com/google/android/horologist/media/ui/components/LoadingMediaDisplayTest.kt b/...rc/test/java/com/google/android/horologist/media/ui/components/LoadingMediaDisplayTest.kt
@@ -24,6 +24,7 @@ import com.google.android.horologist.media.ui.components.display.TextMediaDispla
 import com.google.android.horologist.screenshots.ScreenshotBaseTest
 import com.google.android.horologist.screenshots.ScreenshotTestRule
 import org.junit.Test
+import org.robolectric.annotation.Config
 
 class LoadingMediaDisplayTest : ScreenshotBaseTest(
     ScreenshotTestRule.screenshotTestRuleParams {
@@ -38,15 +39,37 @@ class LoadingMediaDisplayTest : ScreenshotBaseTest(
     }
 
     @Test
-    fun loadingMediaDisplay_textMediaDisplay_overlay() {
+    fun loadingMediaDisplay_textMediaDisplay_overlay_largeScreen() {
         screenshotTestRule.setContent(isComponent = true, takeScreenshot = true) {
             LoadingMediaDisplay(modifier = Modifier.alpha(0.5f))
             TextMediaDisplay(title = "Sorrow", subtitle = "David Bowie")
         }
     }
 
     @Test
-    fun loadingMediaDisplay_marqueeTextMediaDisplay_overlay() {
+    fun loadingMediaDisplay_marqueeTextMediaDisplay_overlay_largeScreen() {
+        screenshotTestRule.setContent(isComponent = true, takeScreenshot = true) {
+            LoadingMediaDisplay(modifier = Modifier.alpha(0.5f))
+            MarqueeTextMediaDisplay(title = "Sorrow", artist = "David Bowie")
+        }
+    }
+
+    @Config(
+        qualifiers = "+w192dp-h192dp",
+    )
+    @Test
+    fun loadingMediaDisplay_textMediaDisplay_overlay_smallScreen() {
+        screenshotTestRule.setContent(isComponent = true, takeScreenshot = true) {
+            LoadingMediaDisplay(modifier = Modifier.alpha(0.5f))
+            TextMediaDisplay(title = "Sorrow", subtitle = "David Bowie")
+        }
+    }
+
+    @Config(
+        qualifiers = "+w192dp-h192dp",
+    )
+    @Test
+    fun loadingMediaDisplay_marqueeTextMediaDisplay_overlay_smallScreen() {
         screenshotTestRule.setContent(isComponent = true, takeScreenshot = true) {
             LoadingMediaDisplay(modifier = Modifier.alpha(0.5f))
             MarqueeTextMediaDisplay(title = "Sorrow", artist = "David Bowie")

diff --git a/...ogle.android.horologist.media.ui.components_LoadingMediaDisplayTest_default.png b/...ogle.android.horologist.media.ui.components_LoadingMediaDisplayTest_default.png
diff --git a/...DisplayTest_loadingMediaDisplay_marqueeTextMediaDisplay_overlay_largeScreen.png b/...DisplayTest_loadingMediaDisplay_marqueeTextMediaDisplay_overlay_largeScreen.png
diff --git a/...DisplayTest_loadingMediaDisplay_marqueeTextMediaDisplay_overlay_smallScreen.png b/...DisplayTest_loadingMediaDisplay_marqueeTextMediaDisplay_overlay_smallScreen.png
diff --git a/...ngMediaDisplayTest_loadingMediaDisplay_textMediaDisplay_overlay_largeScreen.png b/...ngMediaDisplayTest_loadingMediaDisplay_textMediaDisplay_overlay_largeScreen.png
diff --git a/...ngMediaDisplayTest_loadingMediaDisplay_textMediaDisplay_overlay_smallScreen.png b/...ngMediaDisplayTest_loadingMediaDisplay_textMediaDisplay_overlay_smallScreen.png
diff --git a/...ts_MarqueeTextMediaDisplayTest_givenPaintableIcon_thenTintedIconIsDisplayed.png b/...ts_MarqueeTextMediaDisplayTest_givenPaintableIcon_thenTintedIconIsDisplayed.png
diff --git a/....components_MarqueeTextMediaDisplayTest_givenPaintable_thenImageIsDisplayed.png b/....components_MarqueeTextMediaDisplayTest_givenPaintable_thenImageIsDisplayed.png
diff --git a/...queeTextMediaDisplayTest_givenTitleAndArtist_thenTitleAndArtistAreDisplayed.png b/...queeTextMediaDisplayTest_givenTitleAndArtist_thenTitleAndArtistAreDisplayed.png
diff --git a/...droid.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerLargeRound.png b/...droid.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerLargeRound.png
diff --git a/...e.android.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerScreen.png b/...e.android.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerScreen.png
diff --git a/...droid.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerSmallRound.png b/...droid.horologist.media.ui_MediaPlayerDeviceScreenTest_mediaPlayerSmallRound.png
diff --git a/...ogist.media.ui_MediaPlayerScreenTest_mediaPlayerScreen[0]_bluedefaultaecbfa.png b/...ogist.media.ui_MediaPlayerScreenTest_mediaPlayerScreen[0]_bluedefaultaecbfa.png
diff --git a/...d.horologist.media.ui_MediaPlayerScreenTest_mediaPlayerScreen[1]_blue7fcfff.png b/...d.horologist.media.ui_MediaPlayerScreenTest_mediaPlayerScreen[1]_blue7fcfff.png