From e6327fb01570698d5578ca505fa6b696c50fe8f5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 22 Jun 2016 11:00:45 +0900 Subject: [PATCH 1/4] Add EventEmitter::CreateEventFromFlags --- atom/browser/api/atom_api_tray.cc | 26 +++----------------------- atom/browser/api/atom_api_tray.h | 2 -- atom/browser/api/event_emitter.cc | 10 ++++++++++ atom/browser/api/event_emitter.h | 11 +++++++++++ 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/atom/browser/api/atom_api_tray.cc b/atom/browser/api/atom_api_tray.cc index 85c0578a8e89..6bbad738b32d 100644 --- a/atom/browser/api/atom_api_tray.cc +++ b/atom/browser/api/atom_api_tray.cc @@ -16,7 +16,6 @@ #include "atom/common/node_includes.h" #include "native_mate/constructor.h" #include "native_mate/dictionary.h" -#include "ui/events/event_constants.h" #include "ui/gfx/image/image.h" namespace atom { @@ -44,24 +43,15 @@ mate::WrappableBase* Tray::New(v8::Isolate* isolate, } void Tray::OnClicked(const gfx::Rect& bounds, int modifiers) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - EmitCustomEvent("click", - ModifiersToObject(isolate(), modifiers), bounds); + EmitWithFlags("click", modifiers, bounds); } void Tray::OnDoubleClicked(const gfx::Rect& bounds, int modifiers) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - EmitCustomEvent("double-click", - ModifiersToObject(isolate(), modifiers), bounds); + EmitWithFlags("double-click", modifiers, bounds); } void Tray::OnRightClicked(const gfx::Rect& bounds, int modifiers) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - EmitCustomEvent("right-click", - ModifiersToObject(isolate(), modifiers), bounds); + EmitWithFlags("right-click", modifiers, bounds); } void Tray::OnBalloonShow() { @@ -163,16 +153,6 @@ gfx::Rect Tray::GetBounds() { return tray_icon_->GetBounds(); } -v8::Local Tray::ModifiersToObject(v8::Isolate* isolate, - int modifiers) { - mate::Dictionary obj(isolate, v8::Object::New(isolate)); - obj.Set("shiftKey", static_cast(modifiers & ui::EF_SHIFT_DOWN)); - obj.Set("ctrlKey", static_cast(modifiers & ui::EF_CONTROL_DOWN)); - obj.Set("altKey", static_cast(modifiers & ui::EF_ALT_DOWN)); - obj.Set("metaKey", static_cast(modifiers & ui::EF_COMMAND_DOWN)); - return obj.GetHandle(); -} - // static void Tray::BuildPrototype(v8::Isolate* isolate, v8::Local prototype) { diff --git a/atom/browser/api/atom_api_tray.h b/atom/browser/api/atom_api_tray.h index 0bc28af818cf..4dd79c467f09 100644 --- a/atom/browser/api/atom_api_tray.h +++ b/atom/browser/api/atom_api_tray.h @@ -68,8 +68,6 @@ class Tray : public mate::TrackableObject, gfx::Rect GetBounds(); private: - v8::Local ModifiersToObject(v8::Isolate* isolate, int modifiers); - v8::Global menu_; std::unique_ptr tray_icon_; diff --git a/atom/browser/api/event_emitter.cc b/atom/browser/api/event_emitter.cc index 7e392fddee34..e98008b85d49 100644 --- a/atom/browser/api/event_emitter.cc +++ b/atom/browser/api/event_emitter.cc @@ -8,6 +8,7 @@ #include "native_mate/arguments.h" #include "native_mate/dictionary.h" #include "native_mate/object_template_builder.h" +#include "ui/events/event_constants.h" namespace mate { @@ -65,6 +66,15 @@ v8::Local CreateCustomEvent( return event; } +v8::Local CreateEventFromFlags(v8::Isolate* isolate, int flags) { + mate::Dictionary obj = mate::Dictionary::CreateEmpty(isolate); + obj.Set("shiftKey", static_cast(flags & ui::EF_SHIFT_DOWN)); + obj.Set("ctrlKey", static_cast(flags & ui::EF_CONTROL_DOWN)); + obj.Set("altKey", static_cast(flags & ui::EF_ALT_DOWN)); + obj.Set("metaKey", static_cast(flags & ui::EF_COMMAND_DOWN)); + return obj.GetHandle(); +} + } // namespace internal } // namespace mate diff --git a/atom/browser/api/event_emitter.h b/atom/browser/api/event_emitter.h index 99f6ed46e48f..ead3beddaac5 100644 --- a/atom/browser/api/event_emitter.h +++ b/atom/browser/api/event_emitter.h @@ -30,6 +30,7 @@ v8::Local CreateCustomEvent( v8::Isolate* isolate, v8::Local object, v8::Local event); +v8::Local CreateEventFromFlags(v8::Isolate* isolate, int flags); } // namespace internal @@ -54,6 +55,16 @@ class EventEmitter : public Wrappable { internal::CreateCustomEvent(isolate(), GetWrapper(), event), args...); } + // this.emit(name, new Event(flags), args...); + template + bool EmitWithFlags(const base::StringPiece& name, + int flags, + const Args&... args) { + return EmitCustomEvent( + name, + internal::CreateEventFromFlags(isolate(), flags), args...); + } + // this.emit(name, new Event(), args...); template bool Emit(const base::StringPiece& name, const Args&... args) { From 8d08e215b2284a66267b2209954b3f37157dac34 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 22 Jun 2016 11:22:14 +0900 Subject: [PATCH 2/4] Add "event" parameter for "click" handler of MenuItem --- atom/browser/api/atom_api_menu.cc | 6 ++++-- atom/browser/api/atom_api_menu.h | 2 +- docs/api/menu-item.md | 4 ++-- lib/browser/api/menu-item.js | 4 ++-- lib/browser/api/menu.js | 4 ++-- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index 996c71739bc4..c9cd37522b82 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -61,8 +61,10 @@ bool Menu::GetAcceleratorForCommandId(int command_id, return mate::ConvertFromV8(isolate(), val, accelerator); } -void Menu::ExecuteCommand(int command_id, int event_flags) { - execute_command_.Run(command_id); +void Menu::ExecuteCommand(int command_id, int flags) { + execute_command_.Run( + mate::internal::CreateEventFromFlags(isolate(), flags), + command_id); } void Menu::MenuWillShow(ui::SimpleMenuModel* source) { diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 9ba4d7a754c4..53c6bdaf4ec9 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -90,7 +90,7 @@ class Menu : public mate::TrackableObject, base::Callback is_enabled_; base::Callback is_visible_; base::Callback(int)> get_accelerator_; - base::Callback execute_command_; + base::Callback, int)> execute_command_; base::Callback menu_will_show_; DISALLOW_COPY_AND_ASSIGN(Menu); diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 13e8317b972d..7cb434288b0a 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -11,8 +11,8 @@ Create a new `MenuItem` with the following method: ### new MenuItem(options) * `options` Object - * `click` Function - Will be called with `click(menuItem, browserWindow)` when - the menu item is clicked + * `click` Function - Will be called with + `click(menuItem, browserWindow, event)` when the menu item is clicked * `role` String - Define the action of the menu item; when specified the `click` property will be ignored * `type` String - Can be `normal`, `separator`, `submenu`, `checkbox` or diff --git a/lib/browser/api/menu-item.js b/lib/browser/api/menu-item.js index 651de147af1b..b0100b7f2ff7 100644 --- a/lib/browser/api/menu-item.js +++ b/lib/browser/api/menu-item.js @@ -72,7 +72,7 @@ const MenuItem = (function () { throw new Error('Unknown menu type ' + this.type) } this.commandId = ++nextCommandId - this.click = (focusedWindow) => { + this.click = (event, focusedWindow) => { // Manually flip the checked flags when clicked. if (this.type === 'checkbox' || this.type === 'radio') { this.checked = !this.checked @@ -91,7 +91,7 @@ const MenuItem = (function () { return webContents != null ? webContents[methodName]() : void 0 } } else if (typeof click === 'function') { - return click(this, focusedWindow) + return click(this, focusedWindow, event) } else if (typeof this.selector === 'string' && process.platform === 'darwin') { return Menu.sendActionToFirstResponder(this.selector) } diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index b122fc36cf37..3b1082193639 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -114,9 +114,9 @@ Menu.prototype._init = function () { var command = this.commandsMap[commandId] return command != null ? command.icon : undefined }, - executeCommand: (commandId) => { + executeCommand: (event, commandId) => { var command = this.commandsMap[commandId] - return command != null ? command.click(BrowserWindow.getFocusedWindow()) : undefined + return command != null ? command.click(event, BrowserWindow.getFocusedWindow()) : undefined }, menuWillShow: () => { // Make sure radio groups have at least one menu item seleted. From 62d0dbea5a2382dacc2231a6150d0a4699498e4b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 22 Jun 2016 13:23:07 +0900 Subject: [PATCH 3/4] docs: Reformat the menu-item.md --- docs/api/menu-item.md | 52 ++++++++++++++++++++++++++----------------- docs/api/session.md | 1 + 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 7cb434288b0a..0d05d81bcd00 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -2,7 +2,7 @@ > Add items to native application menus and context menus. -See [`menu`](menu.md) for examples. +See [`Menu`](menu.md) for examples. ## Class: MenuItem @@ -12,11 +12,11 @@ Create a new `MenuItem` with the following method: * `options` Object * `click` Function - Will be called with - `click(menuItem, browserWindow, event)` when the menu item is clicked - * `role` String - Define the action of the menu item; when specified the - `click` property will be ignored + `click(menuItem, browserWindow, event)` when the menu item is clicked. + * `role` String - Define the action of the menu item, when specified the + `click` property will be ignored. * `type` String - Can be `normal`, `separator`, `submenu`, `checkbox` or - `radio` + `radio`. * `label` String * `sublabel` String * `accelerator` [Accelerator](accelerator.md) @@ -25,15 +25,15 @@ Create a new `MenuItem` with the following method: unclickable. * `visible` Boolean - If false, the menu item will be entirely hidden. * `checked` Boolean - Should only be specified for `checkbox` or `radio` type - menu items. + menu items. * `submenu` Menu - Should be specified for `submenu` type menu items. If - `submenu` is specified, the `type: 'submenu'` can be omitted. If the value - is not a `Menu` then it will be automatically converted to one using - `Menu.buildFromTemplate`. + `submenu` is specified, the `type: 'submenu'` can be omitted. If the value + is not a `Menu` then it will be automatically converted to one using + `Menu.buildFromTemplate`. * `id` String - Unique within a single menu. If defined then it can be used - as a reference to this item by the position attribute. + as a reference to this item by the position attribute. * `position` String - This field allows fine-grained definition of the - specific location within a given menu. + specific location within a given menu. It is best to specify `role` for any menu item that matches a standard role, rather than trying to manually implement the behavior in a `click` function. @@ -69,19 +69,29 @@ On macOS `role` can also have following additional values: When specifying `role` on macOS, `label` and `accelerator` are the only options that will affect the MenuItem. All other options will be ignored. -## Instance Properties +### Instance Properties -The following properties (and no others) can be updated on an existing `MenuItem`: +The following properties are available on instances of `MenuItem`: - * `enabled` Boolean - * `visible` Boolean - * `checked` Boolean +#### `menuItem.enabled` -Their meanings are as described above. +A Boolean indicating whether the item is enabled, this property can be +dynamically changed. -A `checkbox` menu item will toggle its `checked` property on and off when -selected. You can add a `click` function to do additional work. +#### `menuItem.visible` + +A Boolean indicating whether the item is visible, this property can be +dynamically changed. + +#### `menuItem.checked` + +A Boolean indicating whether the item is checked, this property can be +dynamically changed. + +A `checkbox` menu item will toggle the `checked` property on and off when +selected. A `radio` menu item will turn on its `checked` property when clicked, and -will turn off that property for all adjacent items in the same menu. Again, -you can add a `click` function for additional behavior. +will turn off that property for all adjacent items in the same menu. + +You can add a `click` function for additional behavior. diff --git a/docs/api/session.md b/docs/api/session.md index 030b2c7542ea..0486b1ddbdd6 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -566,3 +566,4 @@ app.on('ready', function () { console.error('Failed to register protocol') }) }) +``` From 90b64504fcc4d72272ee12a89133a45904739999 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 22 Jun 2016 13:36:10 +0900 Subject: [PATCH 4/4] spec: Fix failing tests of Menu --- spec/api-menu-spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 6866448e0fd1..a8cc5b518cba 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -231,7 +231,7 @@ describe('menu module', function () { } } ]) - menu.delegate.executeCommand(menu.items[0].commandId) + menu.delegate.executeCommand({}, menu.items[0].commandId) }) }) @@ -244,7 +244,7 @@ describe('menu module', function () { } ]) assert.equal(menu.items[0].checked, false) - menu.delegate.executeCommand(menu.items[0].commandId) + menu.delegate.executeCommand({}, menu.items[0].commandId) assert.equal(menu.items[0].checked, true) }) @@ -255,9 +255,9 @@ describe('menu module', function () { type: 'radio' } ]) - menu.delegate.executeCommand(menu.items[0].commandId) + menu.delegate.executeCommand({}, menu.items[0].commandId) assert.equal(menu.items[0].checked, true) - menu.delegate.executeCommand(menu.items[0].commandId) + menu.delegate.executeCommand({}, menu.items[0].commandId) assert.equal(menu.items[0].checked, true) })