diff options
author | sky@google.com <sky@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-03 23:14:01 +0000 |
---|---|---|
committer | sky@google.com <sky@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-03 23:14:01 +0000 |
commit | 3ec1a3af311b95b325a3302679be1eb68e89e58e (patch) | |
tree | 61049c73993b824ce537cf62cea8acfb74cbf14a /chrome/browser/bookmarks | |
parent | 32f720c5a051885d45b79533522c06870e7037ff (diff) | |
download | chromium_src-3ec1a3af311b95b325a3302679be1eb68e89e58e.zip chromium_src-3ec1a3af311b95b325a3302679be1eb68e89e58e.tar.gz chromium_src-3ec1a3af311b95b325a3302679be1eb68e89e58e.tar.bz2 |
Miscellaneous bookmark manager polish:
. Adds a menu button on the bookmark manager. If I finish up import
I'll add a second one for importing.
. Creating new page/folder from menu items selects new item in table.
. I changed around the FolderBookmarkTableModel to copy the contents
into a vector (now extends VectorBackedTabledModel). This is
necessitated by TableView not providing a moved notification. The
problem with previous approach is that I sent out ModelChanged on
any change, which loses selection and causes things to snap around.
I considered adding a moved method to TableView, but it's too much
work at this time.
. Added persisting of divider location in bookmark manager.
. When focus is on table pressing enter on a folder descends into the
folder, and pressing backspace goes back up a folder.
BUG=674
TEST=none
Review URL: http://codereview.chromium.org/8967
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@4512 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/bookmarks')
-rw-r--r-- | chrome/browser/bookmarks/bookmark_context_menu.cc | 130 | ||||
-rw-r--r-- | chrome/browser/bookmarks/bookmark_context_menu.h | 3 | ||||
-rw-r--r-- | chrome/browser/bookmarks/bookmark_table_model.cc | 161 |
3 files changed, 180 insertions, 114 deletions
diff --git a/chrome/browser/bookmarks/bookmark_context_menu.cc b/chrome/browser/bookmarks/bookmark_context_menu.cc index 4f033ac..0e45493 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu.cc +++ b/chrome/browser/bookmarks/bookmark_context_menu.cc @@ -56,10 +56,11 @@ class EditFolderController : public InputWindowDelegate, static void Show(Profile* profile, HWND hwnd, BookmarkNode* node, - bool is_new) { + bool is_new, + bool show_in_manager) { // EditFolderController deletes itself when done. EditFolderController* controller = - new EditFolderController(profile, hwnd, node, is_new); + new EditFolderController(profile, hwnd, node, is_new, show_in_manager); controller->Show(); } @@ -67,11 +68,13 @@ class EditFolderController : public InputWindowDelegate, EditFolderController(Profile* profile, HWND hwnd, BookmarkNode* node, - bool is_new) + bool is_new, + bool show_in_manager) : profile_(profile), model_(profile->GetBookmarkModel()), node_(node), - is_new_(is_new) { + is_new_(is_new), + show_in_manager_(show_in_manager) { DCHECK(is_new_ || node); window_ = CreateInputWindow(hwnd, this); model_->AddObserver(this); @@ -97,10 +100,17 @@ class EditFolderController : public InputWindowDelegate, } virtual void InputAccepted(const std::wstring& text) { - if (is_new_) - model_->AddGroup(node_, node_->GetChildCount(), text); - else + if (is_new_) { + BookmarkNode* node = + model_->AddGroup(node_, node_->GetChildCount(), text); + if (show_in_manager_) { + BookmarkManagerView* manager = BookmarkManagerView::current(); + if (manager && manager->profile() == profile_) + manager->SelectInTree(node); + } + } else { model_->SetTitle(node_, text); + } } virtual void InputCanceled() { @@ -164,11 +174,38 @@ class EditFolderController : public InputWindowDelegate, BookmarkNode* node_; bool is_new_; + + // If is_new_ is true and a new node is created, it is selected in the + // bookmark manager. + bool show_in_manager_; views::Window* window_; DISALLOW_COPY_AND_ASSIGN(EditFolderController); }; +// SelectOnCreationHandler ---------------------------------------------------- + +// Used when adding a new bookmark. If a new bookmark is created it is selected +// in the bookmark manager. +class SelectOnCreationHandler : public BookmarkEditorView::Handler { + public: + explicit SelectOnCreationHandler(Profile* profile) : profile_(profile) { + } + + virtual void NodeCreated(BookmarkNode* new_node) { + BookmarkManagerView* manager = BookmarkManagerView::current(); + if (!manager || manager->profile() != profile_) + return; // Manager no longer showing, or showing a different profile. + + manager->SelectInTree(new_node); + } + + private: + Profile* profile_; + + DISALLOW_COPY_AND_ASSIGN(SelectOnCreationHandler); +}; + } // namespace // BookmarkContextMenu ------------------------------------------- @@ -192,36 +229,39 @@ BookmarkContextMenu::BookmarkContextMenu( DCHECK(profile_); DCHECK(model_->IsLoaded()); menu_.reset(new views::MenuItemView(this)); - if (selection.size() == 1 && selection[0]->is_url()) { - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB)); - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW)); - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_INCOGNITO)); - } else { - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL)); - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); - menu_->AppendMenuItemWithLabel( - IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, - l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + if (configuration != BOOKMARK_MANAGER_ORGANIZE_MENU) { + if (selection.size() == 1 && selection[0]->is_url()) { + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB)); + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW)); + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_INCOGNITO)); + } else { + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL)); + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW)); + menu_->AppendMenuItemWithLabel( + IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO, + l10n_util::GetString(IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO)); + } + menu_->AppendSeparator(); } - menu_->AppendSeparator(); menu_->AppendMenuItemWithLabel(IDS_BOOKMARK_BAR_EDIT, - l10n_util::GetString(IDS_BOOKMARK_BAR_EDIT)); + l10n_util::GetString(IDS_BOOKMARK_BAR_EDIT)); menu_->AppendMenuItemWithLabel( IDS_BOOKMARK_BAR_REMOVE, l10n_util::GetString(IDS_BOOKMARK_BAR_REMOVE)); - if (configuration != BOOKMARK_BAR) { + if (configuration == BOOKMARK_MANAGER_TABLE || + configuration == BOOKMARK_MANAGER_ORGANIZE_MENU) { menu_->AppendMenuItemWithLabel( IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER, l10n_util::GetString(IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER)); @@ -239,10 +279,10 @@ BookmarkContextMenu::BookmarkContextMenu( if (configuration == BOOKMARK_BAR) { menu_->AppendSeparator(); menu_->AppendMenuItemWithLabel(IDS_BOOKMARK_MANAGER, - l10n_util::GetString(IDS_BOOKMARK_MANAGER)); + l10n_util::GetString(IDS_BOOKMARK_MANAGER)); menu_->AppendMenuItem(IDS_BOOMARK_BAR_ALWAYS_SHOW, - l10n_util::GetString(IDS_BOOMARK_BAR_ALWAYS_SHOW), - views::MenuItemView::CHECKBOX); + l10n_util::GetString(IDS_BOOMARK_BAR_ALWAYS_SHOW), + views::MenuItemView::CHECKBOX); } model_->AddObserver(this); } @@ -304,9 +344,10 @@ void BookmarkContextMenu::ExecuteCommand(int id) { else editor_config = BookmarkEditorView::NO_TREE; BookmarkEditorView::Show(hwnd_, profile_, NULL, selection_[0], - editor_config); + editor_config, NULL); } else { - EditFolderController::Show(profile_, hwnd_, selection_[0], false); + EditFolderController::Show(profile_, hwnd_, selection_[0], false, + false); } break; @@ -326,12 +367,16 @@ void BookmarkContextMenu::ExecuteCommand(int id) { UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Add", profile_); BookmarkEditorView::Configuration editor_config; - if (configuration_ == BOOKMARK_BAR) + BookmarkEditorView::Handler* handler = NULL; + if (configuration_ == BOOKMARK_BAR) { editor_config = BookmarkEditorView::SHOW_TREE; - else + } else { editor_config = BookmarkEditorView::NO_TREE; + // This is owned by the BookmarkEditorView. + handler = new SelectOnCreationHandler(profile_); + } BookmarkEditorView::Show(hwnd_, profile_, GetParentForNewNodes(), NULL, - editor_config); + editor_config, handler); break; } @@ -340,7 +385,7 @@ void BookmarkContextMenu::ExecuteCommand(int id) { profile_); EditFolderController::Show(profile_, hwnd_, GetParentForNewNodes(), - true); + true, (configuration_ != BOOKMARK_BAR)); break; } @@ -361,6 +406,10 @@ void BookmarkContextMenu::ExecuteCommand(int id) { BookmarkManagerView::current()->SelectInTree(selection_[0]); break; + case IDS_BOOKMARK_MANAGER: + BookmarkManagerView::Show(profile_); + break; + default: NOTREACHED(); } @@ -393,7 +442,8 @@ bool BookmarkContextMenu::IsCommandEnabled(int id) const { return !selection_.empty() && !is_root_node; case IDS_BOOKMARK_MANAGER_SHOW_IN_FOLDER: - return configuration_ == BOOKMARK_MANAGER_TABLE && + return (configuration_ == BOOKMARK_MANAGER_TABLE || + configuration_ == BOOKMARK_MANAGER_ORGANIZE_MENU) && selection_.size() == 1; case IDS_BOOMARK_BAR_NEW_FOLDER: diff --git a/chrome/browser/bookmarks/bookmark_context_menu.h b/chrome/browser/bookmarks/bookmark_context_menu.h index 5b48627..2991637 100644 --- a/chrome/browser/bookmarks/bookmark_context_menu.h +++ b/chrome/browser/bookmarks/bookmark_context_menu.h @@ -23,7 +23,8 @@ class BookmarkContextMenu : public views::MenuDelegate, enum ConfigurationType { BOOKMARK_BAR, BOOKMARK_MANAGER_TABLE, - BOOKMARK_MANAGER_TREE + BOOKMARK_MANAGER_TREE, + BOOKMARK_MANAGER_ORGANIZE_MENU }; // Creates the bookmark context menu. diff --git a/chrome/browser/bookmarks/bookmark_table_model.cc b/chrome/browser/bookmarks/bookmark_table_model.cc index c55f6ff..a9122fa 100644 --- a/chrome/browser/bookmarks/bookmark_table_model.cc +++ b/chrome/browser/bookmarks/bookmark_table_model.cc @@ -20,22 +20,72 @@ namespace { // Number of bookmarks shown in recently bookmarked. const int kRecentlyBookmarkedCount = 50; -// FolderBookmarkTableModel ---------------------------------------------------- +// VectorBackedBookmarkTableModel ---------------------------------------------- -class FolderBookmarkTableModel : public BookmarkTableModel { +class VectorBackedBookmarkTableModel : public BookmarkTableModel { public: - FolderBookmarkTableModel(BookmarkModel* model, BookmarkNode* root_node) - : BookmarkTableModel(model), - root_node_(root_node) { + explicit VectorBackedBookmarkTableModel(BookmarkModel* model) + : BookmarkTableModel(model) { + } + + virtual BookmarkNode* GetNodeForRow(int row) { + return nodes_[row]; } virtual int RowCount() { - return root_node_ ? root_node_->GetChildCount() : 0; + return static_cast<int>(nodes_.size()); } - virtual BookmarkNode* GetNodeForRow(int row) { - DCHECK(root_node_); - return root_node_->GetChild(row); + virtual void BookmarkNodeMoved(BookmarkModel* model, + BookmarkNode* old_parent, + int old_index, + BookmarkNode* new_parent, + int new_index) { + NotifyObserverOfChange(new_parent->GetChild(new_index)); + } + + virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model, + BookmarkNode* node) { + NotifyObserverOfChange(node); + } + + virtual void BookmarkNodeChanged(BookmarkModel* model, + BookmarkNode* node) { + NotifyObserverOfChange(node); + } + + protected: + void NotifyObserverOfChange(BookmarkNode* node) { + if (!observer()) + return; + + int index = IndexOfNode(node); + if (index != -1) + observer()->OnItemsChanged(index, 1); + } + + typedef std::vector<BookmarkNode*> Nodes; + Nodes& nodes() { return nodes_; } + + private: + Nodes nodes_; + + DISALLOW_COPY_AND_ASSIGN(VectorBackedBookmarkTableModel); +}; + +// FolderBookmarkTableModel ---------------------------------------------------- + +// FolderBookmarkTableModel is a TableModel implementation backed by the +// contents of a bookmark folder. FolderBookmarkTableModel caches the contents +// of the folder so that it can send out the correct events when a bookmark +// node is moved. +class FolderBookmarkTableModel : public VectorBackedBookmarkTableModel { + public: + FolderBookmarkTableModel(BookmarkModel* model, BookmarkNode* root_node) + : VectorBackedBookmarkTableModel(model), + root_node_(root_node) { + for (int i = 0; i < root_node->GetChildCount(); ++i) + nodes().push_back(root_node->GetChild(i)); } virtual void BookmarkNodeMoved(BookmarkModel* model, @@ -43,15 +93,27 @@ class FolderBookmarkTableModel : public BookmarkTableModel { int old_index, BookmarkNode* new_parent, int new_index) { - if (observer() && (old_parent == root_node_ || new_parent == root_node_)) - observer()->OnModelChanged(); + if (old_parent == root_node_) { + nodes().erase(nodes().begin() + old_index); + if (observer()) + observer()->OnItemsRemoved(old_index, 1); + } + if (new_parent == root_node_) { + nodes().insert(nodes().begin() + new_index, + root_node_->GetChild(new_index)); + if (observer()) + observer()->OnItemsAdded(new_index, 1); + } } virtual void BookmarkNodeAdded(BookmarkModel* model, BookmarkNode* parent, int index) { - if (root_node_ == parent && observer()) - observer()->OnItemsAdded(index, 1); + if (root_node_ == parent) { + nodes().insert(nodes().begin() + index, parent->GetChild(index)); + if (observer()) + observer()->OnItemsAdded(index, 1); + } } virtual void BookmarkNodeRemoved(BookmarkModel* model, @@ -59,14 +121,18 @@ class FolderBookmarkTableModel : public BookmarkTableModel { int index, BookmarkNode* node) { if (root_node_->HasAncestor(node)) { - // We, our one of our ancestors was removed. + // We, or one of our ancestors was removed. root_node_ = NULL; + nodes().clear(); if (observer()) observer()->OnModelChanged(); return; } - if (root_node_ == parent && observer()) - observer()->OnItemsRemoved(index, 1); + if (root_node_ == parent) { + nodes().erase(nodes().begin() + index); + if (observer()) + observer()->OnItemsRemoved(index, 1); + } } virtual void BookmarkNodeChanged(BookmarkModel* model, @@ -92,57 +158,6 @@ class FolderBookmarkTableModel : public BookmarkTableModel { DISALLOW_COPY_AND_ASSIGN(FolderBookmarkTableModel); }; -// VectorBackedBookmarkTableModel ---------------------------------------------- - -class VectorBackedBookmarkTableModel : public BookmarkTableModel { - public: - explicit VectorBackedBookmarkTableModel(BookmarkModel* model) - : BookmarkTableModel(model) { - } - - virtual BookmarkNode* GetNodeForRow(int row) { - return nodes_[row]; - } - - virtual int RowCount() { - return static_cast<int>(nodes_.size()); - } - - virtual void BookmarkNodeMoved(BookmarkModel* model, - BookmarkNode* old_parent, - int old_index, - BookmarkNode* new_parent, - int new_index) { - NotifyObserverOfChange(new_parent->GetChild(new_index)); - } - - virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model, - BookmarkNode* node) { - NotifyObserverOfChange(node); - } - - virtual void BookmarkNodeChanged(BookmarkModel* model, - BookmarkNode* node) { - NotifyObserverOfChange(node); - } - - protected: - void NotifyObserverOfChange(BookmarkNode* node) { - if (!observer()) - return; - - int index = IndexOfNode(node); - if (index != -1) - observer()->OnItemsChanged(index, 1); - } - - typedef std::vector<BookmarkNode*> Nodes; - Nodes nodes_; - - private: - DISALLOW_COPY_AND_ASSIGN(VectorBackedBookmarkTableModel); -}; - // RecentlyBookmarkedTableModel ------------------------------------------------ class RecentlyBookmarkedTableModel : public VectorBackedBookmarkTableModel { @@ -169,8 +184,8 @@ class RecentlyBookmarkedTableModel : public VectorBackedBookmarkTableModel { private: void UpdateRecentlyBookmarked() { - nodes_.clear(); - model()->GetMostRecentlyAddedEntries(kRecentlyBookmarkedCount, &nodes_); + nodes().clear(); + model()->GetMostRecentlyAddedEntries(kRecentlyBookmarkedCount, &nodes()); if (observer()) observer()->OnModelChanged(); } @@ -190,7 +205,7 @@ class BookmarkSearchTableModel : public VectorBackedBookmarkTableModel { model->GetBookmarksMatchingText(search_text, std::numeric_limits<int>::max(), &matches); for (size_t i = 0; i < matches.size(); ++i) - nodes_.push_back(matches[i].node); + nodes().push_back(matches[i].node); } virtual void BookmarkNodeAdded(BookmarkModel* model, @@ -198,9 +213,9 @@ class BookmarkSearchTableModel : public VectorBackedBookmarkTableModel { int index) { BookmarkNode* node = parent->GetChild(index); if (model->DoesBookmarkMatchText(search_text_, node)) { - nodes_.push_back(node); + nodes().push_back(node); if (observer()) - observer()->OnItemsAdded(static_cast<int>(nodes_.size() - 1), 1); + observer()->OnItemsAdded(static_cast<int>(nodes().size() - 1), 1); } } @@ -212,7 +227,7 @@ class BookmarkSearchTableModel : public VectorBackedBookmarkTableModel { if (internal_index == -1) return; - nodes_.erase(nodes_.begin() + static_cast<int>(internal_index)); + nodes().erase(nodes().begin() + static_cast<int>(internal_index)); if (observer()) observer()->OnItemsRemoved(internal_index, 1); } |