Fix multi-line reactions blocking message content (#785)

Fixes #753 --------- Co-authored-by: ElementBot <[email protected]>
element-hq · Jul 6, 2023 · 5b7c42a · 5b7c42a
1 parent 7e8228e
commit 5b7c42a
Show file tree

Hide file tree

Showing 49 changed files with 207 additions and 135 deletions.
diff --git a/features/messages/impl/build.gradle.kts b/features/messages/impl/build.gradle.kts
@@ -54,6 +54,8 @@ dependencies {
     implementation(libs.accompanist.flowlayout)
     implementation(libs.androidx.recyclerview)
     implementation(libs.jsoup)
+    implementation(libs.androidx.constraintlayout)
+    implementation(libs.androidx.constraintlayout.compose)
     implementation(libs.androidx.media3.exoplayer)
     implementation(libs.androidx.media3.ui)
     implementation(libs.accompanist.systemui)

diff --git a/...c/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt b/...c/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt
@@ -139,10 +139,12 @@ fun aTimelineItemReactions(
     count: Int = 1,
     isHighlighted: Boolean = false,
 ): TimelineItemReactions {
+    val emojis = arrayOf("👍", "😀️", "😁️", "😆️", "😅️", "🤣️", "🥰️", "😇️", "😊️", "😉️", "🙃️", "🙂️", "😍️", "🤗️", "🤭️")
     return TimelineItemReactions(
         reactions = buildList {
-            repeat(count) {
-                add(AggregatedReaction(key = "👍", count = 1 + it, isHighlighted = isHighlighted))
+            repeat(count) { index ->
+                val key = emojis[index % emojis.size]
+                add(AggregatedReaction(key = key, count = 1 + index, isHighlighted = isHighlighted))
             }
         }.toPersistentList()
     )

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
@@ -54,8 +54,13 @@ import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.text.style.TextOverflow
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
+import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
+import androidx.compose.ui.zIndex
+import androidx.constraintlayout.compose.ConstrainScope
+import androidx.constraintlayout.compose.ConstraintLayout
+import com.google.accompanist.flowlayout.FlowMainAxisAlignment
 import io.element.android.features.messages.impl.timeline.aTimelineItemEvent
 import io.element.android.features.messages.impl.timeline.aTimelineItemReactions
 import io.element.android.features.messages.impl.timeline.components.event.TimelineItemEventContentView
@@ -183,67 +188,78 @@ private fun TimelineItemEventRowContent(
     onMoreReactionsClicked: (event: TimelineItem.Event) -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    // To avoid using negative offset, we display in this Box a column with:
-    // - Spacer to give room to the Sender information if they must be displayed;
-    // - The message bubble;
-    // - Spacer for the reactions if there are some.
-    // Then the Sender information and the reactions are displayed on top of it.
-    // This fixes some clickable issue and some unexpected margin on top and bottom of each message row
-    Box(
+    fun ConstrainScope.linkStartOrEnd(event: TimelineItem.Event) = if (event.isMine) {
+        end.linkTo(parent.end)
+    } else {
+        start.linkTo(parent.start)
+    }
+
+    ConstraintLayout(
         modifier = modifier
-            .fillMaxWidth()
-            .wrapContentHeight(),
-        contentAlignment = if (event.isMine) Alignment.CenterEnd else Alignment.CenterStart
+            .wrapContentHeight()
+            .fillMaxWidth(),
     ) {
-        Column {
-            if (event.showSenderInformation) {
-                Spacer(modifier = Modifier.height(event.senderAvatar.size.dp - 8.dp))
-            }
-            val bubbleState = BubbleState(
-                groupPosition = event.groupPosition,
-                isMine = event.isMine,
-                isHighlighted = isHighlighted,
-            )
-            MessageEventBubble(
-                state = bubbleState,
-                interactionSource = interactionSource,
-                onClick = onClick,
-                onLongClick = onLongClick,
-            ) {
-                MessageEventBubbleContent(
-                    event = event,
-                    interactionSource = interactionSource,
-                    onMessageClick = onClick,
-                    onMessageLongClick = onLongClick,
-                    inReplyToClick = inReplyToClicked,
-                    onTimestampClicked = {
-                        onTimestampClicked(event)
-                    }
-                )
-            }
-            if (event.reactionsState.reactions.isNotEmpty()) {
-                Spacer(modifier = Modifier.height(28.dp))
-            }
-        }
-        // Align to the top of the box
+        val (sender, message, reactions) = createRefs()
+
+        // Sender
+        val avatarStrokeSize = 3.dp
         if (event.showSenderInformation) {
             MessageSenderInformation(
                 event.safeSenderName,
                 event.senderAvatar,
+                avatarStrokeSize,
                 Modifier
+                    .constrainAs(sender) {
+                        top.linkTo(parent.top)
+                    }
                     .padding(horizontal = 16.dp)
-                    .align(Alignment.TopStart)
+                    .zIndex(1f)
                     .clickable(onClick = onUserDataClicked)
             )
         }
-        // Align to the bottom of the box
+
+        // Message bubble
+        val bubbleState = BubbleState(
+            groupPosition = event.groupPosition,
+            isMine = event.isMine,
+            isHighlighted = isHighlighted,
+        )
+        MessageEventBubble(
+            modifier = Modifier
+                .constrainAs(message) {
+                    top.linkTo(sender.bottom, margin = -avatarStrokeSize - 8.dp)
+                    this.linkStartOrEnd(event)
+                },
+            state = bubbleState,
+            interactionSource = interactionSource,
+            onClick = onClick,
+            onLongClick = onLongClick,
+        ) {
+            MessageEventBubbleContent(
+                event = event,
+                interactionSource = interactionSource,
+                onMessageClick = onClick,
+                onMessageLongClick = onLongClick,
+                inReplyToClick = inReplyToClicked,
+                onTimestampClicked = {
+                    onTimestampClicked(event)
+                }
+            )
+        }
+
+        // Reactions
         if (event.reactionsState.reactions.isNotEmpty()) {
             TimelineItemReactionsView(
                 reactionsState = event.reactionsState,
+                mainAxisAlignment = if (event.isMine) FlowMainAxisAlignment.End else FlowMainAxisAlignment.Start,
                 onReactionClicked = onReactionClicked,
                 onMoreReactionsClicked = { onMoreReactionsClicked(event) },
                 modifier = Modifier
-                    .align(if (event.isMine) Alignment.BottomEnd else Alignment.BottomStart)
+                    .constrainAs(reactions) {
+                        top.linkTo(message.bottom, margin = (-4).dp)
+                        this.linkStartOrEnd(event)
+                    }
+                    .zIndex(1f)
                     .padding(start = if (event.isMine) 16.dp else 36.dp, end = 16.dp)
             )
         }
@@ -262,9 +278,9 @@ private fun DismissState.toSwipeProgress(): Float {
 private fun MessageSenderInformation(
     sender: String,
     senderAvatar: AvatarData,
+    avatarStrokeSize: Dp,
     modifier: Modifier = Modifier
 ) {
-    val avatarStrokeSize = 3.dp
     val avatarStrokeColor = MaterialTheme.colorScheme.background
     val avatarSize = senderAvatar.size.dp
     Box(
@@ -527,7 +543,7 @@ private fun ContentToPreview() {
                     content = aTimelineItemTextContent().copy(
                         body = "A long text which will be displayed on several lines and" +
                             " hopefully can be manually adjusted to test different behaviors."
-                    )
+                    ),
                 ),
                 isHighlighted = false,
                 canReply = true,
@@ -545,7 +561,7 @@ private fun ContentToPreview() {
                     isMine = it,
                     content = aTimelineItemImageContent().copy(
                         aspectRatio = 5f
-                    )
+                    ),
                 ),
                 isHighlighted = false,
                 canReply = true,
@@ -682,3 +698,42 @@ private fun ContentTimestampToPreview(event: TimelineItem.Event) {
         }
     }
 }
+
+
+@Preview
+@Composable
+internal fun TimelineItemEventRowWithManyReactionsLightPreview() =
+    ElementPreviewLight { ContentWithManyReactionsToPreview() }
+
+@Preview
+@Composable
+internal fun TimelineItemEventRowWithManyReactionsDarkPreview() =
+    ElementPreviewDark { ContentWithManyReactionsToPreview() }
+
+@Composable
+private fun ContentWithManyReactionsToPreview() {
+    Column {
+        listOf(false, true).forEach { isMine ->
+            TimelineItemEventRow(
+                event = aTimelineItemEvent(
+                    isMine = isMine,
+                    content = aTimelineItemTextContent().copy(
+                        body = "A couple of multi-line messages with many reactions attached." +
+                            " One sent by me and another from someone else."
+                    ),
+                    timelineItemReactions = aTimelineItemReactions(count = 20),
+                ),
+                isHighlighted = false,
+                canReply = true,
+                onClick = {},
+                onLongClick = {},
+                onUserDataClick = {},
+                inReplyToClick = {},
+                onReactionClick = { _, _ -> },
+                onMoreReactionsClick = {},
+                onSwipeToReply = {},
+                onTimestampClicked = {},
+            )
+        }
+    }
+}
diff --git a/...o/element/android/features/messages/impl/timeline/components/TimelineItemReactionsView.kt b/...o/element/android/features/messages/impl/timeline/components/TimelineItemReactionsView.kt
@@ -20,6 +20,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
+import com.google.accompanist.flowlayout.FlowMainAxisAlignment
 import com.google.accompanist.flowlayout.FlowRow
 import io.element.android.features.messages.impl.timeline.model.TimelineItemReactions
 import io.element.android.features.messages.impl.timeline.model.aTimelineItemReactions
@@ -29,14 +30,16 @@ import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 @Composable
 fun TimelineItemReactionsView(
     reactionsState: TimelineItemReactions,
+    mainAxisAlignment: FlowMainAxisAlignment,
     onReactionClicked: (emoji: String) -> Unit,
     onMoreReactionsClicked: () -> Unit,
     modifier: Modifier = Modifier,
 ) {
     FlowRow(
         modifier = modifier,
-        mainAxisSpacing = 2.dp,
-        crossAxisSpacing = 8.dp,
+        mainAxisSpacing = 4.dp,
+        crossAxisSpacing = 4.dp,
+        mainAxisAlignment = mainAxisAlignment,
     ) {
         reactionsState.reactions.forEach { reaction ->
             MessagesReactionButton(
@@ -64,6 +67,7 @@ internal fun TimelineItemReactionsViewDarkPreview() =
 private fun ContentToPreview() {
     TimelineItemReactionsView(
         reactionsState = aTimelineItemReactions(),
+        mainAxisAlignment = FlowMainAxisAlignment.Center,
         onReactionClicked = {},
         onMoreReactionsClicked = {},
     )

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -13,6 +13,7 @@ material = "1.9.0"
 core = "1.10.1"
 datastore = "1.0.0"
 constraintlayout = "2.1.4"
+constraintlayout_compose = "1.0.1"
 recyclerview = "1.3.0"
 lifecycle = "2.6.1"
 activity = "1.7.2"
@@ -74,6 +75,8 @@ androidx_datastore_preferences = { module = "androidx.datastore:datastore-prefer
 androidx_datastore_datastore = { module = "androidx.datastore:datastore", version.ref = "datastore" }
 androidx_exifinterface = "androidx.exifinterface:exifinterface:1.3.6"
 androidx_constraintlayout = { module = "androidx.constraintlayout:constraintlayout", version.ref = "constraintlayout" }
+androidx_constraintlayout_compose = { module = "androidx.constraintlayout:constraintlayout-compose", version.ref = "constraintlayout_compose" }
+
 androidx_recyclerview = { module = "androidx.recyclerview:recyclerview", version.ref = "recyclerview" }
 androidx_browser = { module = "androidx.browser:browser", version.ref = "browser" }
 androidx_lifecycle_runtime = { module = "androidx.lifecycle:lifecycle-runtime-ktx", version.ref = "lifecycle" }

diff --git a/...ts_null_DefaultGroup_TimelineItemEventRowDarkPreview_0_null,NEXUS_5,1.0,en].png b/...ts_null_DefaultGroup_TimelineItemEventRowDarkPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...s_null_DefaultGroup_TimelineItemEventRowLightPreview_0_null,NEXUS_5,1.0,en].png b/...s_null_DefaultGroup_TimelineItemEventRowLightPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...oup_TimelineItemEventRowWithManyReactionsDarkPreview_0_null,NEXUS_5,1.0,en].png b/...oup_TimelineItemEventRowWithManyReactionsDarkPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...up_TimelineItemEventRowWithManyReactionsLightPreview_0_null,NEXUS_5,1.0,en].png b/...up_TimelineItemEventRowWithManyReactionsLightPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...efaultGroup_TimelineItemEventRowWithReplyDarkPreview_0_null,NEXUS_5,1.0,en].png b/...efaultGroup_TimelineItemEventRowWithReplyDarkPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...faultGroup_TimelineItemEventRowWithReplyLightPreview_0_null,NEXUS_5,1.0,en].png b/...faultGroup_TimelineItemEventRowWithReplyLightPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...ll_DefaultGroup_TimelineItemReactionsViewDarkPreview_0_null,NEXUS_5,1.0,en].png b/...ll_DefaultGroup_TimelineItemReactionsViewDarkPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...l_DefaultGroup_TimelineItemReactionsViewLightPreview_0_null,NEXUS_5,1.0,en].png b/...l_DefaultGroup_TimelineItemReactionsViewLightPreview_0_null,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_0,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_0,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_1,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_1,NEXUS_5,1.0,en].png
diff --git a/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_10,NEXUS_5,1.0,en].png b/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_10,NEXUS_5,1.0,en].png
diff --git a/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_11,NEXUS_5,1.0,en].png b/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_11,NEXUS_5,1.0,en].png
diff --git a/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_12,NEXUS_5,1.0,en].png b/...imeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_12,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_3,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_3,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_4,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_4,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_5,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_5,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_6,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_6,NEXUS_5,1.0,en].png
diff --git a/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_7,NEXUS_5,1.0,en].png b/...timeline_null_DefaultGroup_TimelineViewDarkPreview_0_null_7,NEXUS_5,1.0,en].png