Skip to content

Commit

Permalink
Implement all room sorting in the SDK, no need to sort twice
Browse files Browse the repository at this point in the history
Change-Id: Ieb1a2336b60e5b4cc59e2276ad680ae7dfb0290b
  • Loading branch information
SpiritCroc committed Sep 1, 2024
1 parent 1fc850d commit 5c310cd
Show file tree
Hide file tree
Showing 15 changed files with 129 additions and 110 deletions.
6 changes: 4 additions & 2 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ Note that following list of changes compared to Element X is likely incomplete,

- Filter for spaces, including support for hierarchical spaces †‡
- Filter for favorites, unreads, DMs, and group chats via our spaces navigation
- Sort room list by unread first ‡
- Configure room list sort order to optionally: †‡
- Show unread on top
- Pin favorites
- Show low priority on bottom
- Non-expanding compact app bar in the chat overview †
- Show unread counts ([MSC2654](https://github.com/matrix-org/matrix-spec-proposals/pull/2654)) ‡[^1]
- Experimental client-side unread sorting to respect client-side mention detection and more †
- Option to show favorites on top †


## Conversation screen
Expand Down
Original file line number Diff line number Diff line change
@@ -1,90 +1,62 @@
package chat.schildi.features.roomlist

import androidx.lifecycle.AtomicReference
import chat.schildi.lib.preferences.ScPreferencesStore
import chat.schildi.lib.preferences.ScPrefs
import io.element.android.features.roomlist.impl.model.RoomListRoomSummary
import io.element.android.features.roomlist.impl.model.RoomSummaryDisplayType
import io.element.android.libraries.core.coroutine.childScope
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder
import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.currentCoroutineContext
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.flatMapLatest
import timber.log.Timber
import javax.inject.Inject

class ScRoomSortOrderSource @Inject constructor(
private val scPreferencesStore: ScPreferencesStore
private val scPreferencesStore: ScPreferencesStore,
private val roomListService: RoomListService,
) {
private val lastCoroutineScope = AtomicReference<CoroutineScope?>(null)

private val _sortOrder = MutableSharedFlow<ScRoomSortOrder>(replay = 1)
val sortOrder: SharedFlow<ScRoomSortOrder> = _sortOrder

fun launchIn(coroutineScope: CoroutineScope) {
// From life space list and current space selection, build the RoomId filter
combine(
@OptIn(ExperimentalCoroutinesApi::class)
fun filteredSummaries(coroutineScope: CoroutineScope): Flow<List<RoomSummary>> {
// Listen to preferences relevant to sort order, then apply these to the dynamic room list
return combine(
scPreferencesStore.settingFlow(ScPrefs.SORT_BY_UNREAD),
scPreferencesStore.settingFlow(ScPrefs.PIN_FAVORITES),
scPreferencesStore.settingFlow(ScPrefs.BURY_LOW_PRIORITY),
scPreferencesStore.settingFlow(ScPrefs.CLIENT_SIDE_SORT),
scPreferencesStore.settingFlow(ScPrefs.SORT_BY_ACTIVITY),
scPreferencesStore.settingFlow(ScPrefs.CLIENT_GENERATED_UNREAD_COUNTS),
) { pinFavorites, buryLowPriority, clientSideSort, activitySort, clientSideUnreadCounts ->
ScRoomSortOrder(pinFavorites, buryLowPriority, clientSideSort, activitySort, clientSideUnreadCounts)
}.onEach { result ->
_sortOrder.emit(result)
}.launchIn(coroutineScope)
}

fun sortRooms(rooms: List<RoomListRoomSummary>, order: ScRoomSortOrder): List<RoomListRoomSummary> {
// TODO: move to SDK?
return if (order.needsAction()) {
// Do activity-based sorting as separate step, since we do not know for sure the range of timestamps,
// but we want to prioritize favorite state above activity
if (order.activitySort) {
rooms.sortedByDescending { it.lastMessageTimestamp ?: 0L }
} else {
rooms
}.sortedBy { room ->
val inviteAdd = if (room.displayType == RoomSummaryDisplayType.INVITE) -10_000 else 0
val favoriteAdd = if (order.pinFavorites && !room.isFavorite) 1000 else 0
val lowPrioAdd = if (order.buryLowPriority && room.isLowPriority) 100 else 0
val unreadAdd = when {
order.activitySort -> 0
!order.clientSideSort -> 0
order.clientSideUnreadCounts -> unreadSort(
room.isMarkedUnread,
room.numberOfUnreadMentions,
room.numberOfUnreadNotifications,
room.numberOfUnreadMessages
)
else -> unreadSort(
room.isMarkedUnread,
room.highlightCount,
room.notificationCount,
room.unreadCount
)
}
inviteAdd + favoriteAdd + lowPrioAdd + unreadAdd
}
} else {
rooms
) { byUnread, pinFavorites, buryLowPriority, clientSideUnreadCounts ->
ScRoomSortOrder(
byUnread = byUnread,
pinFavourites = pinFavorites,
buryLowPriority = buryLowPriority,
clientSideUnreadCounts = clientSideUnreadCounts,
)
}.flatMapLatest { sortOrder ->
// TODO: would be nice to teach the SDK to update sort order without recreating the whole list
// Cancel jobs for previous list
val scope = coroutineScope.childScope(Dispatchers.Default, "sc-sorted-room-list")
lastCoroutineScope.getAndSet(scope)?.cancel()
Timber.d("Create new filtered and sorted room list for $sortOrder")
roomListService.scCreateRoomList(
pageSize = 30,
initialFilter = RoomListFilter.all(),
source = RoomList.Source.All,
sortOrder = sortOrder,
coroutineScope = scope,
).also {
it.loadAllIncrementally(scope)
}.filteredSummaries
}
}

private fun unreadSort(markedUnread: Boolean, mentionCount: Int, notificationCount: Int, unreadCount: Int): Int {
return when {
markedUnread || notificationCount > 0 || mentionCount > 0 -> 0
unreadCount > 0 -> 1
else -> 2
}
}
}

data class ScRoomSortOrder(
val pinFavorites: Boolean,
val buryLowPriority: Boolean,
val clientSideSort: Boolean,
val activitySort: Boolean,
val clientSideUnreadCounts: Boolean,
) {
fun needsAction() = pinFavorites || buryLowPriority || clientSideSort || activitySort
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package io.element.android.features.roomlist.impl.datasource

import chat.schildi.features.roomlist.ScRoomSortOrder
import chat.schildi.features.roomlist.ScRoomSortOrderSource
import io.element.android.features.roomlist.impl.model.RoomListRoomSummary
import io.element.android.libraries.androidutils.diff.DiffCacheUpdater
Expand All @@ -32,7 +31,6 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
Expand Down Expand Up @@ -71,18 +69,12 @@ class RoomListDataSource @Inject constructor(
roomListService
.allRooms
.filteredSummaries
*/
scRoomSortOrderSource.filteredSummaries(coroutineScope)
.onEach { roomSummaries ->
replaceWith(roomSummaries)
}
.launchIn(coroutineScope)
*/
scRoomSortOrderSource.launchIn(coroutineScope)
combine(
roomListService.allRooms.filteredSummaries,
scRoomSortOrderSource.sortOrder,
) { roomSummaries, sortOrder ->
replaceWith(roomSummaries, sortOrder)
}.launchIn(coroutineScope)
}

suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
Expand All @@ -99,17 +91,17 @@ class RoomListDataSource @Inject constructor(
.launchIn(appScope)
}

private suspend fun replaceWith(roomSummaries: List<RoomSummary>, sortOrder: ScRoomSortOrder) = withContext(coroutineDispatchers.computation) {
private suspend fun replaceWith(roomSummaries: List<RoomSummary>) = withContext(coroutineDispatchers.computation) {
lock.withLock {
diffCacheUpdater.updateWith(roomSummaries)
buildAndEmitAllRooms(roomSummaries, sortOrder)
buildAndEmitAllRooms(roomSummaries)
}
}

private suspend fun buildAndEmitAllRooms(roomSummaries: List<RoomSummary>, sortOrder: ScRoomSortOrder) {
private suspend fun buildAndEmitAllRooms(roomSummaries: List<RoomSummary>) {
val roomListRoomSummaries = diffCache.indices().mapNotNull { index ->
diffCache.get(index) ?: buildAndCacheItem(roomSummaries, index)
}.let { scRoomSortOrderSource.sortRooms(it, sortOrder) }
}
_allRooms.emit(roomListRoomSummaries.toImmutableList())
}

Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ jsoup = "org.jsoup:jsoup:1.18.1"
appyx_core = { module = "com.bumble.appyx:core", version.ref = "appyx" }
molecule-runtime = "app.cash.molecule:molecule-runtime:2.0.0"
timber = "com.jakewharton.timber:timber:5.0.1"
matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.25"
matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.26"
matrix_richtexteditor = { module = "chat.schildi:wysiwyg", version.ref = "wysiwyg" }
matrix_richtexteditor_compose = { module = "chat.schildi:wysiwyg-compose", version.ref = "wysiwyg" }
sqldelight-driver-android = { module = "app.cash.sqldelight:android-driver", version.ref = "sqldelight" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package io.element.android.libraries.matrix.api.roomlist

import androidx.compose.runtime.Immutable
import io.element.android.libraries.matrix.api.core.RoomId
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterIsInstance
Expand Down Expand Up @@ -54,6 +55,14 @@ interface RoomListService {
source: RoomList.Source,
): DynamicRoomList

fun scCreateRoomList(
pageSize: Int,
initialFilter: RoomListFilter,
source: RoomList.Source,
coroutineScope: CoroutineScope,
sortOrder: ScRoomSortOrder = ScRoomSortOrder(),
): DynamicRoomList

/**
* Subscribes to sync requests for the visible rooms.
* @param roomIds the list of visible room ids to subscribe to.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.element.android.libraries.matrix.api.roomlist


data class ScRoomSortOrder(
val byUnread: Boolean = false,
val pinFavourites: Boolean = false,
val buryLowPriority: Boolean = false,
val clientSideUnreadCounts: Boolean = false,
)
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package io.element.android.libraries.matrix.impl.roomlist

import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder
import io.element.android.libraries.matrix.impl.util.cancelAndDestroy
import io.element.android.libraries.matrix.impl.util.mxCallbackFlow
import kotlinx.coroutines.channels.Channel
Expand Down Expand Up @@ -66,7 +67,8 @@ fun RoomListInterface.loadingStateFlow(): Flow<RoomListLoadingState> =
internal fun RoomListInterface.entriesFlow(
pageSize: Int,
roomListDynamicEvents: Flow<RoomListDynamicEvents>,
initialFilterKind: RoomListEntriesDynamicFilterKind
initialFilterKind: RoomListEntriesDynamicFilterKind,
sortOrder: ScRoomSortOrder,
): Flow<List<RoomListEntriesUpdate>> =
callbackFlow {
val listener = object : RoomListEntriesListener {
Expand All @@ -77,6 +79,7 @@ internal fun RoomListInterface.entriesFlow(
val result = entriesWithDynamicAdapters(pageSize.toUInt(), listener)
val controller = result.controller()
controller.setFilter(initialFilterKind)
controller.setSortOrder(sortOrder.toSdkSortOrder())
roomListDynamicEvents.onEach { controllerEvents ->
when (controllerEvents) {
is RoomListDynamicEvents.SetFilter -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.element.android.libraries.matrix.api.roomlist.DynamicRoomList
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
Expand Down Expand Up @@ -50,6 +51,7 @@ internal class RoomListFactory(
coroutineScope: CoroutineScope = sessionCoroutineScope,
coroutineContext: CoroutineContext = EmptyCoroutineContext,
initialFilter: RoomListFilter = RoomListFilter.all(),
sortOrder: ScRoomSortOrder = ScRoomSortOrder(),
innerProvider: suspend () -> InnerRoomList
): DynamicRoomList {
val loadingStateFlow: MutableStateFlow<RoomList.LoadingState> = MutableStateFlow(RoomList.LoadingState.NotLoaded)
Expand All @@ -68,7 +70,8 @@ internal class RoomListFactory(
innerRoomList.entriesFlow(
pageSize = pageSize,
roomListDynamicEvents = dynamicEvents,
initialFilterKind = RoomListEntriesDynamicFilterKind.NonLeft
initialFilterKind = RoomListEntriesDynamicFilterKind.NonLeft,
sortOrder = sortOrder,
).onEach { update ->
processor.postUpdate(update)
}.launchIn(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.roomlist.DynamicRoomList
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder
import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally
import io.element.android.libraries.matrix.impl.room.RoomSyncSubscriber
import kotlinx.coroutines.CoroutineDispatcher
Expand Down Expand Up @@ -62,6 +63,26 @@ internal class RustRoomListService(
}
}

override fun scCreateRoomList( // SC: add coroutineScope and sortOrder - tricky to have coroutineScope with default fallback for child classes
pageSize: Int,
initialFilter: RoomListFilter,
source: RoomList.Source,
coroutineScope: CoroutineScope,
sortOrder: ScRoomSortOrder
): DynamicRoomList {
return roomListFactory.createRoomList(
pageSize = pageSize,
initialFilter = initialFilter,
coroutineScope = coroutineScope,
sortOrder = sortOrder,
) {
when (source) {
RoomList.Source.All -> innerRoomListService.allRooms()
RoomList.Source.SPACES -> innerRoomListService.allSpaces()
}
}
}

override suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
val toSubscribe = roomIds.filterNot { roomSyncSubscriber.isSubscribedTo(it) }
if (toSubscribe.isNotEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.element.android.libraries.matrix.impl.roomlist

import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder

fun ScRoomSortOrder.toSdkSortOrder() = uniffi.matrix_sdk_ui.ScSortOrder(
byUnread = byUnread,
pinFavorites = pinFavourites,
buryLowPriority = buryLowPriority,
clientGeneratedUnread = clientSideUnreadCounts,
)
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.roomlist.ScRoomSortOrder
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow

Expand Down Expand Up @@ -58,9 +60,13 @@ class FakeRoomListService(
): DynamicRoomList {
return when (source) {
RoomList.Source.All -> allRooms
RoomList.Source.SPACES -> allSpaces
}
}

override fun scCreateRoomList(pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source, coroutineScope: CoroutineScope, sortOrder: ScRoomSortOrder)
= createRoomList(pageSize, initialFilter, source)

override suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
subscribeToVisibleRoomsLambda(roomIds)
}
Expand All @@ -71,7 +77,7 @@ class FakeRoomListService(
MutableStateFlow(RoomListFilter.all())
)

override val allSpaces: RoomList = SimplePagedRoomList(
override val allSpaces: DynamicRoomList = SimplePagedRoomList(
allSpacesSummariesFlow,
allSpacesLoadingStateFlow,
MutableStateFlow(RoomListFilter.all())
Expand Down
Loading

0 comments on commit 5c310cd

Please sign in to comment.