Let TimelineItemsFactory group the item if necessary, so it's done on a computation dispatcher. Let the View manage the expanded/grouped state.

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/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,
+                    isExpanded = isExpanded.value,
-                    isHighlighted = !timelineItem.expanded && timelineItem.events.any { it.identifier() == highlightedItem },
+                    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 = {}
                             )
                         }
                     }

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/groups/TimelineItemGrouper.kt

@@ -29,16 +29,14 @@ import io.element.android.features.messages.impl.timeline.model.event.TimelineIt
 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,14 +46,14 @@ 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
     }
@@ -82,8 +80,7 @@ class TimelineItemGrouper @Inject constructor() {
  * 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>,
+    group: MutableList<TimelineItem.Event>
-    expandedGroups: Map<String, Boolean>,
 ) {
     if (group.size == 1) {
         // Do not create a group with just 1 item, just add the item to the result
@@ -91,7 +88,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
+        // use last id with a suffix. Last will not change in cas of new event from backpagination.
-        val id = events.first().id + "_group"
+        val id = events.last().id + "_group"
     }
 }

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/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/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,