From 75b1c2219761a84b3aa9f89d6c4756b01cfb5d1c Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 14 Jun 2024 19:04:11 +0200 Subject: [PATCH] Timeline : let FocusOnEvent be cancellable and refactor a bit focus states. --- changelog.d/2876.misc | 1 + .../messages/impl/timeline/TimelineEvents.kt | 1 + .../impl/timeline/TimelinePresenter.kt | 57 ++++++++++++------- .../messages/impl/timeline/TimelineState.kt | 24 ++++++-- .../impl/timeline/TimelineStateProvider.kt | 27 +++++---- .../messages/impl/timeline/TimelineView.kt | 28 +++++---- .../focus/FocusRequestStateProvider.kt | 4 +- .../timeline/focus/FocusRequestStateView.kt | 2 +- .../impl/timeline/TimelinePresenterTest.kt | 8 +-- .../matrix/impl/room/RustMatrixRoom.kt | 5 ++ 10 files changed, 105 insertions(+), 52 deletions(-) create mode 100644 changelog.d/2876.misc diff --git a/changelog.d/2876.misc b/changelog.d/2876.misc new file mode 100644 index 0000000000..b75bdf0e1c --- /dev/null +++ b/changelog.d/2876.misc @@ -0,0 +1 @@ +Allow cancelling jump to event in timeline. diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvents.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvents.kt index 65600f48cf..24007530e6 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvents.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineEvents.kt @@ -23,6 +23,7 @@ sealed interface TimelineEvents { data class OnScrollFinished(val firstIndex: Int) : TimelineEvents data class FocusOnEvent(val eventId: EventId) : TimelineEvents data object ClearFocusRequestState : TimelineEvents + data object OnFocusEventRender : TimelineEvents data object JumpToLive : TimelineEvents /** diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt index 61a445b187..d25069c24d 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt @@ -76,9 +76,6 @@ class TimelinePresenter @AssistedInject constructor( @Composable override fun present(): TimelineState { val localScope = rememberCoroutineScope() - val focusedEventId: MutableState = rememberSaveable { - mutableStateOf(null) - } val focusRequestState: MutableState = remember { mutableStateOf(FocusRequestState.None) } @@ -139,23 +136,16 @@ class TimelinePresenter @AssistedInject constructor( navigator.onEditPollClick(event.pollStartId) } is TimelineEvents.FocusOnEvent -> localScope.launch { - focusedEventId.value = event.eventId if (timelineItemIndexer.isKnown(event.eventId)) { val index = timelineItemIndexer.indexOf(event.eventId) - focusRequestState.value = FocusRequestState.Cached(index) + focusRequestState.value = FocusRequestState.Success(eventId = event.eventId, index = index) } else { - focusRequestState.value = FocusRequestState.Fetching - timelineController.focusOnEvent(event.eventId) - .fold( - onSuccess = { - focusRequestState.value = FocusRequestState.Fetched - }, - onFailure = { - focusRequestState.value = FocusRequestState.Failure(it) - } - ) + focusRequestState.value = FocusRequestState.Loading(eventId = event.eventId) } } + is TimelineEvents.OnFocusEventRender -> { + focusRequestState.value = focusRequestState.value.onFocusEventRender() + } is TimelineEvents.ClearFocusRequestState -> { focusRequestState.value = FocusRequestState.None } @@ -165,16 +155,33 @@ class TimelinePresenter @AssistedInject constructor( } } + LaunchedEffect(focusRequestState.value) { + val currentFocusRequestState = focusRequestState.value + if (currentFocusRequestState 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) + } + ) + } + } + LaunchedEffect(timelineItems.size) { computeNewItemState(timelineItems, prevMostRecentItemId, newEventState) } - LaunchedEffect(timelineItems.size, focusRequestState.value, focusedEventId.value) { - val currentFocusedEventId = focusedEventId.value - if (focusRequestState.value is FocusRequestState.Fetched && currentFocusedEventId != null) { - if (timelineItemIndexer.isKnown(currentFocusedEventId)) { - val index = timelineItemIndexer.indexOf(currentFocusedEventId) - focusRequestState.value = FocusRequestState.Cached(index) + LaunchedEffect(timelineItems.size, focusRequestState.value) { + val currentFocusRequestState = focusRequestState.value + if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.isIndexed) { + val eventId = currentFocusRequestState.eventId + if (timelineItemIndexer.isKnown(eventId)) { + val index = timelineItemIndexer.indexOf(eventId) + focusRequestState.value = FocusRequestState.Success(eventId = eventId, index = index) } } } @@ -208,7 +215,6 @@ class TimelinePresenter @AssistedInject constructor( renderReadReceipts = renderReadReceipts, newEventState = newEventState.value, isLive = isLive, - focusedEventId = focusedEventId.value, focusRequestState = focusRequestState.value, eventSink = { handleEvents(it) } ) @@ -278,3 +284,10 @@ class TimelinePresenter @AssistedInject constructor( return null } } + +private fun FocusRequestState.onFocusEventRender(): FocusRequestState { + return when (this) { + is FocusRequestState.Success -> copy(rendered = true) + else -> this + } +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt index 971f231546..eb8622d0bc 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineState.kt @@ -29,20 +29,36 @@ data class TimelineState( val renderReadReceipts: Boolean, val newEventState: NewEventState, val isLive: Boolean, - val focusedEventId: EventId?, val focusRequestState: FocusRequestState, val eventSink: (TimelineEvents) -> Unit, ) { val hasAnyEvent = timelineItems.any { it is TimelineItem.Event } + val focusedEventId = focusRequestState.eventId() } @Immutable sealed interface FocusRequestState { data object None : FocusRequestState - data class Cached(val index: Int) : FocusRequestState - data object Fetching : FocusRequestState - data object Fetched : FocusRequestState + data class Loading(val eventId: EventId) : FocusRequestState + data class Success( + val eventId: EventId, + val index: Int = -1, + // This is used to know if the event has been rendered yet. + val rendered: Boolean = false, + ) : FocusRequestState { + val isIndexed + get() = index != -1 + } + data class Failure(val throwable: Throwable) : FocusRequestState + + fun eventId(): EventId? { + return when (this) { + is Loading -> eventId + is Success -> eventId + else -> null + } + } } @Immutable diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt index 964a5d2ef7..a15763648d 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt @@ -51,16 +51,23 @@ fun aTimelineState( focusedEventIndex: Int = -1, isLive: Boolean = true, eventSink: (TimelineEvents) -> Unit = {}, -) = TimelineState( - timelineItems = timelineItems, - timelineRoomInfo = timelineRoomInfo, - renderReadReceipts = renderReadReceipts, - newEventState = NewEventState.None, - isLive = isLive, - focusedEventId = timelineItems.filterIsInstance().getOrNull(focusedEventIndex)?.eventId, - focusRequestState = FocusRequestState.None, - eventSink = eventSink, -) +): TimelineState { + val focusedEventId = timelineItems.filterIsInstance().getOrNull(focusedEventIndex)?.eventId + val focusRequestState = if (focusedEventId != null) { + FocusRequestState.Success(focusedEventId, focusedEventIndex) + } else { + FocusRequestState.None + } + return TimelineState( + timelineItems = timelineItems, + timelineRoomInfo = timelineRoomInfo, + renderReadReceipts = renderReadReceipts, + newEventState = NewEventState.None, + isLive = isLive, + focusRequestState = focusRequestState, + eventSink = eventSink, + ) +} internal fun aTimelineItemList(content: TimelineItemEventContent): ImmutableList { return persistentListOf( diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index 370d7bc517..eff2019a11 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt @@ -100,6 +100,14 @@ fun TimelineView( state.eventSink(TimelineEvents.OnScrollFinished(firstVisibleIndex)) } + fun onFocusEventRender() { + state.eventSink(TimelineEvents.OnFocusEventRender) + } + + fun onJumpToLive() { + state.eventSink(TimelineEvents.JumpToLive) + } + val context = LocalContext.current val lazyListState = rememberLazyListState() // Disable reverse layout when TalkBack is enabled to avoid incorrect ordering issues seen in the current Compose UI version @@ -167,8 +175,8 @@ fun TimelineView( isLive = state.isLive, focusRequestState = state.focusRequestState, onScrollFinishAt = ::onScrollFinishAt, - onClearFocusRequestState = ::clearFocusRequestState, - onJumpToLive = { state.eventSink(TimelineEvents.JumpToLive) }, + onJumpToLive = ::onJumpToLive, + onFocusEventRender = ::onFocusEventRender, ) } } @@ -182,9 +190,9 @@ private fun BoxScope.TimelineScrollHelper( isLive: Boolean, forceJumpToBottomVisibility: Boolean, focusRequestState: FocusRequestState, - onClearFocusRequestState: () -> Unit, onScrollFinishAt: (Int) -> Unit, onJumpToLive: () -> Unit, + onFocusEventRender: () -> Unit, ) { val coroutineScope = rememberCoroutineScope() val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } } @@ -212,15 +220,15 @@ private fun BoxScope.TimelineScrollHelper( } } - val latestOnClearFocusRequestState by rememberUpdatedState(onClearFocusRequestState) + val latestOnFocusEventRender by rememberUpdatedState(onFocusEventRender) LaunchedEffect(focusRequestState) { - if (focusRequestState is FocusRequestState.Cached) { + if (focusRequestState is FocusRequestState.Success && focusRequestState.isIndexed) { if (abs(lazyListState.firstVisibleItemIndex - focusRequestState.index) < 10) { lazyListState.animateScrollToItem(focusRequestState.index) } else { lazyListState.scrollToItem(focusRequestState.index) } - latestOnClearFocusRequestState() + latestOnFocusEventRender() } } @@ -243,8 +251,8 @@ private fun BoxScope.TimelineScrollHelper( // Use inverse of canAutoScroll otherwise we might briefly see the before the scroll animation is triggered isVisible = !canAutoScroll || forceJumpToBottomVisibility || !isLive, modifier = Modifier - .align(Alignment.BottomEnd) - .padding(end = 24.dp, bottom = 12.dp), + .align(Alignment.BottomEnd) + .padding(end = 24.dp, bottom = 12.dp), onClick = { jumpToBottom() }, ) } @@ -271,8 +279,8 @@ private fun JumpToBottomButton( ) { Icon( modifier = Modifier - .size(24.dp) - .rotate(90f), + .size(24.dp) + .rotate(90f), imageVector = CompoundIcons.ArrowRight(), contentDescription = stringResource(id = CommonStrings.a11y_jump_to_bottom) ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateProvider.kt index cb5b4a10f2..37097fbb05 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateProvider.kt @@ -24,7 +24,9 @@ import io.element.android.libraries.matrix.api.room.errors.FocusEventException open class FocusRequestStateProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( - FocusRequestState.Fetching, + FocusRequestState.Loading( + eventId = EventId("\$anEventId"), + ), FocusRequestState.Failure( FocusEventException.EventNotFound( eventId = EventId("\$anEventId"), diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateView.kt index 3c2892784d..23002a9d57 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/focus/FocusRequestStateView.kt @@ -49,7 +49,7 @@ fun FocusRequestStateView( modifier = modifier, ) } - FocusRequestState.Fetching -> { + is FocusRequestState.Loading -> { ProgressDialog( modifier = modifier, properties = DialogProperties(dismissOnBackPress = true, dismissOnClickOutside = true), diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt index 265e3ef636..ad52af11ca 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt @@ -495,11 +495,11 @@ private const val FAKE_UNIQUE_ID_2 = "FAKE_UNIQUE_ID_2" initialState.eventSink.invoke(TimelineEvents.FocusOnEvent(AN_EVENT_ID)) awaitItem().also { state -> assertThat(state.focusedEventId).isEqualTo(AN_EVENT_ID) - assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Fetching) + assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Loading(AN_EVENT_ID)) } skipItems(2) awaitItem().also { state -> - assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Fetched) + assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Success(AN_EVENT_ID)) assertThat(state.timelineItems).isNotEmpty() } initialState.eventSink.invoke(TimelineEvents.JumpToLive) @@ -539,7 +539,7 @@ private const val FAKE_UNIQUE_ID_2 = "FAKE_UNIQUE_ID_2" initialState.eventSink.invoke(TimelineEvents.FocusOnEvent(AN_EVENT_ID)) awaitItem().also { state -> assertThat(state.focusedEventId).isEqualTo(AN_EVENT_ID) - assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Cached(0)) + assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Success(AN_EVENT_ID, 0)) } } } @@ -562,7 +562,7 @@ private const val FAKE_UNIQUE_ID_2 = "FAKE_UNIQUE_ID_2" initialState.eventSink(TimelineEvents.FocusOnEvent(AN_EVENT_ID)) awaitItem().also { state -> assertThat(state.focusedEventId).isEqualTo(AN_EVENT_ID) - assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Fetching) + assertThat(state.focusRequestState).isEqualTo(FocusRequestState.Loading(AN_EVENT_ID)) } awaitItem().also { state -> assertThat(state.focusRequestState).isInstanceOf(FocusRequestState.Failure::class.java) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 28318540ce..f10938cc19 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -87,6 +87,7 @@ import org.matrix.rustcomponents.sdk.use import timber.log.Timber import uniffi.matrix_sdk.RoomPowerLevelChanges import java.io.File +import kotlin.coroutines.cancellation.CancellationException import org.matrix.rustcomponents.sdk.Room as InnerRoom import org.matrix.rustcomponents.sdk.Timeline as InnerTimeline @@ -183,6 +184,10 @@ class RustMatrixRoom( } }.mapFailure { it.toFocusEventException() + }.onFailure { + if (it is CancellationException) { + throw it + } } }