From d830badc571502fe3ea713531f0f98d6042bc8ed Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 1 Sep 2015 19:48:11 +0800 Subject: [PATCH] Add role property for MenuItem --- atom/browser/api/atom_api_menu.cc | 5 ++ atom/browser/api/atom_api_menu.h | 1 + atom/browser/api/lib/menu-item.coffee | 3 +- atom/browser/api/lib/menu.coffee | 1 + atom/browser/default_app/main.js | 29 +++++----- atom/browser/ui/atom_menu_model.cc | 13 +++++ atom/browser/ui/atom_menu_model.h | 6 ++ atom/browser/ui/cocoa/atom_menu_controller.h | 13 ----- atom/browser/ui/cocoa/atom_menu_controller.mm | 56 +++++++++++++++---- 9 files changed, 89 insertions(+), 38 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index 356a4d4ce494..9bd724a9612e 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -107,6 +107,10 @@ void Menu::SetSublabel(int index, const base::string16& sublabel) { model_->SetSublabel(index, sublabel); } +void Menu::SetRole(int index, const base::string16& role) { + model_->SetRole(index, role); +} + void Menu::Clear() { model_->Clear(); } @@ -154,6 +158,7 @@ void Menu::BuildPrototype(v8::Isolate* isolate, .SetMethod("insertSubMenu", &Menu::InsertSubMenuAt) .SetMethod("setIcon", &Menu::SetIcon) .SetMethod("setSublabel", &Menu::SetSublabel) + .SetMethod("setRole", &Menu::SetRole) .SetMethod("clear", &Menu::Clear) .SetMethod("getIndexOfCommandId", &Menu::GetIndexOfCommandId) .SetMethod("getItemCount", &Menu::GetItemCount) diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 33aafbc45d3b..0d93c0d46be6 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -73,6 +73,7 @@ class Menu : public mate::Wrappable, Menu* menu); void SetIcon(int index, const gfx::Image& image); void SetSublabel(int index, const base::string16& sublabel); + void SetRole(int index, const base::string16& role); void Clear(); int GetIndexOfCommandId(int command_id); int GetItemCount() const; diff --git a/atom/browser/api/lib/menu-item.coffee b/atom/browser/api/lib/menu-item.coffee index d8e896f8ffa1..7663b32996d8 100644 --- a/atom/browser/api/lib/menu-item.coffee +++ b/atom/browser/api/lib/menu-item.coffee @@ -9,12 +9,13 @@ class MenuItem constructor: (options) -> Menu = require 'menu' - {click, @selector, @type, @label, @sublabel, @accelerator, @icon, @enabled, @visible, @checked, @submenu} = options + {click, @selector, @type, @role, @label, @sublabel, @accelerator, @icon, @enabled, @visible, @checked, @submenu} = options @type = 'submenu' if not @type? and @submenu? throw new Error('Invalid submenu') if @type is 'submenu' and @submenu?.constructor isnt Menu @overrideReadOnlyProperty 'type', 'normal' + @overrideReadOnlyProperty 'role' @overrideReadOnlyProperty 'accelerator' @overrideReadOnlyProperty 'icon' @overrideReadOnlyProperty 'submenu' diff --git a/atom/browser/api/lib/menu.coffee b/atom/browser/api/lib/menu.coffee index af70af740aaf..35335ffcebe5 100644 --- a/atom/browser/api/lib/menu.coffee +++ b/atom/browser/api/lib/menu.coffee @@ -115,6 +115,7 @@ Menu::insert = (pos, item) -> @setSublabel pos, item.sublabel if item.sublabel? @setIcon pos, item.icon if item.icon? + @setRole pos, item.role if item.role? # Make menu accessable to items. item.overrideReadOnlyProperty 'menu', this diff --git a/atom/browser/default_app/main.js b/atom/browser/default_app/main.js index fd3a6b596b6b..99c4ad03320b 100644 --- a/atom/browser/default_app/main.js +++ b/atom/browser/default_app/main.js @@ -44,13 +44,14 @@ app.once('ready', function() { submenu: [ { label: 'About Electron', - selector: 'orderFrontStandardAboutPanel:' + role: 'about' }, { type: 'separator' }, { label: 'Services', + role: 'services', submenu: [] }, { @@ -59,16 +60,16 @@ app.once('ready', function() { { label: 'Hide Electron', accelerator: 'Command+H', - selector: 'hide:' + role: 'hide' }, { label: 'Hide Others', accelerator: 'Command+Shift+H', - selector: 'hideOtherApplications:' + role: 'hideothers:' }, { label: 'Show All', - selector: 'unhideAllApplications:' + role: 'unhide:' }, { type: 'separator' @@ -86,12 +87,12 @@ app.once('ready', function() { { label: 'Undo', accelerator: 'Command+Z', - selector: 'undo:' + role: 'undo' }, { label: 'Redo', accelerator: 'Shift+Command+Z', - selector: 'redo:' + role: 'redo' }, { type: 'separator' @@ -99,22 +100,22 @@ app.once('ready', function() { { label: 'Cut', accelerator: 'Command+X', - selector: 'cut:' + role: 'cut' }, { label: 'Copy', accelerator: 'Command+C', - selector: 'copy:' + role: 'copy' }, { label: 'Paste', accelerator: 'Command+V', - selector: 'paste:' + role: 'paste' }, { label: 'Select All', accelerator: 'Command+A', - selector: 'selectAll:' + role: 'selectall' }, ] }, @@ -152,28 +153,30 @@ app.once('ready', function() { }, { label: 'Window', + role: 'window', submenu: [ { label: 'Minimize', accelerator: 'Command+M', - selector: 'performMiniaturize:' + role: 'minimize' }, { label: 'Close', accelerator: 'Command+W', - selector: 'performClose:' + role: 'close' }, { type: 'separator' }, { label: 'Bring All to Front', - selector: 'arrangeInFront:' + role: 'front' }, ] }, { label: 'Help', + role: 'help', submenu: [ { label: 'Learn More', diff --git a/atom/browser/ui/atom_menu_model.cc b/atom/browser/ui/atom_menu_model.cc index 7d2d5e1b6a1f..9add7a22715e 100644 --- a/atom/browser/ui/atom_menu_model.cc +++ b/atom/browser/ui/atom_menu_model.cc @@ -4,6 +4,8 @@ #include "atom/browser/ui/atom_menu_model.h" +#include "base/stl_util.h" + namespace atom { AtomMenuModel::AtomMenuModel(Delegate* delegate) @@ -14,6 +16,17 @@ AtomMenuModel::AtomMenuModel(Delegate* delegate) AtomMenuModel::~AtomMenuModel() { } +void AtomMenuModel::SetRole(int index, const base::string16& role) { + roles_[index] = role; +} + +base::string16 AtomMenuModel::GetRoleAt(int index) { + if (ContainsKey(roles_, index)) + return roles_[index]; + else + return base::string16(); +} + void AtomMenuModel::MenuClosed() { ui::SimpleMenuModel::MenuClosed(); FOR_EACH_OBSERVER(Observer, observers_, MenuClosed()); diff --git a/atom/browser/ui/atom_menu_model.h b/atom/browser/ui/atom_menu_model.h index 42e0e5dbc53a..fe19a8dc2518 100644 --- a/atom/browser/ui/atom_menu_model.h +++ b/atom/browser/ui/atom_menu_model.h @@ -5,6 +5,8 @@ #ifndef ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_ #define ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_ +#include + #include "base/observer_list.h" #include "ui/base/models/simple_menu_model.h" @@ -31,12 +33,16 @@ class AtomMenuModel : public ui::SimpleMenuModel { void AddObserver(Observer* obs) { observers_.AddObserver(obs); } void RemoveObserver(Observer* obs) { observers_.RemoveObserver(obs); } + void SetRole(int index, const base::string16& role); + base::string16 GetRoleAt(int index); + // ui::SimpleMenuModel: void MenuClosed() override; private: Delegate* delegate_; // weak ref. + std::map roles_; ObserverList observers_; DISALLOW_COPY_AND_ASSIGN(AtomMenuModel); diff --git a/atom/browser/ui/cocoa/atom_menu_controller.h b/atom/browser/ui/cocoa/atom_menu_controller.h index da10e1b0ba66..f8c48aa5dcb5 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.h +++ b/atom/browser/ui/cocoa/atom_menu_controller.h @@ -59,17 +59,4 @@ class MenuModel; @end -// Exposed only for unit testing, do not call directly. -@interface AtomMenuController (PrivateExposedForTesting) -- (BOOL)validateUserInterfaceItem:(id)item; -@end - -// Protected methods that subclassers can override. -@interface AtomMenuController (Protected) -- (void)addItemToMenu:(NSMenu*)menu - atIndex:(NSInteger)index - fromModel:(ui::MenuModel*)model; -- (NSMenu*)menuFromModel:(ui::MenuModel*)model; -@end - #endif // ATOM_BROWSER_UI_COCOA_ATOM_MENU_CONTROLLER_H_ diff --git a/atom/browser/ui/cocoa/atom_menu_controller.mm b/atom/browser/ui/cocoa/atom_menu_controller.mm index d799a9f9b64d..e3aa78aa248c 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.mm +++ b/atom/browser/ui/cocoa/atom_menu_controller.mm @@ -8,16 +8,36 @@ #include "atom/browser/ui/atom_menu_model.h" #include "base/logging.h" #include "base/strings/sys_string_conversions.h" +#include "base/strings/utf_string_conversions.h" #include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/platform_accelerator_cocoa.h" #include "ui/base/l10n/l10n_util_mac.h" #include "ui/events/cocoa/cocoa_event_utils.h" #include "ui/gfx/image/image.h" -@interface AtomMenuController (Private) -- (void)addSeparatorToMenu:(NSMenu*)menu - atIndex:(int)index; -@end +namespace { + +struct Role { + SEL selector; + const char* role; +}; +Role kRolesMap[] = { + { @selector(orderFrontStandardAboutPanel:), "about" }, + { @selector(hide:), "hide" }, + { @selector(hideOtherApplications:), "hideothers" }, + { @selector(unhideAllApplications:), "unhide" }, + { @selector(arrangeInFront:), "front" }, + { @selector(undo:), "undo" }, + { @selector(redo:), "redo" }, + { @selector(cut:), "cut" }, + { @selector(copy:), "copy" }, + { @selector(paste:), "paste" }, + { @selector(selectAll:), "selectall" }, + { @selector(performMiniaturize:), "minimize" }, + { @selector(performClose:), "close" }, +}; + +} // namespace @implementation AtomMenuController @@ -101,7 +121,9 @@ // associated with the entry in the model identified by |modelIndex|. - (void)addItemToMenu:(NSMenu*)menu atIndex:(NSInteger)index - fromModel:(ui::MenuModel*)model { + fromModel:(ui::MenuModel*)ui_model { + atom::AtomMenuModel* model = static_cast(ui_model); + base::string16 label16 = model->GetLabelAt(index); NSString* label = l10n_util::FixUpWindowsStyleLabel(label16); base::scoped_nsobject item( @@ -124,13 +146,13 @@ [submenu setTitle:[item title]]; [item setSubmenu:submenu]; - // Hack to set window and help menu. - if ([[item title] isEqualToString:@"Window"] && [submenu numberOfItems] > 0) + // Set submenu's role. + base::string16 role = model->GetRoleAt(index); + if (role == base::ASCIIToUTF16("window")) [NSApp setWindowsMenu:submenu]; - else if ([[item title] isEqualToString:@"Help"]) + else if (role == base::ASCIIToUTF16("help")) [NSApp setHelpMenu:submenu]; - if ([[item title] isEqualToString:@"Services"] && - [submenu numberOfItems] == 0) + if (role == base::ASCIIToUTF16("services")) [NSApp setServicesMenu:submenu]; } else { // The MenuModel works on indexes so we can't just set the command id as the @@ -139,7 +161,6 @@ // model. Setting the target to |self| allows this class to participate // in validation of the menu items. [item setTag:index]; - [item setTarget:self]; NSValue* modelObject = [NSValue valueWithPointer:model]; [item setRepresentedObject:modelObject]; // Retains |modelObject|. ui::Accelerator accelerator; @@ -153,6 +174,19 @@ platformAccelerator->modifier_mask()]; } } + + // Set menu item's role. + base::string16 role = model->GetRoleAt(index); + if (role.empty()) { + [item setTarget:self]; + } else { + for (const Role& pair : kRolesMap) { + if (role == base::ASCIIToUTF16(pair.role)) { + [item setAction:pair.selector]; + break; + } + } + } } [menu insertItem:item atIndex:index]; }