Adding accessibility to the close button in sign-in promo

ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm

Index: ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm b/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm
index edf209f874ca54fdaa99eb4334d3301e6873fe0a..9be5ae3ab23294f9b10c431267a75f66d5df5250 100644
--- a/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm
+++ b/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.mm
@@ -13,11 +13,6 @@
 #error "This file requires ARC support."
 #endif
 
-namespace {
-// Close button size.
-const CGFloat kCloseButtonSize = 24;
-}
-
 @implementation BookmarkSigninPromoCell {
   SigninPromoView* _signinPromoView;
   UIButton* _closeButton;
@@ -43,19 +38,9 @@ const CGFloat kCloseButtonSize = 24;
     [contentView addSubview:_signinPromoView];
     AddSameConstraints(_signinPromoView, contentView);
 
-    _closeButton = [[UIButton alloc]
-        initWithFrame:CGRectMake(0, 0, kCloseButtonSize, kCloseButtonSize)];
-    [_closeButton addTarget:self
-                     action:@selector(closeButtonAction:)
-           forControlEvents:UIControlEventTouchUpInside];
-    _closeButton.translatesAutoresizingMaskIntoConstraints = NO;
-    [contentView addSubview:_closeButton];
-    [_closeButton setImage:[UIImage imageNamed:@"signin_promo_close_gray"]
-                  forState:UIControlStateNormal];
-    NSArray* buttonVisualConstraints =
-        @[ @"H:[closeButton]-|", @"V:|-[closeButton]" ];
-    NSDictionary* views = @{ @"closeButton" : _closeButton };
-    ApplyVisualConstraints(buttonVisualConstraints, views);
+    [_signinPromoView.closeButton addTarget:self

[msarda, 2017/05/24 08:47:58]

I think we should also make the close button visible here:
_signinPromoView.closeButton.hidden = NO;

[jlebel, 2017/05/24 11:20:17]

Done.

+                                     action:@selector(closeButtonAction:)
+                           forControlEvents:UIControlEventTouchUpInside];
 
     _signinPromoView.backgroundColor = [UIColor whiteColor];
     _signinPromoView.textLabel.text =
@@ -78,9 +63,15 @@ const CGFloat kCloseButtonSize = 24;
 
 - (void)prepareForReuse {

[msarda, 2017/05/24 08:47:58]

Just call [self setCloseButtonAction:nil] here.

[jlebel, 2017/05/24 11:20:17]

I guess there is no point for that, |_closeButtonAction = nil| is fine.

   _closeButtonAction = nil;
+  _signinPromoView.closeButton.hidden = NO;

[msarda, 2017/05/24 08:47:58]

This should probably be YES (in the header it says that the close button is
hidden when the closeButtonAction is nil).

However I think that for bookmarks the close button should always be visible,
right? So, I would just move _signinPromoView.closeButton.hidden = NO to the
initializer.

[jlebel, 2017/05/24 11:20:17]

Done.

   _signinPromoView.delegate = nil;
 }
 
+- (void)setCloseButtonAction:(CloseButtonCallback)closeButtonAction {
+  _closeButtonAction = [closeButtonAction copy];
+  _signinPromoView.closeButton.hidden = _closeButtonAction == nil;

[msarda, 2017/05/24 08:47:58]

I would remove this setter and  _signinPromoView.closeButton.hidden = NO to the
initializer.

[jlebel, 2017/05/24 11:20:17]

Done.

+}
+
 - (void)closeButtonAction:(id)sender {
   if (_closeButtonAction) {
     _closeButtonAction();