From 8649b260332a1279f01e4cfe45dc2d0de0186f2e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 16 Jun 2017 15:25:55 -0400 Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20auto-scroll=20after=20?= =?UTF-8?q?=E2=80=9Cloading=20more=20messages=E2=80=9D=20unless=20we=20hav?= =?UTF-8?q?e=20=E2=80=9Cmore=20unseen=20messages=E2=80=9D.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // FREEBIE --- .../ConversationView/MessagesViewController.m | 18 ++++++++++++------ Signal/src/util/ThreadUtil.h | 2 ++ Signal/src/util/ThreadUtil.m | 8 ++++---- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index ea8e7fe3e..b19f66fe8 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -2184,6 +2184,9 @@ typedef enum : NSUInteger { header:(JSQMessagesLoadEarlierHeaderView *)headerView didTapLoadEarlierMessagesButton:(UIButton *)sender { + OWSAssert(!self.isUserScrolling); + + BOOL hasMoreUnseenMessages = self.dynamicInteractions.hasMoreUnseenMessages; // We want to restore the current scroll state after we update the range, update // the dynamic interactions and re-layout. Here we take a "before" snapshot. @@ -2229,13 +2232,16 @@ typedef enum : NSUInteger { [self.collectionView layoutSubviews]; self.collectionView.contentOffset = CGPointMake(0, self.collectionView.contentSize.height - scrollDistanceToBottom); + [self.scrollLaterTimer invalidate]; - // We want to scroll to the bottom _after_ the layout has been updated. - self.scrollLaterTimer = [NSTimer weakScheduledTimerWithTimeInterval:0.001f - target:self - selector:@selector(scrollToUnreadIndicatorAnimated) - userInfo:nil - repeats:NO]; + if (hasMoreUnseenMessages) { + // We want to scroll to the bottom _after_ the layout has been updated. + self.scrollLaterTimer = [NSTimer weakScheduledTimerWithTimeInterval:0.001f + target:self + selector:@selector(scrollToUnreadIndicatorAnimated) + userInfo:nil + repeats:NO]; + } [self updateLoadEarlierVisible]; } diff --git a/Signal/src/util/ThreadUtil.h b/Signal/src/util/ThreadUtil.h index a79199b57..5a4bf67e8 100644 --- a/Signal/src/util/ThreadUtil.h +++ b/Signal/src/util/ThreadUtil.h @@ -36,6 +36,8 @@ NS_ASSUME_NONNULL_BEGIN // YES in ensureDynamicInteractionsForThread:... @property (nonatomic, nullable) NSNumber *firstUnseenInteractionTimestamp; +@property (nonatomic) BOOL hasMoreUnseenMessages; + - (void)clearUnreadIndicatorState; @end diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index e3ea27574..6b5ed208b 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -24,6 +24,7 @@ NS_ASSUME_NONNULL_BEGIN { self.unreadIndicatorPosition = nil; self.firstUnseenInteractionTimestamp = nil; + self.hasMoreUnseenMessages = NO; } @end @@ -200,7 +201,6 @@ NS_ASSUME_NONNULL_BEGIN // so that it can widen its "load window" to always show // the unread indicator. __block long visibleUnseenMessageCount = 0; - __block BOOL hasMoreUnseenMessages = NO; __block TSInteraction *interactionAfterUnreadIndicator = nil; NSUInteger missingUnseenSafetyNumberChangeCount = 0; if (result.firstUnseenInteractionTimestamp != nil) { @@ -243,13 +243,13 @@ NS_ASSUME_NONNULL_BEGIN // messages view, show the unread indicator at the top of the // displayed messages. *stop = YES; - hasMoreUnseenMessages = YES; + result.hasMoreUnseenMessages = YES; } }]; OWSAssert(interactionAfterUnreadIndicator); - if (hasMoreUnseenMessages) { + if (result.hasMoreUnseenMessages) { NSMutableSet *missingUnseenSafetyNumberChanges = [NSMutableSet set]; for (TSInvalidIdentityKeyErrorMessage *safetyNumberChange in blockingSafetyNumberChanges) { BOOL isUnseen = safetyNumberChange.timestampForSorting @@ -403,7 +403,7 @@ NS_ASSUME_NONNULL_BEGIN TSUnreadIndicatorInteraction *indicator = [[TSUnreadIndicatorInteraction alloc] initWithTimestamp:indicatorTimestamp thread:thread - hasMoreUnseenMessages:hasMoreUnseenMessages + hasMoreUnseenMessages:result.hasMoreUnseenMessages missingUnseenSafetyNumberChangeCount:missingUnseenSafetyNumberChangeCount]; [indicator saveWithTransaction:transaction]; From e14b9b511dd66062aca8c66f116acf057f20b76d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Sat, 17 Jun 2017 14:03:44 -0400 Subject: [PATCH 2/2] Respond to CR. // FREEBIE --- .../ConversationView/MessagesViewController.m | 7 +++++-- Signal/src/util/ThreadUtil.h | 6 +++--- Signal/src/util/ThreadUtil.m | 12 ++++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index b19f66fe8..f7c0c053d 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -2186,7 +2186,7 @@ typedef enum : NSUInteger { { OWSAssert(!self.isUserScrolling); - BOOL hasMoreUnseenMessages = self.dynamicInteractions.hasMoreUnseenMessages; + BOOL hasEarlierUnseenMessages = self.dynamicInteractions.hasMoreUnseenMessages; // We want to restore the current scroll state after we update the range, update // the dynamic interactions and re-layout. Here we take a "before" snapshot. @@ -2234,7 +2234,10 @@ typedef enum : NSUInteger { self.collectionView.contentOffset = CGPointMake(0, self.collectionView.contentSize.height - scrollDistanceToBottom); [self.scrollLaterTimer invalidate]; - if (hasMoreUnseenMessages) { + // Don’t auto-scroll after “loading more messages” unless we have “more unseen messages”. + // + // Otherwise, tapping on "load more messages" autoscrolls you downward which is completely wrong. + if (hasEarlierUnseenMessages) { // We want to scroll to the bottom _after_ the layout has been updated. self.scrollLaterTimer = [NSTimer weakScheduledTimerWithTimeInterval:0.001f target:self diff --git a/Signal/src/util/ThreadUtil.h b/Signal/src/util/ThreadUtil.h index 5a4bf67e8..41c84ab78 100644 --- a/Signal/src/util/ThreadUtil.h +++ b/Signal/src/util/ThreadUtil.h @@ -23,7 +23,7 @@ NS_ASSUME_NONNULL_BEGIN // This is used by MessageViewController to increase the // range size of the mappings (the load window of the conversation) // to include the unread indicator. -@property (nonatomic, nullable) NSNumber *unreadIndicatorPosition; +@property (nonatomic, nullable, readonly) NSNumber *unreadIndicatorPosition; // If there are unseen messages in the thread, this is the timestamp // of the oldest unseen messaage. @@ -34,9 +34,9 @@ NS_ASSUME_NONNULL_BEGIN // repeatedly. The unread indicator should continue to show up until // it has been cleared, at which point hideUnreadMessagesIndicator is // YES in ensureDynamicInteractionsForThread:... -@property (nonatomic, nullable) NSNumber *firstUnseenInteractionTimestamp; +@property (nonatomic, nullable, readonly) NSNumber *firstUnseenInteractionTimestamp; -@property (nonatomic) BOOL hasMoreUnseenMessages; +@property (nonatomic, readonly) BOOL hasMoreUnseenMessages; - (void)clearUnreadIndicatorState; diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 6b5ed208b..7dfb85018 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -18,6 +18,18 @@ NS_ASSUME_NONNULL_BEGIN +@interface ThreadDynamicInteractions () + +@property (nonatomic, nullable) NSNumber *unreadIndicatorPosition; + +@property (nonatomic, nullable) NSNumber *firstUnseenInteractionTimestamp; + +@property (nonatomic) BOOL hasMoreUnseenMessages; + +@end + +#pragma mark - + @implementation ThreadDynamicInteractions - (void)clearUnreadIndicatorState