From 3399480304e22c91c5eba07b52a665aa37f0983b Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 09:35:58 -0500 Subject: [PATCH 1/8] first pass at menu event emission --- atom/browser/api/atom_api_menu.cc | 8 ++++++++ atom/browser/api/atom_api_menu.h | 4 ++++ atom/browser/ui/atom_menu_model.cc | 12 ++++++++++-- atom/browser/ui/atom_menu_model.h | 6 +++++- atom/browser/ui/tray_icon_cocoa.h | 2 +- atom/browser/ui/tray_icon_cocoa.mm | 2 +- 6 files changed, 29 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index da208b3659..f8964421b8 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -153,6 +153,14 @@ bool Menu::IsVisibleAt(int index) const { return model_->IsVisibleAt(index); } +void Menu::OnMenuWillClose() { + Emit("menu-will-close"); +} + +void Menu::OnMenuWillShow() { + Emit("menu-will-show"); +} + // static void Menu::BuildPrototype(v8::Isolate* isolate, v8::Local prototype) { diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 574d218024..316dbd36d3 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -60,6 +60,10 @@ class Menu : public mate::TrackableObject, std::unique_ptr model_; Menu* parent_; + // Observable: + void OnMenuWillClose() override; + void OnMenuWillShow() override; + private: void InsertItemAt(int index, int command_id, const base::string16& label); void InsertSeparatorAt(int index); diff --git a/atom/browser/ui/atom_menu_model.cc b/atom/browser/ui/atom_menu_model.cc index 1ec6b3c09f..835e8d3dc4 100644 --- a/atom/browser/ui/atom_menu_model.cc +++ b/atom/browser/ui/atom_menu_model.cc @@ -42,8 +42,16 @@ bool AtomMenuModel::GetAcceleratorAtWithParams( void AtomMenuModel::MenuWillClose() { ui::SimpleMenuModel::MenuWillClose(); - for (Observer& observer : observers_) - observer.MenuWillClose(); + for (Observer& observer : observers_) { + observer.OnMenuWillClose(); + } +} + +void AtomMenuModel::MenuWillShow() { + ui::SimpleMenuModel::MenuWillShow(); + for (Observer& observer : observers_) { + observer.OnMenuWillShow(); + } } AtomMenuModel* AtomMenuModel::GetSubmenuModelAt(int index) { diff --git a/atom/browser/ui/atom_menu_model.h b/atom/browser/ui/atom_menu_model.h index 7bf734b74c..f20cf2586d 100644 --- a/atom/browser/ui/atom_menu_model.h +++ b/atom/browser/ui/atom_menu_model.h @@ -36,8 +36,11 @@ class AtomMenuModel : public ui::SimpleMenuModel { public: virtual ~Observer() {} + // Notifies the menu will open. + virtual void OnMenuWillShow() {} + // Notifies the menu has been closed. - virtual void MenuWillClose() {} + virtual void OnMenuWillClose() {} }; explicit AtomMenuModel(Delegate* delegate); @@ -54,6 +57,7 @@ class AtomMenuModel : public ui::SimpleMenuModel { // ui::SimpleMenuModel: void MenuWillClose() override; + void MenuWillShow() override; using SimpleMenuModel::GetSubmenuModelAt; AtomMenuModel* GetSubmenuModelAt(int index); diff --git a/atom/browser/ui/tray_icon_cocoa.h b/atom/browser/ui/tray_icon_cocoa.h index 7680b6f30f..d0b124acab 100644 --- a/atom/browser/ui/tray_icon_cocoa.h +++ b/atom/browser/ui/tray_icon_cocoa.h @@ -35,7 +35,7 @@ class TrayIconCocoa : public TrayIcon, protected: // AtomMenuModel::Observer: - void MenuWillClose() override; + void OnMenuWillClose() override; private: // Atom custom view for NSStatusItem. diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm index 6c2def356c..a358f676a8 100644 --- a/atom/browser/ui/tray_icon_cocoa.mm +++ b/atom/browser/ui/tray_icon_cocoa.mm @@ -461,7 +461,7 @@ gfx::Rect TrayIconCocoa::GetBounds() { return bounds; } -void TrayIconCocoa::MenuWillClose() { +void TrayIconCocoa::OnMenuWillClose() { [status_item_view_ setNeedsDisplay:YES]; } From bef4c8479988ea2d09384804c6b604c4a976b1d1 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 10:40:50 -0500 Subject: [PATCH 2/8] turn class into observer --- atom/browser/api/atom_api_menu.cc | 4 ++++ atom/browser/api/atom_api_menu.h | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_menu.cc b/atom/browser/api/atom_api_menu.cc index f8964421b8..651e2067e1 100644 --- a/atom/browser/api/atom_api_menu.cc +++ b/atom/browser/api/atom_api_menu.cc @@ -23,9 +23,13 @@ Menu::Menu(v8::Isolate* isolate, v8::Local wrapper) : model_(new AtomMenuModel(this)), parent_(nullptr) { InitWith(isolate, wrapper); + model_->AddObserver(this); } Menu::~Menu() { + if (model_) { + model_->RemoveObserver(this); + } } void Menu::AfterInit(v8::Isolate* isolate) { diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h index 316dbd36d3..aafd8f8939 100644 --- a/atom/browser/api/atom_api_menu.h +++ b/atom/browser/api/atom_api_menu.h @@ -18,7 +18,8 @@ namespace atom { namespace api { class Menu : public mate::TrackableObject, - public AtomMenuModel::Delegate { + public AtomMenuModel::Delegate, + public AtomMenuModel::Observer { public: static mate::WrappableBase* New(mate::Arguments* args); From e345342e3652a55be2634d274ad37a0abc575f32 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 11:23:46 -0500 Subject: [PATCH 3/8] add first pass at menu event specs --- spec/api-menu-spec.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 08835f3ad8..84ceac34b0 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -284,6 +284,17 @@ describe('Menu module', () => { return closeWindow(w).then(() => { w = null }) }) + it('should emit menu-will-show event', (done) => { + menu.popup(w) + menu.on('menu-will-show', () => { done() }) + }) + + it('should emit menu-will-close event', (done) => { + menu.popup(w) + menu.closePopup() + menu.on('menu-will-close', () => { done() }) + }) + it('returns immediately', () => { const { browserWindow, x, y } = menu.popup(w, {x: 100, y: 101}) From e81265bc7fa531cc04ec8d638e393fd0c3ccfc7b Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 11:28:42 -0500 Subject: [PATCH 4/8] add documentation for new menu events --- docs/api/menu.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/api/menu.md b/docs/api/menu.md index d6323ac178..0a5ecf734d 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -99,6 +99,29 @@ Returns `MenuItem` the item with the specified `id` Inserts the `menuItem` to the `pos` position of the menu. +### Instance Events + +Objects created with `new Menu` emit the following events: + +**Note:** Some events are only available on specific operating systems and are +labeled as such. + +#### Event: 'menu-will-show' _macOS_ + +Returns: + +* `event` Event + +Emitted when `menu.popup()` is called. + +#### Event: 'menu-will-close' _macOS_ + +Returns: + +* `event` Event + +Emitted when a popup is close either manually or with `menu.closePopup()` + ### Instance Properties `menu` objects also have the following properties: From 3679a9c37af9e8bc3ef21ef054e4a4ad59be8d13 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 12:36:51 -0500 Subject: [PATCH 5/8] fix event callback placement in spec --- spec/api-menu-spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 84ceac34b0..6c837fac28 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -261,7 +261,7 @@ describe('Menu module', () => { }) }) - describe('Menu.popup', () => { + describe.only('Menu.popup', () => { let w = null let menu @@ -285,14 +285,14 @@ describe('Menu module', () => { }) it('should emit menu-will-show event', (done) => { - menu.popup(w) menu.on('menu-will-show', () => { done() }) + menu.popup(w) }) it('should emit menu-will-close event', (done) => { + menu.on('menu-will-close', () => { done() }) menu.popup(w) menu.closePopup() - menu.on('menu-will-close', () => { done() }) }) it('returns immediately', () => { From 3d032c2b5732bb2294657351065173bdc6eb6e15 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 27 Jan 2018 12:38:55 -0500 Subject: [PATCH 6/8] forgot to remove .only from spec --- spec/api-menu-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 6c837fac28..3fa0050920 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -261,7 +261,7 @@ describe('Menu module', () => { }) }) - describe.only('Menu.popup', () => { + describe('Menu.popup', () => { let w = null let menu From a9dd4c927de78b63574af6be8752f3c0a46da127 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 28 Jan 2018 13:59:53 -0500 Subject: [PATCH 7/8] update menu docs description string --- docs/api/menu.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/menu.md b/docs/api/menu.md index 0a5ecf734d..ffc9ef5b17 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -120,7 +120,7 @@ Returns: * `event` Event -Emitted when a popup is close either manually or with `menu.closePopup()` +Emitted when a popup is closed either manually or with `menu.closePopup()`. ### Instance Properties From c886803d0f7b39a62716abb13724b9bd1224c236 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 28 Jan 2018 18:57:44 -0500 Subject: [PATCH 8/8] change doc to show working on all platforms --- docs/api/menu.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/menu.md b/docs/api/menu.md index ffc9ef5b17..60491b26f2 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -106,7 +106,7 @@ Objects created with `new Menu` emit the following events: **Note:** Some events are only available on specific operating systems and are labeled as such. -#### Event: 'menu-will-show' _macOS_ +#### Event: 'menu-will-show' Returns: @@ -114,7 +114,7 @@ Returns: Emitted when `menu.popup()` is called. -#### Event: 'menu-will-close' _macOS_ +#### Event: 'menu-will-close' Returns: