Do not group timeline messages sent more than five minutes apart

Consecutive messages from the same sender were grouped into one bubble block no matter how far apart they were sent, so a reply hours later looked like part of the earlier burst and lost its own timestamp header. computeGroupPosition now folds a five minute limit into the two groupability flags it already computes. The delta is symmetric between a pair of neighbours, so both items always agree on where the block splits and every branch of the existing decision table keeps working unchanged. Breaking a group on reactions, which the iOS client also does, is a separate divergence and is not part of this. Relates to https://github.com/element-hq/element-x-android/issues/4757
2026-11-02 12:07:28 +00:00 · 2026-08-09 03:51:38 +03:00
parent 044737a3a7
commit b5e95e27d4
2 changed files with 139 additions and 2 deletions
@@ -38,6 +38,9 @@ import io.element.android.libraries.matrix.ui.messages.reply.map
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList

+/** Events further apart than this are never grouped together, even when sent by the same person. */
+private const val GROUPING_TIMEOUT_MS = 5 * 60 * 1000L
+
@AssistedInject
 class TimelineItemEventFactory(
    @Assisted private val config: TimelineItemsFactoryConfig,
@@ -221,8 +224,18 @@ class TimelineItemEventFactory(
        val previousSender = prevTimelineItem?.event?.sender
        val nextSender = nextTimelineItem?.event?.sender

-        val previousIsGroupable = prevTimelineItem?.canBeDisplayedInBubbleBlock().orTrue()
-        val nextIsGroupable = nextTimelineItem?.canBeDisplayedInBubbleBlock().orTrue()
+        // Two events from the same sender are only grouped together if they were sent within
+        // GROUPING_TIMEOUT_MS of each other. The delta is symmetric between a pair of neighbours, so
+        // both items always agree on where the group is split.
+        // See https://github.com/element-hq/element-x-android/issues/4757
+        val previousIsGroupable = prevTimelineItem?.let {
+            it.canBeDisplayedInBubbleBlock() &&
+                currentTimelineItem.event.timestamp - it.event.timestamp <= GROUPING_TIMEOUT_MS
+        }.orTrue()
+        val nextIsGroupable = nextTimelineItem?.let {
+            it.canBeDisplayedInBubbleBlock() &&
+                it.event.timestamp - currentTimelineItem.event.timestamp <= GROUPING_TIMEOUT_MS
+        }.orTrue()

        return when {
            previousSender != currentSender && nextSender == currentSender -> {
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.features.messages.impl.timeline.factories
+
+import app.cash.turbine.test
+import com.google.common.truth.Truth.assertThat
+import io.element.android.features.messages.impl.fixtures.aTimelineItemsFactory
+import io.element.android.features.messages.impl.timeline.model.TimelineItem
+import io.element.android.features.messages.impl.timeline.model.TimelineItemGroupPosition
+import io.element.android.libraries.matrix.api.core.UniqueId
+import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
+import io.element.android.libraries.matrix.test.A_USER_ID
+import io.element.android.libraries.matrix.test.timeline.aMessageContent
+import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem
+import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+
+class TimelineItemsFactoryTest {
+    @Test
+    fun `messages sent close together are grouped`() = runTest {
+        val positions = groupPositionsOf(
+            timestamps = listOf(0L, ONE_MINUTE, 2 * ONE_MINUTE),
+        )
+        assertThat(positions).containsExactly(
+            TimelineItemGroupPosition.First,
+            TimelineItemGroupPosition.Middle,
+            TimelineItemGroupPosition.Last,
+        ).inOrder()
+    }
+
+    @Test
+    fun `messages sent more than five minutes apart are not grouped`() = runTest {
+        val positions = groupPositionsOf(
+            timestamps = listOf(0L, 6 * ONE_MINUTE, 12 * ONE_MINUTE),
+        )
+        assertThat(positions).containsExactly(
+            TimelineItemGroupPosition.None,
+            TimelineItemGroupPosition.None,
+            TimelineItemGroupPosition.None,
+        ).inOrder()
+    }
+
+    @Test
+    fun `a gap of more than five minutes splits the group`() = runTest {
+        val positions = groupPositionsOf(
+            timestamps = listOf(0L, ONE_MINUTE, 8 * ONE_MINUTE, 9 * ONE_MINUTE),
+        )
+        assertThat(positions).containsExactly(
+            TimelineItemGroupPosition.First,
+            TimelineItemGroupPosition.Last,
+            TimelineItemGroupPosition.First,
+            TimelineItemGroupPosition.Last,
+        ).inOrder()
+    }
+
+    @Test
+    fun `a gap of exactly five minutes still groups`() = runTest {
+        val positions = groupPositionsOf(
+            timestamps = listOf(0L, FIVE_MINUTES),
+        )
+        assertThat(positions).containsExactly(
+            TimelineItemGroupPosition.First,
+            TimelineItemGroupPosition.Last,
+        ).inOrder()
+    }
+
+    @Test
+    fun `messages with the same timestamp are grouped`() = runTest {
+        val positions = groupPositionsOf(
+            timestamps = listOf(0L, 0L, 0L),
+        )
+        assertThat(positions).containsExactly(
+            TimelineItemGroupPosition.First,
+            TimelineItemGroupPosition.Middle,
+            TimelineItemGroupPosition.Last,
+        ).inOrder()
+    }
+
+    private suspend fun TestScope.groupPositionsOf(timestamps: List<Long>): List<TimelineItemGroupPosition> {
+        val factory = aTimelineItemsFactory(
+            config = TimelineItemsFactoryConfig(
+                computeReadReceipts = false,
+                computeReactions = false,
+            )
+        )
+        val items = timestamps.mapIndexed { index, timestamp ->
+            MatrixTimelineItem.Event(
+                uniqueId = UniqueId("event-$index"),
+                event = anEventTimelineItem(
+                    sender = A_USER_ID,
+                    timestamp = timestamp,
+                    content = aMessageContent(body = "Message $index"),
+                ),
+            )
+        }
+        var positions: List<TimelineItemGroupPosition> = emptyList()
+        factory.timelineItems.test {
+            factory.replaceWith(
+                timelineItems = items,
+                roomMembers = emptyList(),
+                renderReadReceipts = false,
+            )
+            // The factory emits the timeline newest-first; reverse it so the assertions below read
+            // in the same chronological order as the timestamps that were fed in.
+            positions = awaitItem()
+                .filterIsInstance<TimelineItem.Event>()
+                .map { it.groupPosition }
+                .reversed()
+            cancelAndIgnoreRemainingEvents()
+        }
+        return positions
+    }
+
+    private companion object {
+        const val ONE_MINUTE = 60 * 1000L
+        const val FIVE_MINUTES = 5 * ONE_MINUTE
+    }
+}