[iOS] Info button inside SettingCheckCell

This change adds Information button to the SettingCheckCell. This button
will be used to display popovers with detailed information about error.
It will be used both by Safety and Password Checks.

Bug: 1075494
Change-Id: Ife3f06614b98929ca7df544032d284d3e62a063a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279883Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Cr-Commit-Position: refs/heads/master@{#788536}

ios/chrome/browser/ui/settings/cells/settings_check_cell.h

@@ -11,14 +11,19 @@
 
 // Cell representation for SettingsCheckItem.
 //  +---------------------------------------------------------+
-//  |                                           +--------+    |
-//  | +--------+                                |trailing|    |
-//  | | leading|  One line title                |image or|    |
-//  | | image  |  Multiline detail text         |spinner |    |
-//  | +--------+                                +--------+    |
+//  | +--------+                                +---------+   |
+//  | |        |  One line title                |trailing |   |
+//  | | leading|                                |image    |   |
+//  | | image  |  Multiline detail text         |spinner  |   |
+//  | |        |  Multiline detail text         |or button|   |
+//  | +--------+                                +---------+   |
 //  +---------------------------------------------------------+
 @interface SettingsCheckCell : TableViewCell
 
+// Button which is used as an anchor to show popover with additional
+// information.
+@property(nonatomic, readonly, strong) UIButton* infoButton;
+
 // Shows |activityIndicator| and starts animation. It will hide |imageView| if
 // it was shown.
 - (void)showActivityIndicator;
@@ -39,6 +44,9 @@
 - (void)setLeadingImage:(UIImage*)leadingImage
           withTintColor:(UIColor*)tintColor;
 
+// Shows/Hides |infoButton|.
+- (void)setInfoButtonHidden:(BOOL)hidden;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_SETTINGS_CELLS_SETTINGS_CHECK_CELL_H_

ios/chrome/browser/ui/settings/cells/settings_check_cell.mm

@@ -8,6 +8,7 @@
 #include "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
 #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
 #import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h"
+#import "ios/chrome/common/ui/colors/semantic_color_names.h"
 #import "ios/chrome/common/ui/util/constraints_ui_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -37,12 +38,12 @@ const CGFloat kIconImageSize = 28;
 @property(nonatomic, strong) UIImageView* leadingImageView;
 
 // Constraint that is used to define trailing text constraint without
-// |trailingImageView| or |activityIndicator|.
+// |trailingImageView| |activityIndicator| and |infoButton|.
 @property(nonatomic, strong)
     NSLayoutConstraint* textNoTrailingContentsConstraint;
 
 // Constraint that is used to define trailing text constraint with either
-// |trailingImageView| or |activityIndicator| showing.
+// |trailingImageView| or |activityIndicator| or |infoButton| showing.
 @property(nonatomic, strong)
     NSLayoutConstraint* textWithTrailingContentsConstraint;
 
@@ -92,8 +93,8 @@ const CGFloat kIconImageSize = 28;
     _detailTextLabel.textColor = UIColor.cr_secondaryLabelColor;
     [contentView addSubview:_detailTextLabel];
 
-    // Only |_trailingImageView| or |_activityIndicator| is shown, not both at
-    // once. |trailingImage| attributes.
+    // Only |_trailingImageView| or |_activityIndicator| or |_infoButton| is
+    // shown, not all at once. |trailingImage| attributes.
     _trailingImageView = [[UIImageView alloc] init];
     _trailingImageView.translatesAutoresizingMaskIntoConstraints = NO;
     _trailingImageView.tintColor = UIColor.cr_labelColor;
@@ -104,6 +105,15 @@ const CGFloat kIconImageSize = 28;
     _activityIndicator.translatesAutoresizingMaskIntoConstraints = NO;
     _activityIndicator.hidden = YES;
     [contentView addSubview:_activityIndicator];
+    // |_infoButton| attribues.
+    _infoButton = [UIButton buttonWithType:UIButtonTypeSystem];
+    _infoButton.translatesAutoresizingMaskIntoConstraints = NO;
+    _infoButton.hidden = YES;
+    UIImage* image = [[UIImage imageNamed:@"settings_info"]
+        imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
+    [_infoButton setImage:image forState:UIControlStateNormal];
+    [_infoButton setTintColor:[UIColor colorNamed:kBlueColor]];
+    [contentView addSubview:_infoButton];
 
     // Constraints.
     UILayoutGuide* textLayoutGuide = [[UILayoutGuide alloc] init];
@@ -147,6 +157,18 @@ const CGFloat kIconImageSize = 28;
       [_trailingImageView.leadingAnchor
           constraintEqualToAnchor:_activityIndicator.leadingAnchor],
 
+      // Constraints for |_infoButton| (same position as
+      // |_trailingImageView|).
+      [_infoButton.trailingAnchor
+          constraintEqualToAnchor:self.contentView.trailingAnchor
+                         constant:-kTableViewHorizontalSpacing],
+      [_infoButton.widthAnchor constraintEqualToConstant:kIconImageSize],
+      [_infoButton.heightAnchor constraintEqualToConstant:kIconImageSize],
+      [_infoButton.centerYAnchor
+          constraintEqualToAnchor:textLayoutGuide.centerYAnchor],
+      [_infoButton.leadingAnchor
+          constraintEqualToAnchor:_activityIndicator.leadingAnchor],
+
       // Constraints for |_activityIndictor| (same position as
       // |_trailingImageView|).
       [_activityIndicator.trailingAnchor
@@ -194,8 +216,8 @@ const CGFloat kIconImageSize = 28;
 - (void)showActivityIndicator {
   if (!self.activityIndicator.hidden)
     return;
-
   self.trailingImageView.hidden = YES;
+  self.infoButton.hidden = YES;
   self.activityIndicator.hidden = NO;
   [self.activityIndicator startAnimating];
   [self updateTrailingImageTextConstraints];
@@ -219,7 +241,8 @@ const CGFloat kIconImageSize = 28;
     return;
   self.trailingImageView.hidden = hidden;
   if (!hidden) {
-    [self hideActivityIndicator];
+    self.activityIndicator.hidden = YES;
+    self.infoButton.hidden = YES;
   }
   [self updateTrailingImageTextConstraints];
 }
@@ -240,14 +263,27 @@ const CGFloat kIconImageSize = 28;
   }
 }
 
+- (void)setInfoButtonHidden:(BOOL)hidden {
+  if (hidden == self.infoButton.hidden)
+    return;
+
+  self.infoButton.hidden = hidden;
+  if (!hidden) {
+    self.trailingImageView.hidden = YES;
+    self.activityIndicator.hidden = YES;
+  }
+  [self updateTrailingImageTextConstraints];
+}
+
 #pragma mark - Private Methods
 
 // Updates the constraints around the trailing image for when |trailingImage| or
-// |activityIndicator| is shown or hidden.
+// |activityIndicator| or |infoButton| is shown or hidden.
 - (void)updateTrailingImageTextConstraints {
   // Active proper |textLayoutGuide| trailing constraint to show
-  // |trailingImageView| or |activityIndicator|.
-  if (self.activityIndicator.hidden && self.trailingImageView.hidden) {
+  // |trailingImageView| or |activityIndicator| or |infoButton|.
+  if (self.activityIndicator.hidden && self.trailingImageView.hidden &&
+      self.infoButton.hidden) {
     _textWithTrailingContentsConstraint.active = NO;
     _textNoTrailingContentsConstraint.active = YES;
   } else {
@@ -262,6 +298,9 @@ const CGFloat kIconImageSize = 28;
   [super prepareForReuse];
 
   self.textLabel.text = nil;
+  [self.infoButton removeTarget:nil
+                         action:nil
+               forControlEvents:UIControlEventAllEvents];
   self.detailTextLabel.text = nil;
   self.accessibilityTraits = UIAccessibilityTraitNone;
   [self setTrailingImage:nil withTintColor:nil];

ios/chrome/browser/ui/settings/cells/settings_check_item.h

@@ -29,18 +29,22 @@
 // The image to display on the trailing side of |text| (required). If this image
 // should be tinted to match the text color (e.g. in dark mode), the provided
 // image should have rendering mode UIImageRenderingModeAlwaysTemplate. Don't
-// set image with |isIndicatorHidden| as only either image or
-// |activityIndicator| will be shown.
+// set image with |isIndicatorHidden| equal to false as image won't be shown
+// in that case.
 @property(nonatomic, strong) UIImage* trailingImage;
 
 // Tint color for |trailingImage|.
 @property(nonatomic, copy) UIColor* trailingImageTintColor;
 
-// Controls visibility of |activityIndicator|, if set true |imageView| will be
-// hidden and activity indicator will be shown. In case both image is provided
-// and this property set to false, only |activityIndicator| will be shown.
+// Controls visibility of |activityIndicator|, if set false |trailingImage| or
+// |infoButton| will be hidden and |activityIndicator| will be shown. This
+// property has the highest priority.
 @property(nonatomic, assign, getter=isIndicatorHidden) BOOL indicatorHidden;
 
+// Controls visibility of |infoButton|. This property has no effect in case
+// |trailingImage| is provided or |indicatorHidden| is false.
+@property(nonatomic, assign, getter=isInfoButtonHidden) BOOL infoButtonHidden;
+
 // Disabled cell are automatically drawn with dimmed text and without
 // |trailingImage| or |activityIndicator|.
 @property(nonatomic, assign, getter=isEnabled) BOOL enabled;

ios/chrome/browser/ui/settings/cells/settings_check_item.mm

@@ -31,6 +31,7 @@
   cell.detailTextLabel.text = self.detailText;
   cell.selectionStyle = UITableViewCellSelectionStyleNone;
   if (self.enabled) {
+    [cell setInfoButtonHidden:self.infoButtonHidden];
     [cell setLeadingImage:self.leadingImage
             withTintColor:self.leadingImageTintColor];
     [cell setTrailingImage:self.trailingImage
@@ -44,6 +45,7 @@
             withTintColor:UIColor.cr_secondaryLabelColor];
     [cell setTrailingImage:nil withTintColor:nil];
     [cell hideActivityIndicator];
+    [cell setInfoButtonHidden:YES];
     cell.textLabel.textColor = UIColor.cr_secondaryLabelColor;
     cell.accessibilityTraits |= UIAccessibilityTraitNotEnabled;
   }

ios/chrome/browser/ui/settings/cells/settings_check_item_unittest.mm

@@ -44,4 +44,43 @@ TEST_F(SettingsCheckItemTest, ConfigureCell) {
   EXPECT_NSEQ(detailText, CheckCell.detailTextLabel.text);
 }
 
+// Tests that cell is configured properly based on infoButtonHidden property of
+// the item.
+TEST_F(SettingsCheckItemTest, InfoButtonVisibility) {
+  SettingsCheckItem* item = [[SettingsCheckItem alloc] initWithType:0];
+  item.text = @"Test Text";
+  item.detailText = @"Test Text";
+  item.enabled = YES;
+  item.indicatorHidden = YES;
+  item.infoButtonHidden = NO;
+
+  id cell = [[[item cellClass] alloc] init];
+  SettingsCheckCell* CheckCell =
+      base::mac::ObjCCastStrict<SettingsCheckCell>(cell);
+
+  [item configureCell:cell withStyler:[[ChromeTableViewStyler alloc] init]];
+  EXPECT_FALSE(CheckCell.infoButton.hidden);
+
+  item.infoButtonHidden = YES;
+  [item configureCell:cell withStyler:[[ChromeTableViewStyler alloc] init]];
+  EXPECT_TRUE(CheckCell.infoButton.hidden);
+}
+
+// Tests that infoButton won't be shown in case of a conflict.
+TEST_F(SettingsCheckItemTest, InfoButtonVisibilityDuringConflict) {
+  SettingsCheckItem* item = [[SettingsCheckItem alloc] initWithType:0];
+  item.text = @"Test Text";
+  item.detailText = @"Test Text";
+  item.enabled = YES;
+  item.indicatorHidden = NO;
+  item.infoButtonHidden = NO;
+
+  id cell = [[[item cellClass] alloc] init];
+  SettingsCheckCell* CheckCell =
+      base::mac::ObjCCastStrict<SettingsCheckCell>(cell);
+
+  [item configureCell:cell withStyler:[[ChromeTableViewStyler alloc] init]];
+  EXPECT_TRUE(CheckCell.infoButton.hidden);
+}
+
 }  // namespace

ios/chrome/browser/ui/settings/table_cell_catalog_view_controller.mm

@@ -12,6 +12,7 @@
 #import "ios/chrome/browser/ui/icons/chrome_icon.h"
 #import "ios/chrome/browser/ui/settings/cells/account_sign_in_item.h"
 #import "ios/chrome/browser/ui/settings/cells/copied_to_chrome_item.h"
+#import "ios/chrome/browser/ui/settings/cells/settings_check_cell.h"
 #import "ios/chrome/browser/ui/settings/cells/settings_check_item.h"
 #import "ios/chrome/browser/ui/settings/cells/settings_image_detail_text_item.h"
 #import "ios/chrome/browser/ui/settings/cells/settings_switch_item.h"
@@ -84,6 +85,7 @@ typedef NS_ENUM(NSInteger, ItemType) {
   ItemTypeCheck3,
   ItemTypeCheck4,
   ItemTypeCheck5,
+  ItemTypeCheck6,
 };
 }
 
@@ -412,8 +414,8 @@ typedef NS_ENUM(NSInteger, ItemType) {
       @"description.";
   checkFinished.enabled = YES;
   checkFinished.indicatorHidden = YES;
-  checkFinished.trailingImage = [[ChromeIcon infoIcon]
-      imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
+  checkFinished.trailingImage =
+      [UIImage imageNamed:@"table_view_cell_check_mark"];
   [model addItem:checkFinished
       toSectionWithIdentifier:SectionIdentifierSettings];
 
@@ -426,9 +428,9 @@ typedef NS_ENUM(NSInteger, ItemType) {
   checkFinishedWithLeadingImage.leadingImage = [[ChromeIcon infoIcon]
       imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
   checkFinishedWithLeadingImage.enabled = YES;
-  checkFinishedWithLeadingImage.indicatorHidden = NO;
-  checkFinishedWithLeadingImage.trailingImage = [[ChromeIcon infoIcon]
-      imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate];
+  checkFinishedWithLeadingImage.indicatorHidden = YES;
+  checkFinishedWithLeadingImage.trailingImage =
+      [UIImage imageNamed:@"table_view_cell_check_mark"];
   [model addItem:checkFinishedWithLeadingImage
       toSectionWithIdentifier:SectionIdentifierSettings];
 
@@ -454,6 +456,18 @@ typedef NS_ENUM(NSInteger, ItemType) {
   [model addItem:checkDisabledWithLeadingImage
       toSectionWithIdentifier:SectionIdentifierSettings];
 
+  SettingsCheckItem* checkWithInfoButton =
+      [[SettingsCheckItem alloc] initWithType:ItemTypeCheck6];
+  checkWithInfoButton.text = @"Check item with info ";
+  checkWithInfoButton.detailText =
+      @"This is very long description of check item. Another line of "
+      @"description.";
+  checkWithInfoButton.enabled = YES;
+  checkWithInfoButton.indicatorHidden = YES;
+  checkWithInfoButton.infoButtonHidden = NO;
+  [model addItem:checkWithInfoButton
+      toSectionWithIdentifier:SectionIdentifierSettings];
+
   TableViewLinkHeaderFooterItem* linkFooter =
       [[TableViewLinkHeaderFooterItem alloc] initWithType:ItemTypeLinkFooter];
   linkFooter.text =
@@ -570,6 +584,26 @@ typedef NS_ENUM(NSInteger, ItemType) {
       UIPopoverArrowDirectionAny;
 }
 
+// Called when the user clicks on the information button of the check item
+// setting's UI. Shows a textual bubble with the detailed information.
+- (void)didTapCheckInfoButton:(UIButton*)buttonView {
+  PopoverLabelViewController* popoverViewController =
+      [[PopoverLabelViewController alloc]
+          initWithMessage:@"You clicked settings check item. Here you can see "
+                          @"detailed information."];
+
+  // Set the anchor and arrow direction of the bubble.
+  popoverViewController.popoverPresentationController.sourceView = buttonView;
+  popoverViewController.popoverPresentationController.sourceRect =
+      buttonView.bounds;
+  popoverViewController.popoverPresentationController.permittedArrowDirections =
+      UIPopoverArrowDirectionAny;
+
+  [self presentViewController:popoverViewController
+                     animated:YES
+                   completion:nil];
+}
+
 #pragma mark - UITableViewDataSource
 
 - (UITableViewCell*)tableView:(UITableView*)tableView
@@ -586,6 +620,12 @@ typedef NS_ENUM(NSInteger, ItemType) {
     [managedCell.trailingButton addTarget:self
                                    action:@selector(didTapManagedUIInfoButton:)
                          forControlEvents:UIControlEventTouchUpInside];
+  } else if (itemType == ItemTypeCheck6) {
+    SettingsCheckCell* checkCell =
+        base::mac::ObjCCastStrict<SettingsCheckCell>(cell);
+    [checkCell.infoButton addTarget:self
+                             action:@selector(didTapCheckInfoButton:)
+                   forControlEvents:UIControlEventTouchUpInside];
   }
   return cell;
 }