jessesquires / JSQMessagesViewController

An elegant messages UI library for iOS
https://www.jessesquires.com/blog/officially-deprecating-jsqmessagesviewcontroller/
Other
11.14k stars 2.81k forks source link

messageBubbleSizeForItemAtIndexPath counts vertical insets against text width #407

Closed ghazel closed 10 years ago

ghazel commented 10 years ago

https://github.com/jessesquires/JSQMessagesViewController/blob/51701ca5ddd695236975777182c1e24759f4b18c/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m#L351

Strangely, boundingRectWithSize is passed maximumTextWidth - textInsetsTotal but textInsetsTotal is based on jsq_messageBubbleTextContainerInsetsTotal which is insets.left + insets.right + insets.bottom + insets.top.

So, doesn't that make the text take less width than it should?

ghazel commented 10 years ago

Same in jsq_configureMessageCellLayoutAttributes:

https://github.com/jessesquires/JSQMessagesViewController/blob/51701ca5ddd695236975777182c1e24759f4b18c/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m#L374

jessesquires commented 10 years ago

thanks @ghazel

this is intentional.

the height is unbounded (CGFLOAT_MAX), so we need to constrict the calculated width by the total amount of padding. if we only used the left and right inset values, the bubbles would not be the correct size.

however, there might be a different/better approach.

closing this issue, but it is related to #347

ghazel commented 10 years ago

In trying to more closely mimic the iMessage bubbles, I had to change the font size to [UIFont systemFontOfSize:17.0f]. The bubbles were noticeably mis-sized still, so I changed messageBubbleTextViewTextContainerInsets = UIEdgeInsetsMake(7.0f, 7.0f, 7.0f, 7.0f). This pointed out the problem with jsq_messageBubbleTextContainerInsetsTotal. Once I fixed that, things worked great. Here's the patch:

diff --git a/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m b/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m
index 45f6369..52ec429 100644
--- a/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m
+++ b/JSQMessagesViewController/Layout/JSQMessagesCollectionViewFlowLayout.m
@@ -346,18 +346,21 @@ const CGFloat kJSQMessagesCollectionViewCellLabelHeightDefault = 20.0f;

     CGFloat maximumTextWidth = self.itemWidth - avatarSize.width - self.messageBubbleLeftRightMargin;

-    CGFloat textInsetsTotal = [self jsq_messageBubbleTextContainerInsetsTotal];
-    
-    CGRect stringRect = [[messageData text] boundingRectWithSize:CGSizeMake(maximumTextWidth - textInsetsTotal, CGFLOAT_MAX)
+    CGFloat horizontalInsets = self.messageBubbleTextViewTextContainerInsets.left + self.messageBubbleTextViewTextContainerInsets.right;
+
+    CGRect stringRect = [[messageData text] boundingRectWithSize:CGSizeMake(maximumTextWidth - horizontalInsets, CGFLOAT_MAX)
                                                          options:(NSStringDrawingUsesLineFragmentOrigin | NSStringDrawingUsesFontLeading)
                                                       attributes:@{ NSFontAttributeName : self.messageBubbleFont }
                                                          context:nil];

     CGSize stringSize = CGRectIntegral(stringRect).size;

+    // XXX: it seems boundingRectWithSize underestimates slightly?
+    stringSize.width += 4;
+    stringSize.height += 1;
+
     CGFloat verticalInsets = self.messageBubbleTextViewTextContainerInsets.top + self.messageBubbleTextViewTextContainerInsets.bottom;

-    CGSize finalSize = CGSizeMake(stringSize.width, stringSize.height + verticalInsets);
+    CGSize finalSize = CGSizeMake(stringSize.width + horizontalInsets, stringSize.height + verticalInsets);

     [self.messageBubbleSizes setObject:[NSValue valueWithCGSize:finalSize] forKey:indexPath];

@@ -370,8 +373,8 @@ const CGFloat kJSQMessagesCollectionViewCellLabelHeightDefault = 20.0f;

     CGSize messageBubbleSize = [self messageBubbleSizeForItemAtIndexPath:indexPath];
     CGFloat remainingItemWidthForBubble = self.itemWidth - [self jsq_avatarSizeForIndexPath:indexPath].width;
-    CGFloat textPadding = [self jsq_messageBubbleTextContainerInsetsTotal];
-    CGFloat messageBubblePadding = remainingItemWidthForBubble - messageBubbleSize.width - textPadding;
+    CGFloat horizontalInsets = self.messageBubbleTextViewTextContainerInsets.left + self.messageBubbleTextViewTextContainerInsets.right;
+    CGFloat messageBubblePadding = remainingItemWidthForBubble - messageBubbleSize.width - horizontalInsets;

     layoutAttributes.messageBubbleLeftRightMargin = messageBubblePadding;

It appears to still render correctly even without my font size and messageBubbleTextViewTextContainerInsets settings.

ghazel commented 10 years ago

So, I don't see what you mean by "if we only used the left and right inset values, the bubbles would not be the correct size." They appear to be the correct size even when leaving top and bottom insets out of the width.