Skip to content

Commit cfcb821

Browse files
committed
Make sure the main timeline focuses on the thread root id too when navigating to a thread
1 parent 059842e commit cfcb821

File tree

6 files changed

+121
-86
lines changed

6 files changed

+121
-86
lines changed

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import io.element.android.features.verifysession.api.IncomingVerificationEntryPo
6868
import io.element.android.libraries.architecture.BackstackView
6969
import io.element.android.libraries.architecture.BaseFlowNode
7070
import io.element.android.libraries.architecture.createNode
71+
import io.element.android.libraries.architecture.waitForChildAttached
7172
import io.element.android.libraries.architecture.waitForNavTargetAttached
7273
import io.element.android.libraries.core.meta.BuildMeta
7374
import io.element.android.libraries.designsystem.theme.ElementThemeApp
@@ -79,7 +80,6 @@ import io.element.android.libraries.matrix.api.core.EventId
7980
import io.element.android.libraries.matrix.api.core.MAIN_SPACE
8081
import io.element.android.libraries.matrix.api.core.RoomId
8182
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
82-
import io.element.android.libraries.matrix.api.core.ThreadId
8383
import io.element.android.libraries.matrix.api.core.UserId
8484
import io.element.android.libraries.matrix.api.core.toRoomIdOrAlias
8585
import io.element.android.libraries.matrix.api.permalink.PermalinkData
@@ -506,16 +506,24 @@ class LoggedInFlowNode(
506506
waitForNavTargetAttached { navTarget ->
507507
navTarget is NavTarget.Home
508508
}
509-
return attachChild<RoomFlowNode> {
509+
attachChild<RoomFlowNode> {
510510
val roomNavTarget = NavTarget.Room(
511511
roomIdOrAlias = roomIdOrAlias,
512512
serverNames = serverNames,
513513
trigger = trigger,
514-
initialElement = RoomNavigationTarget.Root(eventId = eventId,
515-
)
514+
initialElement = RoomNavigationTarget.Root(eventId = eventId)
516515
)
517516
backstack.accept(AttachRoomOperation(roomNavTarget, clearBackstack))
518517
}
518+
519+
// If we don't do this check, we might be returning while a previous node with the same type is still displayed
520+
// This means we may attach some new nodes to that one, which will be quickly replaced by the one instantiated above
521+
return waitForChildAttached<RoomFlowNode, NavTarget> {
522+
it is NavTarget.Room &&
523+
it.roomIdOrAlias == roomIdOrAlias &&
524+
it.initialElement is RoomNavigationTarget.Root &&
525+
it.initialElement.eventId == eventId
526+
}
519527
}
520528

521529
suspend fun attachUser(userId: UserId) {

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import io.element.android.libraries.architecture.BaseFlowNode
4646
import io.element.android.libraries.architecture.appyx.rememberDelegateTransitionHandler
4747
import io.element.android.libraries.architecture.createNode
4848
import io.element.android.libraries.architecture.waitForChildAttached
49+
import io.element.android.libraries.architecture.waitForNavTargetAttached
4950
import io.element.android.libraries.core.uri.ensureProtocol
5051
import io.element.android.libraries.deeplink.api.DeeplinkData
5152
import io.element.android.libraries.featureflag.api.FeatureFlagService
@@ -392,6 +393,12 @@ class RootFlowNode(
392393
is PermalinkData.FallbackLink -> Unit
393394
is PermalinkData.RoomEmailInviteLink -> Unit
394395
is PermalinkData.RoomLink -> {
396+
// If there is a thread id, focus on it in the main timeline
397+
val focusedEventId = if (permalinkData.threadId != null) {
398+
permalinkData.threadId?.asEventId()
399+
} else {
400+
permalinkData.eventId
401+
}
395402
attachRoom(
396403
roomIdOrAlias = permalinkData.roomIdOrAlias,
397404
trigger = JoinedRoom.Trigger.MobilePermalink,
@@ -408,6 +415,7 @@ class RootFlowNode(
408415

409416
private suspend fun RoomFlowNode.maybeAttachThread(threadId: ThreadId?, focusedEventId: EventId?) {
410417
if (threadId != null) {
418+
waitForNavTargetAttached { it is RoomFlowNode.NavTarget.JoinedRoom }
411419
attachThread(threadId, focusedEventId)
412420
}
413421
}
@@ -417,11 +425,13 @@ class RootFlowNode(
417425
attachSession(deeplinkData.sessionId).let { loggedInFlowNode ->
418426
when (deeplinkData) {
419427
is DeeplinkData.Root -> Unit // The room list will always be shown, observing FtueState
420-
is DeeplinkData.Room -> loggedInFlowNode.attachRoom(
421-
roomIdOrAlias = deeplinkData.roomId.toRoomIdOrAlias(),
422-
eventId = if (deeplinkData.threadId != null) deeplinkData.threadId?.asEventId() else deeplinkData.eventId,
423-
clearBackstack = true
424-
).maybeAttachThread(deeplinkData.threadId, deeplinkData.eventId)
428+
is DeeplinkData.Room -> {
429+
loggedInFlowNode.attachRoom(
430+
roomIdOrAlias = deeplinkData.roomId.toRoomIdOrAlias(),
431+
eventId = if (deeplinkData.threadId != null) deeplinkData.threadId?.asEventId() else deeplinkData.eventId,
432+
clearBackstack = true,
433+
).maybeAttachThread(deeplinkData.threadId, deeplinkData.eventId)
434+
}
425435
}
426436
}
427437
}

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesFlowNode.kt

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import io.element.android.features.location.api.LocationService
3131
import io.element.android.features.location.api.SendLocationEntryPoint
3232
import io.element.android.features.location.api.ShowLocationEntryPoint
3333
import io.element.android.features.messages.api.MessagesEntryPoint
34+
import io.element.android.features.messages.api.MessagesEntryPointNode
3435
import io.element.android.features.messages.impl.attachments.Attachment
3536
import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewNode
3637
import io.element.android.features.messages.impl.forward.ForwardMessagesNode
@@ -86,10 +87,12 @@ import io.element.android.libraries.textcomposer.mentions.MentionSpanUpdater
8687
import io.element.android.services.analytics.api.AnalyticsService
8788
import io.element.android.services.analyticsproviders.api.trackers.captureInteraction
8889
import kotlinx.collections.immutable.ImmutableList
90+
import kotlinx.coroutines.delay
8991
import kotlinx.coroutines.flow.launchIn
9092
import kotlinx.coroutines.flow.onEach
9193
import kotlinx.coroutines.withContext
9294
import kotlinx.parcelize.Parcelize
95+
import kotlin.time.Duration.Companion.milliseconds
9396

9497
@ContributesNode(RoomScope::class)
9598
@AssistedInject
@@ -171,7 +174,7 @@ class MessagesFlowNode(
171174
data object KnockRequestsList : NavTarget
172175

173176
@Parcelize
174-
data class OpenThread(val threadRootId: ThreadId, val focusedEventId: EventId?) : NavTarget
177+
data class Thread(val threadRootId: ThreadId, val focusedEventId: EventId?) : NavTarget
175178
}
176179

177180
private val callbacks = plugins<MessagesEntryPoint.Callback>()
@@ -283,7 +286,7 @@ class MessagesFlowNode(
283286
}
284287

285288
override fun onOpenThread(threadRootId: ThreadId, focusedEventId: EventId?) {
286-
backstack.push(NavTarget.OpenThread(threadRootId, focusedEventId))
289+
backstack.push(NavTarget.Thread(threadRootId, focusedEventId))
287290
}
288291
}
289292
val inputs = MessagesNode.Inputs(focusedEventId = navTarget.focusedEventId)
@@ -405,7 +408,7 @@ class MessagesFlowNode(
405408
NavTarget.KnockRequestsList -> {
406409
knockRequestsListEntryPoint.createNode(this, buildContext)
407410
}
408-
is NavTarget.OpenThread -> {
411+
is NavTarget.Thread -> {
409412
val inputs = ThreadedMessagesNode.Inputs(
410413
threadRootEventId = navTarget.threadRootId,
411414
focusedEventId = navTarget.focusedEventId,
@@ -470,7 +473,7 @@ class MessagesFlowNode(
470473
}
471474

472475
override fun onOpenThread(threadRootId: ThreadId, focusedEventId: EventId?) {
473-
backstack.push(NavTarget.OpenThread(threadRootId, focusedEventId))
476+
backstack.push(NavTarget.Thread(threadRootId, focusedEventId))
474477
}
475478
}
476479
createNode<ThreadedMessagesNode>(buildContext, listOf(inputs, callback))
@@ -587,7 +590,7 @@ class MessagesFlowNode(
587590
override suspend fun attachThread(threadId: ThreadId, focusedEventId: EventId?) {
588591
// Wait until we have the UI for the main timeline attached
589592
waitForChildAttached<MessagesNode>()
590-
// Give enough time for the items in the main timeline to be received, otherwise loading the focused thread root id won't work
593+
// Give some time for the items in the main timeline to be received, otherwise loading the focused thread root id won't work
591594
// (look at TimelineItemIndexer and firstProcessLatch for more info)
592595
delay(10.milliseconds)
593596
// Then push the new threads screen on top

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

Lines changed: 72 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ class TimelinePresenter(
8888
private val markAsFullyRead: MarkAsFullyRead,
8989
private val featureFlagService: FeatureFlagService,
9090
) : Presenter<TimelineState> {
91+
private val tag = "TimelinePresenter"
9192
@AssistedFactory
9293
interface Factory {
9394
fun create(
@@ -104,14 +105,14 @@ class TimelinePresenter(
104105
)
105106
private var timelineItems by mutableStateOf<ImmutableList<TimelineItem>>(persistentListOf())
106107

108+
private val focusRequestState: MutableState<FocusRequestState> = mutableStateOf(FocusRequestState.None)
109+
107110
@Composable
108111
override fun present(): TimelineState {
109112
val localScope = rememberCoroutineScope()
110113

111114
val timelineMode = remember { timelineController.mainTimelineMode() }
112115

113-
var focusRequestState: FocusRequestState by remember { mutableStateOf(FocusRequestState.None) }
114-
115116
val lastReadReceiptId = rememberSaveable { mutableStateOf<EventId?>(null) }
116117

117118
val roomInfo by room.roomInfoFlow.collectAsState()
@@ -157,7 +158,7 @@ class TimelinePresenter(
157158
if (event.firstIndex == 0) {
158159
newEventState.value = NewEventState.None
159160
}
160-
Timber.d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
161+
Timber.tag(tag).d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
161162
sessionCoroutineScope.sendReadReceiptIfNeeded(
162163
firstVisibleIndex = event.firstIndex,
163164
timelineItems = timelineItems,
@@ -188,14 +189,17 @@ class TimelinePresenter(
188189
is TimelineEvents.EditPoll -> {
189190
navigator.onEditPollClick(event.pollStartId)
190191
}
191-
is TimelineEvents.FocusOnEvent -> {
192-
focusRequestState = FocusRequestState.Requested(event.eventId, event.debounce)
193-
}
192+
is TimelineEvents.FocusOnEvent -> sessionCoroutineScope.launch {
193+
focusRequestState.value = FocusRequestState.Requested(event.eventId, event.debounce)
194+
delay(event.debounce)
195+
Timber.tag(tag).d("Started focus on ${event.eventId}")
196+
focusOnEvent(event.eventId, focusRequestState)
197+
}.start()
194198
is TimelineEvents.OnFocusEventRender -> {
195-
focusRequestState = focusRequestState.onFocusEventRender()
199+
focusRequestState.value = focusRequestState.value.onFocusEventRender()
196200
}
197201
is TimelineEvents.ClearFocusRequestState -> {
198-
focusRequestState = FocusRequestState.None
202+
focusRequestState.value = FocusRequestState.None
199203
}
200204
is TimelineEvents.JumpToLive -> {
201205
timelineController.focusOnLive()
@@ -244,69 +248,19 @@ class TimelinePresenter(
244248
.launchIn(this)
245249
}
246250

247-
LaunchedEffect(focusRequestState) {
248-
Timber.d("## focusRequestState: $focusRequestState")
249-
when (val currentFocusRequestState = focusRequestState) {
250-
is FocusRequestState.Requested -> {
251-
delay(currentFocusRequestState.debounce)
252-
if (timelineItemIndexer.isKnown(currentFocusRequestState.eventId)) {
253-
val index = timelineItemIndexer.indexOf(currentFocusRequestState.eventId)
254-
focusRequestState = FocusRequestState.Success(eventId = currentFocusRequestState.eventId, index = index)
255-
} else {
256-
focusRequestState = FocusRequestState.Loading(eventId = currentFocusRequestState.eventId)
257-
}
258-
}
259-
is FocusRequestState.Loading -> {
260-
val eventId = currentFocusRequestState.eventId
261-
val threadId = room.threadRootIdForEvent(eventId).getOrElse {
262-
focusRequestState = FocusRequestState.Failure(it)
263-
return@LaunchedEffect
264-
}
265-
266-
if (timelineController.mainTimelineMode() is Timeline.Mode.Thread && threadId == null) {
267-
// We are in a thread timeline, and the event isn't part of a thread, we need to navigate back to the room
268-
focusRequestState = FocusRequestState.None
269-
navigator.onNavigateToRoom(room.roomId, eventId, calculateServerNamesForRoom(room))
270-
} else {
271-
timelineController.focusOnEvent(eventId, threadId)
272-
.onSuccess { result ->
273-
when (result) {
274-
is EventFocusResult.FocusedOnLive -> {
275-
focusRequestState = FocusRequestState.Success(eventId = eventId)
276-
}
277-
is EventFocusResult.IsInThread -> {
278-
val currentThreadId = (timelineController.mainTimelineMode() as? Timeline.Mode.Thread)?.threadRootId
279-
if (currentThreadId == result.threadId) {
280-
// It's the same thread, we just focus on the event
281-
focusRequestState = FocusRequestState.Success(eventId = eventId)
282-
} else {
283-
focusRequestState = FocusRequestState.Success(eventId = result.threadId.asEventId())
284-
// It's part of a thread we're not in, let's open it in another timeline
285-
navigator.onOpenThread(result.threadId, eventId)
286-
}
287-
}
288-
}
289-
}
290-
.onFailure {
291-
focusRequestState = FocusRequestState.Failure(it)
292-
}
293-
}
294-
}
295-
else -> Unit
296-
}
297-
}
298-
299251
LaunchedEffect(timelineItems.size) {
300252
computeNewItemState(timelineItems, prevMostRecentItemId, newEventState)
301253
}
302254

303-
LaunchedEffect(timelineItems.size, focusRequestState) {
304-
val currentFocusRequestState = focusRequestState
255+
LaunchedEffect(timelineItems.size, focusRequestState.value) {
256+
val currentFocusRequestState = focusRequestState.value
305257
if (currentFocusRequestState is FocusRequestState.Success && !currentFocusRequestState.rendered) {
306258
val eventId = currentFocusRequestState.eventId
307259
if (timelineItemIndexer.isKnown(eventId)) {
308260
val index = timelineItemIndexer.indexOf(eventId)
309-
focusRequestState = FocusRequestState.Success(eventId = eventId, index = index)
261+
focusRequestState.value = FocusRequestState.Success(eventId = eventId, index = index)
262+
} else {
263+
Timber.w("Unknown timeline item for focused item, can't render focus")
310264
}
311265
}
312266
}
@@ -327,21 +281,75 @@ class TimelinePresenter(
327281
)
328282
}
329283
}
284+
285+
LaunchedEffect(focusRequestState.value) {
286+
Timber.tag(tag).d("Timeline: $timelineMode | focus state: ${focusRequestState.value}")
287+
}
288+
330289
return TimelineState(
331290
timelineItems = timelineItems,
332291
timelineMode = timelineMode,
333292
timelineRoomInfo = timelineRoomInfo,
334293
renderReadReceipts = renderReadReceipts,
335294
newEventState = newEventState.value,
336295
isLive = isLive,
337-
focusRequestState = focusRequestState,
296+
focusRequestState = focusRequestState.value,
338297
messageShield = messageShield.value,
339298
resolveVerifiedUserSendFailureState = resolveVerifiedUserSendFailureState,
340299
displayThreadSummaries = displayThreadSummaries,
341300
eventSink = { handleEvents(it) }
342301
)
343302
}
344303

304+
private suspend fun focusOnEvent(
305+
eventId: EventId,
306+
focusRequestState: MutableState<FocusRequestState>,
307+
) {
308+
if (timelineItemIndexer.isKnown(eventId)) {
309+
val index = timelineItemIndexer.indexOf(eventId)
310+
focusRequestState.value = FocusRequestState.Success(eventId = eventId, index = index)
311+
return
312+
}
313+
314+
Timber.tag(tag).d("Event $eventId not found in the loaded timeline, loading a focused timeline")
315+
focusRequestState.value = FocusRequestState.Loading(eventId = eventId)
316+
317+
val threadId = room.threadRootIdForEvent(eventId).getOrElse {
318+
focusRequestState.value = FocusRequestState.Failure(it)
319+
return
320+
}
321+
322+
if (timelineController.mainTimelineMode() is Timeline.Mode.Thread && threadId == null) {
323+
// We are in a thread timeline, and the event isn't part of a thread, we need to navigate back to the room
324+
focusRequestState.value = FocusRequestState.None
325+
navigator.onNavigateToRoom(room.roomId, eventId, calculateServerNamesForRoom(room))
326+
} else {
327+
Timber.tag(tag).d("Focusing on event $eventId - thread $threadId")
328+
timelineController.focusOnEvent(eventId, threadId)
329+
.onSuccess { result ->
330+
when (result) {
331+
is EventFocusResult.FocusedOnLive -> {
332+
focusRequestState.value = FocusRequestState.Success(eventId = eventId)
333+
}
334+
is EventFocusResult.IsInThread -> {
335+
val currentThreadId = (timelineController.mainTimelineMode() as? Timeline.Mode.Thread)?.threadRootId
336+
if (currentThreadId == result.threadId) {
337+
// It's the same thread, we just focus on the event
338+
focusRequestState.value = FocusRequestState.Success(eventId = eventId)
339+
} else {
340+
focusRequestState.value = FocusRequestState.Success(eventId = result.threadId.asEventId())
341+
// It's part of a thread we're not in, let's open it in another timeline
342+
navigator.onOpenThread(result.threadId, eventId)
343+
}
344+
}
345+
}
346+
}
347+
.onFailure {
348+
focusRequestState.value = FocusRequestState.Failure(it)
349+
}
350+
}
351+
}
352+
345353
/**
346354
* This method compute the hasNewItem state passed as a [MutableState] each time the timeline items size changes.
347355
* Basically, if we got new timeline event from sync or local, either from us or another user, we update the state so we tell we have new items.

0 commit comments

Comments
 (0)