Use UITextView for better bulleted list presentation

ios/chrome/browser/ui/sad_tab/sad_tab_view.mm

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/sad_tab/sad_tab_view.h"
 
 #include "base/strings/sys_string_conversions.h"
 #include "components/grit/components_scaled_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
@@ skipping 12 matching lines @@
 #include "url/gurl.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
 namespace {
 // Color constants.
 const CGFloat kBackgroundColorBrightness = 247.0f / 255.0f;
 const CGFloat kTitleLabelTextColorBrightness = 22.0f / 255.0f;
-const CGFloat kMessageLabelTextColorBrightness = 80.0f / 255.0f;
+const CGFloat kGeneralTextColorBrightness = 80.0f / 255.0f;
 // Layout constants.
 const UIEdgeInsets kLayoutInsets = {24.0f, 24.0f, 24.0f, 24.0f};
 const CGFloat kLayoutBoundsMaxWidth = 600.0f;
 const CGFloat kContainerViewLandscapeTopPadding = 22.0f;
 const CGFloat kTitleLabelTopPadding = 26.0f;
-const CGFloat kMessageLabelTopPadding = 16.0f;
+const CGFloat kMessageTextViewTopPadding = 16.0f;
 const CGFloat kFooterLabelTopPadding = 16.0f;
 const CGFloat kActionButtonHeight = 48.0f;
 const CGFloat kActionButtonTopPadding = 16.0f;
 // Label font sizes.
 const CGFloat kTitleLabelFontSize = 23.0f;
-const CGFloat kMessageLabelFontSize = 14.0f;
+const CGFloat kMessageTextViewFontSize = 14.0f;
 const CGFloat kFooterLabelFontSize = 14.0f;
-// String constants for formatting bullets.
-// "<5xSpace><Bullet><4xSpace><Content>".
-NSString* const kMessageLabelBulletPrefix = @"     \u2022    ";
-// "<newline>".
-NSString* const kMessageLabelBulletSuffix = @"\n";
+// Feedback message bullet indentation.
+const CGFloat kBulletIndent = 17.0f;        // Left margin to bullet indent.
+const CGFloat kBulletedTextIndent = 15.0f;  // Bullet to text indent.
+// Format for bulleted line (<tab><bullet><tab><string>).
+NSString* const kMessageTextViewBulletPrefix = @"\t\u2022\t";
+// Separator for each new bullet line.
+NSString* const kMessageTextViewBulletSuffix = @"\n";
 // "<RTL Begin Indicator><NSString Token><RTL End Indicator>".
-NSString* const kMessageLabelBulletRTLFormat = @"\u202E%@\u202C";
+NSString* const kMessageTextViewBulletRTLFormat = @"\u202E%@\u202C";
 }  // namespace
 
 @interface SadTabView ()
 
 // Container view that displays all other subviews.
 @property(nonatomic, readonly, strong) UIView* containerView;
 // Displays the Sad Tab face.
 @property(nonatomic, readonly, strong) UIImageView* imageView;
 // Displays the Sad Tab title.
 @property(nonatomic, readonly, strong) UILabel* titleLabel;
 // Displays the Sad Tab message.
-@property(nonatomic, readonly, strong) UILabel* messageLabel;
+@property(nonatomic, readonly, strong) UITextView* messageTextView;
 // Displays the Sad Tab footer message (including a link to more help).
 @property(nonatomic, readonly, strong) UILabel* footerLabel;
 // Provides Link functionality to the footerLabel.
 @property(nonatomic, readonly, strong)
     LabelLinkController* footerLabelLinkController;
 // Triggers a reload or feedback action.
 @property(nonatomic, readonly, strong) MDCFlatButton* actionButton;
 // The bounds of |containerView|, with a height updated to CGFLOAT_MAX to allow
 // text to be laid out using as many lines as necessary.
 @property(nonatomic, readonly) CGRect containerBounds;
 // Allows this view to perform navigation actions such as reloading.
 @property(nonatomic, readonly) web::NavigationManager* navigationManager;
 
 // Subview layout methods.  Must be called in the following order, as subsequent
 // layouts reference the values set in previous functions.
 - (void)layoutImageView;
 - (void)layoutTitleLabel;
-- (void)layoutMessageLabel;
+- (void)layoutMessageTextView;
 - (void)layoutFooterLabel;
 - (void)layoutActionButton;
 - (void)layoutContainerView;
 
 // Takes an array of strings and bulletizes them into a single multi-line string
-// for display.
-+ (nonnull NSString*)bulletedStringFromStrings:
+// for display. The string has NSParagraphStyle attributes for tab alignment.
++ (nonnull NSAttributedString*)bulletedAttributedStringFromStrings:
     (nonnull NSArray<NSString*>*)strings;
 
 // Returns the appropriate title for the view, e.g. 'Aw Snap!'.
 - (nonnull NSString*)titleLabelText;
-// Returns the appropriate message label body for the view, this will typically
-// be a larger body of explanation or help text.
-- (nonnull NSString*)messageLabelText;
+// Returns the appropriate message text body for the view, this will typically
+// be a larger body of explanation or help text. Returns an attributed string
+// to allow for text formatting and layout to be applied to the returned string.
+- (nonnull NSAttributedString*)messageTextViewAttributedText;
 // Returns the full footer string containing a link, intended to be the last
 // piece of text.
 - (nonnull NSString*)footerLabelText;
 // Returns the substring of the footer string which is to be the underlined link
 // text. (May be the entire footer label string).
 - (nonnull NSString*)footerLinkText;
 // Returns the string to be used for the main action button.
 - (nonnull NSString*)buttonText;
 
 // Attaches a link controller to |label|, finding the |linkString|
 // within the |label| text to use as the link.
 - (void)attachLinkControllerToLabel:(nonnull UILabel*)label
                         forLinkText:(nonnull NSString*)linkText;
 
 // The action selector for |_actionButton|.
 - (void)handleActionButtonTapped:(id)sender;
 
 // Returns the desired background color.
 + (UIColor*)sadTabBackgroundColor;
 
 @end
 
 #pragma mark - SadTabView
 
 @implementation SadTabView
 
 @synthesize imageView = _imageView;
 @synthesize containerView = _containerView;
 @synthesize titleLabel = _titleLabel;
-@synthesize messageLabel = _messageLabel;
+@synthesize messageTextView = _messageTextView;
 @synthesize footerLabel = _footerLabel;
 @synthesize footerLabelLinkController = _footerLabelLinkController;
 @synthesize actionButton = _actionButton;
 @synthesize mode = _mode;
 @synthesize navigationManager = _navigationManager;
 
 - (instancetype)initWithMode:(SadTabViewMode)mode
            navigationManager:(web::NavigationManager*)navigationManager {
   self = [super initWithFrame:CGRectZero];
   if (self) {
@@ skipping 14 matching lines @@
   return nil;
 }
 
 - (instancetype)initWithCoder:(NSCoder*)aDecoder {
   NOTREACHED();
   return nil;
 }
 
 #pragma mark - Text Utilities
 
-+ (nonnull NSString*)bulletedStringFromStrings:
++ (nonnull NSAttributedString*)bulletedAttributedStringFromStrings:
     (nonnull NSArray<NSString*>*)strings {
   // Ensures the bullet string is appropriately directional.
   NSString* directionalBulletPrefix =
       base::i18n::IsRTL()
-          ? [NSString stringWithFormat:kMessageLabelBulletRTLFormat,
-                                       kMessageLabelBulletPrefix]
-          : kMessageLabelBulletPrefix;
-  NSMutableString* bulletedString = [NSMutableString string];
+          ? [NSString stringWithFormat:kMessageTextViewBulletRTLFormat,
+                                       kMessageTextViewBulletPrefix]
+          : kMessageTextViewBulletPrefix;
+
+  // Assemble the strings into a single string with each line preceded by a
+  // bullet point.
+  NSMutableString* bulletedString = [[NSMutableString string];

[pkl (ping after 24h if needed), 2017/05/26 21:51:17]

Looks like you have one too many (stray) [ here.

[PL, 2017/05/26 21:56:30]

Ha wow, thanks amazing spot! This was a last minute change I didn't compile
(obviously), I would like to change this back to [[NSMutableString alloc] init]
actually to avoid using the string convenience method as it's less definitive
what you will get back.

   for (NSString* string in strings) {
     // If content line has been added to the bulletedString already, ensure the
     // suffix is applied, otherwise don't (e.g. don't for the first item).
     NSArray* newStringArray =
         bulletedString.length
-            ? @[ kMessageLabelBulletSuffix, directionalBulletPrefix, string ]
+            ? @[ kMessageTextViewBulletSuffix, directionalBulletPrefix, string ]
             : @[ directionalBulletPrefix, string ];
     [bulletedString appendString:[newStringArray componentsJoinedByString:@""]];
   }
-  DCHECK(bulletedString);
-  return bulletedString;
+
+  // Prepare a paragraph style that will allow for the alignment of lines of
+  // text separately to the alignment of the bullet-points.
+  NSMutableParagraphStyle* paragraphStyle =
+      [[NSParagraphStyle defaultParagraphStyle] mutableCopy];
+  paragraphStyle.tabStops = @[
+    [[NSTextTab alloc] initWithTextAlignment:NSTextAlignmentNatural
+                                    location:kBulletIndent
+                                     options:@{}],
+    [[NSTextTab alloc] initWithTextAlignment:NSTextAlignmentNatural
+                                    location:kBulletIndent + kBulletedTextIndent
+                                     options:@{}]
+  ];
+  paragraphStyle.firstLineHeadIndent = 0.0f;
+  paragraphStyle.headIndent = kBulletIndent + kBulletedTextIndent;
+
+  // Use the paragraph style on the full string.
+  NSAttributedString* bulletedAttributedString = [[NSAttributedString alloc]
+      initWithString:bulletedString
+          attributes:@{NSParagraphStyleAttributeName : paragraphStyle}];
+
+  DCHECK(bulletedAttributedString);
+  return bulletedAttributedString;
 }
 
 #pragma mark - Label Text
 
 - (nonnull NSString*)titleLabelText {
   NSString* label = nil;
   switch (self.mode) {
     case SadTabViewMode::RELOAD:
       label = l10n_util::GetNSString(IDS_SAD_TAB_TITLE);
       break;
     case SadTabViewMode::FEEDBACK:
       label = l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_TITLE);
       break;
   }
   DCHECK(label);
   return label;
 }
 
-- (nonnull NSString*)messageLabelText {
-  NSString* label = nil;
+- (nonnull NSAttributedString*)messageTextViewAttributedText {
+  NSAttributedString* label = nil;
   switch (self.mode) {
     case SadTabViewMode::RELOAD:
-      label = l10n_util::GetNSString(IDS_SAD_TAB_MESSAGE);
+      label = [[NSAttributedString alloc]
+          initWithString:l10n_util::GetNSString(IDS_SAD_TAB_MESSAGE)];
       break;
-    case SadTabViewMode::FEEDBACK:
-      label = l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_TRY);
-      label = [label
-          stringByAppendingFormat:
-              @"\n\n%@", [[self class] bulletedStringFromStrings:@[
-                l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_CLOSE_NOTABS),
-                l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_INCOGNITO),
-                l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_RESTART_BROWSER),
-                l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_RESTART_DEVICE)
-              ]]];
+    case SadTabViewMode::FEEDBACK: {
+      NSMutableAttributedString* feedbackString =
+          [[NSMutableAttributedString alloc]
+              initWithString:[NSString
+                                 stringWithFormat:@"%@\n\n",
+                                                  l10n_util::GetNSString(
+                                                      IDS_SAD_TAB_RELOAD_TRY)]];

[kkhorimoto, 2017/05/26 21:40:22]

Can we break this out into some local variables to make the formatting a little
better?

[PL, 2017/05/26 21:56:30]

Absolutely, will do, thanks!

+      NSAttributedString* bulletedListString =
+          [[self class] bulletedAttributedStringFromStrings:@[
+            l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_CLOSE_NOTABS),
+            l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_INCOGNITO),
+            l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_RESTART_BROWSER),
+            l10n_util::GetNSString(IDS_SAD_TAB_RELOAD_RESTART_DEVICE)
+          ]];
+      [feedbackString appendAttributedString:bulletedListString];
+      label = feedbackString;
+    }
 
-      break;
+    break;

[pkl (ping after 24h if needed), 2017/05/26 21:51:17]

"break" should go inside the {} blocks, based on my read of C++ style guide
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

[PL, 2017/05/26 21:56:30]

Thanks! Will do.

   }
   DCHECK(label);
   return label;
 }
 
 - (nonnull NSString*)footerLabelText {
   NSString* label = nil;
   switch (self.mode) {
     case SadTabViewMode::RELOAD: {
       base::string16 footerLinkText(
@@ skipping 88 matching lines @@
     [_titleLabel setNumberOfLines:0];
     [_titleLabel
         setTextColor:[UIColor colorWithWhite:kTitleLabelTextColorBrightness
                                        alpha:1.0]];
     [_titleLabel setFont:[[MDFRobotoFontLoader sharedInstance]
                              regularFontOfSize:kTitleLabelFontSize]];
   }
   return _titleLabel;
 }
 
-- (UILabel*)messageLabel {
-  if (!_messageLabel) {
-    _messageLabel = [[UILabel alloc] initWithFrame:CGRectZero];
-    [_messageLabel setBackgroundColor:self.backgroundColor];
-    [_messageLabel setText:[self messageLabelText]];
-    [_messageLabel setLineBreakMode:NSLineBreakByWordWrapping];
-    [_messageLabel setNumberOfLines:0];
-    [_messageLabel
-        setTextColor:[UIColor colorWithWhite:kMessageLabelTextColorBrightness
+- (UITextView*)messageTextView {
+  if (!_messageTextView) {
+    _messageTextView = [[UITextView alloc] initWithFrame:CGRectZero];
+    [_messageTextView setBackgroundColor:self.backgroundColor];
+    [_messageTextView setAttributedText:[self messageTextViewAttributedText]];
+    _messageTextView.textContainer.lineFragmentPadding = 0.0f;
+    [_messageTextView
+        setTextColor:[UIColor colorWithWhite:kGeneralTextColorBrightness
                                        alpha:1.0]];
-    [_messageLabel setFont:[[MDFRobotoFontLoader sharedInstance]
-                               regularFontOfSize:kMessageLabelFontSize]];
+    [_messageTextView setFont:[[MDFRobotoFontLoader sharedInstance]
+                                  regularFontOfSize:kMessageTextViewFontSize]];
   }
-  return _messageLabel;
+  return _messageTextView;
 }
 
 - (UILabel*)footerLabel {
   if (!_footerLabel) {
     _footerLabel = [[UILabel alloc] initWithFrame:CGRectZero];
     [_footerLabel setBackgroundColor:self.backgroundColor];
     [_footerLabel setNumberOfLines:0];
     [_footerLabel setFont:[[MDFRobotoFontLoader sharedInstance]
                               regularFontOfSize:kFooterLabelFontSize]];
     [_footerLabel
-        setTextColor:[UIColor colorWithWhite:kMessageLabelTextColorBrightness
+        setTextColor:[UIColor colorWithWhite:kGeneralTextColorBrightness
                                        alpha:1.0]];
 
     [_footerLabel setText:[self footerLabelText]];
     [self attachLinkControllerToLabel:_footerLabel
                           forLinkText:[self footerLinkText]];
   }
   return _footerLabel;
 }
 
 - (UIButton*)actionButton {
@@ skipping 30 matching lines @@
   [super willMoveToSuperview:newSuperview];
 
   if (self.containerView.superview) {
     DCHECK_EQ(self.containerView.superview, self);
     return;
   }
 
   [self addSubview:self.containerView];
   [self.containerView addSubview:self.imageView];
   [self.containerView addSubview:self.titleLabel];
-  [self.containerView addSubview:self.messageLabel];
+  [self.containerView addSubview:self.messageTextView];
   [self.containerView addSubview:self.footerLabel];
 }
 
 - (void)layoutSubviews {
   [super layoutSubviews];
 
   [self layoutImageView];
   [self layoutTitleLabel];
-  [self layoutMessageLabel];
+  [self layoutMessageTextView];
   [self layoutFooterLabel];
   [self layoutActionButton];
   [self layoutContainerView];
 }
 
 - (CGSize)sizeThatFits:(CGSize)size {
   return size;
 }
 
 - (void)layoutImageView {
   LayoutRect imageViewLayout = LayoutRectZero;
   imageViewLayout.boundingWidth = CGRectGetWidth(self.containerBounds);
   imageViewLayout.size = self.imageView.bounds.size;
   self.imageView.frame =
       AlignRectOriginAndSizeToPixels(LayoutRectGetRect(imageViewLayout));
 }
 
 - (void)layoutTitleLabel {
   CGRect containerBounds = self.containerBounds;
   LayoutRect titleLabelLayout = LayoutRectZero;
   titleLabelLayout.boundingWidth = CGRectGetWidth(containerBounds);
   titleLabelLayout.size = [self.titleLabel sizeThatFits:containerBounds.size];
   titleLabelLayout.position.originY =
       CGRectGetMaxY(self.imageView.frame) + kTitleLabelTopPadding;
   self.titleLabel.frame =
       AlignRectOriginAndSizeToPixels(LayoutRectGetRect(titleLabelLayout));
 }
 
-- (void)layoutMessageLabel {
+- (void)layoutMessageTextView {
   CGRect containerBounds = self.containerBounds;
-  LayoutRect messageLabelLayout = LayoutRectZero;
-  messageLabelLayout.boundingWidth = CGRectGetWidth(containerBounds);
-  messageLabelLayout.size =
-      [self.messageLabel sizeThatFits:containerBounds.size];
-  messageLabelLayout.position.originY =
-      CGRectGetMaxY(self.titleLabel.frame) + kMessageLabelTopPadding;
-  self.messageLabel.frame =
-      AlignRectOriginAndSizeToPixels(LayoutRectGetRect(messageLabelLayout));
+  LayoutRect messageTextViewLayout = LayoutRectZero;
+  messageTextViewLayout.boundingWidth = CGRectGetWidth(containerBounds);
+  messageTextViewLayout.size =
+      [self.messageTextView sizeThatFits:containerBounds.size];
+  messageTextViewLayout.position.originY =
+      CGRectGetMaxY(self.titleLabel.frame) + kMessageTextViewTopPadding;
+  self.messageTextView.frame =
+      AlignRectOriginAndSizeToPixels(LayoutRectGetRect(messageTextViewLayout));
 }
 
 - (void)layoutFooterLabel {
   CGRect containerBounds = self.containerBounds;
   LayoutRect footerLabelLayout = LayoutRectZero;
   footerLabelLayout.boundingWidth = CGRectGetWidth(containerBounds);
   footerLabelLayout.size = [self.footerLabel sizeThatFits:containerBounds.size];
   footerLabelLayout.position.originY =
-      CGRectGetMaxY(self.messageLabel.frame) + kFooterLabelTopPadding;
+      CGRectGetMaxY(self.messageTextView.frame) + kFooterLabelTopPadding;
   self.footerLabel.frame =
       AlignRectOriginAndSizeToPixels(LayoutRectGetRect(footerLabelLayout));
 }
 
 - (void)layoutActionButton {
   CGRect containerBounds = self.containerBounds;
   BOOL isIPadIdiom = IsIPadIdiom();
   BOOL isPortrait = IsPortrait();
   BOOL shouldAddActionButtonToContainer = isIPadIdiom || !isPortrait;
   LayoutRect actionButtonLayout = LayoutRectZero;
@@ skipping 64 matching lines @@
       break;
     }
   };
 }
 
 + (UIColor*)sadTabBackgroundColor {
   return [UIColor colorWithWhite:kBackgroundColorBrightness alpha:1.0];
 }
 
 @end