Mac: give visual feedback for bookmark button drags.

Draw a "ugly black bar" (where "black" is actually a theme colour) to indicate
where a bookmark button that's being dragged will end up if dropped. This is a
stopgap measure for the beta. Later, we will make the buttons move around
instead (even if implementable in the time remaining, such animations would be a
crash risk).

BUG=17608
TEST=Drag bookmark bar buttons around. Make sure that the bar indicating where a button will be dropped is visible enough and accurate. Repeat with various themes.

Review URL: http://codereview.chromium.org/437051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33079 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 151 insertions, 13 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index 1f7bd8c..e80cfa1 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -167,11 +167,18 @@ willAnimateFromState:(bookmarks::VisualState)oldState
 // need it for animations. Try not to propagate its use.
 - (void)layoutSubviews;
 
-// Complete a drag of a bookmark button to this location on the main bar.
+// Complete a drag of a bookmark button to the given point (given in window
+// coordinates) on the main bar.
 // TODO(jrg): submenu DnD.
 // Returns YES on success.
 - (BOOL)dragButton:(BookmarkButton*)sourceButton to:(NSPoint)point;
 
+// The x-coordinate of (the middle of) the indicator to draw for a drag of the
+// source button to the given point (given in window coordinates).
+// TODO(viettrungluu,jrg): instead of this, make buttons move around.
+- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton
+                               toPoint:(NSPoint)point;
+
 // Actions for manipulating bookmarks.
 // From a button, ...
 - (IBAction)openBookmark:(id)sender;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index 7b8bf6a..79e505a 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -110,6 +110,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 }  // namespace
 
 @interface BookmarkBarController(Private)
+
 // Determines the appropriate state for the given situation.
 + (bookmarks::VisualState)visualStateToShowNormalBar:(BOOL)showNormalBar
                                      showDetachedBar:(BOOL)showDetachedBar;
@@ -142,6 +143,13 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 // the animation, NO if not (and hence it should be done instantly).
 - (BOOL)doBookmarkBarAnimation;
 
+// Returns the index in the model for a drag of the given button to the given
+// location; currently, only the x-coordinate of |point| is considered. I
+// reserve the right to check for errors, in which case this would return
+// negative value; callers should check for this.
+- (int)indexForDragOfButton:(BookmarkButton*)sourceButton
+                    toPoint:(NSPoint)point;
+
 - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu;
 - (void)addFolderNode:(const BookmarkNode*)node toMenu:(NSMenu*)menu;
 - (void)tagEmptyMenu:(NSMenu*)menu;
@@ -153,6 +161,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 - (void)centerNoItemsLabel;
 - (NSImage*)getFavIconForNode:(const BookmarkNode*)node;
 - (void)setNodeForBarMenu;
+
 @end
 
 @implementation BookmarkBarController
@@ -489,21 +498,25 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   return YES;
 }
 
-- (BOOL)dragButton:(BookmarkButton*)sourceButton to:(NSPoint)point {
+- (int)indexForDragOfButton:(BookmarkButton*)sourceButton
+                    toPoint:(NSPoint)point {
   DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]);
 
   void* pointer = [[[sourceButton cell] representedObject] pointerValue];
   const BookmarkNode* sourceNode = static_cast<const BookmarkNode*>(pointer);
-  DCHECK(sourceNode);
+  if (!sourceNode) {
+    NOTREACHED();
+    return -1;
+  }
 
   // Identify which buttons we are between.  For now, assume a button
   // location is at the center point of its view, and that an exact
   // match means "place before".
   // TODO(jrg): revisit position info based on UI team feedback.
   // dropLocation is in bar local coordinates.
-  NSPoint dropLocation = [[self view] convertPoint:point
-                                          fromView:[[[self view] window]
-                                                     contentView]];
+  NSPoint dropLocation =
+      [[self view] convertPoint:point
+                       fromView:[[[self view] window] contentView]];
   NSButton* buttonToTheRightOfDraggedButton = nil;
   for (NSButton* button in buttons_.get()) {
     CGFloat midpoint = NSMidX([button frame]);
@@ -516,12 +529,25 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
     pointer = [[[buttonToTheRightOfDraggedButton cell]
                  representedObject] pointerValue];
     const BookmarkNode* afterNode = static_cast<const BookmarkNode*>(pointer);
-    bookmarkModel_->Move(sourceNode, sourceNode->GetParent(),
-                         afterNode->GetParent()->IndexOfChild(afterNode));
+    return afterNode->GetParent()->IndexOfChild(afterNode);
+  }
+
+  // If nothing is to my right I am at the end!
+  return sourceNode->GetParent()->GetChildCount();
+}
+
+- (BOOL)dragButton:(BookmarkButton*)sourceButton to:(NSPoint)point {
+  DCHECK([sourceButton isKindOfClass:[BookmarkButton class]]);
+
+  void* pointer = [[[sourceButton cell] representedObject] pointerValue];
+  const BookmarkNode* sourceNode = static_cast<const BookmarkNode*>(pointer);
+  DCHECK(sourceNode);
+
+  int destIndex = [self indexForDragOfButton:sourceButton toPoint:point];
+  if (destIndex >= 0 && sourceNode) {
+    bookmarkModel_->Move(sourceNode, sourceNode->GetParent(), destIndex);
   } else {
-    // If nothing is to my right I am at the end!
-    bookmarkModel_->Move(sourceNode, sourceNode->GetParent(),
-                         sourceNode->GetParent()->GetChildCount());
+    NOTREACHED();
   }
 
   // Movement of a node triggers observers (like us) to rebuild the
@@ -530,6 +556,44 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   return YES;
 }
 
+- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton
+                               toPoint:(NSPoint)point {
+  CGFloat x = 0;
+  int destIndex = [self indexForDragOfButton:sourceButton toPoint:point];
+  int numButtons = static_cast<int>([buttons_ count]);
+
+  // If it's a drop strictly between existing buttons ...
+  if (destIndex >= 0 && destIndex < numButtons) {
+    // ... put the indicator right between the buttons.
+    BookmarkButton* button =
+        [buttons_ objectAtIndex:static_cast<NSUInteger>(destIndex)];
+    DCHECK(button);
+    NSRect buttonFrame = [button frame];
+    x = buttonFrame.origin.x - 0.5 * bookmarks::kBookmarkHorizontalPadding;
+
+  // If it's a drop at the end (past the last button, if there are any) ...
+  } else if (destIndex == numButtons) {
+    // and if it's past the last button ...
+    if (numButtons > 0) {
+      // ... find the last button, and put the indicator to its right.
+      BookmarkButton* button =
+          [buttons_ objectAtIndex:static_cast<NSUInteger>(destIndex - 1)];
+      DCHECK(button);
+      NSRect buttonFrame = [button frame];
+      x = buttonFrame.origin.x + buttonFrame.size.width +
+          0.5 * bookmarks::kBookmarkHorizontalPadding;
+
+    // Otherwise, put it right at the beginning.
+    } else {
+      x = 0.5 * bookmarks::kBookmarkHorizontalPadding;
+    }
+  } else {
+    NOTREACHED();
+  }
+
+  return x;
+}
+
 - (int)currentTabContentsHeight {
   return browser_->GetSelectedTabContents() ?
       browser_->GetSelectedTabContents()->view()->GetContainerSize().height() :
diff --git a/chrome/browser/cocoa/bookmark_bar_view.h b/chrome/browser/cocoa/bookmark_bar_view.h
index 8a9cc3f..567544d 100644
--- a/chrome/browser/cocoa/bookmark_bar_view.h
+++ b/chrome/browser/cocoa/bookmark_bar_view.h
@@ -14,6 +14,9 @@
 
 @interface BookmarkBarView : NSView {
  @private
+  BOOL dropIndicatorShown_;
+  CGFloat dropIndicatorPosition_;
+
   IBOutlet BookmarkBarController* controller_;
   IBOutlet NSTextField* noItemTextfield_;
 }
diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm
index 31d167f..2b6d69c 100644
--- a/chrome/browser/cocoa/bookmark_bar_view.mm
+++ b/chrome/browser/cocoa/bookmark_bar_view.mm
@@ -72,16 +72,75 @@
   return noItemTextfield_;
 }
 
+-(void)drawRect:(NSRect)dirtyRect {
+  [super drawRect:dirtyRect];
+
+  // Draw the bookmark-button-dragging drop indicator if necessary.
+  if (dropIndicatorShown_) {
+    const CGFloat kBarWidth = 1;
+    const CGFloat kBarHalfWidth = kBarWidth / 2.0;
+    const CGFloat kBarVertPad = 4;
+    const CGFloat kBarOpacity = 0.85;
+
+    // Prevent the indicator from being clipped on the left.
+    CGFloat xLeft = MAX(dropIndicatorPosition_ - kBarHalfWidth, 0);
+
+    NSRect uglyBlackBar =
+        NSMakeRect(xLeft, kBarVertPad,
+                   kBarWidth, NSHeight([self bounds]) - 2 * kBarVertPad);
+    NSColor* uglyBlackBarColor =
+        [[self gtm_theme] textColorForStyle:GTMThemeStyleBookmarksBarButton
+                                      state:GTMThemeStateActiveWindow];
+    [[uglyBlackBarColor colorWithAlphaComponent:kBarOpacity] setFill];
+    [[NSBezierPath bezierPathWithRect:uglyBlackBar] fill];
+  }
+}
+
 // NSDraggingDestination methods
 
 - (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info {
   if ([[info draggingPasteboard] containsURLData])
     return NSDragOperationCopy;
-  if ([[info draggingPasteboard] dataForType:kBookmarkButtonDragType])
+  if ([[info draggingPasteboard] dataForType:kBookmarkButtonDragType]) {
+    NSData* data = [[info draggingPasteboard]
+                     dataForType:kBookmarkButtonDragType];
+    // [info draggingSource] is nil if not the same application.
+    if (data && [info draggingSource]) {
+      // Find the position of the drop indicator.
+      BookmarkButton* button = nil;
+      [data getBytes:&button length:sizeof(button)];
+      CGFloat x =
+          [controller_ indicatorPosForDragOfButton:button
+                                           toPoint:[info draggingLocation]];
+
+      // Need an update if the indicator wasn't previously shown or if it has
+      // moved.
+      if (!dropIndicatorShown_ || dropIndicatorPosition_ != x) {
+        dropIndicatorShown_ = YES;
+        dropIndicatorPosition_ = x;
+        [self setNeedsDisplay:YES];
+      }
+    }
+
     return NSDragOperationMove;
+  }
   return NSDragOperationNone;
 }
 
+- (void)draggingExited:(id<NSDraggingInfo>)info {
+  // Regardless of the type of dragging which ended, we need to get rid of the
+  // drop indicator if one was shown.
+  if (dropIndicatorShown_) {
+    dropIndicatorShown_ = NO;
+    [self setNeedsDisplay:YES];
+  }
+}
+
+- (void)draggingEnded:(id<NSDraggingInfo>)info {
+  // For now, we just call |-draggingExited:|.
+  [self draggingExited:info];
+}
+
 - (BOOL)wantsPeriodicDraggingUpdates {
   // TODO(port): This should probably return |YES| and the controller should
   // slide the existing bookmark buttons interactively to the side to make
diff --git a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
index c4f48ae..fabe97d 100644
--- a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
@@ -60,6 +60,11 @@
   return pong_;
 }
 
+- (CGFloat)indicatorPosForDragOfButton:(BookmarkButton*)sourceButton
+                               toPoint:(NSPoint)point {
+  return 0;
+}
+
 @end
 
 namespace {
diff --git a/chrome/browser/cocoa/bookmark_button.mm b/chrome/browser/cocoa/bookmark_button.mm
index 1fdf4a3..38b549f 100644
--- a/chrome/browser/cocoa/bookmark_button.mm
+++ b/chrome/browser/cocoa/bookmark_button.mm
@@ -18,7 +18,7 @@ const CGFloat kWebDragStartHysteresisX = 5.0;
 const CGFloat kWebDragStartHysteresisY = 5.0;
 
 // The opacity of the bookmark button drag image.
-const CGFloat kDragImageOpacity = 0.8;
+const CGFloat kDragImageOpacity = 0.7;
 
 }