Sulkta-Coop/element-x-ada
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source
This commit is contained in:
Benoit Marty 2023-05-26 12:45:43 +02:00 • committed by Benoit Marty
parent 09be5f4a6a
commit 6000a7ca5b
11 changed files with 29 additions and 117 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -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(

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

@ -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
}

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

@ -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
)
}

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

@ -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 = {}
)
}
}

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

@ -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>) {

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

@ -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>,
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 +88,6 @@ private fun MutableList<TimelineItem>.addGroup(
} else {
add(
TimelineItem.GroupedEvents(
expanded = expandedGroups[group.first().id + "_group"].orFalse(),
events = group.toImmutableList()
)
)

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

@ -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"
}
}

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

@ -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()

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

@ -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(),
)
}

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

@ -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()
}
}
}

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

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