Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeline better jump to behaviours #3597

Merged
merged 5 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemPollContent
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateContent
import io.element.android.features.messages.impl.timeline.model.event.TimelineItemTextBasedContent
import io.element.android.features.messages.impl.typing.TypingNotificationPresenter
import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerPresenter
import io.element.android.features.networkmonitor.api.NetworkMonitor
import io.element.android.features.networkmonitor.api.NetworkStatus
Expand Down Expand Up @@ -91,7 +90,6 @@ class MessagesPresenter @AssistedInject constructor(
private val composerPresenter: MessageComposerPresenter,
private val voiceMessageComposerPresenter: VoiceMessageComposerPresenter,
timelinePresenterFactory: TimelinePresenter.Factory,
private val typingNotificationPresenter: TypingNotificationPresenter,
private val actionListPresenterFactory: ActionListPresenter.Factory,
private val customReactionPresenter: CustomReactionPresenter,
private val reactionSummaryPresenter: ReactionSummaryPresenter,
Expand Down Expand Up @@ -125,7 +123,6 @@ class MessagesPresenter @AssistedInject constructor(
val composerState = composerPresenter.present()
val voiceMessageComposerState = voiceMessageComposerPresenter.present()
val timelineState = timelinePresenter.present()
val typingNotificationState = typingNotificationPresenter.present()
val actionListState = actionListPresenter.present()
val customReactionState = customReactionPresenter.present()
val reactionSummaryState = reactionSummaryPresenter.present()
Expand Down Expand Up @@ -216,7 +213,6 @@ class MessagesPresenter @AssistedInject constructor(
userEventPermissions = userEventPermissions,
voiceMessageComposerState = voiceMessageComposerState,
timelineState = timelineState,
typingNotificationState = typingNotificationState,
actionListState = actionListState,
customReactionState = customReactionState,
reactionSummaryState = reactionSummaryState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import io.element.android.features.messages.impl.timeline.TimelineState
import io.element.android.features.messages.impl.timeline.components.customreaction.CustomReactionState
import io.element.android.features.messages.impl.timeline.components.reactionsummary.ReactionSummaryState
import io.element.android.features.messages.impl.timeline.components.receipt.bottomsheet.ReadReceiptBottomSheetState
import io.element.android.features.messages.impl.typing.TypingNotificationState
import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerState
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.designsystem.components.avatar.AvatarData
Expand All @@ -33,7 +32,6 @@ data class MessagesState(
val composerState: MessageComposerState,
val voiceMessageComposerState: VoiceMessageComposerState,
val timelineState: TimelineState,
val typingNotificationState: TypingNotificationState,
val actionListState: ActionListState,
val customReactionState: CustomReactionState,
val reactionSummaryState: ReactionSummaryState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import io.element.android.features.messages.impl.timeline.components.receipt.bot
import io.element.android.features.messages.impl.timeline.components.receipt.bottomsheet.ReadReceiptBottomSheetState
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.features.messages.impl.typing.aTypingNotificationState
import io.element.android.features.messages.impl.voicemessages.composer.VoiceMessageComposerState
import io.element.android.features.messages.impl.voicemessages.composer.aVoiceMessageComposerState
import io.element.android.features.messages.impl.voicemessages.composer.aVoiceMessagePreviewState
Expand Down Expand Up @@ -122,7 +121,6 @@ fun aMessagesState(
userEventPermissions = userEventPermissions,
composerState = composerState,
voiceMessageComposerState = voiceMessageComposerState,
typingNotificationState = aTypingNotificationState(),
timelineState = timelineState,
readReceiptBottomSheetState = readReceiptBottomSheetState,
actionListState = actionListState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ private fun MessagesViewContent(
val scrollBehavior = PinnedMessagesBannerViewDefaults.rememberExitOnScrollBehavior()
TimelineView(
state = state.timelineState,
typingNotificationState = state.typingNotificationState,
onUserDataClick = onUserDataClick,
onLinkClick = onLinkClick,
onMessageClick = onMessageClick,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import io.element.android.features.messages.impl.crypto.sendfailure.resolve.Reso
import io.element.android.features.messages.impl.crypto.sendfailure.resolve.ResolveVerifiedUserSendFailureState
import io.element.android.features.messages.impl.pinned.banner.PinnedMessagesBannerPresenter
import io.element.android.features.messages.impl.pinned.banner.PinnedMessagesBannerState
import io.element.android.features.messages.impl.typing.TypingNotificationPresenter
import io.element.android.features.messages.impl.typing.TypingNotificationState
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.di.RoomScope

Expand All @@ -25,4 +27,7 @@ interface MessagesModule {

@Binds
fun bindResolveVerifiedUserSendFailurePresenter(presenter: ResolveVerifiedUserSendFailurePresenter): Presenter<ResolveVerifiedUserSendFailureState>

@Binds
fun bindTypingNotificationPresenter(presenter: TypingNotificationPresenter): Presenter<TypingNotificationState>
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import io.element.android.features.messages.impl.timeline.TimelineRoomInfo
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.features.messages.impl.typing.TypingNotificationState
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
Expand All @@ -44,6 +45,7 @@ import io.element.android.libraries.ui.strings.CommonStrings
import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.analyticsproviders.api.trackers.captureInteraction
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.flowOf
Expand Down Expand Up @@ -87,7 +89,12 @@ class PinnedMessagesListPresenter @AssistedInject constructor(
userHasPermissionToSendReaction = false,
isCallOngoing = false,
// don't compute this value or the pin icon will be shown
pinnedEventIds = emptyList()
pinnedEventIds = emptyList(),
typingNotificationState = TypingNotificationState(
renderTypingNotifications = false,
typingMembers = persistentListOf(),
reserveSpace = false,
)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,39 @@
package io.element.android.features.messages.impl.timeline

import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.matrix.api.core.EventId
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import timber.log.Timber
import javax.inject.Inject

@SingleIn(RoomScope::class)
class TimelineItemIndexer @Inject constructor() {
// This is a latch to wait for the first process call
private val firstProcessLatch = CompletableDeferred<Unit>()
private val timelineEventsIndexes = mutableMapOf<EventId, Int>()

fun isKnown(eventId: EventId): Boolean {
return timelineEventsIndexes.containsKey(eventId).also {
Timber.d("$eventId isKnown = $it")
private val mutex = Mutex()

suspend fun isKnown(eventId: EventId): Boolean {
firstProcessLatch.await()
return mutex.withLock {
timelineEventsIndexes.containsKey(eventId).also {
Timber.d("$eventId isKnown = $it")
}
}
}

fun indexOf(eventId: EventId): Int {
return (timelineEventsIndexes[eventId] ?: -1).also {
Timber.d("indexOf $eventId= $it")
suspend fun indexOf(eventId: EventId): Int {
firstProcessLatch.await()
return mutex.withLock {
(timelineEventsIndexes[eventId] ?: -1).also {
Timber.d("indexOf $eventId= $it")
}
}
}

fun process(timelineItems: List<TimelineItem>) {
suspend fun process(timelineItems: List<TimelineItem>) = mutex.withLock {
Timber.d("process ${timelineItems.size} items")
timelineEventsIndexes.clear()
timelineItems.forEachIndexed { index, timelineItem ->
Expand All @@ -46,6 +56,7 @@ class TimelineItemIndexer @Inject constructor() {
else -> Unit
}
}
firstProcessLatch.complete(Unit)
}

private fun processEvent(event: TimelineItem.Event, index: Int) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactoryConfig
import io.element.android.features.messages.impl.timeline.model.NewEventState
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.features.messages.impl.typing.TypingNotificationState
import io.element.android.features.messages.impl.voicemessages.timeline.RedactedVoiceMessageManager
import io.element.android.features.poll.api.actions.EndPollAction
import io.element.android.features.poll.api.actions.SendPollResponseAction
Expand All @@ -54,12 +55,12 @@
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import timber.log.Timber

const val FOCUS_ON_PINNED_EVENT_DEBOUNCE_DURATION_IN_MILLIS = 200L

class TimelinePresenter @AssistedInject constructor(
timelineItemsFactoryCreator: TimelineItemsFactory.Creator,
private val timelineItemIndexer: TimelineItemIndexer,
private val room: MatrixRoom,
private val dispatchers: CoroutineDispatchers,
private val appScope: CoroutineScope,
Expand All @@ -69,7 +70,9 @@
private val endPollAction: EndPollAction,
private val sessionPreferencesStore: SessionPreferencesStore,
private val timelineController: TimelineController,
private val timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
private val resolveVerifiedUserSendFailurePresenter: Presenter<ResolveVerifiedUserSendFailureState>,
private val typingNotificationPresenter: Presenter<TypingNotificationState>,
) : Presenter<TimelineState> {
@AssistedFactory
interface Factory {
Expand All @@ -87,13 +90,7 @@
@Composable
override fun present(): TimelineState {
val localScope = rememberCoroutineScope()
val focusRequestState: MutableState<FocusRequestState> = remember {
mutableStateOf(FocusRequestState.None)
}

LaunchedEffect(Unit) {
timelineItemsFactory.timelineItems.collect { timelineItems = it }
}
var focusRequestState: FocusRequestState by remember { mutableStateOf(FocusRequestState.None) }

val lastReadReceiptId = rememberSaveable { mutableStateOf<EventId?>(null) }

Expand Down Expand Up @@ -152,13 +149,13 @@
navigator.onEditPollClick(event.pollStartId)
}
is TimelineEvents.FocusOnEvent -> {
focusRequestState.value = FocusRequestState.Requested(event.eventId, event.debounce)
focusRequestState = FocusRequestState.Requested(event.eventId, event.debounce)
}
is TimelineEvents.OnFocusEventRender -> {
focusRequestState.value = focusRequestState.value.onFocusEventRender()
focusRequestState = focusRequestState.onFocusEventRender()

Check warning on line 155 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt

View check run for this annotation

Codecov / codecov/patch

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

Added line #L155 was not covered by tests
}
is TimelineEvents.ClearFocusRequestState -> {
focusRequestState.value = FocusRequestState.None
focusRequestState = FocusRequestState.None
}
is TimelineEvents.JumpToLive -> {
timelineController.focusOnLive()
Expand All @@ -171,28 +168,46 @@
}
}

LaunchedEffect(focusRequestState.value) {
when (val currentFocusRequestState = focusRequestState.value) {
LaunchedEffect(Unit) {
timelineItemsFactory.timelineItems
.onEach { newTimelineItems ->
timelineItemIndexer.process(newTimelineItems)
timelineItems = newTimelineItems
}
.launchIn(this)

combine(timelineController.timelineItems(), room.membersStateFlow) { items, membersState ->
timelineItemsFactory.replaceWith(
timelineItems = items,
roomMembers = membersState.roomMembers().orEmpty()
)
items
}
.onEach(redactedVoiceMessageManager::onEachMatrixTimelineItem)
.launchIn(this)
}

LaunchedEffect(focusRequestState) {
Timber.d("## focusRequestState: $focusRequestState")
when (val currentFocusRequestState = focusRequestState) {
is FocusRequestState.Requested -> {
delay(currentFocusRequestState.debounce)
if (timelineItemIndexer.isKnown(currentFocusRequestState.eventId)) {
val index = timelineItemIndexer.indexOf(currentFocusRequestState.eventId)
focusRequestState.value = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
focusRequestState = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
} else {
focusRequestState.value = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
focusRequestState = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
}
}
is FocusRequestState.Loading -> {
val eventId = currentFocusRequestState.eventId
timelineController.focusOnEvent(eventId)
.fold(
onSuccess = {
focusRequestState.value = FocusRequestState.Success(eventId = eventId)
},
onFailure = {
focusRequestState.value = FocusRequestState.Failure(throwable = it)
}
)
.onSuccess {
focusRequestState = FocusRequestState.Success(eventId = eventId)
}
.onFailure {
focusRequestState = FocusRequestState.Failure(it)
}
}
else -> Unit
}
Expand All @@ -202,30 +217,19 @@
computeNewItemState(timelineItems, prevMostRecentItemId, newEventState)
}

LaunchedEffect(timelineItems.size, focusRequestState.value) {
val currentFocusRequestState = focusRequestState.value
if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.isIndexed) {
LaunchedEffect(timelineItems.size, focusRequestState) {
val currentFocusRequestState = focusRequestState
if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.rendered) {
val eventId = currentFocusRequestState.eventId
if (timelineItemIndexer.isKnown(eventId)) {
val index = timelineItemIndexer.indexOf(eventId)
focusRequestState.value = FocusRequestState.Success(eventId = eventId, index = index)
focusRequestState = FocusRequestState.Success(eventId = eventId, index = index)
}
}
}

LaunchedEffect(Unit) {
combine(timelineController.timelineItems(), room.membersStateFlow) { items, membersState ->
timelineItemsFactory.replaceWith(
timelineItems = items,
roomMembers = membersState.roomMembers().orEmpty()
)
items
}
.onEach(redactedVoiceMessageManager::onEachMatrixTimelineItem)
.launchIn(this)
}

val timelineRoomInfo by remember {
val typingNotificationState = typingNotificationPresenter.present()
val timelineRoomInfo by remember(typingNotificationState) {
derivedStateOf {
TimelineRoomInfo(
name = room.displayName,
Expand All @@ -234,6 +238,7 @@
userHasPermissionToSendReaction = userHasPermissionToSendReaction,
isCallOngoing = roomInfo?.hasRoomCall.orFalse(),
pinnedEventIds = roomInfo?.pinnedEventIds.orEmpty(),
typingNotificationState = typingNotificationState,
)
}
}
Expand All @@ -243,7 +248,7 @@
renderReadReceipts = renderReadReceipts,
newEventState = newEventState.value,
isLive = isLive,
focusRequestState = focusRequestState.value,
focusRequestState = focusRequestState,
messageShield = messageShield.value,
resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState,
eventSink = { handleEvents(it) }
Expand Down
Loading
Loading