Size error messages correctly.

michaelkirk · michaelkirk · commit 94a23021f807 · 2016-10-12T09:30:25.000-04:00
* 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
diff --git 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 <JSQMessagesViewController/JSQMessagesCollectionViewFlowLayout.h>
 
@@ -41,8 +42,14 @@ - (CGSize)messageBubbleSizeForMessageData:(id<JSQMessageData>)messageData
                               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 @@ - (CGSize)messageBubbleSizeForMessageData:(id<JSQMessageData>)messageData
     }
     // 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 @@ - (CGSize)withiOS10EmojiFixSuperMessageBubbleSizeForMessageData:(id<JSQMessageDa
     return finalSize;
 }
 
+
+- (CGSize)messageBubbleSizeForInfoMessageData:(id<JSQMessageData>)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
@@ -158,6 +158,8 @@ - (UIDeviceFamily)deviceFamily {
     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
@@ -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
@@ -37,6 +37,11 @@ + (UIFont *)ows_dynamicTypeBodyFont {
     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
@@ -110,6 +110,8 @@ @interface MessagesViewController () {
 @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 @@ - (void)viewDidLoad
 {
     [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 @@ - (void)viewDidLoad
     [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 @@ - (void)didPressSendButton:(UIButton *)button
             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 @@ - (OWSDisplayedMessageCollectionViewCell *)loadDisplayedMessageCollectionViewCel
                                                                                                         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 @@ - (OWSDisplayedMessageCollectionViewCell *)loadInfoMessageCellForMessage:(OWSInf
         [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 @@ - (OWSDisplayedMessageCollectionViewCell *)loadErrorMessageCellForMessage:(OWSEr
                                                               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
@@ -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
@@ -3,14 +3,13 @@
 //  Portions Copyright (c) 2016 Open Whisper Systems. All rights reserved.
 
 #import "OWSDisplayedMessageCollectionViewCell.h"
-
+#import "UIFont+OWS.h"
 #import <JSQMessagesViewController/UIView+JSQMessages.h>
 
 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 @@ - (void)awakeFromNib
     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 @@ - (void)prepareForReuse
 {
     [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
@@ -27,21 +27,18 @@
                     </label>
                     <view contentMode="scaleToFill" translatesAutoresizingMaskIntoConstraints="NO" id="qCf-bs-dBd" userLabel="Bubble container">
                         <subviews>
-                            <label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="Info Message" textAlignment="center" lineBreakMode="wordWrap" numberOfLines="0" baselineAdjustment="alignBaselines" minimumScaleFactor="0.5" translatesAutoresizingMaskIntoConstraints="NO" id="OVa-Xw-5vl" customClass="JSQMessagesLabel">
-                                <constraints>
-                                    <constraint firstAttribute="height" relation="greaterThanOrEqual" constant="14" id="fed-2c-dqd"/>
-                                </constraints>
-                                <fontDescription key="fontDescription" style="UICTFontTextStyleBody"/>
-                                <color key="textColor" red="0.0" green="0.0" blue="0.0" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
-                                <nil key="highlightedColor"/>
-                            </label>
+                            <textView clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="scaleToFill" text="Lorem ipsum dolor sit er elit lamet" textAlignment="center" translatesAutoresizingMaskIntoConstraints="NO" id="gbd-vF-h3e" customClass="JSQMessagesCellTextView">
+                                <color key="backgroundColor" white="1" alpha="1" colorSpace="calibratedWhite"/>
+                                <fontDescription key="fontDescription" style="UICTFontTextStyleSubhead"/>
+                                <textInputTraits key="textInputTraits" autocapitalizationType="sentences"/>
+                            </textView>
                         </subviews>
                         <color key="backgroundColor" red="1" green="1" blue="1" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
                         <constraints>
-                            <constraint firstItem="OVa-Xw-5vl" firstAttribute="leading" secondItem="qCf-bs-dBd" secondAttribute="leading" constant="8" id="2IE-8k-czI"/>
-                            <constraint firstAttribute="bottom" secondItem="OVa-Xw-5vl" secondAttribute="bottom" constant="8" id="MtI-jW-t1x"/>
-                            <constraint firstAttribute="trailing" secondItem="OVa-Xw-5vl" secondAttribute="trailing" constant="8" id="Y8z-8G-PLt"/>
-                            <constraint firstItem="OVa-Xw-5vl" firstAttribute="top" secondItem="qCf-bs-dBd" secondAttribute="top" constant="12" id="v5B-tB-pOB"/>
+                            <constraint firstAttribute="trailing" secondItem="gbd-vF-h3e" secondAttribute="trailing" constant="6" id="WME-2Q-rFT"/>
+                            <constraint firstItem="gbd-vF-h3e" firstAttribute="top" secondItem="qCf-bs-dBd" secondAttribute="top" constant="6" id="Zee-be-E6m"/>
+                            <constraint firstAttribute="bottom" secondItem="gbd-vF-h3e" secondAttribute="bottom" id="aCy-5N-gI4"/>
+                            <constraint firstItem="gbd-vF-h3e" firstAttribute="leading" secondItem="qCf-bs-dBd" secondAttribute="leading" constant="6" id="wL5-SN-GFl"/>
                         </constraints>
                     </view>
                     <imageView userInteractionEnabled="NO" contentMode="scaleToFill" horizontalHuggingPriority="251" verticalHuggingPriority="251" image="warning_white.png" translatesAutoresizingMaskIntoConstraints="NO" id="ePO-Cy-jUE">
@@ -65,11 +62,11 @@
             </constraints>
             <size key="customSize" width="320" height="55"/>
             <connections>
-                <outlet property="cellLabel" destination="OVa-Xw-5vl" id="7PC-oj-dQZ"/>
                 <outlet property="cellTopLabel" destination="gcR-Rk-KDC" id="Ogk-hD-ge8"/>
                 <outlet property="cellTopLabelHeightConstraint" destination="ckj-xD-FJI" id="wBH-pQ-Wc7"/>
                 <outlet property="headerImageView" destination="ePO-Cy-jUE" id="4uq-2C-V7U"/>
                 <outlet property="messageBubbleContainerView" destination="qCf-bs-dBd" id="WMx-Di-LZG"/>
+                <outlet property="textView" destination="gbd-vF-h3e" id="Ynt-WC-VrK"/>
             </connections>
             <point key="canvasLocation" x="219" y="433"/>
         </collectionViewCell>

Original file line number	Diff line number	Diff line change
`@@ -158,6 +158,8 @@ - (UIDeviceFamily)deviceFamily {`
`158`	`158`	`return UIDeviceFamilyUnknown;`
`159`	`159`	`}`
`160`	`160`
	`161`	`+// FIXME this is probably broken for new iPhones =(`
	`162`	`+// Who could have guessed that there would ever be a new iPhone model?`
`161`	`163`	`- (BOOL)isiPhoneVersionSixOrMore {`
`162`	`164`	`return`
`163`	`165`	`[[self modelIdentifier] isEqualToString:@"iPhone7,1"] \|\| [[self modelIdentifier] isEqualToString:@"iPhone7,2"];`