Adjustments to bookmark bar folder menu placement issues....

chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm

 // Copyright (c) 2010 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 "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h"
 
 #include "base/mac/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
@@ skipping 11 matching lines @@
 using bookmarks::kBookmarkBarMenuCornerRadius;
 
 namespace {
 
 // Frequency of the scrolling timer in seconds.
 const NSTimeInterval kBookmarkBarFolderScrollInterval = 0.1;
 
 // Amount to scroll by per timer fire.  We scroll rather slowly; to
 // accomodate we do several at a time.
 const CGFloat kBookmarkBarFolderScrollAmount =
-    3 * bookmarks::kBookmarkButtonVerticalSpan;
+    3 * bookmarks::kBookmarkFolderButtonHeight;
 
 // Amount to scroll for each scroll wheel roll.
 const CGFloat kBookmarkBarFolderScrollWheelAmount =
-    1 * bookmarks::kBookmarkButtonVerticalSpan;
+    1 * bookmarks::kBookmarkFolderButtonHeight;
 
 // Determining adjustments to the layout of the folder menu window in response
 // to resizing and scrolling relies on many visual factors. The following
 // struct is used to pass around these factors to the several support
 // functions involved in the adjustment calculations and application.
 struct LayoutMetrics {
   // Metrics applied during the final layout adjustments to the window,
   // the main visible content view, and the menu content view (i.e. the
   // scroll view).
   CGFloat windowLeft;
@@ skipping 419 matching lines @@
     // BELOW the parent button.  Our left is the button left; our top
     // is bottom of button's parent view.
     NSPoint buttonBottomLeftInScreen =
         [[parentButton_ window]
             convertBaseToScreen:[parentButton_
                                     convertPoint:NSZeroPoint toView:nil]];
     NSPoint bookmarkBarBottomLeftInScreen =
         [[parentButton_ window]
             convertBaseToScreen:[[parentButton_ superview]
                                     convertPoint:NSZeroPoint toView:nil]];
-    newWindowTopLeft = NSMakePoint(buttonBottomLeftInScreen.x,
+    newWindowTopLeft = NSMakePoint(buttonBottomLeftInScreen.x +

[mafv, 2011/01/19 22:39:39]

Indentation in this function looks weird. I see what  you're aiming to do
(treating each function argument as a block, and aligning those blocks at the
open paren), but I find it confusing to look at.  

[mrossetti, 2011/01/20 01:41:42]

Done.

+                                      bookmarks::kBookmarkBarButtonOffset,
                                    bookmarkBarBottomLeftInScreen.y +
                                       bookmarks::kBookmarkBarMenuOffset);
     // Make sure the window is on-screen; if not, push left.  It is
     // intentional that top level folders "push left" slightly
     // different than subfolders.
     NSRect screenFrame = [[[parentButton_ window] screen] frame];
     CGFloat spillOff = (newWindowTopLeft.x + windowWidth) - NSMaxX(screenFrame);
     if (spillOff > 0.0) {
       newWindowTopLeft.x = std::max(newWindowTopLeft.x - spillOff,
                                     NSMinX(screenFrame));
     }
   } else {
-    // Parent is a folder; grow right/left.
+    // Parent is a folder: expose as much as we can vertically; grow right/left.
     newWindowTopLeft.x = [self childFolderWindowLeftForWidth:windowWidth];
     NSPoint topOfWindow = NSMakePoint(0,
-                                      (NSMaxY([parentButton_ frame]) +
-                                          bookmarks::kBookmarkVerticalPadding));
+                                      NSMaxY([parentButton_ frame]) -
+                                          bookmarks::kBookmarkVerticalPadding);
     topOfWindow = [[parentButton_ window]
                    convertBaseToScreen:[[parentButton_ superview]
                                         convertPoint:topOfWindow toView:nil]];
     newWindowTopLeft.y = topOfWindow.y;
   }
   return newWindowTopLeft;
 }
 
 // Set our window level to the right spot so we're above the menubar, dock, etc.
 // Factored out so we can override/noop in a unit test.
 - (void)configureWindowLevel {
   [[self window] setLevel:NSPopUpMenuWindowLevel];
 }
 
 - (int)menuHeightForButtonCount:(int)buttonCount {
   // This does not take into account any padding which may be required at the
   // top and/or bottom of the window.
-  return (buttonCount * bookmarks::kBookmarkButtonVerticalSpan) +
-      bookmarks::kBookmarkVerticalPadding;
+  return (buttonCount * bookmarks::kBookmarkFolderButtonHeight) +
+      2 * bookmarks::kBookmarkVerticalPadding;
 }
 
 - (void)adjustWindowLeft:(CGFloat)windowLeft
                     size:(NSSize)windowSize
              scrollingBy:(CGFloat)scrollDelta {
   // Callers of this function should make adjustments to the vertical
   // attributes of the folder view only (height, scroll position).
   // This function will then make appropriate layout adjustments in order
   // to accommodate screen/dock margins, scroll-up and scroll-down arrow
   // presentation, etc.
@@ skipping 39 matching lines @@
   NSScreen* screen = [[self window] screen];
   CGFloat effectiveFolderY = metrics.folderY;
   if (!metrics.couldScrollUp && !metrics.couldScrollDown)
     effectiveFolderY -= metrics.windowSize.height;
   metrics.canScrollUp = effectiveFolderY < metrics.minimumY;
   CGFloat maximumY =
       NSMaxY([screen frame]) - bookmarks::kScrollWindowVerticalMargin;
   metrics.canScrollDown = metrics.folderTop > maximumY;
 
   // Accommodate changes in the bottom of the menu.
-  [self adjustMetricsForMenuTopChanges:layoutMetrics];
+  [self adjustMetricsForMenuBottomChanges:layoutMetrics];
 
   // Accommodate changes in the top of the menu.
-  [self adjustMetricsForMenuBottomChanges:layoutMetrics];
+  [self adjustMetricsForMenuTopChanges:layoutMetrics];
 
   metrics.scrollerFrame.origin.y += metrics.deltaScrollerY;
   metrics.scrollerFrame.size.height += metrics.deltaScrollerHeight;
   metrics.visibleFrame.origin.y += metrics.deltaVisibleY;
   metrics.visibleFrame.size.height += metrics.deltaVisibleHeight;
   metrics.preScroll = metrics.canScrollUp && !metrics.couldScrollUp &&
       metrics.scrollDelta == 0.0 && metrics.deltaWindowHeight >= 0.0;
   metrics.windowFrame.origin.y += metrics.deltaWindowY;
   metrics.windowFrame.origin.x = metrics.windowLeft;
   metrics.windowFrame.size.height += metrics.deltaWindowHeight;
   metrics.windowFrame.size.width = metrics.windowSize.width;
 }
 
-- (void)adjustMetricsForMenuTopChanges:(LayoutMetrics*)layoutMetrics {
+- (void)adjustMetricsForMenuBottomChanges:(LayoutMetrics*)layoutMetrics {
   LayoutMetrics& metrics(*layoutMetrics);
   if (metrics.canScrollUp) {
     if (!metrics.couldScrollUp) {
       // Couldn't -> Can
       metrics.deltaWindowY = -metrics.oldWindowY;
       metrics.deltaWindowHeight = -metrics.deltaWindowY;
       metrics.deltaVisibleY = metrics.minimumY;
       metrics.deltaVisibleHeight = -metrics.deltaVisibleY;
       metrics.deltaScrollerY = verticalScrollArrowHeight_;
       metrics.deltaScrollerHeight = -metrics.deltaScrollerY;
       // Adjust the scroll delta if we've grown the window and it is
-      // now scroll-up-able, but don't adjust it factor if we've
+      // now scroll-up-able, but don't adjust it if we've
       // scrolled down and it wasn't scroll-up-able but now is.
       if (metrics.canScrollDown == metrics.couldScrollDown) {
         CGFloat deltaScroll = metrics.deltaWindowY + metrics.deltaScrollerY +
                               metrics.deltaVisibleY;
         metrics.scrollPoint.y += deltaScroll + metrics.windowSize.height;
       }
     } else if (!metrics.canScrollDown && metrics.windowSize.height > 0.0) {
       metrics.scrollPoint.y += metrics.windowSize.height;
     }
   } else {
     if (metrics.couldScrollUp) {
       // Could -> Can't
       metrics.deltaWindowY = metrics.folderY - metrics.oldWindowY;
       metrics.deltaWindowHeight = -metrics.deltaWindowY;
-      metrics.deltaVisibleY = -bookmarks::kScrollWindowVerticalMargin;
+      metrics.deltaVisibleY = -metrics.visibleFrame.origin.y;
       metrics.deltaVisibleHeight = -metrics.deltaVisibleY;
       metrics.deltaScrollerY = -verticalScrollArrowHeight_;
       metrics.deltaScrollerHeight = -metrics.deltaScrollerY;
-      // Adjust the scroll delta if we are no longer scroll-up-able
-      // and the scroll-down-able-ness hasn't changed.
-      if (metrics.canScrollDown == metrics.couldScrollDown) {
-        CGFloat deltaScroll = metrics.deltaWindowY + metrics.deltaScrollerY +
-                              metrics.deltaVisibleY;
-        metrics.scrollPoint.y += deltaScroll;
-      }
+      // We are no longer scroll-up-able so the scroll point drops to zero.
+      metrics.scrollPoint.y = 0.0;
     } else {
       // Couldn't -> Can't
       // Check for menu height change by looking at the relative tops of the
       // menu folder and the window folder, which previously would have been
       // the same.
       metrics.deltaWindowY = NSMaxY(metrics.windowFrame) - metrics.folderTop;
       metrics.deltaWindowHeight = -metrics.deltaWindowY;
     }
   }
 }
 
-- (void)adjustMetricsForMenuBottomChanges:(LayoutMetrics*)layoutMetrics {
+- (void)adjustMetricsForMenuTopChanges:(LayoutMetrics*)layoutMetrics {
   LayoutMetrics& metrics(*layoutMetrics);
   if (metrics.canScrollDown == metrics.couldScrollDown) {
     if (!metrics.canScrollDown) {
       // Not scroll-down-able but the menu top has changed.
       metrics.deltaWindowHeight += metrics.scrollDelta;
     }
   } else {
     if (metrics.canScrollDown) {
       // Couldn't -> Can
       metrics.deltaWindowHeight += (NSMaxY([[[self window] screen] frame]) -
@@ skipping 77 matching lines @@
 
   // Prelim height of the window.  We'll trim later as needed.
   int height = [self menuHeightForButtonCount:buttons];
   // We'll need this soon...
   [self window];
 
   // TODO(jrg): combine with frame code in bookmark_bar_controller.mm
   // http://crbug.com/35966
   NSRect buttonsOuterFrame = NSMakeRect(
       0,
-      (height - bookmarks::kBookmarkButtonVerticalSpan),
+      height - bookmarks::kBookmarkFolderButtonHeight -
+          bookmarks::kBookmarkVerticalPadding,
       bookmarks::kDefaultBookmarkWidth,
-      bookmarks::kBookmarkButtonHeight);
+      bookmarks::kBookmarkFolderButtonHeight);
 
   // TODO(jrg): combine with addNodesToButtonList: code from
   // bookmark_bar_controller.mm (but use y offset)
   // http://crbug.com/35966
   if (!node->GetChildCount()) {
     // If no children we are the empty button.
     BookmarkButton* button = [self makeButtonForNode:nil
                                                frame:buttonsOuterFrame];
     [buttons_ addObject:button];
     [folderView_ addSubview:button];
   } else {
     for (int i = startingIndex;
          i < node->GetChildCount();
          i++) {
       const BookmarkNode* child = node->GetChild(i);
       BookmarkButton* button = [self makeButtonForNode:child
                                                  frame:buttonsOuterFrame];
       [buttons_ addObject:button];
       [folderView_ addSubview:button];
-      buttonsOuterFrame.origin.y -= bookmarks::kBookmarkButtonVerticalSpan;
+      buttonsOuterFrame.origin.y -= bookmarks::kBookmarkFolderButtonHeight;
     }
   }
   [self layOutWindowWithHeight:height];
 }
 
 - (void)layOutWindowWithHeight:(CGFloat)height {
   // Lay out the window by adjusting all button widths to be consistent, then
   // base the window width on this ideal button width.
   CGFloat buttonWidth = [self adjustButtonWidths];
   CGFloat windowWidth = buttonWidth + padding_;
   NSPoint newWindowTopLeft = [self windowTopLeftForWidth:windowWidth];
   // Make sure as much of a submenu is exposed (which otherwise would be a
   // problem if the parent button is close to the bottom of the screen).
   if ([parentController_ isKindOfClass:[self class]]) {
-    newWindowTopLeft.y = MAX(newWindowTopLeft.y,
-                             height + bookmarks::kScrollWindowVerticalMargin);
+    CGFloat minimumY = NSMinY([[[self window] screen] visibleFrame]) +
+                       bookmarks::kScrollWindowVerticalMargin +
+                       height;
+    newWindowTopLeft.y = MAX(newWindowTopLeft.y, minimumY);
   }
   NSWindow* window = [self window];
   NSRect windowFrame = NSMakeRect(newWindowTopLeft.x,
                                   newWindowTopLeft.y - height,
                                   windowWidth, height);
   [window setFrame:windowFrame display:NO];
   NSRect folderFrame = NSMakeRect(0, 0, windowWidth, height);
   [folderView_ setFrame:folderFrame];
   NSSize newSize = NSMakeSize(windowWidth, 0.0);
   [self adjustWindowLeft:newWindowTopLeft.x size:newSize scrollingBy:0.0];
@@ skipping 657 matching lines @@
     disposition:(WindowOpenDisposition)disposition {
   [barController_ openAll:node disposition:disposition];
 }
 
 - (void)addButtonForNode:(const BookmarkNode*)node
                  atIndex:(NSInteger)buttonIndex {
   // Propose the frame for the new button. By default, this will be set to the
   // topmost button's frame (and there will always be one) offset upward in
   // anticipation of insertion.
   NSRect newButtonFrame = [[buttons_ objectAtIndex:0] frame];
-  newButtonFrame.origin.y += bookmarks::kBookmarkButtonVerticalSpan;
+  newButtonFrame.origin.y += bookmarks::kBookmarkFolderButtonHeight;
   // When adding a button to an empty folder we must remove the 'empty'
   // placeholder button. This can be detected by checking for a parent
   // child count of 1.
   const BookmarkNode* parentNode = node->GetParent();
   if (parentNode->GetChildCount() == 1) {
     BookmarkButton* emptyButton = [buttons_ lastObject];
     newButtonFrame = [emptyButton frame];
     [emptyButton setDelegate:nil];
     [emptyButton removeFromSuperview];
     [buttons_ removeLastObject];
   }
 
   if (buttonIndex == -1 || buttonIndex > (NSInteger)[buttons_ count])
     buttonIndex = [buttons_ count];
 
   // Offset upward by one button height all buttons above insertion location.
   BookmarkButton* button = nil;  // Remember so it can be de-highlighted.
   for (NSInteger i = 0; i < buttonIndex; ++i) {
     button = [buttons_ objectAtIndex:i];
     // Remember this location in case it's the last button being moved
     // which is where the new button will be located.
     newButtonFrame = [button frame];
     NSRect buttonFrame = [button frame];
-    buttonFrame.origin.y += bookmarks::kBookmarkButtonVerticalSpan;
+    buttonFrame.origin.y += bookmarks::kBookmarkFolderButtonHeight;
     [button setFrame:buttonFrame];
   }
   [[button cell] mouseExited:nil];  // De-highlight.
   BookmarkButton* newButton = [self makeButtonForNode:node
                                                 frame:newButtonFrame];
   [buttons_ insertObject:newButton atIndex:buttonIndex];
   [folderView_ addSubview:newButton];
 
   // Close any child folder(s) which may still be open.
   [self closeBookmarkFolder:self];
@@ skipping 51 matching lines @@
     [buttons_ removeObjectAtIndex:fromIndex];
     NSRect movedFrame = [movedButton frame];
     NSPoint toOrigin = movedFrame.origin;
     [movedButton setHidden:YES];
     if (fromIndex < toIndex) {
       BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex - 1];
       toOrigin = [targetButton frame].origin;
       for (NSInteger i = fromIndex; i < toIndex; ++i) {
         BookmarkButton* button = [buttons_ objectAtIndex:i];
         NSRect frame = [button frame];
-        frame.origin.y += bookmarks::kBookmarkButtonVerticalSpan;
+        frame.origin.y += bookmarks::kBookmarkFolderButtonHeight;
         [button setFrameOrigin:frame.origin];
       }
     } else {
       BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex];
       toOrigin = [targetButton frame].origin;
       for (NSInteger i = fromIndex - 1; i >= toIndex; --i) {
         BookmarkButton* button = [buttons_ objectAtIndex:i];
         NSRect buttonFrame = [button frame];
-        buttonFrame.origin.y -= bookmarks::kBookmarkButtonVerticalSpan;
+        buttonFrame.origin.y -= bookmarks::kBookmarkFolderButtonHeight;
         [button setFrameOrigin:buttonFrame.origin];
       }
     }
     [buttons_ insertObject:movedButton atIndex:toIndex];
     [movedButton setFrameOrigin:toOrigin];
     [movedButton setHidden:NO];
   }
 }
 
 // TODO(jrg): Refactor BookmarkBarFolder common code. http://crbug.com/35966
@@ skipping 21 matching lines @@
       [button setShowsBorderOnlyWhileMouseInside:YES];
     }
   }
 
   [oldButton setDelegate:nil];
   [oldButton removeFromSuperview];
   [buttons_ removeObjectAtIndex:buttonIndex];
   for (NSInteger i = 0; i < buttonIndex; ++i) {
     BookmarkButton* button = [buttons_ objectAtIndex:i];
     NSRect buttonFrame = [button frame];
-    buttonFrame.origin.y -= bookmarks::kBookmarkButtonVerticalSpan;
+    buttonFrame.origin.y -= bookmarks::kBookmarkFolderButtonHeight;
     [button setFrame:buttonFrame];
   }
   // Search for and adjust submenus, if necessary.
   NSInteger buttonCount = [buttons_ count];
   if (buttonCount) {
     BookmarkButton* subButton = [folderController_ parentButton];
     for (NSInteger i = buttonIndex; i < buttonCount; ++i) {
       BookmarkButton* aButton = [buttons_ objectAtIndex:i];
       // If this button is showing its menu then we need to move the menu, too.
       if (aButton == subButton)
         [folderController_ offsetFolderMenuWindow:NSMakeSize(0.0,
          bookmarks::kBookmarkBarHeight)];
     }
   } else {
     // If all nodes have been removed from this folder then add in the
     // 'empty' placeholder button.
     NSRect buttonFrame =
-        NSMakeRect(0,
-                   bookmarks::kBookmarkButtonHeight -
-                       (bookmarks::kBookmarkBarHeight -
-                        bookmarks::kBookmarkVerticalPadding),
-                   bookmarks::kDefaultBookmarkWidth,
-                   (bookmarks::kBookmarkBarHeight -
-                       2 * bookmarks::kBookmarkVerticalPadding));
+        NSMakeRect(0.0, 0.0, bookmarks::kDefaultBookmarkWidth,
+                   bookmarks::kBookmarkFolderButtonHeight);
     BookmarkButton* button = [self makeButtonForNode:nil
                                                frame:buttonFrame];
     [buttons_ addObject:button];
     [folderView_ addSubview:button];
     buttonCount = 1;
   }
 
   [self adjustWindowForButtonCount:buttonCount];
 
   if (animate && !ignoreAnimations_)
@@ skipping 38 matching lines @@
 
 - (void)setIgnoreAnimations:(BOOL)ignore {
   ignoreAnimations_ = ignore;
 }
 
 - (BookmarkButton*)buttonThatMouseIsIn {
   return buttonThatMouseIsIn_;
 }
 
 @end  // BookmarkBarFolderController