Merge pull request #467 from vector-im/feature/bma/reworkGrouper

Improve timeline rendering for message and state event

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt

@@ -153,11 +153,6 @@ fun MessagesView(
         }
     }
 
-    fun onExpandGroupClick(event: TimelineItem.GroupedEvents) {
-        Timber.v("onExpandGroupClick= ${event.id}")
-        state.timelineState.eventSink(TimelineEvents.ToggleExpandGroup(event))
-    }
-
     fun onActionSelected(action: TimelineItemAction, event: TimelineItem.Event) {
         state.eventSink(MessagesEvents.HandleAction(action, event))
     }
@@ -208,7 +203,6 @@ fun MessagesView(
                         .consumeWindowInsets(padding),
                     onMessageClicked = ::onMessageClicked,
                     onMessageLongClicked = ::onMessageLongClicked,
-                    onExpandGroupClick = ::onExpandGroupClick,
                 )
             },
             snackbarHost = {
@@ -247,7 +241,6 @@ fun MessagesViewContent(
     modifier: Modifier = Modifier,
     onMessageClicked: (TimelineItem.Event) -> Unit = {},
     onMessageLongClicked: (TimelineItem.Event) -> Unit = {},
-    onExpandGroupClick: (TimelineItem.GroupedEvents) -> Unit = {},
 ) {
     Column(
         modifier = modifier
@@ -262,7 +255,6 @@ fun MessagesViewContent(
                 modifier = Modifier.weight(1f),
                 onMessageClicked = onMessageClicked,
                 onMessageLongClicked = onMessageLongClicked,
-                onExpandGroupClick = onExpandGroupClick,
             )
         }
         MessageComposerView(

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvents.kt

@@ -16,11 +16,9 @@
 
 package io.element.android.features.messages.impl.timeline
 
-import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.libraries.matrix.api.core.EventId
 
 sealed interface TimelineEvents {
     object LoadMore : TimelineEvents
     data class SetHighlightedEvent(val eventId: EventId?) : TimelineEvents
-    data class ToggleExpandGroup(val event: TimelineItem.GroupedEvents) : TimelineEvents
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt

@@ -20,19 +20,14 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.collectAsState
-import androidx.compose.runtime.mutableStateMapOf
 import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
 import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
-import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
 import io.element.android.libraries.architecture.Presenter
-import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
-import kotlinx.collections.immutable.toImmutableList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
@@ -45,7 +40,6 @@ private const val backPaginationPageSize = 50
 
 class TimelinePresenter @Inject constructor(
     private val timelineItemsFactory: TimelineItemsFactory,
-    private val timelineItemGrouper: TimelineItemGrouper,
     room: MatrixRoom,
 ) : Presenter<TimelineState> {
 
@@ -57,7 +51,6 @@ class TimelinePresenter @Inject constructor(
         val highlightedEventId: MutableState<EventId?> = rememberSaveable {
             mutableStateOf(null)
         }
-        val expandedGroups = remember { mutableStateMapOf<String, Boolean>() }
 
         val timelineItems = timelineItemsFactory
             .flow()
@@ -71,9 +64,6 @@ class TimelinePresenter @Inject constructor(
             when (event) {
                 TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState.value)
                 is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
-                is TimelineEvents.ToggleExpandGroup -> {
-                    expandedGroups[event.event.identifier()] = expandedGroups[event.event.identifier()].orFalse().not()
-                }
             }
         }
 
@@ -92,7 +82,7 @@ class TimelinePresenter @Inject constructor(
         return TimelineState(
             highlightedEventId = highlightedEventId.value,
             paginationState = paginationState.value,
-            timelineItems = timelineItemGrouper.group(timelineItems.value, expandedGroups).toImmutableList(),
+            timelineItems = timelineItems.value,
             eventSink = ::handleEvents
         )
     }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt

@@ -94,7 +94,7 @@ internal fun aTimelineItemEvent(
     eventId: EventId = EventId("\$" + Random.nextInt().toString()),
     isMine: Boolean = false,
     content: TimelineItemEventContent = aTimelineItemTextContent(),
-    groupPosition: TimelineItemGroupPosition = TimelineItemGroupPosition.First,
+    groupPosition: TimelineItemGroupPosition = TimelineItemGroupPosition.None,
     sendState: EventSendState = EventSendState.Sent(eventId),
 ): TimelineItem.Event {
     return TimelineItem.Event(

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt

@@ -47,8 +47,10 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.derivedStateOf
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.snapshotFlow
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -95,7 +97,6 @@ fun TimelineView(
     modifier: Modifier = Modifier,
     onMessageClicked: (TimelineItem.Event) -> Unit = {},
     onMessageLongClicked: (TimelineItem.Event) -> Unit = {},
-    onExpandGroupClick: (TimelineItem.GroupedEvents) -> Unit = {},
 ) {
 
     fun onReachedLoadMore() {
@@ -119,7 +120,6 @@ fun TimelineView(
                     highlightedItem = state.highlightedEventId?.value,
                     onClick = onMessageClicked,
                     onLongClick = onMessageLongClicked,
-                    onExpandGroupClick = onExpandGroupClick,
                 )
                 if (index == state.timelineItems.lastIndex) {
                     onReachedLoadMore()
@@ -141,7 +141,6 @@ fun TimelineItemRow(
     highlightedItem: String?,
     onClick: (TimelineItem.Event) -> Unit,
     onLongClick: (TimelineItem.Event) -> Unit,
-    onExpandGroupClick: (TimelineItem.GroupedEvents) -> Unit,
     modifier: Modifier = Modifier
 ) {
     when (timelineItem) {
@@ -179,8 +178,10 @@ fun TimelineItemRow(
             }
         }
         is TimelineItem.GroupedEvents -> {
+            val isExpanded = rememberSaveable(key = timelineItem.identifier()) { mutableStateOf(false) }
+
             fun onExpandGroupClick() {
-                onExpandGroupClick(timelineItem)
+                isExpanded.value = !isExpanded.value
             }
 
             Column(modifier = modifier.animateContentSize()) {
@@ -190,11 +191,11 @@ fun TimelineItemRow(
                         count = timelineItem.events.size,
                         timelineItem.events.size
                     ),
-                    isExpanded = timelineItem.expanded,
-                    isHighlighted = !timelineItem.expanded && timelineItem.events.any { it.identifier() == highlightedItem },
+                    isExpanded = isExpanded.value,
+                    isHighlighted = !isExpanded.value && timelineItem.events.any { it.identifier() == highlightedItem },
                     onClick = ::onExpandGroupClick,
                 )
-                if (timelineItem.expanded) {
+                if (isExpanded.value) {
                     Column {
                         timelineItem.events.forEach { subGroupEvent ->
                             TimelineItemRow(
@@ -202,7 +203,6 @@ fun TimelineItemRow(
                                 highlightedItem = highlightedItem,
                                 onClick = onClick,
                                 onLongClick = onLongClick,
-                                onExpandGroupClick = {}
                             )
                         }
                     }
@@ -247,14 +247,16 @@ fun TimelineItemEventRow(
     ) {
         Row {
             if (!event.isMine) {
-                Spacer(modifier = Modifier.width(16.dp))
+                Spacer(modifier = Modifier.width(4.dp))
             }
             Column(horizontalAlignment = contentAlignment) {
                 if (event.showSenderInformation) {
                     MessageSenderInformation(
                         event.safeSenderName,
                         event.senderAvatar,
-                        Modifier.zIndex(1f)
+                        Modifier
+                            .zIndex(1f)
+                            .offset(y = 12.dp)
                     )
                 }
                 val bubbleState = BubbleState(
@@ -282,7 +284,7 @@ fun TimelineItemEventRow(
                     reactionsState = event.reactionsState,
                     modifier = Modifier
                         .zIndex(1f)
-                        .offset(x = if (event.isMine) 0.dp else 20.dp, y = -(16.dp))
+                        .offset(x = if (event.isMine) 0.dp else 20.dp, y = -(4.dp))
                 )
             }
             if (event.isMine) {

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/MessageEventBubble.kt

@@ -20,7 +20,6 @@ import androidx.compose.foundation.ExperimentalFoundationApi
 import androidx.compose.foundation.combinedClickable
 import androidx.compose.foundation.interaction.MutableInteractionSource
 import androidx.compose.foundation.layout.Box
-import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.offset
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
@@ -35,13 +34,16 @@ import androidx.compose.ui.graphics.Shape
 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.sp
 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.preview.ElementPreviewDark
 import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.theme.ElementTheme
 import io.element.android.libraries.designsystem.theme.components.Surface
+import io.element.android.libraries.designsystem.theme.components.Text
 
 private val BUBBLE_RADIUS = 16.dp
 
@@ -84,10 +86,9 @@ fun MessageEventBubble(
 
     fun Modifier.offsetForItem(): Modifier {
         return if (state.isMine) {
-            // FIXME setting y offset to -12.dp can overlap a state event displayed above.
-            offset(y = -(12.dp))
+            this
         } else {
-            offset(x = 20.dp, y = -(12.dp))
+            offset(x = 20.dp)
         }
     }
 
@@ -130,7 +131,7 @@ internal fun MessageEventBubbleDarkPreview(@PreviewParameter(BubbleStateProvider
 
 @Composable
 private fun ContentToPreview(state: BubbleState) {
-    // Due to y offset, surround with a Box
+    // Due to position offset, surround with a Box
     Box(
         modifier = Modifier
             .size(width = 240.dp, height = 64.dp)
@@ -141,7 +142,18 @@ private fun ContentToPreview(state: BubbleState) {
             state = state,
             interactionSource = MutableInteractionSource(),
         ) {
-            Spacer(modifier = Modifier.size(width = 120.dp, height = 32.dp))
+            // Render the state as a text to better understand the previews
+            Box(
+                modifier = Modifier
+                    .size(width = 120.dp, height = 32.dp)
+                    .padding(horizontal = 12.dp, vertical = 6.dp),
+                contentAlignment = Alignment.Center,
+            ) {
+                Text(
+                    fontSize = 10.sp,
+                    text = "${state.groupPosition.javaClass.simpleName} m:${state.isMine.to01()} h:${state.isHighlighted.to01()}"
+                )
+            }
         }
     }
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineEventTimestampView.kt

@@ -25,13 +25,18 @@ import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Error
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
+import androidx.compose.ui.tooling.preview.Preview
+import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContent
 import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.designsystem.ElementTextStyles
+import io.element.android.libraries.designsystem.preview.ElementPreviewDark
+import io.element.android.libraries.designsystem.preview.ElementPreviewLight
 import io.element.android.libraries.designsystem.theme.ElementTheme
 import io.element.android.libraries.designsystem.theme.components.Icon
 import io.element.android.libraries.designsystem.theme.components.Text
@@ -48,7 +53,10 @@ fun TimelineEventTimestampView(
     val hasMessageSendingFailed = event.sendState is EventSendState.SendingFailed
     val isMessageEdited = (event.content as? TimelineItemTextBasedContent)?.isEdited.orFalse()
     val tint = if (hasMessageSendingFailed) ElementTheme.colors.textActionCritical else null
-    Row(modifier = modifier.clickable(onClick = onClick)) {
+    Row(
+        modifier = modifier.clickable(onClick = onClick),
+        verticalAlignment = Alignment.CenterVertically,
+    ) {
         if (isMessageEdited) {
             Text(
                 stringResource(R.string.common_edited_suffix),
@@ -68,3 +76,21 @@ fun TimelineEventTimestampView(
         }
     }
 }
+
+@Preview
+@Composable
+internal fun TimelineEventTimestampViewLightPreview(@PreviewParameter(TimelineItemEventForTimestampViewProvider::class) event: TimelineItem.Event) =
+    ElementPreviewLight { ContentToPreview(event) }
+
+@Preview
+@Composable
+internal fun TimelineEventTimestampViewDarkPreview(@PreviewParameter(TimelineItemEventForTimestampViewProvider::class) event: TimelineItem.Event) =
+    ElementPreviewDark { ContentToPreview(event) }
+
+@Composable
+private fun ContentToPreview(event: TimelineItem.Event) {
+    TimelineEventTimestampView(
+        event = event,
+        onClick = {}
+    )
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemEventForTimestampViewProvider.kt

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * 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
+ *
+ *     http://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 io.element.android.features.messages.impl.timeline.components
+
+import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.features.messages.impl.timeline.aTimelineItemEvent
+import io.element.android.features.messages.impl.timeline.model.TimelineItem
+import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
+import io.element.android.libraries.matrix.api.timeline.item.event.EventSendState
+
+class TimelineItemEventForTimestampViewProvider : PreviewParameterProvider<TimelineItem.Event> {
+    override val values: Sequence<TimelineItem.Event>
+        get() = sequenceOf(
+            aTimelineItemEvent(),
+            // Sending failed
+            aTimelineItemEvent().copy(sendState = EventSendState.SendingFailed("AN_ERROR")),
+            // Edited
+            aTimelineItemEvent().copy(content = aTimelineItemTextContent().copy(isEdited = true)),
+            // Sending failed + Edited (not sure this is possible IRL, but should be covered by test)
+            aTimelineItemEvent().copy(
+                sendState = EventSendState.SendingFailed("AN_ERROR"),
+                content = aTimelineItemTextContent().copy(isEdited = true),
+            ),
+        )
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/TimelineItemsFactory.kt

@@ -21,9 +21,13 @@ import io.element.android.features.messages.impl.timeline.diff.CacheInvalidator
 import io.element.android.features.messages.impl.timeline.diff.MatrixTimelineItemsDiffCallback
 import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemEventFactory
 import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory
+import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
+import kotlinx.collections.immutable.ImmutableList
+import kotlinx.collections.immutable.toImmutableList
+import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
@@ -38,9 +42,10 @@ class TimelineItemsFactory @Inject constructor(
     private val dispatchers: CoroutineDispatchers,
     private val eventItemFactory: TimelineItemEventFactory,
     private val virtualItemFactory: TimelineItemVirtualFactory,
+    private val timelineItemGrouper: TimelineItemGrouper,
 ) {
 
-    private val timelineItems = MutableStateFlow<List<TimelineItem>>(emptyList())
+    private val timelineItems = MutableStateFlow(emptyList<TimelineItem>().toImmutableList())
     private val timelineItemsCache = arrayListOf<TimelineItem?>()
 
     // Items from rust sdk, used for diffing
@@ -49,7 +54,7 @@ class TimelineItemsFactory @Inject constructor(
     private val lock = Mutex()
     private val cacheInvalidator = CacheInvalidator(timelineItemsCache)
 
-    fun flow(): StateFlow<List<TimelineItem>> = timelineItems.asStateFlow()
+    fun flow(): StateFlow<ImmutableList<TimelineItem>> = timelineItems.asStateFlow()
 
     suspend fun replaceWith(
         timelineItems: List<MatrixTimelineItem>,
@@ -72,7 +77,8 @@ class TimelineItemsFactory @Inject constructor(
                 newTimelineItemStates.add(cacheItem)
             }
         }
-        this.timelineItems.emit(newTimelineItemStates)
+        val result = timelineItemGrouper.group(newTimelineItemStates).toPersistentList()
+        this.timelineItems.emit(result)
     }
 
     private fun calculateAndApplyDiff(newTimelineItems: List<MatrixTimelineItem>) {

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt

@@ -16,10 +16,12 @@
 
 package io.element.android.features.messages.impl.timeline.factories.event
 
+import io.element.android.features.messages.impl.timeline.groups.canBeDisplayedInBubbleBlock
 import io.element.android.features.messages.impl.timeline.model.AggregatedReaction
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
 import io.element.android.features.messages.impl.timeline.model.TimelineItemGroupPosition
 import io.element.android.features.messages.impl.timeline.model.TimelineItemReactions
+import io.element.android.libraries.core.bool.orTrue
 import io.element.android.libraries.designsystem.components.avatar.AvatarData
 import io.element.android.libraries.designsystem.components.avatar.AvatarSize
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
@@ -102,10 +104,39 @@ class TimelineItemEventFactory @Inject constructor(
         val previousSender = prevTimelineItem?.event?.sender
         val nextSender = nextTimelineItem?.event?.sender
 
+        val previousIsGroupable = prevTimelineItem?.canBeDisplayedInBubbleBlock().orTrue()
+        val nextIsGroupable = nextTimelineItem?.canBeDisplayedInBubbleBlock().orTrue()
+
         return when {
-            previousSender != currentSender && nextSender == currentSender -> TimelineItemGroupPosition.First
-            previousSender == currentSender && nextSender == currentSender -> TimelineItemGroupPosition.Middle
-            previousSender == currentSender && nextSender != currentSender -> TimelineItemGroupPosition.Last
+            previousSender != currentSender && nextSender == currentSender -> {
+                if (nextIsGroupable) {
+                    TimelineItemGroupPosition.First
+                } else {
+                    TimelineItemGroupPosition.None
+                }
+            }
+            previousSender == currentSender && nextSender == currentSender -> {
+                if (previousIsGroupable) {
+                    if (nextIsGroupable) {
+                        TimelineItemGroupPosition.Middle
+                    } else {
+                        TimelineItemGroupPosition.Last
+                    }
+                } else {
+                    if (nextIsGroupable) {
+                        TimelineItemGroupPosition.First
+                    } else {
+                        TimelineItemGroupPosition.None
+                    }
+                }
+            }
+            previousSender == currentSender /* && nextSender != currentSender (== true) */ -> {
+                if (previousIsGroupable) {
+                    TimelineItemGroupPosition.Last
+                } else {
+                    TimelineItemGroupPosition.None
+                }
+            }
             else -> TimelineItemGroupPosition.None
         }
     }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/Groupability.kt

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * 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
+ *
+ *     http://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 io.element.android.features.messages.impl.timeline.groups
+
+import io.element.android.features.messages.impl.timeline.model.TimelineItem
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEncryptedContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemFileContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemProfileChangeContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRoomMembershipContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateEventContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemUnknownContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
+import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
+import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseMessageLikeContent
+import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseStateContent
+import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent
+import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent
+import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent
+import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent
+import io.element.android.libraries.matrix.api.timeline.item.event.StateContent
+import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent
+import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent
+import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent
+
+/**
+ * Return true if the Event can be grouped in a collapse/expand block
+ * When [canBeGrouped] returns a value, [canBeDisplayedInBubbleBlock] MUST return the opposite value.
+ * Since the receiving type are not the same, the two functions exist.
+ */
+internal fun TimelineItem.Event.canBeGrouped(): Boolean {
+    return when (content) {
+        is TimelineItemTextBasedContent,
+        is TimelineItemEncryptedContent,
+        is TimelineItemImageContent,
+        is TimelineItemFileContent,
+        is TimelineItemVideoContent,
+        TimelineItemRedactedContent,
+        TimelineItemUnknownContent -> false
+        is TimelineItemProfileChangeContent,
+        is TimelineItemRoomMembershipContent,
+        is TimelineItemStateEventContent -> true
+    }
+}
+
+/**
+ * Return true if the Event can be grouped in a block of message bubbles.
+ * When [canBeDisplayedInBubbleBlock] returns a value, [canBeGrouped] MUST return the opposite value.
+ * Since the receiving type are not the same, the two functions exist.
+ */
+internal fun MatrixTimelineItem.Event.canBeDisplayedInBubbleBlock(): Boolean {
+    return when (event.content) {
+        is FailedToParseMessageLikeContent,
+        is MessageContent,
+        RedactedContent,
+        is StickerContent,
+        is UnableToDecryptContent -> true
+        is FailedToParseStateContent,
+        is ProfileChangeContent,
+        is RoomMembershipContent,
+        UnknownContent,
+        is StateContent -> false
+    }
+}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouper.kt

@@ -17,28 +17,14 @@
 package io.element.android.features.messages.impl.timeline.groups
 
 import io.element.android.features.messages.impl.timeline.model.TimelineItem
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEmoteContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEncryptedContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemFileContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemImageContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemNoticeContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemProfileChangeContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRoomMembershipContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateEventContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemUnknownContent
-import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVideoContent
-import io.element.android.libraries.core.bool.orFalse
 import kotlinx.collections.immutable.toImmutableList
-
 import javax.inject.Inject
 
 class TimelineItemGrouper @Inject constructor() {
     /**
      * Create a new list of [TimelineItem] by grouping some of them into [TimelineItem.GroupedEvents].
      */
-    fun group(from: List<TimelineItem>, expandedGroups: Map<String, Boolean>): List<TimelineItem> {
+    fun group(from: List<TimelineItem>): List<TimelineItem> {
         val result = mutableListOf<TimelineItem>()
         val currentGroup = mutableListOf<TimelineItem.Event>()
         from.forEach { timelineItem ->
@@ -48,42 +34,24 @@ class TimelineItemGrouper @Inject constructor() {
                 // timelineItem cannot be grouped
                 if (currentGroup.isNotEmpty()) {
                     // There is a pending group, create a TimelineItem.GroupedEvents if there is more than 1 Event in the pending group.
-                    result.addGroup(currentGroup, expandedGroups)
+                    result.addGroup(currentGroup)
                     currentGroup.clear()
                 }
                 result.add(timelineItem)
             }
         }
         if (currentGroup.isNotEmpty()) {
-            result.addGroup(currentGroup, expandedGroups)
+            result.addGroup(currentGroup)
         }
         return result
     }
-
-    private fun TimelineItem.Event.canBeGrouped(): Boolean {
-        return when (content) {
-            is TimelineItemEncryptedContent,
-            is TimelineItemImageContent,
-            TimelineItemRedactedContent,
-            is TimelineItemEmoteContent,
-            is TimelineItemNoticeContent,
-            is TimelineItemTextContent,
-            is TimelineItemFileContent,
-            is TimelineItemVideoContent,
-            TimelineItemUnknownContent -> false
-            is TimelineItemProfileChangeContent,
-            is TimelineItemRoomMembershipContent,
-            is TimelineItemStateEventContent -> true
-        }
-    }
 }
 
 /**
  * Will add a group if there is more than 1 item, else add the item to the list.
  */
 private fun MutableList<TimelineItem>.addGroup(
-    group: MutableList<TimelineItem.Event>,
-    expandedGroups: Map<String, Boolean>,
+    group: MutableList<TimelineItem.Event>
 ) {
     if (group.size == 1) {
         // Do not create a group with just 1 item, just add the item to the result
@@ -91,7 +59,6 @@ private fun MutableList<TimelineItem>.addGroup(
     } else {
         add(
             TimelineItem.GroupedEvents(
-                expanded = expandedGroups[group.first().id + "_group"].orFalse(),
                 events = group.toImmutableList()
             )
         )

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/TimelineItem.kt

@@ -68,10 +68,9 @@ sealed interface TimelineItem {
 
     @Immutable
     data class GroupedEvents(
-        val expanded: Boolean,
         val events: ImmutableList<Event>,
     ) : TimelineItem {
-        // use first id with a suffix
-        val id = events.first().id + "_group"
+        // use last id with a suffix. Last will not change in cas of new event from backpagination.
+        val id = events.last().id + "_group"
     }
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/TimelineItemGroupPosition.kt

@@ -18,13 +18,48 @@ package io.element.android.features.messages.impl.timeline.model
 
 import androidx.compose.runtime.Immutable
 
+/**
+ * Attribute for a TimelineItem, used to render successive events from the same sender differently.
+ *
+ * Possible sequences in the timeline will be:
+ *
+ * Only one Event:
+ * - [None]
+ *
+ * Two Events
+ * - [First]
+ * - [Last]
+ *
+ * Many Events:
+ * - [First]
+ * - [Middle] (repeated if necessary)
+ * - [Last]
+ */
 @Immutable
 sealed interface TimelineItemGroupPosition {
+    /**
+     * The event is part of a group of events from the same sender and is the first sent Event.
+     */
     object First : TimelineItemGroupPosition
+
+    /**
+     * The event is part of a group of events from the same sender and is neither the first nor the last sent Event.
+     */
     object Middle : TimelineItemGroupPosition
+
+    /**
+     * The event is part of a group of events from the same sender and is the last sent Event.
+     */
     object Last : TimelineItemGroupPosition
+
+    /**
+     * The event is not part of a group of events. Sender of previous event is different, and sender of next event is different.
+     */
     object None : TimelineItemGroupPosition
 
+    /**
+     * Return true if the previous sender of the event is a different sender.
+     */
     fun isNew(): Boolean = when (this) {
         First, None -> true
         else -> false

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/bubble/BubbleStateProvider.kt

@@ -25,6 +25,7 @@ open class BubbleStateProvider : PreviewParameterProvider<BubbleState> {
             TimelineItemGroupPosition.First,
             TimelineItemGroupPosition.Middle,
             TimelineItemGroupPosition.Last,
+            TimelineItemGroupPosition.None,
         ).map { groupPosition ->
             sequenceOf(false, true).map { isMine ->
                 sequenceOf(false, true).map { isHighlighted ->

features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt

@@ -28,7 +28,6 @@ import io.element.android.features.messages.impl.actionlist.ActionListPresenter
 import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction
 import io.element.android.features.messages.impl.messagecomposer.MessageComposerPresenter
 import io.element.android.features.messages.impl.timeline.TimelinePresenter
-import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
 import io.element.android.features.messages.media.FakeLocalMediaFactory
 import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
 import io.element.android.libraries.designsystem.utils.SnackbarDispatcher
@@ -142,7 +141,6 @@ class MessagesPresenterTest {
         )
         val timelinePresenter = TimelinePresenter(
             timelineItemsFactory = aTimelineItemsFactory(),
-            timelineItemGrouper = TimelineItemGrouper(),
             room = matrixRoom,
         )
         val actionListPresenter = ActionListPresenter()

features/messages/impl/src/test/kotlin/io/element/android/features/messages/attachments/AttachmentsPreviewPresenterTest.kt

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+@file:OptIn(ExperimentalCoroutinesApi::class)
+
 package io.element.android.features.messages.attachments
 
 import androidx.media3.common.MimeTypes
@@ -33,6 +35,7 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.mediaupload.api.MediaPreProcessor
 import io.element.android.libraries.mediaupload.api.MediaSender
 import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 

features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/timelineItemsFactory.kt

@@ -30,6 +30,7 @@ import io.element.android.features.messages.impl.timeline.factories.event.Timeli
 import io.element.android.features.messages.impl.timeline.factories.event.TimelineItemEventFactory
 import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemDaySeparatorFactory
 import io.element.android.features.messages.impl.timeline.factories.virtual.TimelineItemVirtualFactory
+import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
 import io.element.android.libraries.dateformatter.test.FakeDaySeparatorFormatter
 import io.element.android.libraries.eventformatter.api.TimelineEventFormatter
 import io.element.android.libraries.matrix.api.timeline.item.event.EventTimelineItem
@@ -56,7 +57,8 @@ internal fun aTimelineItemsFactory(): TimelineItemsFactory {
             daySeparatorFactory = TimelineItemDaySeparatorFactory(
                 FakeDaySeparatorFormatter()
             ),
-        )
+        ),
+        timelineItemGrouper = TimelineItemGrouper(),
     )
 }
 

features/messages/impl/src/test/kotlin/io/element/android/features/messages/media/viewer/MediaViewerPresenterTest.kt

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+@file:OptIn(ExperimentalCoroutinesApi::class)
+
 package io.element.android.features.messages.media.viewer
 
 import androidx.media3.common.MimeTypes
@@ -29,6 +31,7 @@ import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.matrix.test.FAKE_DELAY_IN_MS
 import io.element.android.libraries.matrix.test.media.FakeMediaLoader
 import io.element.android.libraries.matrix.test.media.aMediaSource
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 

features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt

@@ -23,13 +23,8 @@ import com.google.common.truth.Truth.assertThat
 import io.element.android.features.messages.fixtures.aTimelineItemsFactory
 import io.element.android.features.messages.impl.timeline.TimelineEvents
 import io.element.android.features.messages.impl.timeline.TimelinePresenter
-import io.element.android.features.messages.impl.timeline.groups.TimelineItemGrouper
-import io.element.android.features.messages.impl.timeline.model.TimelineItem
-import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
-import io.element.android.libraries.matrix.test.room.anEventTimelineItem
-import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
@@ -38,7 +33,6 @@ class TimelinePresenterTest {
     fun `present - initial state`() = runTest {
         val presenter = TimelinePresenter(
             timelineItemsFactory = aTimelineItemsFactory(),
-            timelineItemGrouper = TimelineItemGrouper(),
             room = FakeMatrixRoom(),
         )
         moleculeFlow(RecompositionClock.Immediate) {
@@ -55,7 +49,6 @@ class TimelinePresenterTest {
     fun `present - load more`() = runTest {
         val presenter = TimelinePresenter(
             timelineItemsFactory = aTimelineItemsFactory(),
-            timelineItemGrouper = TimelineItemGrouper(),
             room = FakeMatrixRoom(),
         )
         moleculeFlow(RecompositionClock.Immediate) {
@@ -78,7 +71,6 @@ class TimelinePresenterTest {
     fun `present - set highlighted event`() = runTest {
         val presenter = TimelinePresenter(
             timelineItemsFactory = aTimelineItemsFactory(),
-            timelineItemGrouper = TimelineItemGrouper(),
             room = FakeMatrixRoom(),
         )
         moleculeFlow(RecompositionClock.Immediate) {
@@ -95,37 +87,4 @@ class TimelinePresenterTest {
             assertThat(withoutHighlightedState.highlightedEventId).isNull()
         }
     }
-
-    @Test
-    fun `present - expand and collapse grouped events`() = runTest {
-        val fakeTimeline = FakeMatrixTimeline(
-            initialTimelineItems = listOf(
-                MatrixTimelineItem.Event(anEventTimelineItem() /* This is a groupable event */),
-                MatrixTimelineItem.Event(anEventTimelineItem() /* This is a groupable event */),
-            )
-        )
-        val fakeRoom = FakeMatrixRoom(matrixTimeline = fakeTimeline)
-        val presenter = TimelinePresenter(
-            timelineItemsFactory = aTimelineItemsFactory(),
-            timelineItemGrouper = TimelineItemGrouper(),
-            room = fakeRoom,
-        )
-        moleculeFlow(RecompositionClock.Immediate) {
-            presenter.present()
-        }.test {
-            skipItems(1)
-            fakeTimeline.updateTimelineItems { it }
-            val loadedState = awaitItem()
-            val group1 = loadedState.timelineItems.first() as TimelineItem.GroupedEvents
-            assertThat(group1.expanded).isFalse()
-            loadedState.eventSink.invoke(TimelineEvents.ToggleExpandGroup(group1))
-            val withExpandedGroup = awaitItem()
-            val group2 = withExpandedGroup.timelineItems.first() as TimelineItem.GroupedEvents
-            assertThat(group2.expanded).isTrue()
-            withExpandedGroup.eventSink.invoke(TimelineEvents.ToggleExpandGroup(group2))
-            val withCollapsedGroup = awaitItem()
-            val group3 = withCollapsedGroup.timelineItems.first() as TimelineItem.GroupedEvents
-            assertThat(group3.expanded).isFalse()
-        }
-    }
 }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/groups/TimelineItemGrouperTest.kt

@@ -49,7 +49,7 @@ class TimelineItemGrouperTest {
 
     @Test
     fun `test empty`() {
-        val result = sut.group(emptyList(), emptyMap())
+        val result = sut.group(emptyList())
         assertThat(result).isEmpty()
     }
 
@@ -60,7 +60,6 @@ class TimelineItemGrouperTest {
                 aNonGroupableItem,
                 aNonGroupableItem,
             ),
-            emptyMap()
         )
         assertThat(result).isEqualTo(
             listOf(
@@ -77,12 +76,10 @@ class TimelineItemGrouperTest {
                 aGroupableItem.copy(id = AN_EVENT_ID_2.value),
                 aGroupableItem,
             ),
-            emptyMap()
         )
         assertThat(result).isEqualTo(
             listOf(
                 TimelineItem.GroupedEvents(
-                    expanded = false,
                     events = listOf(
                         aGroupableItem,
                         aGroupableItem.copy(id = AN_EVENT_ID_2.value),
@@ -92,28 +89,6 @@ class TimelineItemGrouperTest {
         )
     }
 
-    @Test
-    fun `test groupables expanded`() {
-        val result = sut.group(
-            listOf(
-                aGroupableItem,
-                aGroupableItem.copy(id = AN_EVENT_ID_2.value),
-            ),
-            mapOf("${AN_EVENT_ID_2.value}_group" to true)
-        )
-        assertThat(result).isEqualTo(
-            listOf(
-                TimelineItem.GroupedEvents(
-                    expanded = true,
-                    events = listOf(
-                        aGroupableItem.copy(id = AN_EVENT_ID_2.value),
-                        aGroupableItem,
-                    ).toImmutableList()
-                ),
-            )
-        )
-    }
-
     @Test
     fun `test 1 groupable, not group must be created`() {
         val listsToTest = listOf(
@@ -130,7 +105,7 @@ class TimelineItemGrouperTest {
             listOf(aNonGroupableItemNoEvent),
         )
         listsToTest.forEach { listToTest ->
-            val result = sut.group(listToTest, emptyMap())
+            val result = sut.group(listToTest)
             assertThat(result).isEqualTo(listToTest)
         }
     }
@@ -146,12 +121,10 @@ class TimelineItemGrouperTest {
                 aGroupableItem,
                 aGroupableItem,
             ),
-            emptyMap()
         )
         assertThat(result).isEqualTo(
             listOf(
                 TimelineItem.GroupedEvents(
-                    expanded = false,
                     events = listOf(
                         aGroupableItem,
                         aGroupableItem,
@@ -159,7 +132,6 @@ class TimelineItemGrouperTest {
                 ),
                 aNonGroupableItem,
                 TimelineItem.GroupedEvents(
-                    expanded = false,
                     events = listOf(
                         aGroupableItem,
                         aGroupableItem,

libraries/core/src/main/kotlin/io/element/android/libraries/core/extensions/BasicExtensions.kt

@@ -17,6 +17,7 @@
 package io.element.android.libraries.core.extensions
 
 fun Boolean.toOnOff() = if (this) "ON" else "OFF"
+fun Boolean.to01() = if (this) "1" else "0"
 
 inline fun <T> T.ooi(block: (T) -> Unit): T = also(block)
 

tests/testutils/src/main/kotlin/io/element/android/tests/testutils/TestCoroutineDispatchers.kt

@@ -14,9 +14,12 @@
  * limitations under the License.
  */
 
+@file:OptIn(ExperimentalCoroutinesApi::class)
+
 package io.element.android.tests.testutils
 
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.TestCoroutineScheduler
 import kotlinx.coroutines.test.UnconfinedTestDispatcher