From 900eef6269a3fcf167bc1a05036b2b1420dd85e1 Mon Sep 17 00:00:00 2001 From: "rsesek@chromium.org" Date: Tue, 8 Dec 2009 00:38:32 +0000 Subject: [Mac] Show group separators in the keyword editor BUG=23570,21640 TEST=Chromium-->Preferences-->Manage. (1) See "Default" group. (2) Add an engine and it appears under an "Other" group header. (3) Make that new engine default and it moves under the "Default" group and the "Other" header disappears. Review URL: http://codereview.chromium.org/466022 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34024 0039d316-1c4b-4281-b951-d872f2087c98 --- .../cocoa/keyword_editor_cocoa_controller.h | 6 ++ .../cocoa/keyword_editor_cocoa_controller.mm | 94 +++++++++++++++++----- .../keyword_editor_cocoa_controller_unittest.mm | 87 +++++++++++++++++++- 3 files changed, 167 insertions(+), 20 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller.h b/chrome/browser/cocoa/keyword_editor_cocoa_controller.h index 6aac616..105e76c 100644 --- a/chrome/browser/cocoa/keyword_editor_cocoa_controller.h +++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller.h @@ -64,6 +64,8 @@ class KeywordEditorModelObserver : public TemplateURLModelObserver, IBOutlet NSButton* removeButton_; IBOutlet NSButton* makeDefaultButton_; + scoped_nsobject groupCell_; + Profile* profile_; // weak scoped_ptr controller_; scoped_ptr observer_; @@ -96,4 +98,8 @@ class KeywordEditorModelObserver : public TemplateURLModelObserver, // or nil if there is none. + (KeywordEditorCocoaController*)sharedInstanceForProfile:(Profile*)profile; +// Converts a row index in our table view (which has group header rows) into +// one in the |controller_|'s model, which does not have them. +- (int)indexInModelForRow:(NSUInteger)row; + @end diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm index 4f20b37..37ffdf3 100644 --- a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm +++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm @@ -22,6 +22,7 @@ @interface KeywordEditorCocoaController (Private) - (void)adjustEditingButtons; - (void)editKeyword:(id)sender; +- (int)indexInModelForRow:(NSUInteger)row; @end // KeywordEditorModelObserver ------------------------------------------------- @@ -176,6 +177,7 @@ typedef std::map ProfileControllerMap; observer_.reset(new KeywordEditorModelObserver(self)); controller_->table_model()->SetObserver(observer_.get()); controller_->url_model()->AddObserver(observer_.get()); + groupCell_.reset([[NSTextFieldCell alloc] init]); } return self; } @@ -224,8 +226,7 @@ typedef std::map ProfileControllerMap; } } -// The last page info window that was moved will determine the location of the -// next new one. +// Remeber the position of the keyword editor. - (void)windowDidMove:(NSNotification*)notif { if (g_browser_process && g_browser_process->local_state()) { NSWindow* window = [self window]; @@ -264,9 +265,10 @@ typedef std::map ProfileControllerMap; - (void)editKeyword:(id)sender { const NSInteger clickedRow = [tableView_ clickedRow]; - if (clickedRow == -1) + if (clickedRow < 0 || [self tableView:tableView_ isGroupRow:clickedRow]) return; - const TemplateURL* url = controller_->GetTemplateURL(clickedRow); + const TemplateURL* url = controller_->GetTemplateURL( + [self indexInModelForRow:clickedRow]); // The controller will release itself when the window closes. EditSearchEngineCocoaController* editor = [[EditSearchEngineCocoaController alloc] initWithProfile:profile_ @@ -284,7 +286,7 @@ typedef std::map ProfileControllerMap; DCHECK_GT([selection count], 0U); NSUInteger index = [selection lastIndex]; while (index != NSNotFound) { - controller_->RemoveTemplateURL(index); + controller_->RemoveTemplateURL([self indexInModelForRow:index]); index = [selection indexLessThanIndex:index]; } } @@ -292,33 +294,50 @@ typedef std::map ProfileControllerMap; - (IBAction)makeDefault:(id)sender { NSIndexSet* selection = [tableView_ selectedRowIndexes]; DCHECK_EQ([selection count], 1U); - controller_->MakeDefaultTemplateURL([selection firstIndex]); + int row = [self indexInModelForRow:[selection firstIndex]]; + controller_->MakeDefaultTemplateURL(row); } // Table View Data Source ----------------------------------------------------- -- (NSInteger)numberOfRowsInTableView:(NSTableView*)aTableView { - return controller_->table_model()->RowCount(); +- (NSInteger)numberOfRowsInTableView:(NSTableView*)table { + int rowCount = controller_->table_model()->RowCount(); + int numGroups = controller_->table_model()->GetGroups().size(); + if ([self tableView:table isGroupRow:rowCount + numGroups - 1]) { + // Don't show a group header with no rows underneath it. + --numGroups; + } + return rowCount + numGroups; } - (id)tableView:(NSTableView*)tv objectValueForTableColumn:(NSTableColumn*)tableColumn row:(NSInteger)row { - if (!tableColumn) - return nil; - NSString* identifier = [tableColumn identifier]; + if ([self tableView:tv isGroupRow:row]) { + DCHECK(!tableColumn); + TableModel::Groups groups = controller_->table_model()->GetGroups(); + if (row == 0) { + return base::SysWideToNSString(groups[0].title); + } else { + return base::SysWideToNSString(groups[1].title); + } + } + NSString* identifier = [tableColumn identifier]; if ([identifier isEqualToString:@"name"]) { // The name column is an NSButtonCell so we can have text and image in the // same cell. As such, the "object value" for a button cell is either on // or off, so we always return off so we don't act like a button. return [NSNumber numberWithInt:NSOffState]; - } else if ([identifier isEqualToString:@"keyword"]) { + } + if ([identifier isEqualToString:@"keyword"]) { // The keyword object value is a normal string. + int index = [self indexInModelForRow:row]; int columnID = IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN; - std::wstring text = controller_->table_model()->GetText(row, columnID); + std::wstring text = controller_->table_model()->GetText(index, columnID); return base::SysWideToNSString(text); } + // And we shouldn't have any other columns... NOTREACHED(); return nil; @@ -331,20 +350,39 @@ typedef std::map ProfileControllerMap; [self adjustEditingButtons]; } +// Disallow selection of the group header rows. +- (BOOL)tableView:(NSTableView*)table shouldSelectRow:(NSInteger)row { + return ![self tableView:table isGroupRow:row]; +} + +- (BOOL)tableView:(NSTableView*)table isGroupRow:(NSInteger)row { + int otherGroupRow = + controller_->table_model()->last_search_engine_index() + 1; + return (row == 0 || row == otherGroupRow); +} + - (NSCell*)tableView:(NSTableView*)tableView dataCellForTableColumn:(NSTableColumn*)tableColumn row:(NSInteger)row { static const CGFloat kCellFontSize = 12.0; + + // Check to see if we are a grouped row. + if ([self tableView:tableView isGroupRow:row]) { + DCHECK(!tableColumn); // This would violate the group row contract. + return groupCell_.get(); + } + NSCell* cell = [tableColumn dataCellForRow:row]; + int offsetRow = [self indexInModelForRow:row]; // Set the favicon and title for the search engine in the name column. if ([[tableColumn identifier] isEqualToString:@"name"]) { DCHECK([cell isKindOfClass:[NSButtonCell class]]); NSButtonCell* buttonCell = static_cast(cell); - std::wstring title = controller_->table_model()->GetText(row, + std::wstring title = controller_->table_model()->GetText(offsetRow, IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN); [buttonCell setTitle:base::SysWideToNSString(title)]; - [buttonCell setImage:observer_->GetImageForRow(row)]; + [buttonCell setImage:observer_->GetImageForRow(offsetRow)]; [buttonCell setRefusesFirstResponder:YES]; // Don't push in like a button. [buttonCell setHighlightsBy:NSNoCellMask]; } @@ -352,7 +390,8 @@ typedef std::map ProfileControllerMap; // The default search engine should be in bold font. const TemplateURL* defaultEngine = controller_->url_model()->GetDefaultSearchProvider(); - if (controller_->table_model()->IndexOfTemplateURL(defaultEngine) == row) { + int rowIndex = controller_->table_model()->IndexOfTemplateURL(defaultEngine); + if (rowIndex == offsetRow) { [cell setFont:[NSFont boldSystemFontOfSize:kCellFontSize]]; } else { [cell setFont:[NSFont systemFontOfSize:kCellFontSize]]; @@ -365,13 +404,15 @@ typedef std::map ProfileControllerMap; // This function appropriately sets the enabled states on the table's editing // buttons. - (void)adjustEditingButtons { - // Delete button. NSIndexSet* selection = [tableView_ selectedRowIndexes]; BOOL canRemove = ([selection count] > 0); NSUInteger index = [selection firstIndex]; + + // Delete button. while (canRemove && index != NSNotFound) { + int modelIndex = [self indexInModelForRow:index]; const TemplateURL& url = - controller_->table_model()->GetTemplateURL(index); + controller_->table_model()->GetTemplateURL(modelIndex); if (!controller_->CanRemove(&url)) canRemove = NO; index = [selection indexGreaterThanIndex:index]; @@ -382,10 +423,25 @@ typedef std::map ProfileControllerMap; if ([selection count] != 1) { [makeDefaultButton_ setEnabled:NO]; } else { + int row = [self indexInModelForRow:[selection firstIndex]]; const TemplateURL& url = - controller_->table_model()->GetTemplateURL([selection firstIndex]); + controller_->table_model()->GetTemplateURL(row); [makeDefaultButton_ setEnabled:controller_->CanMakeDefault(&url)]; } } +// This converts a row index in our table view to an index in the model by +// computing the group offsets. +- (int)indexInModelForRow:(NSUInteger)row { + DCHECK_GT(row, 0U); + unsigned otherGroupId = + controller_->table_model()->last_search_engine_index() + 1; + DCHECK_NE(row, otherGroupId); + if (row >= otherGroupId) { + return row - 2; // Other group. + } else { + return row - 1; // Default group. + } +} + @end diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm b/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm index e551175..aad19ed 100644 --- a/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm +++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm @@ -35,6 +35,10 @@ return observer_.get(); } +- (NSTableView*)tableView { + return tableView_; +} + @end // TODO(rsesek): Figure out a good way to test this class (crbug.com/21640). @@ -49,7 +53,8 @@ class KeywordEditorCocoaControllerTest : public CocoaTest { static_cast(browser_helper_.profile()); profile->CreateTemplateURLModel(); - controller_ = [[FakeKeywordEditorController alloc] initWithProfile:profile]; + controller_ = + [[FakeKeywordEditorController alloc] initWithProfile:profile]; } virtual void TearDown() { @@ -132,4 +137,84 @@ TEST_F(KeywordEditorCocoaControllerTest, ShowKeywordEditor) { [newSharedInstance close]; } +TEST_F(KeywordEditorCocoaControllerTest, IndexInModelForRowMixed) { + [controller_ window]; // Force |-awakeFromNib|. + TemplateURLModel* template_model = [controller_ controller]->url_model(); + + // Add a default engine. + TemplateURL* t_url = new TemplateURL(); + t_url->SetURL(L"http://test1/{searchTerms}", 0, 0); + t_url->set_keyword(L"test1"); + t_url->set_short_name(L"Test1"); + t_url->set_show_in_default_list(true); + template_model->Add(t_url); + + // Add a non-default engine. + t_url = new TemplateURL(); + t_url->SetURL(L"http://test2/{searchTerms}", 0, 0); + t_url->set_keyword(L"test2"); + t_url->set_short_name(L"Test2"); + t_url->set_show_in_default_list(false); + template_model->Add(t_url); + + // Two headers with a single row underneath each. + NSTableView* table = [controller_ tableView]; + [table reloadData]; + ASSERT_EQ(4, [[controller_ tableView] numberOfRows]); + + // Index 0 is the group header, index 1 should be the first engine. + ASSERT_EQ(0, [controller_ indexInModelForRow:1]); + + // Index 2 should be the group header, so index 3 should be the non-default + // engine. + ASSERT_EQ(1, [controller_ indexInModelForRow:3]); + + ASSERT_TRUE([controller_ tableView:table isGroupRow:0]); + ASSERT_FALSE([controller_ tableView:table isGroupRow:1]); + ASSERT_TRUE([controller_ tableView:table isGroupRow:2]); + ASSERT_FALSE([controller_ tableView:table isGroupRow:3]); + + ASSERT_FALSE([controller_ tableView:table shouldSelectRow:0]); + ASSERT_TRUE([controller_ tableView:table shouldSelectRow:1]); + ASSERT_FALSE([controller_ tableView:table shouldSelectRow:2]); + ASSERT_TRUE([controller_ tableView:table shouldSelectRow:3]); +} + +TEST_F(KeywordEditorCocoaControllerTest, IndexInModelForDefault) { + [controller_ window]; // Force |-awakeFromNib|. + TemplateURLModel* template_model = [controller_ controller]->url_model(); + + // Add 2 default engines. + TemplateURL* t_url = new TemplateURL(); + t_url->SetURL(L"http://test1/{searchTerms}", 0, 0); + t_url->set_keyword(L"test1"); + t_url->set_short_name(L"Test1"); + t_url->set_show_in_default_list(true); + template_model->Add(t_url); + + t_url = new TemplateURL(); + t_url->SetURL(L"http://test2/{searchTerms}", 0, 0); + t_url->set_keyword(L"test2"); + t_url->set_short_name(L"Test2"); + t_url->set_show_in_default_list(true); + template_model->Add(t_url); + + // One header and two rows. + NSTableView* table = [controller_ tableView]; + [table reloadData]; + ASSERT_EQ(3, [[controller_ tableView] numberOfRows]); + + // Index 0 is the group header, index 1 should be the first engine. + ASSERT_EQ(0, [controller_ indexInModelForRow:1]); + ASSERT_EQ(1, [controller_ indexInModelForRow:2]); + + ASSERT_TRUE([controller_ tableView:table isGroupRow:0]); + ASSERT_FALSE([controller_ tableView:table isGroupRow:1]); + ASSERT_FALSE([controller_ tableView:table isGroupRow:2]); + + ASSERT_FALSE([controller_ tableView:table shouldSelectRow:0]); + ASSERT_TRUE([controller_ tableView:table shouldSelectRow:1]); + ASSERT_TRUE([controller_ tableView:table shouldSelectRow:2]); +} + } // namespace -- cgit v1.1