Refactor media galleries dialogs on cocoa to extract common code

chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.h"
 
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/ui/chrome_style.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.h"
@@ skipping 10 matching lines @@
 #include "ui/base/resource/resource_bundle.h"
 
 // Controller for UI events on items in the media galleries dialog.
 @interface MediaGalleriesCocoaController : NSObject {
  @private
   MediaGalleriesDialogCocoa* dialog_;
 }
 
 @property(assign, nonatomic) MediaGalleriesDialogCocoa* dialog;
 
+- (void)onAcceptButton:(id)sender;
+- (void)onCancelButton:(id)sender;
+- (void)onAddFolderClicked:(id)sender;
+
 @end
 
 @implementation MediaGalleriesCocoaController
 
 @synthesize dialog = dialog_;
 
 - (void)onAcceptButton:(id)sender {
   dialog_->OnAcceptClicked();
 }
 
 - (void)onCancelButton:(id)sender {
   dialog_->OnCancelClicked();
 }
 
 - (void)onAddFolderClicked:(id)sender {
   DCHECK(dialog_);
   dialog_->OnAddFolderClicked();
 }
 
-- (void)onCheckboxToggled:(id)sender {
-  DCHECK(dialog_);
-  dialog_->OnCheckboxToggled(sender);
-}
-
-@end
-
-
-@interface MediaGalleriesCheckbox : NSButton {
- @private
-  MediaGalleriesDialogCocoa* dialog_;  // |dialog_| owns |this|.
-  GalleryDialogId galleryId_;
-  base::scoped_nsobject<MenuController> menuController_;
-}
-
-- (id)initWithFrame:(NSRect)frameRect
-             dialog:(MediaGalleriesDialogCocoa*)dialog
-          galleryId:(GalleryDialogId)galleryId;
-- (NSMenu*)menuForEvent:(NSEvent*)theEvent;
-
-@end
-
-@implementation MediaGalleriesCheckbox
-
-- (id)initWithFrame:(NSRect)frameRect
-             dialog:(MediaGalleriesDialogCocoa*)dialog
-          galleryId:(GalleryDialogId)galleryId {
-  if ((self = [super initWithFrame:frameRect])) {
-    dialog_ = dialog;
-    galleryId_ = galleryId;
-  }
-  return self;
-}
-
-- (NSMenu*)menuForEvent:(NSEvent*)theEvent {
-  menuController_.reset(
-    [[MenuController alloc] initWithModel:dialog_->GetContextMenu(galleryId_)
-                   useWithPopUpButtonCell:NO]);
-  return [menuController_ menu];
-}
-
 @end
 
 namespace {
 
 const CGFloat kCheckboxMargin = 10;
 const CGFloat kCheckboxMaxWidth = 440;
 const CGFloat kScrollAreaHeight = 220;
 
 }  // namespace
 
 MediaGalleriesDialogCocoa::MediaGalleriesDialogCocoa(
     MediaGalleriesDialogController* controller,
     MediaGalleriesCocoaController* cocoa_controller)
     : controller_(controller),
       accepted_(false),
       cocoa_controller_([cocoa_controller retain]) {
   [cocoa_controller_ setDialog:this];
 
   alert_.reset([[ConstrainedWindowAlert alloc] init]);
 
   [alert_ setMessageText:base::SysUTF16ToNSString(controller_->GetHeader())];
   [alert_ setInformativeText:
       base::SysUTF16ToNSString(controller_->GetSubtext())];
   [alert_ addButtonWithTitle:
-    l10n_util::GetNSString(IDS_MEDIA_GALLERIES_DIALOG_CONFIRM)
+      l10n_util::GetNSString(IDS_MEDIA_GALLERIES_DIALOG_CONFIRM)
                keyEquivalent:kKeyEquivalentReturn
                       target:cocoa_controller_
                       action:@selector(onAcceptButton:)];
   [alert_ addButtonWithTitle:
       l10n_util::GetNSString(IDS_MEDIA_GALLERIES_DIALOG_CANCEL)
                keyEquivalent:kKeyEquivalentEscape
                       target:cocoa_controller_
                       action:@selector(onCancelButton:)];
   [alert_ addButtonWithTitle:
       l10n_util::GetNSString(IDS_MEDIA_GALLERIES_DIALOG_ADD_GALLERY)
@@ skipping 35 matching lines @@
       [[NSScrollView alloc] initWithFrame:
           NSMakeRect(0, 0, kCheckboxMaxWidth, kScrollAreaHeight)]);
   [scroll_view setHasVerticalScroller:YES];
   [scroll_view setHasHorizontalScroller:NO];
   [scroll_view setBorderType:NSNoBorder];
   [scroll_view setAutohidesScrollers:YES];
   [[main_container_ contentView] addSubview:scroll_view];
 
   // Add gallery permission checkboxes inside the scrolling view.
   checkbox_container_.reset([[FlippedView alloc] initWithFrame:NSZeroRect]);
-  checkboxes_.reset([[NSMutableArray alloc] init]);
-  [scroll_view setDocumentView:checkbox_container_];
 
-  CGFloat y_pos = 0;
-
-  y_pos = CreateCheckboxes(y_pos, controller_->AttachedPermissions());
+  CGFloat y_pos = CreateCheckboxes(0, controller_->AttachedPermissions());
 
   if (!controller_->UnattachedPermissions().empty()) {
     y_pos = CreateCheckboxSeparator(y_pos);
     y_pos = CreateCheckboxes(y_pos, controller_->UnattachedPermissions());
   }
 
-  [checkbox_container_ setFrame:NSMakeRect(0, 0, kCheckboxMaxWidth, y_pos + 2)];
+  // Give the container a reasonable initial size so that the scroll_view can
+  // figure out the content size.
+  [checkbox_container_ setFrame:NSMakeRect(0, 0, kCheckboxMaxWidth, y_pos)];
+  [scroll_view setDocumentView:checkbox_container_];
+
+  NSRect checkboxes_frame =
+      NSMakeRect(0, 0, [scroll_view contentSize].width, y_pos);
+  [checkbox_container_ setFrame:checkboxes_frame];
 
   // Resize to pack the scroll view if possible.
   NSRect scroll_frame = [scroll_view frame];
-  if (NSHeight(scroll_frame) > NSHeight([checkbox_container_ frame])) {
-    scroll_frame.size.height = NSHeight([checkbox_container_ frame]);
+  if (NSHeight(scroll_frame) > NSHeight(checkboxes_frame)) {
+    scroll_frame.size.height = NSHeight(checkboxes_frame);
     [scroll_view setFrame:scroll_frame];
   }
 
   [main_container_ setFrameFromContentFrame:scroll_frame];
+  NSRect main_frame = [main_container_ frame];

[groby-ooo-7-16, 2014/04/23 21:57:58]

[main_container_ setFrameOrigin:NSZeroPoint];

[vandebo (ex-Chrome), 2014/04/25 20:10:56]

Done.

+  main_frame.origin = NSZeroPoint;
+  [main_container_ setFrame:main_frame];
   [alert_ setAccessoryView:main_container_];
 
   // As a safeguard against the user skipping reading over the dialog and just
   // confirming, the button will be unavailable for dialogs without any checks
   // until the user toggles something.
   [[[alert_ buttons] objectAtIndex:0] setEnabled:
       controller_->IsAcceptAllowed()];
 
   [alert_ layout];
 }
 
 CGFloat MediaGalleriesDialogCocoa::CreateCheckboxes(
     CGFloat y_pos,
     const MediaGalleriesDialogController::GalleryPermissionsVector&
         permissions) {
-  y_pos += kCheckboxMargin;
-
   for (MediaGalleriesDialogController::GalleryPermissionsVector::
        const_iterator iter = permissions.begin();
        iter != permissions.end(); iter++) {
     const MediaGalleriesDialogController::GalleryPermission& permission = *iter;
-    UpdateGalleryCheckbox(permission, y_pos);
-    y_pos = NSMaxY([[checkboxes_ lastObject] frame]) + kCheckboxMargin;
+    base::scoped_nsobject<MediaGalleryListEntry> checkbox_entry(
+        [[MediaGalleryListEntry alloc]
+            initWithFrame:NSZeroRect
+               controller:this
+                  prefId:permission.gallery_id

[groby-ooo-7-16, 2014/04/23 21:57:58]

I'd suggest just initializing it with the permission, and teasing it apart in
init - avoids endless parameter lists :)

[vandebo (ex-Chrome), 2014/04/25 20:10:56]

For the two call sites, there are two of these parameters that are handled
differently, prefId and showFolderViewer, so I could pass in the pref_info in
addition to those two things, which saves two parameters. The reason I haven't
already done that is that pref_info has a prefId field (matches in one instance,
but not in the other). For that reason my preference would be to not pass in
pref_info.  

I could move some of the parameters to setters, but it would increase the code
length a bit in order to properly handle all combinations.

+             galleryName:permission.pref_info.GetGalleryDisplayName()
+               subscript:permission.pref_info.GetGalleryAdditionalDetails()
+                 tooltip:permission.pref_info.GetGalleryTooltip()
+        showFolderViewer:false]);
+
+    [checkbox_entry setState:permission.allowed];

[groby-ooo-7-16, 2014/04/23 21:57:58]

... which'd also allow to roll the setState: into init.

+
+    NSRect entry_rect = [checkbox_entry frame];
+    entry_rect.origin.y = y_pos;
+    [checkbox_entry setFrame:entry_rect];

[groby-ooo-7-16, 2014/04/23 21:57:58]

setFrameOrigin: does just set the origin.

[vandebo (ex-Chrome), 2014/04/25 20:10:56]

Done.

+    [checkbox_container_ addSubview:checkbox_entry];
+    y_pos = NSMaxY(entry_rect) + kCheckboxMargin;
   }
 
   return y_pos;
 }
 
 CGFloat MediaGalleriesDialogCocoa::CreateCheckboxSeparator(CGFloat y_pos) {
   base::scoped_nsobject<NSBox> separator(
       [[NSBox alloc] initWithFrame:NSMakeRect(
           0, y_pos + kCheckboxMargin * 0.5, kCheckboxMaxWidth, 1.0)]);
   [separator setBoxType:NSBoxSeparator];
@@ skipping 35 matching lines @@
 
 void MediaGalleriesDialogCocoa::OnCancelClicked() {
   if (window_)
     window_->CloseWebContentsModalDialog();
 }
 
 void MediaGalleriesDialogCocoa::OnAddFolderClicked() {
   controller_->OnAddFolderClicked();
 }
 
-void MediaGalleriesDialogCocoa::OnCheckboxToggled(NSButton* checkbox) {
-  GalleryDialogId gallery_id =
-      [[[checkbox cell] representedObject] longLongValue];
-  controller_->DidToggleGallery(gallery_id, [checkbox state] == NSOnState);
-
-  [[[alert_ buttons] objectAtIndex:0] setEnabled:
-      controller_->IsAcceptAllowed()];
-}
-
-void MediaGalleriesDialogCocoa::UpdateGalleryCheckbox(
-    const MediaGalleriesDialogController::GalleryPermission& gallery,
-    CGFloat y_pos) {
-  // Checkbox.
-  base::scoped_nsobject<MediaGalleriesCheckbox> checkbox(
-      [[MediaGalleriesCheckbox alloc] initWithFrame:NSZeroRect
-                                             dialog:this
-                                          galleryId:gallery.gallery_id]);
-  NSNumber* gallery_id_object =
-      [NSNumber numberWithLongLong:gallery.gallery_id];
-  [[checkbox cell] setRepresentedObject:gallery_id_object];
-  [[checkbox cell] setLineBreakMode:NSLineBreakByTruncatingMiddle];
-  [checkbox setButtonType:NSSwitchButton];
-  [checkbox setTarget:cocoa_controller_];
-  [checkbox setAction:@selector(onCheckboxToggled:)];
-  [checkboxes_ addObject:checkbox];
-
-  [checkbox setTitle:base::SysUTF16ToNSString(
-      gallery.pref_info.GetGalleryDisplayName())];
-  [checkbox setToolTip:base::SysUTF16ToNSString(
-      gallery.pref_info.GetGalleryTooltip())];
-  [checkbox setState:gallery.allowed ? NSOnState : NSOffState];
-
-  [checkbox sizeToFit];
-  NSRect rect = [checkbox bounds];
-
-  // Detail text.
-  base::scoped_nsobject<NSTextField> details(
-      [[NSTextField alloc] initWithFrame:NSZeroRect]);
-  [details setEditable:NO];
-  [details setSelectable:NO];
-  [details setBezeled:NO];
-  [details setAttributedStringValue:
-      constrained_window::GetAttributedLabelString(
-          base::SysUTF16ToNSString(
-              gallery.pref_info.GetGalleryAdditionalDetails()),
-          chrome_style::kTextFontStyle,
-          NSNaturalTextAlignment,
-          NSLineBreakByClipping
-      )];
-  [details setTextColor:[NSColor colorWithCalibratedRed:0.625
-                                                  green:0.625
-                                                   blue:0.625
-                                                  alpha:1.0]];
-  [details sizeToFit];
-  NSRect details_rect = [details bounds];
-
-  // The checkbox will elide so reduce its size so it will all fit.
-  rect.size.width =
-      std::min(NSWidth(rect),
-               kCheckboxMaxWidth - 3 * kCheckboxMargin - NSWidth(details_rect));
-  rect.origin = NSMakePoint(kCheckboxMargin, y_pos);
-  [checkbox setFrame:rect];
-  [checkbox_container_ addSubview:checkbox];
-
-  details_rect.origin = NSMakePoint(NSMaxX(rect) + kCheckboxMargin, y_pos - 1);
-  [details setFrame:details_rect];
-
-  [checkbox_container_ addSubview:details];
-}
-
 void MediaGalleriesDialogCocoa::UpdateGalleries() {
   InitDialogControls();
 }
 
 void MediaGalleriesDialogCocoa::OnConstrainedWindowClosed(
     ConstrainedWindowMac* window) {
   controller_->DialogFinished(accepted_);
 }
 
+void MediaGalleriesDialogCocoa::OnCheckboxToggled(GalleryDialogId gallery_id,
+                                                  bool checked) {
+  controller_->DidToggleGallery(gallery_id, checked);
+
+  [[[alert_ buttons] objectAtIndex:0] setEnabled:
+      controller_->IsAcceptAllowed()];
+}
+
 ui::MenuModel* MediaGalleriesDialogCocoa::GetContextMenu(
     GalleryDialogId gallery_id) {
   return controller_->GetContextMenu(gallery_id);
 }
 
 // static
 MediaGalleriesDialog* MediaGalleriesDialog::Create(
     MediaGalleriesDialogController* controller) {
   base::scoped_nsobject<MediaGalleriesCocoaController> cocoa_controller(
       [[MediaGalleriesCocoaController alloc] init]);
   return new MediaGalleriesDialogCocoa(controller, cocoa_controller);
 }