Merge pull request #3080 from element-hq/feature/bma/improveBubbleLayout

Improve the way we cut the bubble layout to give space for the sender Avatar
element-hq · Jun 24, 2024 · cb14be2 · cb14be2
2 parents 010a90f + d14b9de
commit cb14be2
Show file tree

Hide file tree

Showing 153 changed files with 353 additions and 340 deletions.
diff --git a/...otlin/io/element/android/features/messages/impl/timeline/components/MessageEventBubble.kt b/...otlin/io/element/android/features/messages/impl/timeline/components/MessageEventBubble.kt
@@ -32,16 +32,24 @@ import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
+import androidx.compose.ui.draw.drawWithContent
+import androidx.compose.ui.geometry.Offset
+import androidx.compose.ui.graphics.BlendMode
+import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.graphics.CompositingStrategy
 import androidx.compose.ui.graphics.Shape
+import androidx.compose.ui.graphics.graphicsLayer
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.messages.impl.timeline.model.TimelineItemGroupPosition
 import io.element.android.features.messages.impl.timeline.model.bubble.BubbleState
 import io.element.android.features.messages.impl.timeline.model.bubble.BubbleStateProvider
 import io.element.android.libraries.core.extensions.to01
+import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.designsystem.preview.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
+import io.element.android.libraries.designsystem.text.toPx
 import io.element.android.libraries.designsystem.theme.components.Surface
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.designsystem.theme.messageFromMeBackground
@@ -51,6 +59,7 @@ import io.element.android.libraries.testtags.testTag
 
 private val BUBBLE_RADIUS = 12.dp
 internal val BUBBLE_INCOMING_OFFSET = 16.dp
+private val avatarRadius = AvatarSize.TimelineSender.dp / 2
 
 // Design says: The maximum width of a bubble is still 3/4 of the screen width. But try with 85% now.
 private const val BUBBLE_WIDTH_RATIO = 0.85f
@@ -66,11 +75,12 @@ fun MessageEventBubble(
     content: @Composable () -> Unit = {},
 ) {
     fun bubbleShape(): Shape {
+        val topLeftCorner = if (state.cutTopStart) 0.dp else BUBBLE_RADIUS
         return when (state.groupPosition) {
             TimelineItemGroupPosition.First -> if (state.isMine) {
                 RoundedCornerShape(BUBBLE_RADIUS, BUBBLE_RADIUS, 0.dp, BUBBLE_RADIUS)
             } else {
-                RoundedCornerShape(BUBBLE_RADIUS, BUBBLE_RADIUS, BUBBLE_RADIUS, 0.dp)
+                RoundedCornerShape(topLeftCorner, BUBBLE_RADIUS, BUBBLE_RADIUS, 0.dp)
             }
             TimelineItemGroupPosition.Middle -> if (state.isMine) {
                 RoundedCornerShape(BUBBLE_RADIUS, 0.dp, 0.dp, BUBBLE_RADIUS)
@@ -84,7 +94,7 @@ fun MessageEventBubble(
             }
             TimelineItemGroupPosition.None ->
                 RoundedCornerShape(
-                    BUBBLE_RADIUS,
+                    topLeftCorner,
                     BUBBLE_RADIUS,
                     BUBBLE_RADIUS,
                     BUBBLE_RADIUS
@@ -106,11 +116,30 @@ fun MessageEventBubble(
         else -> ElementTheme.colors.messageFromOtherBackground
     }
     val bubbleShape = bubbleShape()
+    val radiusPx = (avatarRadius + SENDER_AVATAR_BORDER_WIDTH).toPx()
+    val yOffsetPx = -(NEGATIVE_MARGIN_FOR_BUBBLE + avatarRadius).toPx()
     Box(
         modifier = modifier
             .fillMaxWidth(BUBBLE_WIDTH_RATIO)
-            .padding(horizontal = 16.dp)
-            .offsetForItem(),
+            .padding(start = avatarRadius, end = 16.dp)
+            .offsetForItem()
+            .graphicsLayer {
+                compositingStrategy = CompositingStrategy.Offscreen
+            }
+            .drawWithContent {
+                drawContent()
+                if (state.cutTopStart) {
+                    drawCircle(
+                        color = Color.Black,
+                        center = Offset(
+                            x = 0f,
+                            y = yOffsetPx,
+                        ),
+                        radius = radiusPx,
+                        blendMode = BlendMode.Clear,
+                    )
+                }
+            },
         // Need to set the contentAlignment again (it's already set in TimelineItemEventRow), for the case
         // when content width is low.
         contentAlignment = if (state.isMine) Alignment.CenterEnd else Alignment.CenterStart

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
@@ -17,7 +17,6 @@
 package io.element.android.features.messages.impl.timeline.components
 
 import android.annotation.SuppressLint
-import androidx.compose.foundation.Canvas
 import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.gestures.Orientation
@@ -46,8 +45,6 @@ import androidx.compose.ui.Alignment
 import androidx.compose.ui.ExperimentalComposeUiApi
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
-import androidx.compose.ui.draw.clipToBounds
-import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.platform.LocalViewConfiguration
 import androidx.compose.ui.platform.ViewConfiguration
 import androidx.compose.ui.res.stringResource
@@ -59,7 +56,6 @@ import androidx.compose.ui.semantics.testTag
 import androidx.compose.ui.text.font.FontStyle
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.text.style.TextOverflow
-import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.DpOffset
 import androidx.compose.ui.unit.IntOffset
 import androidx.compose.ui.unit.dp
@@ -118,6 +114,13 @@ import kotlinx.coroutines.launch
 import kotlin.math.abs
 import kotlin.math.roundToInt
 
+// The bubble has a negative margin to be placed a bit upper regarding the sender
+// information and overlap the avatar.
+val NEGATIVE_MARGIN_FOR_BUBBLE = (-8).dp
+
+// Width of the transparent border around the sender avatar
+val SENDER_AVATAR_BORDER_WIDTH = 3.dp
+
 @Composable
 fun TimelineItemEventRow(
     event: TimelineItem.Event,
@@ -289,13 +292,11 @@ private fun TimelineItemEventRowContent(
         ) = createRefs()
 
         // Sender
-        val avatarStrokeSize = 3.dp
         if (event.showSenderInformation && !timelineRoomInfo.isDm) {
             MessageSenderInformation(
                 event.senderId,
                 event.senderProfile,
                 event.senderAvatar,
-                avatarStrokeSize,
                 Modifier
                     .constrainAs(sender) {
                         top.linkTo(parent.top)
@@ -321,7 +322,7 @@ private fun TimelineItemEventRowContent(
         MessageEventBubble(
             modifier = Modifier
                 .constrainAs(message) {
-                    top.linkTo(sender.bottom, margin = -avatarStrokeSize - 8.dp)
+                    top.linkTo(sender.bottom, margin = NEGATIVE_MARGIN_FOR_BUBBLE)
                     this.linkStartOrEnd(event)
                 },
             state = bubbleState,
@@ -373,37 +374,17 @@ private fun MessageSenderInformation(
     senderId: UserId,
     senderProfile: ProfileTimelineDetails,
     senderAvatar: AvatarData,
-    avatarStrokeSize: Dp,
     modifier: Modifier = Modifier
 ) {
-    val avatarStrokeColor = MaterialTheme.colorScheme.background
-    val avatarSize = senderAvatar.size.dp
     val avatarColors = AvatarColorsProvider.provide(senderAvatar.id, ElementTheme.isLightTheme)
-    Box(
-        modifier = modifier
-    ) {
-        // Background of Avatar, to erase the corner of the message content
-        Canvas(
-            modifier = Modifier
-                .size(size = avatarSize + avatarStrokeSize)
-                .clipToBounds()
-        ) {
-            drawCircle(
-                color = avatarStrokeColor,
-                center = Offset(x = (avatarSize / 2).toPx(), y = (avatarSize / 2).toPx()),
-                radius = (avatarSize / 2 + avatarStrokeSize).toPx()
-            )
-        }
-        // Content
-        Row {
-            Avatar(senderAvatar)
-            Spacer(modifier = Modifier.width(4.dp))
-            SenderName(
-                senderId = senderId,
-                senderProfile = senderProfile,
-                senderNameMode = SenderNameMode.Timeline(avatarColors.foreground),
-            )
-        }
+    Row(modifier = modifier) {
+        Avatar(senderAvatar)
+        Spacer(modifier = Modifier.width(4.dp))
+        SenderName(
+            senderId = senderId,
+            senderProfile = senderProfile,
+            senderNameMode = SenderNameMode.Timeline(avatarColors.foreground),
+        )
     }
 }
 

diff --git a/...ain/kotlin/io/element/android/features/messages/impl/timeline/model/bubble/BubbleState.kt b/...ain/kotlin/io/element/android/features/messages/impl/timeline/model/bubble/BubbleState.kt
@@ -24,4 +24,7 @@ data class BubbleState(
     val isMine: Boolean,
     val isHighlighted: Boolean,
     val timelineRoomInfo: TimelineRoomInfo,
-)
+) {
+    /** True to cut out the top start corner of the bubble, to give margin for the sender avatar. */
+    val cutTopStart: Boolean = groupPosition.isNew() && !isMine && !timelineRoomInfo.isDm
+}
diff --git a/...hCaptionRow_null_TimelineImageWithCaptionRow-Day-41_41_null,NEXUS_5,1.0,en].png b/...hCaptionRow_null_TimelineImageWithCaptionRow-Day-41_41_null,NEXUS_5,1.0,en].png
diff --git a/...aptionRow_null_TimelineImageWithCaptionRow-Night-41_42_null,NEXUS_5,1.0,en].png b/...aptionRow_null_TimelineImageWithCaptionRow-Night-41_42_null,NEXUS_5,1.0,en].png
diff --git a/...hCaptionRow_null_TimelineVideoWithCaptionRow-Day-52_52_null,NEXUS_5,1.0,en].png b/...hCaptionRow_null_TimelineVideoWithCaptionRow-Day-52_52_null,NEXUS_5,1.0,en].png
diff --git a/...aptionRow_null_TimelineVideoWithCaptionRow-Night-52_53_null,NEXUS_5,1.0,en].png b/...aptionRow_null_TimelineVideoWithCaptionRow-Night-52_53_null,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_0,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_0,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_1,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_1,NEXUS_5,1.0,en].png
diff --git a/...essageEventBubble_null_MessageEventBubble-Day-10_10_null_12,NEXUS_5,1.0,en].png b/...essageEventBubble_null_MessageEventBubble-Day-10_10_null_12,NEXUS_5,1.0,en].png
diff --git a/...essageEventBubble_null_MessageEventBubble-Day-10_10_null_13,NEXUS_5,1.0,en].png b/...essageEventBubble_null_MessageEventBubble-Day-10_10_null_13,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_4,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_4,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_5,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_5,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_8,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_8,NEXUS_5,1.0,en].png
diff --git a/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_9,NEXUS_5,1.0,en].png b/...MessageEventBubble_null_MessageEventBubble-Day-10_10_null_9,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_0,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_0,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_1,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_1,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_10,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_10,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_11,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_11,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_12,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_12,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_13,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_13,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_14,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_14,NEXUS_5,1.0,en].png
diff --git a/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_15,NEXUS_5,1.0,en].png b/...sageEventBubble_null_MessageEventBubble-Night-10_11_null_15,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_2,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_2,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_3,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_3,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_4,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_4,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_5,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_5,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_6,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_6,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_7,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_7,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_8,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_8,NEXUS_5,1.0,en].png
diff --git a/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_9,NEXUS_5,1.0,en].png b/...ssageEventBubble_null_MessageEventBubble-Night-10_11_null_9,NEXUS_5,1.0,en].png
diff --git a/...ted_null_TimelineItemEventRowDisambiguated-Day-19_19_null_0,NEXUS_5,1.0,en].png b/...ted_null_TimelineItemEventRowDisambiguated-Day-19_19_null_0,NEXUS_5,1.0,en].png
diff --git a/...d_null_TimelineItemEventRowDisambiguated-Night-19_20_null_0,NEXUS_5,1.0,en].png b/...d_null_TimelineItemEventRowDisambiguated-Night-19_20_null_0,NEXUS_5,1.0,en].png
diff --git a/...tRoom_null_TimelineItemEventRowForDirectRoom-Day-20_20_null,NEXUS_5,1.0,en].png b/...tRoom_null_TimelineItemEventRowForDirectRoom-Day-20_20_null,NEXUS_5,1.0,en].png
diff --git a/...oom_null_TimelineItemEventRowForDirectRoom-Night-20_21_null,NEXUS_5,1.0,en].png b/...oom_null_TimelineItemEventRowForDirectRoom-Night-20_21_null,NEXUS_5,1.0,en].png