From a3f3a35fd17349ce2d0e3c3b213e6d8f10855c99 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Aug 2015 12:18:00 +0800 Subject: [PATCH 1/5] mac: Don't emit "clicked" event if there is menu attached --- atom/browser/ui/tray_icon_cocoa.mm | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 055ac12c4046..3ac865cce5ab 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -207,23 +207,26 @@ const CGFloat kVerticalTitleMargin = 2; } inMouseEventSequence_ = NO; - // Single click - if (event.clickCount == 1) { - if (menuController_) { - [statusItem_ popUpStatusItemMenu:[menuController_ menu]]; - } + // Show menu when single clicked on the icon. + if (event.clickCount == 1 && menuController_) + [statusItem_ popUpStatusItemMenu:[menuController_ menu]]; + // Don't emit click events when menu is showing. + if (menuController_) + return; + + // Single click event. + if (event.clickCount == 1) trayIcon_->NotifyClicked( [self getBoundsFromEvent:event], ui::EventFlagsFromModifiers([event modifierFlags])); - } - // Double click - if (event.clickCount == 2 && !menuController_) { + // Double click event. + if (event.clickCount == 2) trayIcon_->NotifyDoubleClicked( [self getBoundsFromEvent:event], ui::EventFlagsFromModifiers([event modifierFlags])); - } + [self setNeedsDisplay:YES]; } From 4b9ff309ece31507862a5279d545a8cb1b2b43b2 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Aug 2015 12:39:05 +0800 Subject: [PATCH 2/5] Add our own MenuModel class --- atom/browser/api/atom_api_menu.cc | 2 +- atom/browser/api/atom_api_menu.h | 14 +++--- atom/browser/ui/atom_menu_model.cc | 22 +++++++++ atom/browser/ui/atom_menu_model.h | 47 +++++++++++++++++++ atom/browser/ui/cocoa/atom_menu_controller.mm | 5 +- filenames.gypi | 2 + 6 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 atom/browser/ui/atom_menu_model.cc create mode 100644 atom/browser/ui/atom_menu_model.h diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index 915e152b3a68..356a4d4ce494 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -20,7 +20,7 @@ namespace atom { namespace api { Menu::Menu() - : model_(new ui::SimpleMenuModel(this)), + : model_(new AtomMenuModel(this)), parent_(NULL) { } diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 0c2743878f78..33aafbc45d3b 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -8,9 +8,9 @@ #include #include "atom/browser/api/atom_api_window.h" +#include "atom/browser/ui/atom_menu_model.h" #include "base/callback.h" #include "base/memory/scoped_ptr.h" -#include "ui/base/models/simple_menu_model.h" #include "native_mate/wrappable.h" namespace atom { @@ -18,7 +18,7 @@ namespace atom { namespace api { class Menu : public mate::Wrappable, - public ui::SimpleMenuModel::Delegate { + public AtomMenuModel::Delegate { public: static mate::Wrappable* Create(); @@ -33,7 +33,7 @@ class Menu : public mate::Wrappable, static void SendActionToFirstResponder(const std::string& action); #endif - ui::SimpleMenuModel* model() const { return model_.get(); } + AtomMenuModel* model() const { return model_.get(); } protected: Menu(); @@ -42,7 +42,7 @@ class Menu : public mate::Wrappable, // mate::Wrappable: void AfterInit(v8::Isolate* isolate) override; - // ui::SimpleMenuModel::Delegate implementations: + // ui::SimpleMenuModel::Delegate: bool IsCommandIdChecked(int command_id) const override; bool IsCommandIdEnabled(int command_id) const override; bool IsCommandIdVisible(int command_id) const override; @@ -54,7 +54,7 @@ class Menu : public mate::Wrappable, virtual void Popup(Window* window) = 0; virtual void PopupAt(Window* window, int x, int y) = 0; - scoped_ptr model_; + scoped_ptr model_; Menu* parent_; private: @@ -102,9 +102,9 @@ class Menu : public mate::Wrappable, namespace mate { template<> -struct Converter { +struct Converter { static bool FromV8(v8::Isolate* isolate, v8::Local val, - ui::SimpleMenuModel** out) { + atom::AtomMenuModel** out) { // null would be tranfered to NULL. if (val->IsNull()) { *out = nullptr; diff --git a/atom/browser/ui/atom_menu_model.cc b/atom/browser/ui/atom_menu_model.cc new file mode 100644 index 000000000000..7d2d5e1b6a1f --- /dev/null +++ b/atom/browser/ui/atom_menu_model.cc @@ -0,0 +1,22 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/ui/atom_menu_model.h" + +namespace atom { + +AtomMenuModel::AtomMenuModel(Delegate* delegate) + : ui::SimpleMenuModel(delegate), + delegate_(delegate) { +} + +AtomMenuModel::~AtomMenuModel() { +} + +void AtomMenuModel::MenuClosed() { + ui::SimpleMenuModel::MenuClosed(); + FOR_EACH_OBSERVER(Observer, observers_, MenuClosed()); +} + +} // namespace atom diff --git a/atom/browser/ui/atom_menu_model.h b/atom/browser/ui/atom_menu_model.h new file mode 100644 index 000000000000..42e0e5dbc53a --- /dev/null +++ b/atom/browser/ui/atom_menu_model.h @@ -0,0 +1,47 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_ +#define ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_ + +#include "base/observer_list.h" +#include "ui/base/models/simple_menu_model.h" + +namespace atom { + +class AtomMenuModel : public ui::SimpleMenuModel { + public: + class Delegate : public ui::SimpleMenuModel::Delegate { + public: + virtual ~Delegate() {} + }; + + class Observer { + public: + virtual ~Observer() {} + + // Notifies the menu has been closed. + virtual void MenuClosed() {} + }; + + explicit AtomMenuModel(Delegate* delegate); + virtual ~AtomMenuModel(); + + void AddObserver(Observer* obs) { observers_.AddObserver(obs); } + void RemoveObserver(Observer* obs) { observers_.RemoveObserver(obs); } + + // ui::SimpleMenuModel: + void MenuClosed() override; + + private: + Delegate* delegate_; // weak ref. + + ObserverList observers_; + + DISALLOW_COPY_AND_ASSIGN(AtomMenuModel); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_UI_ATOM_MENU_MODEL_H_ diff --git a/atom/browser/ui/cocoa/atom_menu_controller.mm b/atom/browser/ui/cocoa/atom_menu_controller.mm index 962dd6b0ad36..d799a9f9b64d 100644 --- a/atom/browser/ui/cocoa/atom_menu_controller.mm +++ b/atom/browser/ui/cocoa/atom_menu_controller.mm @@ -5,12 +5,12 @@ #import "atom/browser/ui/cocoa/atom_menu_controller.h" +#include "atom/browser/ui/atom_menu_model.h" #include "base/logging.h" #include "base/strings/sys_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/base/models/simple_menu_model.h" #include "ui/events/cocoa/cocoa_event_utils.h" #include "ui/gfx/image/image.h" @@ -120,8 +120,7 @@ [item setTarget:nil]; [item setAction:nil]; ui::MenuModel* submenuModel = model->GetSubmenuModelAt(index); - NSMenu* submenu = - [self menuFromModel:(ui::SimpleMenuModel*)submenuModel]; + NSMenu* submenu = [self menuFromModel:submenuModel]; [submenu setTitle:[item title]]; [item setSubmenu:submenu]; diff --git a/filenames.gypi b/filenames.gypi index b062c6e932b9..40af1ebb1fe7 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -166,6 +166,8 @@ 'atom/browser/ui/accelerator_util.h', 'atom/browser/ui/accelerator_util_mac.mm', 'atom/browser/ui/accelerator_util_views.cc', + 'atom/browser/ui/atom_menu_model.cc', + 'atom/browser/ui/atom_menu_model.h', 'atom/browser/ui/cocoa/atom_menu_controller.h', 'atom/browser/ui/cocoa/atom_menu_controller.mm', 'atom/browser/ui/cocoa/event_processing_window.h', From 58dee04d5c2317cd9c597b3e0b4b2a885512ef36 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Aug 2015 12:48:22 +0800 Subject: [PATCH 3/5] mac: Redraw icon when menu is closed --- atom/browser/ui/tray_icon_cocoa.h | 11 ++++++++++- atom/browser/ui/tray_icon_cocoa.mm | 14 +++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/atom/browser/ui/tray_icon_cocoa.h b/atom/browser/ui/tray_icon_cocoa.h index 9aa801ec5ead..db53adcbe85d 100644 --- a/atom/browser/ui/tray_icon_cocoa.h +++ b/atom/browser/ui/tray_icon_cocoa.h @@ -9,6 +9,7 @@ #include +#include "atom/browser/ui/atom_menu_model.h" #include "atom/browser/ui/tray_icon.h" #include "base/mac/scoped_nsobject.h" @@ -17,7 +18,8 @@ namespace atom { -class TrayIconCocoa : public TrayIcon { +class TrayIconCocoa : public TrayIcon, + public AtomMenuModel::Observer { public: TrayIconCocoa(); virtual ~TrayIconCocoa(); @@ -30,6 +32,10 @@ class TrayIconCocoa : public TrayIcon { void PopContextMenu(const gfx::Point& pos) override; void SetContextMenu(ui::SimpleMenuModel* menu_model) override; + protected: + // AtomMenuModel::Observer: + void MenuClosed() override; + private: // Atom custom view for NSStatusItem. base::scoped_nsobject status_item_view_; @@ -37,6 +43,9 @@ class TrayIconCocoa : public TrayIcon { // Status menu shown when right-clicking the system icon. base::scoped_nsobject menu_; + // Used for unregistering observer. + AtomMenuModel* menu_model_; // weak ref. + DISALLOW_COPY_AND_ASSIGN(TrayIconCocoa); }; diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 3ac865cce5ab..7c2214e06399 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -281,11 +281,13 @@ const CGFloat kVerticalTitleMargin = 2; namespace atom { -TrayIconCocoa::TrayIconCocoa() { +TrayIconCocoa::TrayIconCocoa() : menu_model_(nullptr) { } TrayIconCocoa::~TrayIconCocoa() { [status_item_view_ removeItem]; + if (menu_model_) + menu_model_->RemoveObserver(this); } void TrayIconCocoa::SetImage(const gfx::Image& image) { @@ -319,10 +321,20 @@ void TrayIconCocoa::PopContextMenu(const gfx::Point& pos) { } void TrayIconCocoa::SetContextMenu(ui::SimpleMenuModel* menu_model) { + // Substribe to MenuClosed event. + if (menu_model_) + menu_model_->RemoveObserver(this); + static_cast(menu_model)->AddObserver(this); + + // Create native menu. menu_.reset([[AtomMenuController alloc] initWithModel:menu_model]); [status_item_view_ setMenuController:menu_.get()]; } +void TrayIconCocoa::MenuClosed() { + [status_item_view_ setNeedsDisplay:YES]; +} + // static TrayIcon* TrayIcon::Create() { return new TrayIconCocoa; From 225140bd64e07b935d498335732b468b24c56c8c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Aug 2015 12:52:55 +0800 Subject: [PATCH 4/5] win: Don't emit right-clicked event when there is menu attached --- atom/browser/ui/win/notify_icon.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/atom/browser/ui/win/notify_icon.cc b/atom/browser/ui/win/notify_icon.cc index 962dc871b5b6..ebc958384363 100644 --- a/atom/browser/ui/win/notify_icon.cc +++ b/atom/browser/ui/win/notify_icon.cc @@ -86,8 +86,10 @@ void NotifyIcon::HandleClickEvent(const gfx::Point& cursor_pos, NotifyClicked(gfx::Rect(rect), modifiers); return; } else if (!double_button_click) { // single right click - NotifyRightClicked(gfx::Rect(rect), modifiers); - PopContextMenu(cursor_pos); + if (menu_model_) + PopContextMenu(cursor_pos); + else + NotifyRightClicked(gfx::Rect(rect), modifiers); } } From 33eadad139136bce7f8c9fe4f152f8576aa7e69b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Aug 2015 13:00:15 +0800 Subject: [PATCH 5/5] popContextMenu => popUpContextMenu --- atom/browser/api/atom_api_tray.cc | 6 +++--- atom/browser/api/atom_api_tray.h | 2 +- atom/browser/api/lib/tray.coffee | 4 ++++ atom/browser/ui/tray_icon.cc | 2 +- atom/browser/ui/tray_icon.h | 2 +- atom/browser/ui/tray_icon_cocoa.h | 2 +- atom/browser/ui/tray_icon_cocoa.mm | 6 +++--- atom/browser/ui/win/notify_icon.cc | 4 ++-- atom/browser/ui/win/notify_icon.h | 2 +- docs/api/tray.md | 4 ++-- 10 files changed, 19 insertions(+), 15 deletions(-) diff --git a/atom/browser/api/atom_api_tray.cc b/atom/browser/api/atom_api_tray.cc index 0f07737da2bf..1382f015a63b 100644 --- a/atom/browser/api/atom_api_tray.cc +++ b/atom/browser/api/atom_api_tray.cc @@ -120,10 +120,10 @@ void Tray::DisplayBalloon(mate::Arguments* args, tray_icon_->DisplayBalloon(icon, title, content); } -void Tray::PopContextMenu(mate::Arguments* args) { +void Tray::PopUpContextMenu(mate::Arguments* args) { gfx::Point pos; args->GetNext(&pos); - tray_icon_->PopContextMenu(pos); + tray_icon_->PopUpContextMenu(pos); } void Tray::SetContextMenu(mate::Arguments* args, Menu* menu) { @@ -151,7 +151,7 @@ void Tray::BuildPrototype(v8::Isolate* isolate, .SetMethod("setTitle", &Tray::SetTitle) .SetMethod("setHighlightMode", &Tray::SetHighlightMode) .SetMethod("displayBalloon", &Tray::DisplayBalloon) - .SetMethod("popContextMenu", &Tray::PopContextMenu) + .SetMethod("popUpContextMenu", &Tray::PopUpContextMenu) .SetMethod("_setContextMenu", &Tray::SetContextMenu); } diff --git a/atom/browser/api/atom_api_tray.h b/atom/browser/api/atom_api_tray.h index 02f7418fe41d..dc9302597cf3 100644 --- a/atom/browser/api/atom_api_tray.h +++ b/atom/browser/api/atom_api_tray.h @@ -60,7 +60,7 @@ class Tray : public mate::EventEmitter, void SetTitle(mate::Arguments* args, const std::string& title); void SetHighlightMode(mate::Arguments* args, bool highlight); void DisplayBalloon(mate::Arguments* args, const mate::Dictionary& options); - void PopContextMenu(mate::Arguments* args); + void PopUpContextMenu(mate::Arguments* args); void SetContextMenu(mate::Arguments* args, Menu* menu); private: diff --git a/atom/browser/api/lib/tray.coffee b/atom/browser/api/lib/tray.coffee index 7d158a9a0104..1c225ddd403c 100644 --- a/atom/browser/api/lib/tray.coffee +++ b/atom/browser/api/lib/tray.coffee @@ -3,8 +3,12 @@ bindings = process.atomBinding 'tray' Tray = bindings.Tray Tray::__proto__ = EventEmitter.prototype + Tray::setContextMenu = (menu) -> @_setContextMenu menu @menu = menu # Keep a strong reference of menu. +# Keep compatibility with old APIs. +Tray::popContextMenu = Tray::popUpContextMenu + module.exports = Tray diff --git a/atom/browser/ui/tray_icon.cc b/atom/browser/ui/tray_icon.cc index cc19be6d8d86..12c6be2ea74e 100644 --- a/atom/browser/ui/tray_icon.cc +++ b/atom/browser/ui/tray_icon.cc @@ -26,7 +26,7 @@ void TrayIcon::DisplayBalloon(const gfx::Image& icon, const base::string16& contents) { } -void TrayIcon::PopContextMenu(const gfx::Point& pos) { +void TrayIcon::PopUpContextMenu(const gfx::Point& pos) { } void TrayIcon::NotifyClicked(const gfx::Rect& bounds, int modifiers) { diff --git a/atom/browser/ui/tray_icon.h b/atom/browser/ui/tray_icon.h index 6d917a53467c..55f1c41d19d7 100644 --- a/atom/browser/ui/tray_icon.h +++ b/atom/browser/ui/tray_icon.h @@ -47,7 +47,7 @@ class TrayIcon { const base::string16& title, const base::string16& contents); - virtual void PopContextMenu(const gfx::Point& pos); + virtual void PopUpContextMenu(const gfx::Point& pos); // Set the context menu for this icon. virtual void SetContextMenu(ui::SimpleMenuModel* menu_model) = 0; diff --git a/atom/browser/ui/tray_icon_cocoa.h b/atom/browser/ui/tray_icon_cocoa.h index db53adcbe85d..7781c93a1c03 100644 --- a/atom/browser/ui/tray_icon_cocoa.h +++ b/atom/browser/ui/tray_icon_cocoa.h @@ -29,7 +29,7 @@ class TrayIconCocoa : public TrayIcon, void SetToolTip(const std::string& tool_tip) override; void SetTitle(const std::string& title) override; void SetHighlightMode(bool highlight) override; - void PopContextMenu(const gfx::Point& pos) override; + void PopUpContextMenu(const gfx::Point& pos) override; void SetContextMenu(ui::SimpleMenuModel* menu_model) override; protected: diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 7c2214e06399..d69fa8636b4f 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -230,7 +230,7 @@ const CGFloat kVerticalTitleMargin = 2; [self setNeedsDisplay:YES]; } -- (void)popContextMenu { +- (void)popUpContextMenu { if (menuController_ && ![menuController_ isMenuOpen]) { // Redraw the dray icon to show highlight if it is enabled. [self setNeedsDisplay:YES]; @@ -316,8 +316,8 @@ void TrayIconCocoa::SetHighlightMode(bool highlight) { [status_item_view_ setHighlight:highlight]; } -void TrayIconCocoa::PopContextMenu(const gfx::Point& pos) { - [status_item_view_ popContextMenu]; +void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos) { + [status_item_view_ popUpContextMenu]; } void TrayIconCocoa::SetContextMenu(ui::SimpleMenuModel* menu_model) { diff --git a/atom/browser/ui/win/notify_icon.cc b/atom/browser/ui/win/notify_icon.cc index ebc958384363..c4c0fe9141e3 100644 --- a/atom/browser/ui/win/notify_icon.cc +++ b/atom/browser/ui/win/notify_icon.cc @@ -87,7 +87,7 @@ void NotifyIcon::HandleClickEvent(const gfx::Point& cursor_pos, return; } else if (!double_button_click) { // single right click if (menu_model_) - PopContextMenu(cursor_pos); + PopUpContextMenu(cursor_pos); else NotifyRightClicked(gfx::Rect(rect), modifiers); } @@ -163,7 +163,7 @@ void NotifyIcon::DisplayBalloon(const gfx::Image& icon, LOG(WARNING) << "Unable to create status tray balloon."; } -void NotifyIcon::PopContextMenu(const gfx::Point& pos) { +void NotifyIcon::PopUpContextMenu(const gfx::Point& pos) { // Returns if context menu isn't set. if (!menu_model_) return; diff --git a/atom/browser/ui/win/notify_icon.h b/atom/browser/ui/win/notify_icon.h index a28a00ff0d6a..136186b689b9 100644 --- a/atom/browser/ui/win/notify_icon.h +++ b/atom/browser/ui/win/notify_icon.h @@ -52,7 +52,7 @@ class NotifyIcon : public TrayIcon { void DisplayBalloon(const gfx::Image& icon, const base::string16& title, const base::string16& contents) override; - void PopContextMenu(const gfx::Point& pos) override; + void PopUpContextMenu(const gfx::Point& pos) override; void SetContextMenu(ui::SimpleMenuModel* menu_model) override; private: diff --git a/docs/api/tray.md b/docs/api/tray.md index 3d3ebc280f5a..a67772746128 100644 --- a/docs/api/tray.md +++ b/docs/api/tray.md @@ -60,7 +60,7 @@ Creates a new tray icon associated with the `image`. Emitted when the tray icon is clicked. -__Note:__ The `bounds` payload is only implemented on OS X and Windows 7 or newer. +__Note:__ The `bounds` payload is only implemented on OS X and Windows. ### Event: 'right-clicked' @@ -173,7 +173,7 @@ Displays a tray balloon. __Note:__ This is only implemented on Windows. -### Tray.popContextMenu([position]) +### Tray.popUpContextMenu([position]) * `position` Object - The pop position * `x` Integer