From 7847db7e1c0dc4a70e71eac8eefa5a2b4d8e2c64 Mon Sep 17 00:00:00 2001 From: Matthew Chen <matthew@signal.org> Date: Fri, 22 Jun 2018 16:52:26 -0400 Subject: [PATCH 1/3] Tweak text insets to reflect dynamic type. --- .../ConversationView/Cells/OWSBubbleView.h | 4 -- .../ConversationView/Cells/OWSBubbleView.m | 4 -- .../Cells/OWSMessageBubbleView.h | 2 - .../Cells/OWSMessageBubbleView.m | 48 ++++++---------- .../ConversationLayoutInfo.swift | 56 +++++++++++++++++++ .../MessageDetailViewController.swift | 1 - 6 files changed, 72 insertions(+), 43 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.h b/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.h index 312006882..bb1a9f4f1 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.h +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.h @@ -8,10 +8,6 @@ NS_ASSUME_NONNULL_BEGIN extern const CGFloat kOWSMessageCellCornerRadius; -extern const CGFloat kBubbleTextHInset; -extern const CGFloat kBubbleTextTopInset; -extern const CGFloat kBubbleTextBottomInset; - @class OWSBubbleView; @protocol OWSBubbleViewPartner <NSObject> diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.m index 85d3c6649..c7be095fd 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSBubbleView.m @@ -9,10 +9,6 @@ NS_ASSUME_NONNULL_BEGIN const CGFloat kOWSMessageCellCornerRadius = 18; -const CGFloat kBubbleTextHInset = 10.f; -const CGFloat kBubbleTextTopInset = 8.f; -const CGFloat kBubbleTextBottomInset = 6.f; - @interface OWSBubbleView () @property (nonatomic) CAShapeLayer *maskLayer; diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.h b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.h index 67131839b..2a8e3748d 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.h +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.h @@ -66,8 +66,6 @@ typedef NS_ENUM(NSUInteger, OWSMessageGestureLocation) { @property (nonatomic, nullable, readonly) UIView *bodyMediaView; -@property (nonatomic) BOOL alwaysShowBubbleTail; - @property (nonatomic, weak) id<OWSMessageBubbleViewDelegate> delegate; - (instancetype)init NS_UNAVAILABLE; diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 51c7d26ee..399f6731c 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -390,6 +390,9 @@ NS_ASSUME_NONNULL_BEGIN } } + OWSTextInsets *textInsets = self.layoutInfo.textInsets; + OWSAssert(textInsets); + OWSMessageTextView *_Nullable bodyTextView = nil; // We render malformed messages as "empty text" messages, // so create a text view if there is no body media view. @@ -399,8 +402,8 @@ NS_ASSUME_NONNULL_BEGIN if (bodyTextView) { [self.bubbleView addSubview:bodyTextView]; [self.viewConstraints addObjectsFromArray:@[ - [bodyTextView autoPinLeadingToSuperviewMarginWithInset:self.textLeadingMargin], - [bodyTextView autoPinTrailingToSuperviewMarginWithInset:self.textTrailingMargin], + [bodyTextView autoPinLeadingToSuperviewMarginWithInset:textInsets.leading], + [bodyTextView autoPinTrailingToSuperviewMarginWithInset:textInsets.trailing], ]]; [self.viewConstraints addObject:[bodyTextView autoSetDimension:ALDimensionHeight toSize:bodyTextContentSize.height]]; @@ -409,13 +412,13 @@ NS_ASSUME_NONNULL_BEGIN [self.viewConstraints addObject:[bodyTextView autoPinEdge:ALEdgeTop toEdge:ALEdgeBottom ofView:lastSubview - withOffset:self.textTopMargin]]; + withOffset:textInsets.top]]; } else { [self.viewConstraints - addObject:[bodyTextView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:self.textTopMargin]]; + addObject:[bodyTextView autoPinEdgeToSuperviewEdge:ALEdgeTop withInset:textInsets.top]]; } lastSubview = bodyTextView; - bottomMargin = self.textBottomMargin; + bottomMargin = textInsets.bottom; } UIView *_Nullable tapForMoreLabel = [self createTapForMoreLabelIfNecessary]; @@ -424,13 +427,13 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(lastSubview == bodyTextView); [self.bubbleView addSubview:tapForMoreLabel]; [self.viewConstraints addObjectsFromArray:@[ - [tapForMoreLabel autoPinLeadingToSuperviewMarginWithInset:self.textLeadingMargin], - [tapForMoreLabel autoPinTrailingToSuperviewMarginWithInset:self.textTrailingMargin], + [tapForMoreLabel autoPinLeadingToSuperviewMarginWithInset:textInsets.leading], + [tapForMoreLabel autoPinTrailingToSuperviewMarginWithInset:textInsets.trailing], [tapForMoreLabel autoPinEdge:ALEdgeTop toEdge:ALEdgeBottom ofView:lastSubview], [tapForMoreLabel autoSetDimension:ALDimensionHeight toSize:self.tapForMoreHeight], ]]; lastSubview = tapForMoreLabel; - bottomMargin = self.textBottomMargin; + bottomMargin = textInsets.bottom; } OWSAssert(lastSubview); @@ -859,7 +862,10 @@ NS_ASSUME_NONNULL_BEGIN return CGSizeZero; } - CGFloat hMargins = self.textTrailingMargin + self.textLeadingMargin; + OWSTextInsets *textInsets = self.layoutInfo.textInsets; + OWSAssert(textInsets); + + CGFloat hMargins = textInsets.leading + textInsets.trailing; const int maxTextWidth = (int)floor(self.layoutInfo.maxMessageWidth - hMargins); @@ -870,7 +876,7 @@ NS_ASSUME_NONNULL_BEGIN if (includeMargins) { result.width += hMargins; - result.height += self.textTopMargin + self.textBottomMargin; + result.height += textInsets.top + textInsets.bottom; } return CGSizeCeil(result); @@ -1018,28 +1024,6 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - -- (CGFloat)textLeadingMargin -{ - CGFloat result = kBubbleTextHInset; - return result; -} - -- (CGFloat)textTrailingMargin -{ - CGFloat result = kBubbleTextHInset; - return result; -} - -- (CGFloat)textTopMargin -{ - return kBubbleTextTopInset; -} - -- (CGFloat)textBottomMargin -{ - return kBubbleTextBottomInset; -} - - (UIColor *)bodyTextColor { return self.isIncoming ? [UIColor blackColor] : [UIColor whiteColor]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift index ddc4b2801..c8adfda1e 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift @@ -4,6 +4,29 @@ import Foundation +@objc +public class OWSTextInsets: NSObject { + + @objc public let leading: CGFloat + @objc public let trailing: CGFloat + @objc public let top: CGFloat + @objc public let bottom: CGFloat + + @objc + public required init(leading: CGFloat = 0, + trailing: CGFloat = 0, + top: CGFloat = 0, + bottom: CGFloat = 0) { + + self.leading = leading + self.trailing = trailing + self.top = top + self.bottom = bottom + + super.init() + } +} + @objc public class ConversationLayoutInfo: NSObject { @@ -40,6 +63,8 @@ public class ConversationLayoutInfo: NSObject { @objc public var maxMessageWidth: CGFloat = 0 @objc public var maxFooterWidth: CGFloat = 0 + @objc public var textInsets = OWSTextInsets() + @objc public required init(thread: TSThread) { @@ -49,8 +74,25 @@ public class ConversationLayoutInfo: NSObject { super.init() updateProperties() + + NotificationCenter.default.addObserver(self, + selector: #selector(uiContentSizeCategoryDidChange), + name: NSNotification.Name.UIContentSizeCategoryDidChange, + object: nil) + } + + deinit { + NotificationCenter.default.removeObserver(self) + } + + @objc func uiContentSizeCategoryDidChange() { + SwiftAssertIsOnMainThread(#function) + + updateProperties() } + // MARK: - + private func updateProperties() { if thread.isGroupThread() { gutterLeading = 40 @@ -70,5 +112,19 @@ public class ConversationLayoutInfo: NSObject { maxMessageWidth = floor(contentWidth * 0.8) // TODO: Should this be different? maxFooterWidth = maxMessageWidth - 10 + + let messageTextFont = UIFont.ows_dynamicTypeBody + // Don't include the distance from the "cap height" to the top of the UILabel + // in the top margin. + let textInsetTop = max(0, 12 - (messageTextFont.ascender - messageTextFont.capHeight)) + // Don't include the distance from the "baseline" to the bottom of the UILabel + // (e.g. the descender) in the top margin. Note that UIFont.descender is a + // negative value. + let textInsetBottom = max(0, 12 - abs(messageTextFont.descender)) + + textInsets = OWSTextInsets(leading: 12, + trailing: 12, + top: textInsetTop, + bottom: textInsetBottom) } } diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index ed36a186a..5ecc93616 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -346,7 +346,6 @@ class MessageDetailViewController: OWSViewController, MediaGalleryDataSourceDele messageBubbleView.viewItem = viewItem messageBubbleView.cellMediaCache = NSCache() messageBubbleView.layoutInfo = self.conversationLayoutInfo - messageBubbleView.alwaysShowBubbleTail = true messageBubbleView.configureViews() messageBubbleView.loadContent() From a31bd16d90eb34295f3df73ed9aeb8f5b9e3de23 Mon Sep 17 00:00:00 2001 From: Matthew Chen <matthew@signal.org> Date: Mon, 25 Jun 2018 13:53:35 -0400 Subject: [PATCH 2/3] Respond to CR. --- .../Cells/OWSMessageBubbleView.m | 4 ++-- .../ConversationLayoutInfo.swift | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 399f6731c..a85bf7cc4 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -390,7 +390,7 @@ NS_ASSUME_NONNULL_BEGIN } } - OWSTextInsets *textInsets = self.layoutInfo.textInsets; + OWSDirectionalEdgeInsets *textInsets = self.layoutInfo.textInsets; OWSAssert(textInsets); OWSMessageTextView *_Nullable bodyTextView = nil; @@ -862,7 +862,7 @@ NS_ASSUME_NONNULL_BEGIN return CGSizeZero; } - OWSTextInsets *textInsets = self.layoutInfo.textInsets; + OWSDirectionalEdgeInsets *textInsets = self.layoutInfo.textInsets; OWSAssert(textInsets); CGFloat hMargins = textInsets.leading + textInsets.trailing; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift index c8adfda1e..c120641de 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift @@ -5,7 +5,7 @@ import Foundation @objc -public class OWSTextInsets: NSObject { +public class OWSDirectionalEdgeInsets: NSObject { @objc public let leading: CGFloat @objc public let trailing: CGFloat @@ -13,10 +13,10 @@ public class OWSTextInsets: NSObject { @objc public let bottom: CGFloat @objc - public required init(leading: CGFloat = 0, - trailing: CGFloat = 0, - top: CGFloat = 0, - bottom: CGFloat = 0) { + public required init(top: CGFloat = 0, + leading: CGFloat = 0, + bottom: CGFloat = 0, + trailing: CGFloat = 0) { self.leading = leading self.trailing = trailing @@ -63,7 +63,7 @@ public class ConversationLayoutInfo: NSObject { @objc public var maxMessageWidth: CGFloat = 0 @objc public var maxFooterWidth: CGFloat = 0 - @objc public var textInsets = OWSTextInsets() + @objc public var textInsets = OWSDirectionalEdgeInsets() @objc public required init(thread: TSThread) { @@ -122,9 +122,9 @@ public class ConversationLayoutInfo: NSObject { // negative value. let textInsetBottom = max(0, 12 - abs(messageTextFont.descender)) - textInsets = OWSTextInsets(leading: 12, - trailing: 12, - top: textInsetTop, - bottom: textInsetBottom) + textInsets = OWSDirectionalEdgeInsets(top: textInsetTop, + leading: 12, + bottom: textInsetBottom, + trailing: 12) } } From 990bb81e4a036e82fdb71e0734889b0915474b60 Mon Sep 17 00:00:00 2001 From: Matthew Chen <matthew@signal.org> Date: Mon, 25 Jun 2018 14:39:48 -0400 Subject: [PATCH 3/3] Respond to CR. --- .../ConversationView/ConversationLayoutInfo.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift index c120641de..5e398ae02 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationLayoutInfo.swift @@ -25,6 +25,11 @@ public class OWSDirectionalEdgeInsets: NSObject { super.init() } + + static var zero = OWSDirectionalEdgeInsets(top: 0, + leading: 0, + bottom: 0, + trailing: 0) } @objc @@ -63,7 +68,7 @@ public class ConversationLayoutInfo: NSObject { @objc public var maxMessageWidth: CGFloat = 0 @objc public var maxFooterWidth: CGFloat = 0 - @objc public var textInsets = OWSDirectionalEdgeInsets() + @objc public var textInsets = OWSDirectionalEdgeInsets.zero @objc public required init(thread: TSThread) {