diff --git a/Riot/Modules/Room/MXKRoomViewController.m b/Riot/Modules/Room/MXKRoomViewController.m index 8d0d2808a..7c014e018 100644 --- a/Riot/Modules/Room/MXKRoomViewController.m +++ b/Riot/Modules/Room/MXKRoomViewController.m @@ -44,6 +44,9 @@ #import "MXKPreviewViewController.h" +// Constant used to determine whether an event is visible at the bottom of the tableview, based on its visible height +static const CGFloat kCellVisibilityMinimumHeight = 8.0; + @interface MXKRoomViewController () { /** @@ -2419,7 +2422,7 @@ contentBottomOffsetY = _bubblesTableView.contentSize.height; } // Be a bit less retrictive, consider visible an event at the bottom even if is partially hidden. - contentBottomOffsetY += 8; + contentBottomOffsetY += kCellVisibilityMinimumHeight; // Reset the current event id currentEventIdAtTableBottom = nil; @@ -2489,24 +2492,26 @@ if (acknowledge && self.isEventsAcknowledgementEnabled) { // Indicate to the homeserver that the user has read this event. - - // Check whether the read marker must be updated. - BOOL updateReadMarker = _updateRoomReadMarker; - if (updateReadMarker && roomDataSource.room.accountData.readMarkerEventId) - { - MXEvent *currentReadMarkerEvent = [roomDataSource.mxSession.store eventWithEventId:roomDataSource.room.accountData.readMarkerEventId inRoom:roomDataSource.roomId]; - if (!currentReadMarkerEvent) - { - currentReadMarkerEvent = [roomDataSource eventWithEventId:roomDataSource.room.accountData.readMarkerEventId]; - } - - // Update the read marker only if the current event is available, and the new event is posterior to it. - updateReadMarker = (currentReadMarkerEvent && (currentReadMarkerEvent.originServerTs <= component.event.originServerTs)); - } - if (self.navigationController.viewControllers.lastObject == self) { - [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:updateReadMarker]; + // Check if the selected event is eligible to be the new read marker position too + if (!bubbleData.collapsed && [self eligibleForReadMarkerUpdate:component.event]) + { + BOOL updateRoomReadMarker = _updateRoomReadMarker && [self isEventPosteriorToCurrentReadMarker:component.event]; + // Acknowledge this event and update the read marker if needed + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:updateRoomReadMarker]; + } + else + { + // Acknowledge only this event. The read marker is handled separately + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:NO]; + + if (_updateRoomReadMarker) + { + // Try to find the best event for the new read marker position + [self updateReadMarkerEvent]; + } + } } } break; @@ -2517,6 +2522,118 @@ } } +- (BOOL)eligibleForReadMarkerUpdate:(MXEvent *)event { + // Prevent the readmarker to be placed on a relatesTo or a redaction event + if (event.relatesTo || event.redacts) + { + return NO; + } + + return YES; +} + +- (BOOL)isEventPosteriorToCurrentReadMarker:(MXEvent *)event { + if (roomDataSource.room.accountData.readMarkerEventId) + { + MXEvent *currentReadMarkerEvent = [roomDataSource.mxSession.store eventWithEventId:roomDataSource.room.accountData.readMarkerEventId inRoom:roomDataSource.roomId]; + if (!currentReadMarkerEvent) + { + currentReadMarkerEvent = [roomDataSource eventWithEventId:roomDataSource.room.accountData.readMarkerEventId]; + } + + // Update the read marker only if the current event is available, and the new event is posterior to it. + return currentReadMarkerEvent && (currentReadMarkerEvent.originServerTs <= event.originServerTs); + } + return YES; +} + +/// Try to update the read marker by looking for an eligible event displayed at the bottom of the tableview +- (void)updateReadMarkerEvent +{ + // Compute the content offset corresponding to the line displayed at the table bottom (just above the toolbar). + CGFloat contentBottomOffsetY = _bubblesTableView.contentOffset.y + (_bubblesTableView.frame.size.height - _bubblesTableView.adjustedContentInset.bottom); + if (contentBottomOffsetY > _bubblesTableView.contentSize.height) + { + contentBottomOffsetY = _bubblesTableView.contentSize.height; + } + // Be a bit less retrictive, consider visible an event at the bottom even if is partially hidden. + contentBottomOffsetY += kCellVisibilityMinimumHeight; + + // Consider the visible cells (starting by those displayed at the bottom) + NSArray *visibleCells = [_bubblesTableView visibleCells]; + NSInteger index = visibleCells.count; + UITableViewCell *cell; + while (index--) + { + cell = visibleCells[index]; + + // Check whether the cell is actually visible + if (!cell || cell.frame.origin.y > contentBottomOffsetY) + { + continue; + } + + if (![cell isKindOfClass:MXKRoomBubbleTableViewCell.class]) + { + continue; + } + MXKRoomBubbleTableViewCell *roomBubbleTableViewCell = (MXKRoomBubbleTableViewCell *)cell; + MXKRoomBubbleCellData *bubbleData = roomBubbleTableViewCell.bubbleData; + if (!bubbleData) + { + continue; + } + + // Prevent to place the read marker on a collapsed cell + if (bubbleData.collapsed) + { + continue; + } + + // Check which bubble component is displayed at the bottom. + // For that update each component position. + [bubbleData prepareBubbleComponentsPosition]; + + NSArray *bubbleComponents = bubbleData.bubbleComponents; + NSInteger componentIndex = bubbleComponents.count; + + CGFloat bottomPositionY = cell.frame.size.height; + + MXKRoomBubbleComponent *component; + + while (componentIndex --) + { + component = bubbleComponents[componentIndex]; + + // Prevent the read marker to be placed on an unsupported event (e.g. redactions, reactions, ...) + if (![self eligibleForReadMarkerUpdate:component.event]) + { + continue; + } + + // Check whether the bottom part of the component is visible. + CGFloat pos = cell.frame.origin.y + bottomPositionY; + if (pos <= contentBottomOffsetY) + { + // We found the component + // Check whether the read marker must be updated. + if ([self isEventPosteriorToCurrentReadMarker:component.event]) + { + // Move the read marker to this event + [roomDataSource.room moveReadMarkerToEventId:component.event.eventId]; + } + + return; + } + + // Prepare the bottom position for the next component + bottomPositionY = roomBubbleTableViewCell.msgTextViewTopConstraint.constant + component.position.y; + } + + // else we consider the previous cell. + } +} + #pragma mark - MXKDataSourceDelegate - (Class)cellViewClassForCellData:(MXKCellData*)cellData diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 3819a7a49..996670081 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -5323,7 +5323,7 @@ static CGSize kThreadListBarButtonItemImageSize; [self dismissKeyboard]; NSString *eventId = self.roomDataSource.room.accountData.readMarkerEventId; NSString *threadId = self.roomDataSource.threadId; - [self reloadRoomWihtEventId:eventId threadId:threadId]; + [self reloadRoomWihtEventId:eventId threadId:threadId forceUpdateRoomMarker:YES]; } else if (sender == self.resetReadMarkerButton) { @@ -6612,8 +6612,12 @@ static CGSize kThreadListBarButtonItemImageSize; // Check whether the read marker exists and has not been rendered yet. if (self.roomDataSource.isLive && !self.roomDataSource.isPeeking && self.roomDataSource.showReadMarker && self.roomDataSource.room.accountData.readMarkerEventId) { - UITableViewCell *cell = [self.bubblesTableView visibleCells].firstObject; - if ([cell isKindOfClass:MXKRoomBubbleTableViewCell.class] && ![cell isKindOfClass:MXKRoomEmptyBubbleTableViewCell.class]) + NSPredicate *predicate = [NSPredicate predicateWithBlock:^BOOL(id evaluatedObject, NSDictionary *bindings) { + return [evaluatedObject isKindOfClass:MXKRoomBubbleTableViewCell.class]; + }]; + NSArray *visibleCells = [[self.bubblesTableView visibleCells] filteredArrayUsingPredicate:predicate]; + UITableViewCell *cell = visibleCells.firstObject; + if (cell) { MXKRoomBubbleTableViewCell *roomBubbleTableViewCell = (MXKRoomBubbleTableViewCell*)cell; // Check whether the read marker is inside the first displayed cell. @@ -6640,6 +6644,9 @@ static CGSize kThreadListBarButtonItemImageSize; else { self.jumpToLastUnreadBannerContainer.hidden = YES; + + // Force the read marker position in order to not depend on the read marker animation (https://github.com/vector-im/element-ios/issues/7420) + self.updateRoomReadMarker = YES; } } } @@ -7919,15 +7926,17 @@ static CGSize kThreadListBarButtonItemImageSize; [[AppDelegate theDelegate] showRoomWithParameters:parameters]; } } + - (void)roomInfoCoordinatorBridgePresenter:(RoomInfoCoordinatorBridgePresenter *)coordinator viewEventInTimeline:(MXEvent *)event { [self.navigationController popToViewController:self animated:true]; - [self reloadRoomWihtEventId:event.eventId threadId:event.threadId]; + [self reloadRoomWihtEventId:event.eventId threadId:event.threadId forceUpdateRoomMarker:NO]; } -(void)reloadRoomWihtEventId:(NSString *)eventId threadId:(NSString *)threadId + forceUpdateRoomMarker:(BOOL)forceUpdateRoomMarker { // Jump to the last unread event by using a temporary room data source initialized with the last unread event id. MXWeakify(self); @@ -7946,6 +7955,9 @@ static CGSize kThreadListBarButtonItemImageSize; // Give the data source ownership to the room view controller. self.hasRoomDataSourceOwnership = YES; + + // Force the read marker update if needed (e.g if we jumped on the last unread message using the banner). + self.updateRoomReadMarker |= forceUpdateRoomMarker; }]; } diff --git a/changelog.d/7420.bugfix b/changelog.d/7420.bugfix new file mode 100644 index 000000000..5238e7dc9 --- /dev/null +++ b/changelog.d/7420.bugfix @@ -0,0 +1 @@ +Update the read marker position even if it is not displayed