OmniboxPopupViewMac refactoring Part 3 (truncation)

chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm

Index: chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
index db647f8f6cf83e360090cc8347955909ec406ba6..5c87f990c0243933f012176dd32697b3c66c361a 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
@@ -4,8 +4,11 @@
 
 #import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h"
 
+#include <algorithm>
 #include <cmath>
 
+#include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
+
 namespace {
 
 // How far to offset image column from the left.
@@ -14,14 +17,46 @@ const CGFloat kImageXOffset = 5.0;
 // How far to offset the text column from the left.
 const CGFloat kTextXOffset = 28.0;
 
+// Maximum fraction of the popup width that can be used to display match
+// contents.
+const CGFloat kMaxContentsFraction = 0.7;
+
 // Rounding radius of selection and hover background on popup items.
 const CGFloat kCellRoundingRadius = 2.0;
 
-NSColor* SelectedBackgroundColor() {
-  return [NSColor selectedControlColor];
-}
-NSColor* HoveredBackgroundColor() {
-  return [NSColor controlHighlightColor];
+void DrawFadeTruncatingTitle(NSAttributedString* title,
+                             NSRect titleRect,
+                             NSColor* backgroundColor) {
+  gfx::ScopedNSGraphicsContextSaveGState scopedGState;
+  NSRectClip(titleRect);
+
+  // Draw the entire text.
+  NSSize textSize = [title size];
+  NSPoint textOrigin = titleRect.origin;
+  textOrigin.y += roundf((NSHeight(titleRect) - textSize.height) / 2.0) - 1.0;
+  [title drawAtPoint:textOrigin];
+
+  // Empirically, Cocoa will draw an extra 2 pixels past NSWidth(titleRect)
+  // before it clips the text.
+  const CGFloat kOverflowBeforeClip = 2.0;
+  BOOL clipping =
+      std::floor(textSize.width) > NSWidth(titleRect) + kOverflowBeforeClip;
+  if (!clipping)
+    return;
+
+  // Gradient is about twice our line height long.
+  CGFloat gradientWidth = std::min(textSize.height * 2, NSWidth(titleRect) / 4);
+
+  // Draw the gradient part.
+  NSColor *alphaColor = [backgroundColor colorWithAlphaComponent:0.0];
+  base::scoped_nsobject<NSGradient> mask(
+      [[NSGradient alloc] initWithStartingColor:alphaColor
+                                    endingColor:backgroundColor]);
+  [mask drawFromPoint:NSMakePoint(NSMaxX(titleRect) - gradientWidth,
+                                  NSMinY(titleRect))
+              toPoint:NSMakePoint(NSMaxX(titleRect),
+                                  NSMinY(titleRect))
+              options:NSGradientDrawsBeforeStartingLocation];
 }
 
 }  // namespace
@@ -29,8 +64,7 @@ NSColor* HoveredBackgroundColor() {
 @implementation OmniboxPopupCell
 
 - (id)init {
-  self = [super init];
-  if (self) {
+  if ((self = [super init])) {
     [self setImagePosition:NSImageLeft];
     [self setBordered:NO];
     [self setButtonType:NSRadioButton];
@@ -41,15 +75,30 @@ NSColor* HoveredBackgroundColor() {
   return self;
 }
 
-// The default NSButtonCell drawing leaves the image flush left and
-// the title next to the image.  This spaces things out to line up
-// with the star button and autocomplete field.
+- (NSAttributedString*)contentText {

[Scott Hess - ex-Googler, 2013/06/28 20:11:34]

Nobody even ever calls the getters.  I'd be fine with removing them entirely,
but if you're going to keep them add a unit test to check that they round-trip
appropriately.

[sail, 2013/06/30 18:49:11]

Done.

+  return contentText_;
+}
+
+- (void)setContentText:(NSAttributedString*)contentText {
+  contentText_.reset([contentText retain]);
+}
+
+- (NSAttributedString*)descriptionText {
+  return descriptionText_;
+}
+
+- (void)setDescriptionText:(NSAttributedString*)descriptionText {
+  descriptionText_.reset([descriptionText retain]);
+}
+
 - (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView *)controlView {
+  NSColor* backgroundColor = [NSColor controlBackgroundColor];
   if ([self state] == NSOnState || [self isHighlighted]) {
     if ([self state] == NSOnState)
-      [SelectedBackgroundColor() set];
+      backgroundColor = [NSColor selectedControlColor];
     else
-      [HoveredBackgroundColor() set];
+      backgroundColor = [NSColor controlHighlightColor];
+    [backgroundColor set];
     NSBezierPath* path =
         [NSBezierPath bezierPathWithRoundedRect:cellFrame
                                         xRadius:kCellRoundingRadius
@@ -74,12 +123,40 @@ NSColor* HoveredBackgroundColor() {
   }
 
   // Adjust the title position to be lined up under the field's text.
-  NSAttributedString* title = [self attributedTitle];
-  if (title && [title length]) {
-    NSRect titleRect = cellFrame;
-    titleRect.size.width -= kTextXOffset;
-    titleRect.origin.x += kTextXOffset;
-    [self drawTitle:title withFrame:titleRect inView:controlView];
+  if ([contentText_ length]) {
+    CGFloat availableWidth = NSWidth(cellFrame) - kTextXOffset;
+    CGFloat contentWidth = [contentText_ size].width;
+    CGFloat descriptionWidth = 0;

[Scott Hess - ex-Googler, 2013/06/28 20:11:34]

I think you should feel free to use abbreviations if descWidth and availWidth
(or totalWidth) would allow things for format more nicely later on.

[sail, 2013/06/30 18:49:11]

Done.

+
+    base::scoped_nsobject<NSMutableAttributedString> dashDescriptionText;
+    if ([descriptionText_ length]) {
+      dashDescriptionText.reset([[NSMutableAttributedString alloc]
+          initWithAttributedString:descriptionText_]);
+      [dashDescriptionText replaceCharactersInRange:NSMakeRange(0, 0)
+                                         withString:@" \u2013 "];
+      descriptionWidth = [dashDescriptionText size].width;
+    }

[Scott Hess - ex-Googler, 2013/06/28 20:11:34]

That's not really what I meant, sorry.  I meant drawing the en-dash separately
from the description, so that the description is just left alone rather than
having arbitrary junk stuffed into it.  Mostly it was about spinning off
unnecessary autorelease values in drawing code.  [That would also imply using
string-drawing with a constant string and the description's leading attributes
rather than creating an autorelease attributed string off the constant string.]

Also, pull back a descriptive variable name for @" \u2013 ".  I don't have a
unicode chart in my head.

Also - feel free to push back on me about pushing the en-dash into the front of
the description text.  You could factor it in/out automatically in the
setter/getter rather than in the drawing code.

[sail, 2013/06/30 18:49:11]

Done.

+
+    CGFloat tempDescWidth = std::min(
+        descriptionWidth, (1.0f - kMaxContentsFraction) * availableWidth);

[Scott Hess - ex-Googler, 2013/06/28 20:11:34]

Now that there is only one ref to this, and it's 1.0 - kMax, maybe it should be
kMinDescriptionFraction.

[sail, 2013/06/30 18:49:11]

Done.

I changed the setter to prepend raw_en_dash to the description.

Drawing the dash separately was a lot more work because I needed to extract the
attribute information from the description anyways.

+    contentWidth = std::min(contentWidth, availableWidth - tempDescWidth);
+    descriptionWidth =
+        std::min(descriptionWidth, availableWidth - contentWidth);
+
+    // Draw content text.
+    NSRect contentRect = cellFrame;
+    contentRect.origin.x = kTextXOffset;
+    contentRect.size.width = contentWidth;

[Scott Hess - ex-Googler, 2013/06/28 20:11:34]

contentRect and descRect could be generated by NSDivideRect() with cellFrame and
contentWidth.

[sail, 2013/06/30 18:49:11]

Done.

+    DrawFadeTruncatingTitle(contentText_, contentRect, backgroundColor);
+
+    // Draw dash and description.
+    if ([dashDescriptionText length]) {
+      NSRect descriptionRect = cellFrame;
+      descriptionRect.origin.x = NSMaxX(contentRect);
+      descriptionRect.size.width = descriptionWidth;
+      DrawFadeTruncatingTitle(
+          dashDescriptionText, descriptionRect, backgroundColor);
+    }
   }
 }