Suggestions UI - Add Data Source and Item

ios/chrome/browser/ui/suggestions/suggestions_item.mm

+// Copyright 2016 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/suggestions/suggestions_item.h"
+
+#import "ios/chrome/browser/ui/suggestions/suggestions_item_actions.h"
+#import "ios/third_party/material_components_ios/src/components/Palettes/src/MaterialPalettes.h"
+#import "ios/third_party/material_roboto_font_loader_ios/src/src/MaterialRobotoFontLoader.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+@implementation SuggestionsItem {
+  NSString* _title;
+  NSString* _subtitle;
+}
+
+- (instancetype)initWithType:(NSInteger)type
+                       title:(NSString*)title
+                    subtitle:(NSString*)subtitle {
+  self = [super initWithType:type];
+  if (self) {
+    self.cellClass = [SuggestionsCell class];
+    _title = title;

[lpromero, 2017/01/11 09:47:05]

You should [title copy] these. To be more idiomatic, I would make properties out
of these ivars. With the copy attribute.

[marq (ping after 24h), 2017/01/11 13:43:09]

+1 to properties, but you shouldn't use this class's properties in its init, so
you will still need to do _title = [title copy];

[gambard, 2017/01/11 15:10:38]

Acknowledged.

[gambard, 2017/01/11 15:10:38]

Done.

+    _subtitle = subtitle;
+  }
+  return self;
+}
+
+- (void)configureCell:(SuggestionsCell*)cell {
+  [super configureCell:cell];
+  cell.title = _title;
+  cell.detailText = _subtitle;
+}
+
+@end
+
+@implementation SuggestionsCell {
+  UIButton* _titleButton;
+  UILabel* _detailTextLabel;
+}
+
+@synthesize title = _title;
+@synthesize detailText = _detailText;
+
+- (instancetype)initWithFrame:(CGRect)frame {
+  self = [super initWithFrame:frame];
+  if (self) {
+    MDFRobotoFontLoader* fontLoader = [MDFRobotoFontLoader sharedInstance];
+    _titleButton = [UIButton buttonWithType:UIButtonTypeSystem];
+    _titleButton.titleLabel.font = [fontLoader mediumFontOfSize:16];
+    _titleButton.titleLabel.textColor = [[MDCPalette greyPalette] tint900];
+    [_titleButton addTarget:nil
+                     action:@selector(addItem:)
+           forControlEvents:UIControlEventTouchUpInside];
+
+    _detailTextLabel = [[UILabel alloc] init];
+    _detailTextLabel.font = [fontLoader mediumFontOfSize:14];
+    _detailTextLabel.textColor = [[MDCPalette greyPalette] tint500];
+
+    UIStackView* labelsStack = [[UIStackView alloc]

[lpromero, 2017/01/11 09:47:05]

I haven't been always happy with stack views in self sizing cells. If that
becomes an issue, prefer using constraints directly.

[gambard, 2017/01/11 15:10:38]

Acknowledged.

+        initWithArrangedSubviews:@[ _titleButton, _detailTextLabel ]];
+    labelsStack.axis = UILayoutConstraintAxisVertical;
+
+    [self.contentView addSubview:labelsStack];
+    labelsStack.layoutMarginsRelativeArrangement = YES;
+    labelsStack.layoutMargins = UIEdgeInsetsMake(16, 16, 16, 16);
+    labelsStack.alignment = UIStackViewAlignmentCenter;
+
+    labelsStack.translatesAutoresizingMaskIntoConstraints = NO;
+    [NSLayoutConstraint activateConstraints:@[
+      [self.contentView.leadingAnchor
+          constraintEqualToAnchor:labelsStack.leadingAnchor],
+      [self.contentView.trailingAnchor
+          constraintEqualToAnchor:labelsStack.trailingAnchor],
+      [self.contentView.topAnchor
+          constraintEqualToAnchor:labelsStack.topAnchor],
+      [self.contentView.bottomAnchor
+          constraintEqualToAnchor:labelsStack.bottomAnchor]
+

[lpromero, 2017/01/11 09:47:05]

Remove empty line.

[gambard, 2017/01/11 15:10:38]

Done.

+    ]];
+
+    self.backgroundColor = [UIColor whiteColor];
+  }
+  return self;
+}
+
+#pragma mark - Properties
+
+- (void)setTitle:(NSString*)title {
+  [_titleButton setTitle:title forState:UIControlStateNormal];

[lpromero, 2017/01/11 09:47:05]

Actually, do you need to store a local copy of the strings? Can't you just
provide a wrapper around the button and label one?

[gambard, 2017/01/11 15:10:38]

Done.

+  _title = title;

[lpromero, 2017/01/11 09:47:05]

Should be [title copy].

[gambard, 2017/01/11 15:10:38]

Removed.

+}
+
+- (void)setDetailText:(NSString*)detailText {
+  _detailTextLabel.text = detailText;
+  _detailText = detailText;

[lpromero, 2017/01/11 09:47:05]

Idem, should be [detailText copy].

[gambard, 2017/01/11 15:10:38]

Removed.

+}
+
+@end