From 411d5a3b4fe1ce2ef884d7e246bf7f275e059d20 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 2 May 2018 11:01:23 -0400 Subject: [PATCH] Respond to CR. --- .../Cells/OWSContactShareView.m | 4 +++- .../Cells/OWSMessageBubbleView.m | 16 +++++++-------- .../ConversationView/ConversationViewItem.h | 2 +- .../ConversationView/ConversationViewItem.m | 16 +++++++-------- .../ViewControllers/DebugUI/DebugUIMessages.m | 20 +++++++++---------- .../ViewControllers/HomeView/HomeViewCell.m | 4 ++-- .../HomeView/HomeViewController.m | 4 +++- .../src/Messages/Interactions/OWSContact.m | 2 +- 8 files changed, 36 insertions(+), 32 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSContactShareView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSContactShareView.m index 77fdf9f50..fad338677 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSContactShareView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSContactShareView.m @@ -160,7 +160,9 @@ NS_ASSUME_NONNULL_BEGIN [stackView autoPinLeadingToSuperviewMarginWithInset:self.iconHMargin]; [stackView autoPinTrailingToSuperviewMarginWithInset:self.iconHMargin]; [stackView autoVCenterInSuperview]; - // NOTE: It's critical that we pin to the superview top and bottom _edge_ and not _margin_. + // Ensure that the cell's contents never overflow the cell bounds. + // We pin pin to the superview _edge_ and not _margin_ for the purposes + // of overflow, so that changes to the margins do not trip these safe guards. [stackView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; [stackView autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 18f39a1b6..50cc326ce 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -203,7 +203,7 @@ NS_ASSUME_NONNULL_BEGIN case OWSMessageCellType_OversizeTextMessage: case OWSMessageCellType_GenericAttachment: case OWSMessageCellType_DownloadingAttachment: - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: return YES; case OWSMessageCellType_StillImage: case OWSMessageCellType_AnimatedImage: @@ -228,7 +228,7 @@ NS_ASSUME_NONNULL_BEGIN case OWSMessageCellType_Video: // Is there a caption? return self.hasBodyText; - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: return NO; } } @@ -331,8 +331,8 @@ NS_ASSUME_NONNULL_BEGIN bodyMediaView = [self loadViewForDownloadingAttachment]; bodyMediaViewHasGreedyWidth = YES; break; - case OWSMessageCellType_ShareContact: - bodyMediaView = [self loadViewForShareContact]; + case OWSMessageCellType_ContactShare: + bodyMediaView = [self loadViewForContactShare]; bodyMediaViewHasGreedyWidth = YES; break; } @@ -792,7 +792,7 @@ NS_ASSUME_NONNULL_BEGIN return customView; } -- (UIView *)loadViewForShareContact +- (UIView *)loadViewForContactShare { OWSAssert(self.viewItem.contactShare); @@ -930,7 +930,7 @@ NS_ASSUME_NONNULL_BEGIN return CGSizeMake(maxMessageWidth, [OWSGenericAttachmentView bubbleHeight]); case OWSMessageCellType_DownloadingAttachment: return CGSizeMake(200, 90); - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: return CGSizeMake(maxMessageWidth, [OWSContactShareView bubbleHeight]); } } @@ -1052,7 +1052,7 @@ NS_ASSUME_NONNULL_BEGIN if (self.cellType == OWSMessageCellType_DownloadingAttachment) { return NO; } - if (self.cellType == OWSMessageCellType_ShareContact) { + if (self.cellType == OWSMessageCellType_ContactShare) { // TODO: Handle this case. return NO; } @@ -1202,7 +1202,7 @@ NS_ASSUME_NONNULL_BEGIN } break; } - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: // TODO: break; } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h index a20c3d1ab..32bfdb4ab 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h @@ -17,7 +17,7 @@ typedef NS_ENUM(NSInteger, OWSMessageCellType) { OWSMessageCellType_Video, OWSMessageCellType_GenericAttachment, OWSMessageCellType_DownloadingAttachment, - OWSMessageCellType_ShareContact, + OWSMessageCellType_ContactShare, }; NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType); diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index bab413b18..909cff909 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -37,8 +37,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) return @"OWSMessageCellType_DownloadingAttachment"; case OWSMessageCellType_Unknown: return @"OWSMessageCellType_Unknown"; - case OWSMessageCellType_ShareContact: - return @"OWSMessageCellType_ShareContact"; + case OWSMessageCellType_ContactShare: + return @"OWSMessageCellType_ContactShare"; } } @@ -407,7 +407,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) TSMessage *message = (TSMessage *)self.interaction; if (message.contactShare) { self.contactShare = message.contactShare; - self.messageCellType = OWSMessageCellType_ShareContact; + self.messageCellType = OWSMessageCellType_ContactShare; return; } TSAttachment *_Nullable attachment = [self firstAttachmentIfAnyOfMessage:message transaction:transaction]; @@ -721,7 +721,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) OWSFail(@"%@ No text to copy", self.logTag); break; } - case OWSMessageCellType_ShareContact: { + case OWSMessageCellType_ContactShare: { // TODO: Implement copy contact. OWSFail(@"%@ Not implemented yet", self.logTag); break; @@ -735,7 +735,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) case OWSMessageCellType_Unknown: case OWSMessageCellType_TextMessage: case OWSMessageCellType_OversizeTextMessage: - case OWSMessageCellType_ShareContact: { + case OWSMessageCellType_ContactShare: { OWSFail(@"%@ No media to copy", self.logTag); break; } @@ -817,7 +817,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) case OWSMessageCellType_Unknown: case OWSMessageCellType_TextMessage: case OWSMessageCellType_OversizeTextMessage: - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: return NO; case OWSMessageCellType_StillImage: case OWSMessageCellType_AnimatedImage: @@ -840,7 +840,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) case OWSMessageCellType_Unknown: case OWSMessageCellType_TextMessage: case OWSMessageCellType_OversizeTextMessage: - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: OWSFail(@"%@ Cannot save text data.", self.logTag); break; case OWSMessageCellType_StillImage: @@ -896,7 +896,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) case OWSMessageCellType_Unknown: case OWSMessageCellType_TextMessage: case OWSMessageCellType_OversizeTextMessage: - case OWSMessageCellType_ShareContact: + case OWSMessageCellType_ContactShare: return NO; case OWSMessageCellType_StillImage: case OWSMessageCellType_AnimatedImage: diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 4fa2784e2..181c97cca 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -97,7 +97,7 @@ NS_ASSUME_NONNULL_BEGIN // Exemplary [DebugUIMessages allFakeAction:thread], [DebugUIMessages allFakeBackDatedAction:thread], - [DebugUIMessages allShareContactAction:thread], + [DebugUIMessages allContactShareAction:thread], ]]]; [items addObjectsFromArray:@[ @@ -2658,7 +2658,7 @@ NS_ASSUME_NONNULL_BEGIN [actions addObjectsFromArray:[self allFakeSequenceActions:thread includeLabels:includeLabels]]; [actions addObjectsFromArray:[self allFakeQuotedReplyActions:thread includeLabels:includeLabels]]; [actions addObjectsFromArray:[self allFakeBackDatedActions:thread includeLabels:includeLabels]]; - [actions addObjectsFromArray:[self allShareContactActions:thread includeLabels:includeLabels]]; + [actions addObjectsFromArray:[self allContactShareActions:thread includeLabels:includeLabels]]; return actions; } @@ -2902,7 +2902,7 @@ NS_ASSUME_NONNULL_BEGIN typedef OWSContact * (^OWSContactBlock)(void); -+ (DebugUIMessagesAction *)fakeShareContactMessageAction:(TSThread *)thread ++ (DebugUIMessagesAction *)fakeContactShareMessageAction:(TSThread *)thread label:(NSString *)label contactBlock:(OWSContactBlock)contactBlock { @@ -2925,7 +2925,7 @@ typedef OWSContact * (^OWSContactBlock)(void); }]; } -+ (NSArray *)allShareContactActions:(TSThread *)thread includeLabels:(BOOL)includeLabels ++ (NSArray *)allContactShareActions:(TSThread *)thread includeLabels:(BOOL)includeLabels { OWSAssert(thread); @@ -2937,7 +2937,7 @@ typedef OWSContact * (^OWSContactBlock)(void); text:@"⚠️ Share Contact ⚠️"]]; } - [actions addObject:[self fakeShareContactMessageAction:thread + [actions addObject:[self fakeContactShareMessageAction:thread label:@"Name & Number" contactBlock:^{ OWSContact *contact = [OWSContact new]; @@ -2950,7 +2950,7 @@ typedef OWSContact * (^OWSContactBlock)(void); ]; return contact; }]]; - [actions addObject:[self fakeShareContactMessageAction:thread + [actions addObject:[self fakeContactShareMessageAction:thread label:@"Name & Email" contactBlock:^{ OWSContact *contact = [OWSContact new]; @@ -2963,7 +2963,7 @@ typedef OWSContact * (^OWSContactBlock)(void); ]; return contact; }]]; - [actions addObject:[self fakeShareContactMessageAction:thread + [actions addObject:[self fakeContactShareMessageAction:thread label:@"Complicated" contactBlock:^{ OWSContact *contact = [OWSContact new]; @@ -3027,12 +3027,12 @@ typedef OWSContact * (^OWSContactBlock)(void); return actions; } -+ (DebugUIMessagesAction *)allShareContactAction:(TSThread *)thread ++ (DebugUIMessagesAction *)allContactShareAction:(TSThread *)thread { OWSAssert(thread); - return [DebugUIMessagesGroupAction allGroupActionWithLabel:@"All Fake Share Contact" - subactions:[self allShareContactActions:thread includeLabels:YES]]; + return [DebugUIMessagesGroupAction allGroupActionWithLabel:@"All Fake Contact Shares" + subactions:[self allContactShareActions:thread includeLabels:YES]]; } #pragma mark - diff --git a/Signal/src/ViewControllers/HomeView/HomeViewCell.m b/Signal/src/ViewControllers/HomeView/HomeViewCell.m index f726826c6..283f35887 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewCell.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewCell.m @@ -84,8 +84,8 @@ NS_ASSUME_NONNULL_BEGIN [self.payloadView autoPinLeadingToTrailingEdgeOfView:self.avatarView offset:self.avatarHSpacing]; [self.payloadView autoVCenterInSuperview]; // Ensure that the cell's contents never overflow the cell bounds. - // - // NOTE: It's critical that we pin to the superview top and bottom _edge_ and not _margin_. + // We pin pin to the superview _edge_ and not _margin_ for the purposes + // of overflow, so that changes to the margins do not trip these safe guards. [self.payloadView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; [self.payloadView autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; // We pin the payloadView traillingEdge later, as part of the "Unread Badge" logic. diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 6345a4a3e..08d1adffa 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -748,7 +748,9 @@ NSString *const kArchivedConversationsReuseIdentifier = @"kArchivedConversations // Constrain to cell margins. [stackView autoPinEdgeToSuperviewMargin:ALEdgeLeading relation:NSLayoutRelationGreaterThanOrEqual]; [stackView autoPinEdgeToSuperviewMargin:ALEdgeTrailing relation:NSLayoutRelationGreaterThanOrEqual]; - // NOTE: It's critical that we pin to the superview top and bottom _edge_ and not _margin_. + // Ensure that the cell's contents never overflow the cell bounds. + // We pin pin to the superview _edge_ and not _margin_ for the purposes + // of overflow, so that changes to the margins do not trip these safe guards. [stackView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; [stackView autoPinEdgeToSuperviewEdge:ALEdgeBottom withInset:0 relation:NSLayoutRelationGreaterThanOrEqual]; diff --git a/SignalServiceKit/src/Messages/Interactions/OWSContact.m b/SignalServiceKit/src/Messages/Interactions/OWSContact.m index 8b08339b3..aa15b985e 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSContact.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSContact.m @@ -204,7 +204,7 @@ NS_ASSUME_NONNULL_BEGIN _displayName = self.organizationName; } if (_displayName.length < 1) { - DDLogError(@"%@ could not derive a valid display name.", self.logTag); + OWSProdLogAndFail(@"%@ could not derive a valid display name.", self.logTag); } }