Index: chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm |
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm |
index 3548b9f9881a2f951b9912894c79b92dfc3d4e94..788c72d7a65041790efbef1c743d4d55ebcd71ae 100644 |
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm |
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.mm |
@@ -5,7 +5,10 @@ |
#import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.h" |
#include "base/logging.h" |
+#include "base/mac/foundation_util.h" |
#import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h" |
+#include "chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h" |
+#include "components/omnibox/autocomplete_result.h" |
namespace { |
@@ -14,9 +17,106 @@ const NSInteger kMiddleButtonNumber = 2; |
} // namespace |
-@interface OmniboxPopupMatrix() |
+@implementation OmniboxPopupTableController |
+ |
+- (id)init { |
+ if ((self = [super init])) { |
+ hoveredIndex_ = -1; |
+ } |
+ return self; |
+} |
+ |
+- (id)initWithMatchResults:(const AutocompleteResult&)result |
+ popupView:(const OmniboxPopupViewMac&)popupView { |
+ if ((self = [super init])) { |
+ hoveredIndex_ = -1; |
+ // Load the results into the popup's matrix. |
+ const size_t start_match = result.ShouldHideTopMatch() ? 1 : 0; |
+ const size_t rows = result.size() - start_match; |
+ DCHECK_GT(rows, 0U); |
Scott Hess - ex-Googler
2015/05/21 20:40:27
IMHO this would be better as:
DCHECK_GT(result.
dschuyler
2015/05/26 18:40:20
Done.
|
+ base::scoped_nsobject<NSMutableArray> array([[NSMutableArray alloc] init]); |
groby-ooo-7-16
2015/05/21 03:01:48
Why not keep it directly in array_?
Scott Hess - ex-Googler
2015/05/21 20:40:27
array_ is NSArray. Which makes me wonder if array
dschuyler
2015/05/26 18:40:20
If there are good reasons to choose style A or B w
dschuyler
2015/05/26 18:40:20
For reasons Scott gives below. :)
groby-ooo-7-16
2015/05/26 21:23:22
Acknowledged.
Scott Hess - ex-Googler
2015/05/29 18:00:26
This is like const functions in C++, where there's
dschuyler
2015/06/09 01:30:46
Thanks
Done.
|
+ CGFloat max_match_contents_width = 0.0f; |
+ CGFloat contents_offset = -1.0f; |
+ for (size_t ii = 0; ii < rows; ++ii) { |
groby-ooo-7-16
2015/05/21 03:01:48
Please don't perpetuate ii :) Best, don't even loo
Scott Hess - ex-Googler
2015/05/21 04:46:36
Yeah, that's some ancient habit of mine which has
dschuyler
2015/05/26 18:40:20
Acknowledged.
dschuyler
2015/05/26 18:40:20
The <should hide top match> stuff is a discontinue
groby-ooo-7-16
2015/05/26 21:23:22
If you can do that as a prior CL and rebase this C
dschuyler
2015/05/28 20:34:22
That CL is now started.
dschuyler
2015/06/10 20:48:31
Done.
|
+ const AutocompleteMatch& match = result.match_at(ii + start_match); |
+ NSImage* image = popupView.ImageForMatch(match); |
groby-ooo-7-16
2015/05/21 03:01:48
Bleh. I hate we have to pass the popupView. I wond
dschuyler
2015/05/26 18:40:21
Which direction is the right way to go for this pa
groby-ooo-7-16
2015/05/26 21:23:22
Yes, I think that's fine. It's not perfect, but it
dschuyler
2015/05/28 20:34:22
Bug entered 492896
|
+ base::scoped_nsobject<OmniboxPopupCellData> cellData( |
+ [[OmniboxPopupCellData alloc] initWithMatch:match image:image]); |
+ [array addObject:cellData]; |
groby-ooo-7-16
2015/05/21 03:01:48
You _could_ just move the alloc/initWithMatch call
Scott Hess - ex-Googler
2015/05/21 20:40:27
Someone needs to own the +alloc ref, though.
groby-ooo-7-16
2015/05/22 05:03:07
Yep:
[array addObject:[[[OmniboxPopupCellData all
Scott Hess - ex-Googler
2015/05/22 05:17:58
I think that means you can do what you wish, eithe
dschuyler
2015/05/26 18:40:21
Acknowledged.
|
+ if (match.type == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) { |
+ max_match_contents_width = std::max(max_match_contents_width, |
groby-ooo-7-16
2015/05/21 03:01:48
So max_match_contents_width stays 0 unless you hav
Scott Hess - ex-Googler
2015/05/21 20:40:27
Yeah, this is a thing we ground on about in the or
dschuyler
2015/05/26 18:40:20
Acknowledged.
dschuyler
2015/05/26 18:40:20
Acknowledged.
dschuyler
2015/05/26 18:40:21
I'd like to leave the TAIL stuff unchanged until A
groby-ooo-7-16
2015/05/26 21:23:22
Acknowledged.
|
+ [cellData getMatchContentsWidth]); |
+ if (contents_offset < 0.0f) { |
Scott Hess - ex-Googler
2015/05/21 20:40:27
Drop the {} here.
dschuyler
2015/05/26 18:40:20
Done.
|
+ contents_offset = [OmniboxPopupCell computeContentsOffset:match]; |
+ } |
+ [cellData setContentsOffset:contents_offset]; |
+ } |
+ } |
+ |
+ for (size_t ii = 0; ii < rows; ++ii) { |
groby-ooo-7-16
2015/05/21 03:01:48
for (OmniboxPopupCellData* cellData : array)
[ce
dschuyler
2015/05/26 18:40:20
Done.
|
+ OmniboxPopupCellData* cellData = [array objectAtIndex:ii]; |
+ [cellData setMaxMatchContentsWidth:max_match_contents_width]; |
+ } |
+ array_.reset([array retain]); |
+ } |
+ return self; |
+} |
+ |
+- (id)initWithArray:(NSArray*)array { |
Scott Hess - ex-Googler
2015/05/21 20:40:27
instancetype.
dschuyler
2015/05/26 18:40:20
Done.
|
+ if ((self = [super init])) { |
+ hoveredIndex_ = -1; |
+ array_.reset([array retain]); |
+ } |
+ return self; |
+} |
+ |
+- (NSInteger)numberOfRowsInTableView:(NSTableView*)tableView { |
+ return [array_ count]; |
+} |
+ |
+- (id)tableView:(NSTableView*)tableView |
+ objectValueForTableColumn:(NSTableColumn*)tableColumn |
+ row:(NSInteger)rowIndex { |
+ return nil; |
+} |
+ |
+- (void)tableView:(NSTableView*)aTableView |
+ setObjectValue:(id)anObject |
+ forTableColumn:(NSTableColumn*)aTableColumn |
+ row:(NSInteger)rowIndex { |
+} |
+ |
+- (void)tableView:(NSTableView*)tableView |
+ willDisplayCell:(id)cell |
+ forTableColumn:(NSTableColumn*)tableColumn |
+ row:(NSInteger)rowIndex { |
+ OmniboxPopupCell* popupCell = |
+ base::mac::ObjCCastStrict<OmniboxPopupCell>(cell); |
+ [popupCell setRepresentedObject:[[array_ objectAtIndex:rowIndex] retain]]; |
+ [popupCell |
+ setState:([tableView selectedRow] == rowIndex) ? NSOnState : NSOffState]; |
+ [popupCell highlight:(hoveredIndex_ == rowIndex) |
+ withFrame:[tableView bounds] |
+ inView:tableView]; |
+} |
+ |
+- (NSInteger)highlightedRow { |
+ return hoveredIndex_; |
+} |
+ |
+- (void)highlightRowAt:(NSInteger)rowIndex { |
+ hoveredIndex_ = rowIndex; |
+} |
+ |
+- (CGFloat)tableView:(NSTableView*)tableView heightOfRow:(NSInteger)row { |
+ OmniboxPopupCellData* cellData = [array_ objectAtIndex:row]; |
groby-ooo-7-16
2015/05/21 03:01:48
chaining methods would be preferred - no need to h
dschuyler
2015/05/26 18:40:20
Done.
|
+ return [cellData rowHeight]; |
groby-ooo-7-16
2015/05/21 03:01:48
Just a general note on tables, no need to implemen
dschuyler
2015/05/26 18:40:21
Acknowledged.
|
+} |
+ |
+@end |
+ |
+@interface OmniboxPopupMatrix () |
- (void)resetTrackingArea; |
-- (void)highlightRowAt:(NSInteger)rowIndex; |
- (void)highlightRowUnder:(NSEvent*)theEvent; |
- (BOOL)selectCellForEvent:(NSEvent*)theEvent; |
@end |
@@ -26,36 +126,34 @@ const NSInteger kMiddleButtonNumber = 2; |
- (id)initWithObserver:(OmniboxPopupMatrixObserver*)observer { |
if ((self = [super initWithFrame:NSZeroRect])) { |
observer_ = observer; |
- [self setCellClass:[OmniboxPopupCell class]]; |
+ |
+ base::scoped_nsobject<NSTableColumn> column( |
+ [[NSTableColumn alloc] initWithIdentifier:@"MainCell"]); |
+ base::scoped_nsobject<OmniboxPopupCell> columnCell( |
+ [[OmniboxPopupCell alloc] init]); |
groby-ooo-7-16
2015/05/21 03:01:48
Again, skip the scoped temp and chain methods.
Scott Hess - ex-Googler
2015/05/21 20:40:27
I think this one also leaves an additional ref unl
groby-ooo-7-16
2015/05/22 05:03:07
Acknowledged.
dschuyler
2015/05/26 18:40:20
Acknowledged.
dschuyler
2015/05/26 18:40:21
Done.
|
+ [column setDataCell:columnCell]; |
+ [self addTableColumn:column]; |
// Cells pack with no spacing. |
[self setIntercellSpacing:NSMakeSize(0.0, 0.0)]; |
- [self setDrawsBackground:YES]; |
+ [self setSelectionHighlightStyle:NSTableViewSelectionHighlightStyleNone]; |
[self setBackgroundColor:[NSColor controlBackgroundColor]]; |
- [self renewRows:0 columns:1]; |
[self setAllowsEmptySelection:YES]; |
- [self setMode:NSRadioModeMatrix]; |
- [self deselectAllCells]; |
+ [self deselectAll:self]; |
[self resetTrackingArea]; |
} |
return self; |
} |
-- (void)setObserver:(OmniboxPopupMatrixObserver*)observer { |
- observer_ = observer; |
+- (OmniboxPopupTableController*)controller { |
+ return base::mac::ObjCCastStrict<OmniboxPopupTableController>( |
+ [self delegate]); |
} |
-- (NSInteger)highlightedRow { |
- NSArray* cells = [self cells]; |
- const NSUInteger count = [cells count]; |
- for(NSUInteger i = 0; i < count; ++i) { |
- if ([[cells objectAtIndex:i] isHighlighted]) { |
- return i; |
- } |
- } |
- return -1; |
+- (void)setObserver:(OmniboxPopupMatrixObserver*)observer { |
+ observer_ = observer; |
} |
- (void)updateTrackingAreas { |
@@ -73,7 +171,7 @@ const NSInteger kMiddleButtonNumber = 2; |
} |
- (void)mouseExited:(NSEvent*)theEvent { |
- [self highlightRowAt:-1]; |
+ [[self controller] highlightRowAt:-1]; |
} |
// The tracking area events aren't forwarded during a drag, so handle |
@@ -103,7 +201,7 @@ const NSInteger kMiddleButtonNumber = 2; |
// make sure the user is getting the right feedback. |
[self highlightRowUnder:theEvent]; |
- const NSInteger highlightedRow = [self highlightedRow]; |
+ const NSInteger highlightedRow = [[self controller] highlightedRow]; |
if (highlightedRow != -1) { |
DCHECK(observer_); |
observer_->OnMatrixRowMiddleClicked(self, highlightedRow); |
@@ -117,7 +215,7 @@ const NSInteger kMiddleButtonNumber = 2; |
NSCell* selectedCell = [self selectedCell]; |
// Clear any existing highlight. |
- [self highlightRowAt:-1]; |
+ [[self controller] highlightRowAt:-1]; |
do { |
if (![self selectCellForEvent:theEvent]) { |
@@ -145,6 +243,21 @@ const NSInteger kMiddleButtonNumber = 2; |
} |
} |
+- (void)selectRowIndex:(NSInteger)rowIndex { |
+ NSIndexSet* indexSet = [NSIndexSet indexSetWithIndex:rowIndex]; |
+ [self selectRowIndexes:indexSet byExtendingSelection:NO]; |
+} |
+ |
+- (NSInteger)highlightedRow { |
+ return [[self controller] highlightedRow]; |
+} |
+ |
+- (void)setController:(OmniboxPopupTableController*)controller { |
+ [self setDelegate:controller]; |
+ [self setDataSource:controller]; |
+ [self reloadData]; |
+} |
+ |
- (void)resetTrackingArea { |
if (trackingArea_.get()) |
[self removeTrackingArea:trackingArea_.get()]; |
@@ -160,33 +273,23 @@ const NSInteger kMiddleButtonNumber = 2; |
[self addTrackingArea:trackingArea_.get()]; |
} |
-- (void)highlightRowAt:(NSInteger)rowIndex { |
- // highlightCell will be nil if rowIndex is out of range, so no cell will be |
- // highlighted. |
- NSCell* highlightCell = [self cellAtRow:rowIndex column:0]; |
- |
- for (NSCell* cell in [self cells]) { |
- [cell setHighlighted:(cell == highlightCell)]; |
- } |
-} |
- |
- (void)highlightRowUnder:(NSEvent*)theEvent { |
NSPoint point = [self convertPoint:[theEvent locationInWindow] fromView:nil]; |
- NSInteger row, column; |
- if ([self getRow:&row column:&column forPoint:point]) { |
- [self highlightRowAt:row]; |
- } else { |
- [self highlightRowAt:-1]; |
+ NSInteger oldRow = [[self controller] highlightedRow]; |
+ NSInteger newRow = [self rowAtPoint:point]; |
+ if (oldRow != newRow) { |
+ [[self controller] highlightRowAt:newRow]; |
+ [self setNeedsDisplayInRect:[self rectOfRow:oldRow]]; |
+ [self setNeedsDisplayInRect:[self rectOfRow:newRow]]; |
} |
} |
-// Select cell under |theEvent|, returning YES if a selection is made. |
- (BOOL)selectCellForEvent:(NSEvent*)theEvent { |
NSPoint point = [self convertPoint:[theEvent locationInWindow] fromView:nil]; |
- NSInteger row, column; |
- if ([self getRow:&row column:&column forPoint:point]) { |
- DCHECK_EQ(column, 0); |
+ NSInteger row = [self rowAtPoint:point]; |
+ [self selectRowIndex:row]; |
+ if (row != -1) { |
DCHECK(observer_); |
observer_->OnMatrixRowSelected(self, row); |
return YES; |