From 94a23021f80746cf06956ea48f6e0d5f137feacd Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Thu, 6 Oct 2016 15:47:45 -0400 Subject: [PATCH] Size error messages correctly. * calculate size of info message using the info message font. * offset by the info message header There were instances of lines getting cropped, or an extra line being added. The previous, more conservative, solution was to just make every bubble too big, but it looked terrible. // FREEBIE --- .../Models/OWSMessagesBubblesSizeCalculator.m | 100 +++++++++++++++--- Signal/src/util/UIDevice+TSHardwareVersion.m | 2 + Signal/src/util/UIFont+OWS.h | 1 + Signal/src/util/UIFont+OWS.m | 5 + .../view controllers/MessagesViewController.m | 32 ++++-- .../OWSDisplayedMessageCollectionViewCell.h | 1 - .../OWSDisplayedMessageCollectionViewCell.m | 8 +- .../OWSDisplayedMessageCollectionViewCell.xib | 23 ++-- 8 files changed, 130 insertions(+), 42 deletions(-) diff --git a/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m b/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m index 53923183c..47e83218f 100644 --- a/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m +++ b/Signal/src/Models/OWSMessagesBubblesSizeCalculator.m @@ -3,6 +3,7 @@ #import "OWSMessagesBubblesSizeCalculator.h" #import "OWSDisplayedMessageCollectionViewCell.h" #import "TSMessageAdapter.h" +#import "UIFont+OWS.h" #import "tgmath.h" // generic math allows fmax to handle CGFLoat correctly on 32 & 64bit. #import @@ -41,8 +42,14 @@ NS_ASSUME_NONNULL_BEGIN atIndexPath:(NSIndexPath *)indexPath withLayout:(JSQMessagesCollectionViewFlowLayout *)layout { - CGSize size; + if ([messageData isKindOfClass:[TSMessageAdapter class]]) { + TSMessageAdapter *message = (TSMessageAdapter *)messageData; + if (message.messageType == TSInfoMessageAdapter || message.messageType == TSErrorMessageAdapter) { + return [self messageBubbleSizeForInfoMessageData:messageData atIndexPath:indexPath withLayout:layout]; + } + } + CGSize size; // BEGIN HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368 BOOL isIOS10OrGreater = [[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:(NSOperatingSystemVersion){.majorVersion = 10 }]; @@ -55,22 +62,8 @@ NS_ASSUME_NONNULL_BEGIN } // END HACK iOS10EmojiBug see: https://github.com/WhisperSystems/Signal-iOS/issues/1368 - if ([messageData isKindOfClass:[TSMessageAdapter class]]) { - TSMessageAdapter *message = (TSMessageAdapter *)messageData; - if (message.messageType == TSInfoMessageAdapter || message.messageType == TSErrorMessageAdapter) { - // DDLogVerbose(@"[OWSMessagesBubblesSizeCalculator] superSize.height:%f, superSize.width:%f", - // superSize.height, - // superSize.width); - - // header icon hangs ouside of the frame a bit. - CGFloat headerIconProtrusion = 30.0f; // too much padding with normal font. - // CGFloat headerIconProtrusion = 18.0f; // clips - size.height += headerIconProtrusion; - } - } - return size; } @@ -173,6 +166,83 @@ NS_ASSUME_NONNULL_BEGIN return finalSize; } + +- (CGSize)messageBubbleSizeForInfoMessageData:(id)messageData + atIndexPath:(NSIndexPath *)indexPath + withLayout:(JSQMessagesCollectionViewFlowLayout *)layout +{ + NSValue *cachedSize = [self.cache objectForKey:@([messageData messageHash])]; + if (cachedSize != nil) { + return [cachedSize CGSizeValue]; + } + + CGSize finalSize = CGSizeZero; + + if ([messageData isMediaMessage]) { + finalSize = [[messageData media] mediaViewDisplaySize]; + } else { + /////////////////// + // BEGIN InfoMessage sizing HACK + // Braindead, and painstakingly produced. + // If you want to change, check for clipping / excess space on 1, 2, and 3 line messages with short and long + // words very near the edge. + +// CGSize avatarSize = [self jsq_avatarSizeForMessageData:messageData withLayout:layout]; +// // from the cell xibs, there is a 2 point space between avatar and bubble +// CGFloat spacingBetweenAvatarAndBubble = 2.0f; +// CGFloat horizontalContainerInsets = layout.messageBubbleTextViewTextContainerInsets.left + layout.messageBubbleTextViewTextContainerInsets.right; +// CGFloat horizontalFrameInsets = layout.messageBubbleTextViewFrameInsets.left + layout.messageBubbleTextViewFrameInsets.right; +// CGFloat horizontalInsetsTotal = horizontalContainerInsets + horizontalFrameInsets + spacingBetweenAvatarAndBubble; +// CGFloat maximumTextWidth = [self textBubbleWidthForLayout:layout] - avatarSize.width - layout.messageBubbleLeftRightMargin - horizontalInsetsTotal; + + // The full layout width, less the textView margins from xib. +// CGFloat horizontalInsetsTotal = 12.0; cropped 3rd line + CGFloat horizontalInsetsTotal = 50.0; + CGFloat maximumTextWidth = [self textBubbleWidthForLayout:layout] - horizontalInsetsTotal; + + CGRect stringRect = [[messageData text] + boundingRectWithSize:CGSizeMake(maximumTextWidth, CGFLOAT_MAX) + options:(NSStringDrawingUsesLineFragmentOrigin | NSStringDrawingUsesFontLeading) + attributes:@{ + NSFontAttributeName : [UIFont ows_dynamicTypeBodyFont] + } // Hack to use a slightly larger than actual font, because I'm seeing messages with higher line count get clipped. + context:nil]; + // END InfoMessage sizing HACK + //////////////////// + + CGSize stringSize = CGRectIntegral(stringRect).size; + + CGFloat verticalContainerInsets = layout.messageBubbleTextViewTextContainerInsets.top + + layout.messageBubbleTextViewTextContainerInsets.bottom; + + CGFloat verticalFrameInsets + = layout.messageBubbleTextViewFrameInsets.top + layout.messageBubbleTextViewFrameInsets.bottom; + /////////////////// + // BEGIN InfoMessage sizing HACK + + CGFloat topIconPortrusion = 28; + + verticalFrameInsets += topIconPortrusion; + + // END InfoMessage sizing HACK + /////////////////// + + // add extra 2 points of space (`self.additionalInset`), because `boundingRectWithSize:` is slightly off + // not sure why. magix. (shrug) if you know, submit a PR + CGFloat verticalInsets = verticalContainerInsets + verticalFrameInsets + self.additionalInset; + + // same as above, an extra 2 points of magix + CGFloat finalWidth + = MAX(stringSize.width + horizontalInsetsTotal, self.minimumBubbleWidth) + self.additionalInset; + + finalSize = CGSizeMake(finalWidth, stringSize.height + verticalInsets); + } + + [self.cache setObject:[NSValue valueWithCGSize:finalSize] forKey:@([messageData messageHash])]; + + return finalSize; +} + @end NS_ASSUME_NONNULL_END diff --git a/Signal/src/util/UIDevice+TSHardwareVersion.m b/Signal/src/util/UIDevice+TSHardwareVersion.m index 0c0e858a3..c33f920ad 100644 --- a/Signal/src/util/UIDevice+TSHardwareVersion.m +++ b/Signal/src/util/UIDevice+TSHardwareVersion.m @@ -158,6 +158,8 @@ return UIDeviceFamilyUnknown; } +// FIXME this is probably broken for new iPhones =( +// Who could have guessed that there would ever be a new iPhone model? - (BOOL)isiPhoneVersionSixOrMore { return [[self modelIdentifier] isEqualToString:@"iPhone7,1"] || [[self modelIdentifier] isEqualToString:@"iPhone7,2"]; diff --git a/Signal/src/util/UIFont+OWS.h b/Signal/src/util/UIFont+OWS.h index e0e2ead36..9e20c7066 100644 --- a/Signal/src/util/UIFont+OWS.h +++ b/Signal/src/util/UIFont+OWS.h @@ -25,5 +25,6 @@ + (UIFont *)ows_dynamicTypeBodyFont; + (UIFont *)ows_dynamicTypeTitle2Font; ++ (UIFont *)ows_infoMessageFont; @end diff --git a/Signal/src/util/UIFont+OWS.m b/Signal/src/util/UIFont+OWS.m index 45ac93794..ea1b68526 100644 --- a/Signal/src/util/UIFont+OWS.m +++ b/Signal/src/util/UIFont+OWS.m @@ -37,6 +37,11 @@ return [UIFont preferredFontForTextStyle:UIFontTextStyleBody]; } ++ (UIFont *)ows_infoMessageFont +{ + return [UIFont preferredFontForTextStyle:UIFontTextStyleSubheadline]; +} + + (UIFont *)ows_dynamicTypeTitle2Font { if (SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(_iOS_9)) { return [UIFont preferredFontForTextStyle:UIFontTextStyleTitle2]; diff --git a/Signal/src/view controllers/MessagesViewController.m b/Signal/src/view controllers/MessagesViewController.m index 32a7d0161..5e3f45150 100644 --- a/Signal/src/view controllers/MessagesViewController.m +++ b/Signal/src/view controllers/MessagesViewController.m @@ -110,6 +110,8 @@ typedef enum : NSUInteger { @property (nonatomic, strong) UILabel *navbarTitleLabel; @property (nonatomic, retain) UIButton *attachButton; +@property (nonatomic) CGFloat previousCollectionViewFrameWidth; + @property NSUInteger page; @property (nonatomic) BOOL composeOnOpen; @property (nonatomic) BOOL peek; @@ -215,12 +217,6 @@ typedef enum : NSUInteger { { [super viewDidLoad]; - // JSQMVC width is 375px at this point (as specified by the xib), but this causes - // our initial bubble calculations to be off since they happen before the containing - // view is layed out. https://github.com/jessesquires/JSQMessagesViewController/issues/1257 - // Resetting here makes sure we've got a good initial width. - [self resetFrame]; - [self.navigationController.navigationBar setTranslucent:NO]; self.messageAdapterCache = [[NSCache alloc] init]; @@ -251,6 +247,23 @@ typedef enum : NSUInteger { [self initializeToolbars]; } +- (void)viewDidLayoutSubviews +{ + [super viewDidLayoutSubviews]; + + // JSQMVC width is initially 375px on iphone6/ios9 (as specified by the xib), which causes + // our initial bubble calculations to be off since they happen before the containing + // view is layed out. https://github.com/jessesquires/JSQMessagesViewController/issues/1257 + if (CGRectGetWidth(self.collectionView.frame) != self.previousCollectionViewFrameWidth) { + // save frame value from next comparison + self.previousCollectionViewFrameWidth = CGRectGetWidth(self.collectionView.frame); + + // invalidate layout + [self.collectionView.collectionViewLayout + invalidateLayoutWithContext:[JSQMessagesCollectionViewFlowLayoutInvalidationContext context]]; + } +} + - (void)didMoveToParentViewController:(UIViewController *)parent { [self setupTitleLabelGestureRecognizer]; @@ -619,7 +632,7 @@ typedef enum : NSUInteger { message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:self.thread messageBody:text - attachmentIds:@[] + attachmentIds:[NSMutableArray new] expiresInSeconds:configuration.durationSeconds]; } else { message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] @@ -831,6 +844,7 @@ typedef enum : NSUInteger { forIndexPath:indexPath]; messageCell.layer.shouldRasterize = YES; messageCell.layer.rasterizationScale = [UIScreen mainScreen].scale; + messageCell.textView.textColor = [UIColor darkGrayColor]; messageCell.cellTopLabel.attributedText = [self.collectionView.dataSource collectionView:self.collectionView attributedTextForCellTopLabelAtIndexPath:indexPath]; return messageCell; @@ -846,7 +860,7 @@ typedef enum : NSUInteger { [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:self.thread.uniqueId]; [self setBarButtonItemsForDisappearingMessagesConfiguration:configuration]; - infoCell.cellLabel.text = [infoMessage text]; + infoCell.textView.text = [infoMessage text]; infoCell.messageBubbleContainerView.layer.borderColor = [[UIColor ows_infoMessageBorderColor] CGColor]; infoCell.headerImageView.image = [UIImage imageNamed:@"warning_white"]; @@ -857,7 +871,7 @@ typedef enum : NSUInteger { atIndexPath:(NSIndexPath *)indexPath { OWSDisplayedMessageCollectionViewCell *errorCell = [self loadDisplayedMessageCollectionViewCellForIndexPath:indexPath]; - errorCell.cellLabel.text = [errorMessage text]; + errorCell.textView.text = [errorMessage text]; errorCell.messageBubbleContainerView.layer.borderColor = [[UIColor ows_errorMessageBorderColor] CGColor]; errorCell.headerImageView.image = [UIImage imageNamed:@"error_white"]; diff --git a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.h b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.h index a42b96a51..03ca9b962 100644 --- a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.h +++ b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.h @@ -9,7 +9,6 @@ extern const CGFloat OWSDisplayedMessageCellMinimumHeight; @interface OWSDisplayedMessageCollectionViewCell : JSQMessagesCollectionViewCell -@property (weak, nonatomic, readonly) JSQMessagesLabel *cellLabel; @property (weak, nonatomic, readonly) UIImageView *headerImageView; @end diff --git a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.m b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.m index 9022ba812..5165c412c 100644 --- a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.m +++ b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.m @@ -3,14 +3,13 @@ // Portions Copyright (c) 2016 Open Whisper Systems. All rights reserved. #import "OWSDisplayedMessageCollectionViewCell.h" - +#import "UIFont+OWS.h" #import const CGFloat OWSDisplayedMessageCellMinimumHeight = 70.0; @interface OWSDisplayedMessageCollectionViewCell () -@property (weak, nonatomic) IBOutlet JSQMessagesLabel *cellLabel; @property (weak, nonatomic) IBOutlet NSLayoutConstraint *cellTopLabelHeightConstraint; @property (weak, nonatomic) IBOutlet UIImageView *headerImageView; @@ -43,7 +42,8 @@ const CGFloat OWSDisplayedMessageCellMinimumHeight = 70.0; self.messageBubbleContainerView.layer.borderColor = [[UIColor lightGrayColor] CGColor]; self.messageBubbleContainerView.layer.borderWidth = 0.75f; self.messageBubbleContainerView.layer.cornerRadius = 5.0f; - self.cellLabel.textColor = [UIColor darkGrayColor]; + self.textView.font = [UIFont ows_infoMessageFont]; + self.textView.textColor = [UIColor darkGrayColor]; } #pragma mark - Collection view cell @@ -52,7 +52,7 @@ const CGFloat OWSDisplayedMessageCellMinimumHeight = 70.0; { [super prepareForReuse]; - self.cellLabel.text = nil; + self.textView.text = nil; } @end diff --git a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.xib b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.xib index 7459982d6..25df9fb4b 100644 --- a/Signal/src/views/OWSDisplayedMessageCollectionViewCell.xib +++ b/Signal/src/views/OWSDisplayedMessageCollectionViewCell.xib @@ -27,21 +27,18 @@ - + + + + + - - - - + + + + @@ -65,11 +62,11 @@ - +