Skip to content

Fix read receipt behavior when the timeline is opened. #4679

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 6, 2025
Merged

Fix read receipt behavior when the timeline is opened. #4679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59f80e5
Use Timber instead of println
bmarty Apr 30, 2025
e74f8bc
Fix ReadReceipt not sent when new event is received:
bmarty May 5, 2025
42c6c46
Improve the fix to fix regression detected by unit tests.
bmarty May 6, 2025
649ee70
Remove unnecessary parenthesis
bmarty May 6, 2025
522aea3
Document parameter.
bmarty May 6, 2025
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
8 changes: 6 additions & 2 deletions ...l/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt
View file
Open in desktop
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.TimelineItem
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.timeline.model.virtual.TimelineItemTypingNotificationModel
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
Expand Down Expand Up @@ -131,7 +132,7 @@ class TimelinePresenter @AssistedInject constructor(
if (event.firstIndex == 0) {
newEventState.value = NewEventState.None
}
println("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
Timber.d("## sendReadReceiptIfNeeded firstVisibleIndex: ${event.firstIndex}")
appScope.sendReadReceiptIfNeeded(
firstVisibleIndex = event.firstIndex,
timelineItems = timelineItems,
Expand Down Expand Up @@ -278,7 +279,10 @@ class TimelinePresenter @AssistedInject constructor(
if (newEventState.value == NewEventState.FromMe) {
return@withContext
}
val newMostRecentItem = timelineItems.firstOrNull()
val newMostRecentItem = timelineItems.firstOrNull {
// Ignore typing item
(it as? TimelineItem.Virtual)?.model !is TimelineItemTypingNotificationModel
}
Comment on lines +282 to +285
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good catch 👌 .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I could also have dropped the first item, but I guess it's more accurate like this.

val prevMostRecentItemIdValue = prevMostRecentItemId.value
val newMostRecentItemId = newMostRecentItem?.identifier()
val hasNewEvent = prevMostRecentItemIdValue != null &&
Expand Down
16 changes: 11 additions & 5 deletions ...s/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,19 +286,24 @@ private fun BoxScope.TimelineScrollHelper(
}
var jumpToLiveHandled by remember { mutableStateOf(true) }

fun scrollToBottom() {
/**
* @param force If true, scroll to the bottom even if the user is already seeing the most recent item.
* This fixes the issue where the user is seeing typing notification and so the read receipt is not sent
* when a new message comes in.
*/
fun scrollToBottom(force: Boolean) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment for why the force param is needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

522aea3

coroutineScope.launch {
if (lazyListState.firstVisibleItemIndex > 10) {
lazyListState.scrollToItem(0)
} else if (lazyListState.firstVisibleItemIndex != 0) {
} else if (force || lazyListState.firstVisibleItemIndex != 0) {
lazyListState.animateScrollToItem(0)
}
}
}

fun jumpToBottom() {
if (isLive) {
scrollToBottom()
scrollToBottom(force = false)
} else {
jumpToLiveHandled = false
onJumpToLive()
Expand All @@ -321,9 +326,10 @@ private fun BoxScope.TimelineScrollHelper(
}

LaunchedEffect(canAutoScroll, newEventState) {
val shouldScrollToBottom = isScrollFinished && (canAutoScroll || newEventState == NewEventState.FromMe)
val shouldScrollToBottom = isScrollFinished &&
(canAutoScroll && newEventState == NewEventState.FromOther || newEventState == NewEventState.FromMe)
if (shouldScrollToBottom) {
scrollToBottom()
scrollToBottom(force = true)
}
}

Expand Down
Loading