From 8efd389a3cf5f2686331fafea171a35fcd53fce5 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Wed, 9 Mar 2022 13:40:05 +0100 Subject: [PATCH 1/4] Fix a case of missing read markers Case: bottom-most loaded event has read marker, but there are messages below it that haven't been loaded yet. --- .../app/features/home/room/detail/TimelineViewModel.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 3bdcbc6529..07c25ce326 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -1120,6 +1120,11 @@ class TimelineViewModel @AssistedInject constructor( } else { UnreadState.Unknown } + // If the read marker is at the bottom-most event, this doesn't mean we read all, in case we just haven't loaded more events. + // Avoid incorrectly returning HasNoUnread in this case. + if (firstDisplayableEventIndex == 0 && timeline.hasMoreToLoad(Timeline.Direction.FORWARDS)) { + return UnreadState.Unknown + } for (i in (firstDisplayableEventIndex - 1) downTo 0) { val timelineEvent = events.getOrNull(i) ?: return UnreadState.Unknown val eventId = timelineEvent.root.eventId ?: return UnreadState.Unknown From 1206c31e3ab01501b02953b28140bd25c65017ee Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Wed, 9 Mar 2022 13:43:52 +0100 Subject: [PATCH 2/4] Fix another case of missing read markers HasUnread might not be correct on the first try while loading the timeline. --- .../vector/app/features/home/room/detail/TimelineViewModel.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 07c25ce326..78e3469a58 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -1099,9 +1099,11 @@ class TimelineViewModel @AssistedInject constructor( computeUnreadState(timelineEvents, roomSummary) } // We don't want live update of unread so we skip when we already had a HasUnread or HasNoUnread + // However, we want to update an existing HasUnread, as we might get additional information during loading of events. .distinctUntilChanged { previous, current -> when { previous is UnreadState.Unknown || previous is UnreadState.ReadMarkerNotLoaded -> false + previous is UnreadState.HasUnread && current is UnreadState.HasUnread -> false current is UnreadState.HasUnread || current is UnreadState.HasNoUnread -> true else -> false } From 884ae1cedd8144e470a1055019d55a4c9eeb7c85 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Wed, 9 Mar 2022 13:50:49 +0100 Subject: [PATCH 3/4] Add changelog.d/5475.bugfix --- changelog.d/5475.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5475.bugfix diff --git a/changelog.d/5475.bugfix b/changelog.d/5475.bugfix new file mode 100644 index 0000000000..03364f6a73 --- /dev/null +++ b/changelog.d/5475.bugfix @@ -0,0 +1 @@ +Fix some cases where the read marker line would not show up From 47dddd706c660c071c8b55e29f2aeb613ef752f3 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Mon, 21 Mar 2022 11:23:03 +0100 Subject: [PATCH 4/4] Only show HasUnread -> HasUnread updates for same readMarker --- .../app/features/home/room/detail/RoomDetailViewState.kt | 2 +- .../app/features/home/room/detail/TimelineViewModel.kt | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt index e2b97b0900..5d545ef4b5 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt @@ -36,7 +36,7 @@ sealed class UnreadState { object Unknown : UnreadState() object HasNoUnread : UnreadState() data class ReadMarkerNotLoaded(val readMarkerId: String) : UnreadState() - data class HasUnread(val firstUnreadEventId: String) : UnreadState() + data class HasUnread(val firstUnreadEventId: String, val readMarkerId: String) : UnreadState() } data class JitsiState( diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 78e3469a58..4caad1bb5e 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -1099,11 +1099,13 @@ class TimelineViewModel @AssistedInject constructor( computeUnreadState(timelineEvents, roomSummary) } // We don't want live update of unread so we skip when we already had a HasUnread or HasNoUnread - // However, we want to update an existing HasUnread, as we might get additional information during loading of events. + // However, we want to update an existing HasUnread, if the readMarkerId hasn't changed, + // as we might be loading new events to fill gaps in the timeline. .distinctUntilChanged { previous, current -> when { previous is UnreadState.Unknown || previous is UnreadState.ReadMarkerNotLoaded -> false - previous is UnreadState.HasUnread && current is UnreadState.HasUnread -> false + previous is UnreadState.HasUnread && current is UnreadState.HasUnread && + previous.readMarkerId == current.readMarkerId -> false current is UnreadState.HasUnread || current is UnreadState.HasNoUnread -> true else -> false } @@ -1132,7 +1134,7 @@ class TimelineViewModel @AssistedInject constructor( val eventId = timelineEvent.root.eventId ?: return UnreadState.Unknown val isFromMe = timelineEvent.root.senderId == session.myUserId if (!isFromMe) { - return UnreadState.HasUnread(eventId) + return UnreadState.HasUnread(eventId, readMarkerIdSnapshot) } } return UnreadState.HasNoUnread