Fix oversize padding on captioned images/videos

Use consistent padding with the InReplyToView for the media, and consistent caption padding with other textual messages. Signed-off-by: Joe Groocock <[email protected]>
element-hq · Oct 23, 2024 · 53100ed · 53100ed
1 parent 14fae6c
commit 53100ed
Show file tree

Hide file tree

Showing 45 changed files with 125 additions and 103 deletions.
diff --git a/...in/kotlin/io/element/android/features/messages/impl/timeline/components/ContentPadding.kt b/...in/kotlin/io/element/android/features/messages/impl/timeline/components/ContentPadding.kt
@@ -0,0 +1,14 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.messages.impl.timeline.components
+
+enum class ContentPadding {
+    Textual,
+    Media,
+    CaptionedMedia;
+}
diff --git a/...lin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt b/...lin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventRow.kt
@@ -522,32 +522,30 @@ private fun MessageEventBubbleContent(
     fun CommonLayout(
         timestampPosition: TimestampPosition,
         showThreadDecoration: Boolean,
+        paddingBehaviour: ContentPadding,
         inReplyToDetails: InReplyToDetails?,
         modifier: Modifier = Modifier,
         canShrinkContent: Boolean = false,
     ) {
-        val timestampLayoutModifier: Modifier
-        val contentModifier: Modifier
-        when {
-            inReplyToDetails != null -> {
-                if (timestampPosition == TimestampPosition.Overlay) {
-                    timestampLayoutModifier = Modifier.padding(start = 8.dp, end = 8.dp, bottom = 8.dp)
-                    contentModifier = Modifier.clip(RoundedCornerShape(12.dp))
-                } else {
-                    contentModifier = Modifier.padding(start = 12.dp, end = 12.dp, top = 0.dp, bottom = 8.dp)
-                    timestampLayoutModifier = Modifier
-                }
-            }
-            timestampPosition != TimestampPosition.Overlay -> {
-                timestampLayoutModifier = Modifier
-                contentModifier = Modifier
-                    .padding(start = 12.dp, end = 12.dp, top = 8.dp, bottom = 8.dp)
+        val timestampLayoutModifier =
+            if (inReplyToDetails != null && timestampPosition == TimestampPosition.Overlay) {
+                Modifier.padding(start = 8.dp, end = 8.dp, bottom = 8.dp)
+            } else {
+                Modifier
             }
-            else -> {
-                timestampLayoutModifier = Modifier
-                contentModifier = Modifier
+
+        val topPadding = if (inReplyToDetails != null) 0.dp else 8.dp
+        val contentModifier = when (paddingBehaviour) {
+            ContentPadding.Textual ->
+                Modifier.padding(start = 12.dp, end = 12.dp, top = topPadding, bottom = 8.dp)
+            ContentPadding.Media -> {
+                if (inReplyToDetails == null) Modifier
+                else Modifier.clip(RoundedCornerShape(10.dp))
             }
+            ContentPadding.CaptionedMedia ->
+                Modifier.padding(start = 8.dp, end = 8.dp, top = topPadding, bottom = 8.dp)
         }
+
         val threadDecoration = @Composable {
             if (showThreadDecoration) {
                 ThreadDecoration(modifier = Modifier.padding(top = 8.dp, start = 12.dp, end = 12.dp))
@@ -601,9 +599,17 @@ private fun MessageEventBubbleContent(
         is TimelineItemPollContent -> TimestampPosition.Below
         else -> TimestampPosition.Default
     }
+    val paddingBehaviour = when (event.content) {
+        is TimelineItemImageContent -> if (event.content.showCaption) ContentPadding.CaptionedMedia else ContentPadding.Media
+        is TimelineItemVideoContent -> if (event.content.showCaption) ContentPadding.CaptionedMedia else ContentPadding.Media
+        is TimelineItemStickerContent,
+        is TimelineItemLocationContent -> ContentPadding.Media
+        else -> ContentPadding.Textual
+    }
     CommonLayout(
         showThreadDecoration = event.isThreaded,
         timestampPosition = timestampPosition,
+        paddingBehaviour = paddingBehaviour,
         inReplyToDetails = event.inReplyTo,
         canShrinkContent = event.content is TimelineItemVoiceContent,
         modifier = bubbleModifier.semantics(mergeDescendants = true) {

diff --git a/...ndroid/features/messages/impl/timeline/components/TimelineItemEventRowWithReplyPreview.kt b/...ndroid/features/messages/impl/timeline/components/TimelineItemEventRowWithReplyPreview.kt
@@ -50,7 +50,9 @@ internal fun TimelineItemEventRowWithReplyContentToPreview(
                     isMine = it,
                     timelineItemReactions = aTimelineItemReactions(count = 0),
                     content = aTimelineItemImageContent(
-                        aspectRatio = 2.5f
+                        aspectRatio = 2.5f,
+                        filename = "image.jpg",
+                        caption = "A reply with an image.",
                     ),
                     inReplyTo = inReplyToDetails,
                     displayNameAmbiguous = displayNameAmbiguous,

diff --git a/...element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt b/...element/android/features/messages/impl/timeline/components/event/TimelineItemImageView.kt
@@ -69,9 +69,7 @@ fun TimelineItemImageView(
         modifier = modifier.semantics { contentDescription = description },
     ) {
         val containerModifier = if (content.showCaption) {
-            Modifier
-                .padding(top = 6.dp)
-                .clip(RoundedCornerShape(6.dp))
+            Modifier.clip(RoundedCornerShape(10.dp))
         } else {
             Modifier
         }
@@ -119,6 +117,7 @@ fun TimelineItemImageView(
                 val aspectRatio = content.aspectRatio ?: DEFAULT_ASPECT_RATIO
                 EditorStyledText(
                     modifier = Modifier
+                        .padding(horizontal = 4.dp)  // This is (12.dp - 8.dp) contentPadding from CommonLayout
                         .widthIn(min = MIN_HEIGHT_IN_DP.dp * aspectRatio, max = MAX_HEIGHT_IN_DP.dp * aspectRatio),
                     text = caption,
                     style = ElementRichTextEditorStyle.textStyle(),

diff --git a/...element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt b/...element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt
@@ -137,6 +137,7 @@ fun TimelineItemVideoView(
                 val aspectRatio = content.aspectRatio ?: DEFAULT_ASPECT_RATIO
                 EditorStyledText(
                     modifier = Modifier
+                        .padding(horizontal = 4.dp)  // This is (12.dp - 8.dp) contentPadding from CommonLayout
                         .widthIn(min = MIN_HEIGHT_IN_DP.dp * aspectRatio, max = MAX_HEIGHT_IN_DP.dp * aspectRatio),
                     text = caption,
                     style = ElementRichTextEditorStyle.textStyle(),

diff --git a/...essages.impl.timeline.components.event_TimelineImageWithCaptionRow_Day_0_en.png b/...essages.impl.timeline.components.event_TimelineImageWithCaptionRow_Day_0_en.png
diff --git a/...sages.impl.timeline.components.event_TimelineImageWithCaptionRow_Night_0_en.png b/...sages.impl.timeline.components.event_TimelineImageWithCaptionRow_Night_0_en.png
diff --git a/...essages.impl.timeline.components.event_TimelineVideoWithCaptionRow_Day_0_en.png b/...essages.impl.timeline.components.event_TimelineVideoWithCaptionRow_Day_0_en.png
diff --git a/...sages.impl.timeline.components.event_TimelineVideoWithCaptionRow_Night_0_en.png b/...sages.impl.timeline.components.event_TimelineVideoWithCaptionRow_Night_0_en.png
diff --git a/...mpl.timeline.components.reactionsummary_ReactionSummaryViewContent_Day_0_en.png b/...mpl.timeline.components.reactionsummary_ReactionSummaryViewContent_Day_0_en.png
diff --git a/...l.timeline.components.reactionsummary_ReactionSummaryViewContent_Night_0_en.png b/...l.timeline.components.reactionsummary_ReactionSummaryViewContent_Night_0_en.png
diff --git a/...essages.impl.timeline.components_TimelineItemEventRowDisambiguated_Day_0_en.png b/...essages.impl.timeline.components_TimelineItemEventRowDisambiguated_Day_0_en.png
diff --git a/...sages.impl.timeline.components_TimelineItemEventRowDisambiguated_Night_0_en.png b/...sages.impl.timeline.components_TimelineItemEventRowDisambiguated_Night_0_en.png
diff --git a/....impl.timeline.components_TimelineItemEventRowWithReplyInformative_Day_0_en.png b/....impl.timeline.components_TimelineItemEventRowWithReplyInformative_Day_0_en.png
diff --git a/....impl.timeline.components_TimelineItemEventRowWithReplyInformative_Day_1_en.png b/....impl.timeline.components_TimelineItemEventRowWithReplyInformative_Day_1_en.png
diff --git a/...mpl.timeline.components_TimelineItemEventRowWithReplyInformative_Night_0_en.png b/...mpl.timeline.components_TimelineItemEventRowWithReplyInformative_Night_0_en.png
diff --git a/...mpl.timeline.components_TimelineItemEventRowWithReplyInformative_Night_1_en.png b/...mpl.timeline.components_TimelineItemEventRowWithReplyInformative_Night_1_en.png
diff --git a/...ssages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Day_0_en.png b/...ssages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Day_0_en.png
diff --git a/...ssages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Day_1_en.png b/...ssages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Day_1_en.png
diff --git a/...ages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Night_0_en.png b/...ages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Night_0_en.png
diff --git a/...ages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Night_1_en.png b/...ages.impl.timeline.components_TimelineItemEventRowWithReplyOther_Night_1_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_0_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_0_en.png
diff --git a/...s.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_10_en.png b/...s.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_10_en.png
diff --git a/...s.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_11_en.png b/...s.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_11_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_1_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_1_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_2_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_2_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_3_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_3_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_4_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_4_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_5_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_5_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_6_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_6_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_7_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_7_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_8_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_8_en.png
diff --git a/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_9_en.png b/...es.messages.impl.timeline.components_TimelineItemEventRowWithReply_Day_9_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_0_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_0_en.png
diff --git a/...messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_10_en.png b/...messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_10_en.png
diff --git a/...messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_11_en.png b/...messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_11_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_1_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_1_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_2_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_2_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_3_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_3_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_4_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_4_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_5_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_5_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_6_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_6_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_7_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_7_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_8_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_8_en.png
diff --git a/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_9_en.png b/....messages.impl.timeline.components_TimelineItemEventRowWithReply_Night_9_en.png