From 49501a5d1b3071b19e2edb3d6cfe11683ee2d23b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 12 Oct 2017 16:19:07 -0400 Subject: [PATCH] Respond to CR. // FREEBIE --- .../Cells/AttachmentUploadView.h | 2 +- .../ConversationView/Cells/OWSMessageCell.m | 28 ++++++------ .../ConversationInputToolbar.m | 2 +- .../ConversationViewController.m | 43 ++++++------------- .../ConversationView/ConversationViewItem.m | 10 ++--- .../ConversationView/ConversationViewLayout.m | 1 + 6 files changed, 32 insertions(+), 54 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/AttachmentUploadView.h b/Signal/src/ViewControllers/ConversationView/Cells/AttachmentUploadView.h index 3d08c5f78..b0e75f7f4 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/AttachmentUploadView.h +++ b/Signal/src/ViewControllers/ConversationView/Cells/AttachmentUploadView.h @@ -8,7 +8,7 @@ NS_ASSUME_NONNULL_BEGIN typedef void (^AttachmentStateBlock)(BOOL isAttachmentReady); -// This entity is used to display download progress for incoming +// This entity is used to display upload progress for outgoing // attachments in conversation view cells. // // During attachment uploads we want to: diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m index fabdcb066..055440176 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m @@ -92,21 +92,33 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSString *)textMessage { + // This should always be valid for the appropriate cell types. + OWSAssert(self.viewItem.textMessage); + return self.viewItem.textMessage; } - (nullable TSAttachmentStream *)attachmentStream { + // This should always be valid for the appropriate cell types. + OWSAssert(self.viewItem.attachmentStream); + return self.viewItem.attachmentStream; } - (nullable TSAttachmentPointer *)attachmentPointer { + // This should always be valid for the appropriate cell types. + OWSAssert(self.viewItem.attachmentPointer); + return self.viewItem.attachmentPointer; } - (CGSize)contentSize { + // This should always be valid for the appropriate cell types. + OWSAssert(self.viewItem.contentSize.width > 0 && self.viewItem.contentSize.height > 0); + return self.viewItem.contentSize; } @@ -116,6 +128,8 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(self.viewItem.interaction); OWSAssert([self.viewItem.interaction isKindOfClass:[TSMessage class]]); + DDLogError(@"%p loadForDisplay: %@", self, NSStringForOWSMessageCellType(self.cellType)); + BOOL isIncoming = self.isIncoming; JSQMessagesBubbleImage *bubbleImageData = isIncoming ? [self.bubbleFactory incoming] : [self.bubbleFactory outgoing]; @@ -152,20 +166,6 @@ NS_ASSUME_NONNULL_BEGIN } } - // If we have an outgoing attachment and we haven't created a - // AttachmentUploadView yet, do so now. - // - // For some attachment types, we may create this view earlier - // so that we can take advantage of its callback. - // if (self.attachmentStream && - // !self.isIncoming && - // !self.attachmentUploadView) { - // self.attachmentUploadView = [[AttachmentUploadView alloc] initWithAttachment:self.attachmentStream - // superview:imageView - // attachmentStateCallback:^(BOOL isAttachmentReady) { - // }]; - // } - // [self.textLabel addBorderWithColor:[UIColor blueColor]]; // [self.bubbleImageView addBorderWithColor:[UIColor greenColor]]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m index 8a6adcaa1..b7b4acc2c 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationInputToolbar.m @@ -83,7 +83,7 @@ NS_ASSUME_NONNULL_BEGIN [self.attachmentButton setImage:[UIImage imageNamed:@"btnAttachments--blue"] forState:UIControlStateNormal]; [self addSubview:self.attachmentButton]; - // TODO: + // TODO: Fix layout in this class. _sendButton = [UIButton buttonWithType:UIButtonTypeCustom]; [self.sendButton setTitle:NSLocalizedString(@"SEND_BUTTON_TITLE", @"Label for the send button in the conversation view.") diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index f7f135887..b044640ff 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -192,7 +192,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { @property (nonatomic, readonly) ContactsViewHelper *contactsViewHelper; @property (nonatomic) BOOL userHasScrolled; -@property (nonatomic) NSDate *lastMessageSentDate; +@property (nonatomic, nullable) NSDate *lastMessageSentDate; @property (nonatomic, nullable) ThreadDynamicInteractions *dynamicInteractions; @property (nonatomic) BOOL hasClearedUnreadMessagesIndicator; @@ -869,7 +869,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { } } -- (void)showUnblockContactUI:(BlockActionCompletionBlock _Nullable)completionBlock +- (void)showUnblockContactUI:(nullable BlockActionCompletionBlock)completionBlock { OWSAssert([self.thread isKindOfClass:[TSContactThread class]]); @@ -2747,7 +2747,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.messageMappings updateWithTransaction:transaction]; }]; - [self reloadViewItems]; return; } @@ -2866,13 +2865,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { - (BOOL)isScrolledToBottom { CGFloat contentHeight = self.safeContentHeight; - if (contentHeight < 1) { - // If the collection view hasn't determined its content size yet, - // scroll state is not yet coherent. Therefore we can't (and don't - // need to) determine whether we're "scrolled to the bottom" until - // the collection view has determined its content size. - return NO; - } // This is a bit subtle. // @@ -3069,7 +3061,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { self.voiceMessageUUID = nil; } -- (void)setAudioRecorder:(AVAudioRecorder *_Nullable)audioRecorder +- (void)setAudioRecorder:(nullable AVAudioRecorder *)audioRecorder { // Prevent device from sleeping while recording a voice message. if (audioRecorder) { @@ -3294,15 +3286,13 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { - (void)loadDraftInCompose { + OWSAssert([NSThread isMainThread]); + __block NSString *draft; - [self.editingDatabaseConnection asyncReadWithBlock:^(YapDatabaseReadTransaction *transaction) { + [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { draft = [_thread currentDraftWithTransaction:transaction]; - } - completionBlock:^{ - dispatch_async(dispatch_get_main_queue(), ^{ - [self.inputToolbar setMessageText:draft]; - }); - }]; + }]; + [self.inputToolbar setMessageText:draft]; } - (void)saveDraft @@ -3468,15 +3458,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { BOOL wasAtBottom = [self isScrolledToBottom]; if (wasAtBottom) { [self scrollToBottomImmediately]; - } else if (self.safeContentHeight < 1) { - // If the collection view hasn't determined its content size yet, - // scroll state is not yet coherent. Therefore we can't (and don't - // need to) determine whether we're "scrolled to the bottom" until - // the collection view has determined its content size. - // - // In this case we should just ensure that scroll state is initialized - // properly. - [self scrollToDefaultPosition]; } [self updateLastVisibleTimestamp]; @@ -3900,10 +3881,9 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { return; } - // TODO: Recycle view items where possible. - // TODO: Distinguish interaction types through some enum. [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - TSInteraction *_Nullable interaction = [TSInteraction fetchObjectWithUniqueID:viewItem.interaction.uniqueId]; + TSInteraction *_Nullable interaction = + [TSInteraction fetchObjectWithUniqueID:viewItem.interaction.uniqueId transaction:transaction]; if (!interaction) { OWSFail(@"%@ could not reload interaction", self.tag); } else { @@ -3915,7 +3895,8 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { - (nullable ConversationViewItem *)viewItemForIndex:(NSUInteger)index { if (index >= self.viewItems.count) { - OWSFail(@"%@ Invalid view item index: %zd", self.tag, index) return nil; + OWSFail(@"%@ Invalid view item index: %zd", self.tag, index); + return nil; } return self.viewItems[index]; } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index 3592fa02f..ea64fb5b3 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -110,30 +110,26 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) { OWSAssert([NSThread isMainThread]); - CGSize cellSize = CGSizeZero; if (!self.cachedCellSize) { ConversationViewCell *_Nullable measurementCell = [self measurementCell]; measurementCell.viewItem = self; - cellSize = [measurementCell cellSizeForViewWidth:viewWidth contentWidth:contentWidth]; + CGSize cellSize = [measurementCell cellSizeForViewWidth:viewWidth contentWidth:contentWidth]; self.cachedCellSize = [NSValue valueWithCGSize:cellSize]; [measurementCell prepareForReuse]; - } else { - cellSize = [self.cachedCellSize CGSizeValue]; } - return cellSize; + return [self.cachedCellSize CGSizeValue]; } - (ConversationViewLayoutAlignment)layoutAlignment { switch (self.interaction.interactionType) { case OWSInteractionType_Unknown: + OWSFail(@"%@ Unknown interaction type: %@", self.tag, self.interaction.debugDescription); return ConversationViewLayoutAlignment_Center; case OWSInteractionType_IncomingMessage: return ConversationViewLayoutAlignment_Incoming; - break; case OWSInteractionType_OutgoingMessage: return ConversationViewLayoutAlignment_Outgoing; - break; case OWSInteractionType_Error: case OWSInteractionType_Info: case OWSInteractionType_Call: diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m index 0019e739a..ae757d5de 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewLayout.m @@ -10,6 +10,7 @@ NS_ASSUME_NONNULL_BEGIN @interface ConversationViewLayout () @property (nonatomic) CGSize contentSize; + @property (nonatomic, readonly) NSMutableDictionary *itemAttributesMap; // This dirty flag may be redundant with logic in UICollectionViewLayout,