From 694b60b71280ba0daf53f77cd3e8d4725418fa34 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Mon, 20 Mar 2023 17:45:10 +0100 Subject: [PATCH 1/5] Fix: Update the read marker position even if it is not displayed --- Riot/Modules/Room/MXKRoomViewController.m | 150 +++++++++++++++++++--- Riot/Modules/Room/RoomViewController.m | 41 +++++- changelog.d/7420.bugfix | 1 + 3 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 changelog.d/7420.bugfix diff --git a/Riot/Modules/Room/MXKRoomViewController.m b/Riot/Modules/Room/MXKRoomViewController.m index 8d0d2808a..90e1d0233 100644 --- a/Riot/Modules/Room/MXKRoomViewController.m +++ b/Riot/Modules/Room/MXKRoomViewController.m @@ -2489,24 +2489,9 @@ 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]; + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:NO]; } } break; @@ -2515,6 +2500,139 @@ } } } + + [self updateReadMarkerEventIdAtTableBottom]; +} + +- (void)updateReadMarkerEventIdAtTableBottom +{ + if (!_updateRoomReadMarker) + { + return; + } + + // Do not update events if the controller is used as context menu preview. + if (self.isContextPreview) + { + return; + } + + // Update the identifier of the event displayed at the bottom of the table, except if a rotation or other size transition is in progress. + if (isSizeTransitionInProgress || self.isBubbleTableViewDisplayInTransition) + { + return; + } + + // 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 += 8; + + // Reset the current event id + currentEventIdAtTableBottom = nil; + + // 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)) + { + if (![cell isKindOfClass:MXKTableViewCell.class]) + { + continue; + } + + MXKCellData *cellData = ((MXKTableViewCell *)cell).mxkCellData; + + // Only 'MXKRoomBubbleCellData' is supported here for the moment. + if (![cellData isKindOfClass:MXKRoomBubbleCellData.class]) + { + continue; + } + + MXKRoomBubbleCellData *bubbleData = (MXKRoomBubbleCellData*)cellData; + + // 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]; + if (![cell isKindOfClass:MXKRoomBubbleTableViewCell.class]) + { + continue; + } + + // Prevent the readmarker to be placed on a relatesTo or a redaction event + if (component.event.relatesTo || component.event.redacts) + { + continue; + } + + MXKRoomBubbleTableViewCell *roomBubbleTableViewCell = (MXKRoomBubbleTableViewCell *)cell; + + // Check whether the bottom part of the component is visible. + CGFloat pos = cell.frame.origin.y + bottomPositionY; + if (pos <= contentBottomOffsetY) + { + // We found the component + currentEventIdAtTableBottom = component.event.eventId; + break; + } + + // Prepare the bottom position for the next component + bottomPositionY = roomBubbleTableViewCell.msgTextViewTopConstraint.constant + component.position.y; + } + + if (currentEventIdAtTableBottom) + { + // Check whether the read marker must be updated. + BOOL updateReadMarker = YES; + 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. + updateReadMarker = currentReadMarkerEvent && (currentReadMarkerEvent.eventId != component.event.eventId) && (currentReadMarkerEvent.originServerTs <= component.event.originServerTs); + } + + if (updateReadMarker && self.navigationController.viewControllers.lastObject == self) + { + [roomDataSource.room moveReadMarkerToEventId:component.event.eventId]; + } + + break; + } + // else we consider the previous cell. + } + } } #pragma mark - MXKDataSourceDelegate diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 3819a7a49..65303d7af 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -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. @@ -6654,6 +6658,39 @@ static CGSize kThreadListBarButtonItemImageSize; // Move the read marker to the current read receipt position by default. [self.roomDataSource.room forgetReadMarker]; } + // Force the update of the read marker if we try to display it (in some cases, we are not able to setup the readMarkerView which prevent us to update the read marker) + else if (!self.roomDataSource.isLive && !self.roomDataSource.isPeeking && self.roomDataSource.showReadMarker && self.roomDataSource.room.accountData.readMarkerEventId && !self.updateRoomReadMarker) + { + // Check if the readmarker's event is in a visible cell + MXEvent *currentReadMarkerEvent = [self.roomDataSource.mxSession.store eventWithEventId:self.roomDataSource.room.accountData.readMarkerEventId inRoom:self.roomDataSource.roomId]; + if (currentReadMarkerEvent) + { + NSPredicate *predicate = [NSPredicate predicateWithBlock:^BOOL(id evaluatedObject, NSDictionary *bindings) { + return [evaluatedObject isKindOfClass:MXKRoomBubbleTableViewCell.class]; + }]; + NSArray *visibleCells = [[self.bubblesTableView visibleCells] filteredArrayUsingPredicate:predicate]; + UITableViewCell *firstCell = visibleCells.firstObject; + UITableViewCell *lastCell = visibleCells.lastObject; + if (firstCell && lastCell) + { + // Get the first displayed event + MXKRoomBubbleTableViewCell *firstRoomBubbleTableViewCell = (MXKRoomBubbleTableViewCell*)firstCell; + MXKRoomBubbleComponent *component = firstRoomBubbleTableViewCell.bubbleData.bubbleComponents.firstObject; + MXEvent *firstDisplayedEvent = component.event; + + // Get the last displayed event + MXKRoomBubbleTableViewCell *lastRoomBubbleTableViewCell = (MXKRoomBubbleTableViewCell*)lastCell; + MXKRoomBubbleComponent *lastComponent = lastRoomBubbleTableViewCell.bubbleData.bubbleComponents.lastObject; + MXEvent *lastDisplayedEvent = lastComponent.event; + + // If the currentReadMarkerEvent is between the first and the last displayed event, force the update of the room read marker to prevent it from being blocked + if (currentReadMarkerEvent.originServerTs > firstDisplayedEvent.originServerTs && currentReadMarkerEvent.originServerTs < lastDisplayedEvent.originServerTs) + { + self.updateRoomReadMarker = YES; + } + } + } + } } } 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 From b5fb7624fd16262337938c83947884edb40f1834 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Wed, 29 Mar 2023 16:06:33 +0200 Subject: [PATCH 2/5] Fix: read marker update --- Riot/Modules/Room/MXKRoomViewController.m | 101 +++++++++++----------- Riot/Modules/Room/RoomViewController.m | 45 +++------- 2 files changed, 60 insertions(+), 86 deletions(-) diff --git a/Riot/Modules/Room/MXKRoomViewController.m b/Riot/Modules/Room/MXKRoomViewController.m index 90e1d0233..9be6b8e3e 100644 --- a/Riot/Modules/Room/MXKRoomViewController.m +++ b/Riot/Modules/Room/MXKRoomViewController.m @@ -2491,7 +2491,22 @@ // Indicate to the homeserver that the user has read this event. if (self.navigationController.viewControllers.lastObject == self) { - [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:NO]; + // If the current event is eligible to be the new read marker position + if (!bubbleData.collapsed && [self eligibleForReadMarkerUpdate:component.event checkTimeStamp:YES]) + { + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:YES]; + } + else + { + // Otherwise, only acknonwledge this event without updating the read marker + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:NO]; + + if (_updateRoomReadMarker) + { + // Try to find the best event for the new read marker position + [self updateReadMarkerEvent]; + } + } } } break; @@ -2500,29 +2515,33 @@ } } } - - [self updateReadMarkerEventIdAtTableBottom]; } -- (void)updateReadMarkerEventIdAtTableBottom +- (BOOL)eligibleForReadMarkerUpdate:(MXEvent *)event checkTimeStamp:(BOOL)checkTimeStamp { + // Prevent the readmarker to be placed on a relatesTo or a redaction event + if (event.relatesTo || event.redacts) + { + return NO; + } + + if (checkTimeStamp && 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.eventId != event.eventId) && (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 { - if (!_updateRoomReadMarker) - { - return; - } - - // Do not update events if the controller is used as context menu preview. - if (self.isContextPreview) - { - return; - } - - // Update the identifier of the event displayed at the bottom of the table, except if a rotation or other size transition is in progress. - if (isSizeTransitionInProgress || self.isBubbleTableViewDisplayInTransition) - { - return; - } - // 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) @@ -2532,9 +2551,6 @@ // Be a bit less retrictive, consider visible an event at the bottom even if is partially hidden. contentBottomOffsetY += 8; - // Reset the current event id - currentEventIdAtTableBottom = nil; - // Consider the visible cells (starting by those displayed at the bottom) NSArray *visibleCells = [_bubblesTableView visibleCells]; NSInteger index = visibleCells.count; @@ -2586,8 +2602,8 @@ continue; } - // Prevent the readmarker to be placed on a relatesTo or a redaction event - if (component.event.relatesTo || component.event.redacts) + // Prevent the read marker to be placed on an unsupported event (e.g. redactions, reactions, ...) + if (![self eligibleForReadMarkerUpdate:component.event checkTimeStamp:NO]) { continue; } @@ -2599,37 +2615,20 @@ if (pos <= contentBottomOffsetY) { // We found the component - currentEventIdAtTableBottom = component.event.eventId; - break; + // Check whether the read marker must be updated. + if ([self eligibleForReadMarkerUpdate:component.event checkTimeStamp:YES]) + { + // 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; } - if (currentEventIdAtTableBottom) - { - // Check whether the read marker must be updated. - BOOL updateReadMarker = YES; - 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. - updateReadMarker = currentReadMarkerEvent && (currentReadMarkerEvent.eventId != component.event.eventId) && (currentReadMarkerEvent.originServerTs <= component.event.originServerTs); - } - - if (updateReadMarker && self.navigationController.viewControllers.lastObject == self) - { - [roomDataSource.room moveReadMarkerToEventId:component.event.eventId]; - } - - break; - } // else we consider the previous cell. } } diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index 65303d7af..d5b093763 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) { @@ -6644,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; } } } @@ -6658,39 +6661,6 @@ static CGSize kThreadListBarButtonItemImageSize; // Move the read marker to the current read receipt position by default. [self.roomDataSource.room forgetReadMarker]; } - // Force the update of the read marker if we try to display it (in some cases, we are not able to setup the readMarkerView which prevent us to update the read marker) - else if (!self.roomDataSource.isLive && !self.roomDataSource.isPeeking && self.roomDataSource.showReadMarker && self.roomDataSource.room.accountData.readMarkerEventId && !self.updateRoomReadMarker) - { - // Check if the readmarker's event is in a visible cell - MXEvent *currentReadMarkerEvent = [self.roomDataSource.mxSession.store eventWithEventId:self.roomDataSource.room.accountData.readMarkerEventId inRoom:self.roomDataSource.roomId]; - if (currentReadMarkerEvent) - { - NSPredicate *predicate = [NSPredicate predicateWithBlock:^BOOL(id evaluatedObject, NSDictionary *bindings) { - return [evaluatedObject isKindOfClass:MXKRoomBubbleTableViewCell.class]; - }]; - NSArray *visibleCells = [[self.bubblesTableView visibleCells] filteredArrayUsingPredicate:predicate]; - UITableViewCell *firstCell = visibleCells.firstObject; - UITableViewCell *lastCell = visibleCells.lastObject; - if (firstCell && lastCell) - { - // Get the first displayed event - MXKRoomBubbleTableViewCell *firstRoomBubbleTableViewCell = (MXKRoomBubbleTableViewCell*)firstCell; - MXKRoomBubbleComponent *component = firstRoomBubbleTableViewCell.bubbleData.bubbleComponents.firstObject; - MXEvent *firstDisplayedEvent = component.event; - - // Get the last displayed event - MXKRoomBubbleTableViewCell *lastRoomBubbleTableViewCell = (MXKRoomBubbleTableViewCell*)lastCell; - MXKRoomBubbleComponent *lastComponent = lastRoomBubbleTableViewCell.bubbleData.bubbleComponents.lastObject; - MXEvent *lastDisplayedEvent = lastComponent.event; - - // If the currentReadMarkerEvent is between the first and the last displayed event, force the update of the room read marker to prevent it from being blocked - if (currentReadMarkerEvent.originServerTs > firstDisplayedEvent.originServerTs && currentReadMarkerEvent.originServerTs < lastDisplayedEvent.originServerTs) - { - self.updateRoomReadMarker = YES; - } - } - } - } } } @@ -7956,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); @@ -7983,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 (this + self.updateRoomReadMarker |= forceUpdateRoomMarker; }]; } From 16c7275bfd770674fcf3c17b18a832b0cf1f17e4 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Wed, 29 Mar 2023 17:29:53 +0200 Subject: [PATCH 3/5] Fix: read marker update --- Riot/Modules/Room/MXKRoomViewController.m | 28 +++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Riot/Modules/Room/MXKRoomViewController.m b/Riot/Modules/Room/MXKRoomViewController.m index 9be6b8e3e..cc28da1e9 100644 --- a/Riot/Modules/Room/MXKRoomViewController.m +++ b/Riot/Modules/Room/MXKRoomViewController.m @@ -2491,14 +2491,15 @@ // Indicate to the homeserver that the user has read this event. if (self.navigationController.viewControllers.lastObject == self) { - // If the current event is eligible to be the new read marker position - if (!bubbleData.collapsed && [self eligibleForReadMarkerUpdate:component.event checkTimeStamp:YES]) + // Check if the selected event is eligible to be the new read marker position too + if (!bubbleData.collapsed && [self eligibleForReadMarkerUpdate:component.event]) { - [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:YES]; + BOOL updateRoomReadMarker = _updateRoomReadMarker && [self isEventPosteriorToCurrentReadMarker:component.event]; + [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:updateRoomReadMarker]; } else { - // Otherwise, only acknonwledge this event without updating the read marker + // Acknowledge only this event. The read marker is handled separately [roomDataSource.room acknowledgeEvent:component.event andUpdateReadMarker:NO]; if (_updateRoomReadMarker) @@ -2517,14 +2518,18 @@ } } -- (BOOL)eligibleForReadMarkerUpdate:(MXEvent *)event checkTimeStamp:(BOOL)checkTimeStamp { +- (BOOL)eligibleForReadMarkerUpdate:(MXEvent *)event { // Prevent the readmarker to be placed on a relatesTo or a redaction event if (event.relatesTo || event.redacts) { return NO; } - if (checkTimeStamp && roomDataSource.room.accountData.readMarkerEventId) + 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) @@ -2533,9 +2538,8 @@ } // Update the read marker only if the current event is available, and the new event is posterior to it. - return currentReadMarkerEvent && (currentReadMarkerEvent.eventId != event.eventId) && (currentReadMarkerEvent.originServerTs <= event.originServerTs); + return currentReadMarkerEvent && (currentReadMarkerEvent.originServerTs <= event.originServerTs); } - return YES; } @@ -2603,7 +2607,7 @@ } // Prevent the read marker to be placed on an unsupported event (e.g. redactions, reactions, ...) - if (![self eligibleForReadMarkerUpdate:component.event checkTimeStamp:NO]) + if (![self eligibleForReadMarkerUpdate:component.event]) { continue; } @@ -2616,13 +2620,13 @@ { // We found the component // Check whether the read marker must be updated. - if ([self eligibleForReadMarkerUpdate:component.event checkTimeStamp:YES]) + if ([self isEventPosteriorToCurrentReadMarker:component.event]) { // Move the read marker to this event [roomDataSource.room moveReadMarkerToEventId:component.event.eventId]; - - return; } + + return; } // Prepare the bottom position for the next component From 8b6970aa6f49476e1ca0506cc177ac3806b8b951 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Thu, 30 Mar 2023 10:31:33 +0200 Subject: [PATCH 4/5] Fix comments --- Riot/Modules/Room/RoomViewController.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Riot/Modules/Room/RoomViewController.m b/Riot/Modules/Room/RoomViewController.m index d5b093763..996670081 100644 --- a/Riot/Modules/Room/RoomViewController.m +++ b/Riot/Modules/Room/RoomViewController.m @@ -7956,7 +7956,7 @@ static CGSize kThreadListBarButtonItemImageSize; // Give the data source ownership to the room view controller. self.hasRoomDataSourceOwnership = YES; - // Force the read marker update if needed (this + // Force the read marker update if needed (e.g if we jumped on the last unread message using the banner). self.updateRoomReadMarker |= forceUpdateRoomMarker; }]; } From 69ec4189855d86a39556038f16afa372877b98a6 Mon Sep 17 00:00:00 2001 From: Nicolas Mauri Date: Fri, 31 Mar 2023 16:45:04 +0200 Subject: [PATCH 5/5] Code cleanup --- Riot/Modules/Room/MXKRoomViewController.m | 138 +++++++++++----------- 1 file changed, 67 insertions(+), 71 deletions(-) diff --git a/Riot/Modules/Room/MXKRoomViewController.m b/Riot/Modules/Room/MXKRoomViewController.m index cc28da1e9..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; @@ -2495,6 +2498,7 @@ 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 @@ -2553,7 +2557,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; // Consider the visible cells (starting by those displayed at the bottom) NSArray *visibleCells = [_bubblesTableView visibleCells]; @@ -2564,77 +2568,69 @@ cell = visibleCells[index]; // Check whether the cell is actually visible - if (cell && (cell.frame.origin.y < contentBottomOffsetY)) + if (!cell || cell.frame.origin.y > contentBottomOffsetY) { - if (![cell isKindOfClass:MXKTableViewCell.class]) - { - continue; - } - - MXKCellData *cellData = ((MXKTableViewCell *)cell).mxkCellData; - - // Only 'MXKRoomBubbleCellData' is supported here for the moment. - if (![cellData isKindOfClass:MXKRoomBubbleCellData.class]) - { - continue; - } - - MXKRoomBubbleCellData *bubbleData = (MXKRoomBubbleCellData*)cellData; - - // 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]; - if (![cell isKindOfClass:MXKRoomBubbleTableViewCell.class]) - { - continue; - } - - // Prevent the read marker to be placed on an unsupported event (e.g. redactions, reactions, ...) - if (![self eligibleForReadMarkerUpdate:component.event]) - { - continue; - } - - MXKRoomBubbleTableViewCell *roomBubbleTableViewCell = (MXKRoomBubbleTableViewCell *)cell; - - // 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. + 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. } }