feat: allow MenuItems to work optionally when hidden (#16853)

* feat: allow MenuItems to work optionally when hidden

* fix: actually include forward_declaration

atom/browser/api/atom_api_menu.cc

@@ -40,6 +40,7 @@ void Menu::AfterInit(v8::Isolate* isolate) {
   delegate.Get("isCommandIdChecked", &is_checked_);
   delegate.Get("isCommandIdEnabled", &is_enabled_);
   delegate.Get("isCommandIdVisible", &is_visible_);
+  delegate.Get("shouldCommandIdWorkWhenHidden", &works_when_hidden_);
   delegate.Get("getAcceleratorForCommandId", &get_accelerator_);
   delegate.Get("shouldRegisterAcceleratorForCommandId",
                &should_register_accelerator_);
@@ -65,6 +66,12 @@ bool Menu::IsCommandIdVisible(int command_id) const {
   return is_visible_.Run(GetWrapper(), command_id);
 }
 
+bool Menu::ShouldCommandIdWorkWhenHidden(int command_id) const {
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+  return works_when_hidden_.Run(GetWrapper(), command_id);
+}
+
 bool Menu::GetAcceleratorForCommandIdWithParams(
     int command_id,
     bool use_default_accelerator,
@@ -181,6 +188,10 @@ bool Menu::IsVisibleAt(int index) const {
   return model_->IsVisibleAt(index);
 }
 
+bool Menu::WorksWhenHiddenAt(int index) const {
+  return model_->WorksWhenHiddenAt(index);
+}
+
 void Menu::OnMenuWillClose() {
   Emit("menu-will-close");
 }
@@ -212,6 +223,7 @@ void Menu::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("getAcceleratorTextAt", &Menu::GetAcceleratorTextAt)
       .SetMethod("isItemCheckedAt", &Menu::IsItemCheckedAt)
       .SetMethod("isEnabledAt", &Menu::IsEnabledAt)
+      .SetMethod("worksWhenHiddenAt", &Menu::WorksWhenHiddenAt)
       .SetMethod("isVisibleAt", &Menu::IsVisibleAt)
       .SetMethod("popupAt", &Menu::PopupAt)
       .SetMethod("closePopupAt", &Menu::ClosePopupAt);

atom/browser/api/atom_api_menu.h

@@ -47,6 +47,7 @@ class Menu : public mate::TrackableObject<Menu>,
   bool IsCommandIdChecked(int command_id) const override;
   bool IsCommandIdEnabled(int command_id) const override;
   bool IsCommandIdVisible(int command_id) const override;
+  bool ShouldCommandIdWorkWhenHidden(int command_id) const override;
   bool GetAcceleratorForCommandIdWithParams(
       int command_id,
       bool use_default_accelerator,
@@ -96,11 +97,13 @@ class Menu : public mate::TrackableObject<Menu>,
   bool IsItemCheckedAt(int index) const;
   bool IsEnabledAt(int index) const;
   bool IsVisibleAt(int index) const;
+  bool WorksWhenHiddenAt(int index) const;
 
   // Stored delegate methods.
   base::Callback<bool(v8::Local<v8::Value>, int)> is_checked_;
   base::Callback<bool(v8::Local<v8::Value>, int)> is_enabled_;
   base::Callback<bool(v8::Local<v8::Value>, int)> is_visible_;
+  base::Callback<bool(v8::Local<v8::Value>, int)> works_when_hidden_;
   base::Callback<v8::Local<v8::Value>(v8::Local<v8::Value>, int, bool)>
       get_accelerator_;
   base::Callback<bool(v8::Local<v8::Value>, int)> should_register_accelerator_;

atom/browser/mac/atom_application.h

@@ -46,6 +46,13 @@ typedef NS_ENUM(NSInteger, AVAuthorizationStatusMac) {
   AVAuthorizationStatusAuthorizedMac = 3,
 };
 
+@interface NSMenuItem (HighSierraSDK)
+@property(atomic, readwrite)
+    BOOL allowsKeyEquivalentWhenHidden API_AVAILABLE(macosx(10.13));
+- (void)setAllowsKeyEquivalentWhenHidden:(BOOL)arg1
+    API_AVAILABLE(macosx(10.13));
+@end
+
 @interface AVCaptureDevice (MojaveSDK)
 + (void)requestAccessForMediaType:(AVMediaType)mediaType
                 completionHandler:(void (^)(BOOL granted))handler

atom/browser/ui/atom_menu_model.cc

@@ -51,6 +51,13 @@ bool AtomMenuModel::ShouldRegisterAcceleratorAt(int index) const {
   return true;
 }
 
+bool AtomMenuModel::WorksWhenHiddenAt(int index) const {
+  if (delegate_) {
+    return delegate_->ShouldCommandIdWorkWhenHidden(GetCommandIdAt(index));
+  }
+  return true;
+}
+
 void AtomMenuModel::MenuWillClose() {
   ui::SimpleMenuModel::MenuWillClose();
   for (Observer& observer : observers_) {

atom/browser/ui/atom_menu_model.h

@@ -27,6 +27,8 @@ class AtomMenuModel : public ui::SimpleMenuModel {
     virtual bool ShouldRegisterAcceleratorForCommandId(
         int command_id) const = 0;
 
+    virtual bool ShouldCommandIdWorkWhenHidden(int command_id) const = 0;
+
    private:
     // ui::SimpleMenuModel::Delegate:
     bool GetAcceleratorForCommandId(
@@ -57,6 +59,7 @@ class AtomMenuModel : public ui::SimpleMenuModel {
                                   bool use_default_accelerator,
                                   ui::Accelerator* accelerator) const;
   bool ShouldRegisterAcceleratorAt(int index) const;
+  bool WorksWhenHiddenAt(int index) const;
 
   // ui::SimpleMenuModel:
   void MenuWillClose() override;

atom/browser/ui/cocoa/atom_menu_controller.mm

@@ -5,6 +5,7 @@
 
 #import "atom/browser/ui/cocoa/atom_menu_controller.h"
 
+#include "atom/browser/mac/atom_application.h"
 #include "atom/browser/ui/atom_menu_model.h"
 #include "base/logging.h"
 #include "base/strings/sys_string_conversions.h"
@@ -291,6 +292,11 @@ static base::scoped_nsobject<NSMenu> recentDocumentsMenuSwap_;
       [item setKeyEquivalentModifierMask:modifier_mask];
     }
 
+    if (@available(macOS 10.13, *)) {
+      [(id)item
+          setAllowsKeyEquivalentWhenHidden:(model->WorksWhenHiddenAt(index))];
+    }
+
     // Set menu item's role.
     [item setTarget:self];
     if (!role.empty()) {
@@ -307,8 +313,7 @@ static base::scoped_nsobject<NSMenu> recentDocumentsMenuSwap_;
 }
 
 // Called before the menu is to be displayed to update the state (enabled,
-// radio, etc) of each item in the menu. Also will update the title if
-// the item is marked as "dynamic".
+// radio, etc) of each item in the menu.
 - (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
   SEL action = [item action];
   if (action != @selector(itemSelected:))

docs/api/menu-item.md

@@ -24,6 +24,7 @@ See [`Menu`](menu.md) for examples.
   * `icon` ([NativeImage](native-image.md) | String) (optional)
   * `enabled` Boolean (optional) - If false, the menu item will be greyed out and
     unclickable.
+  * `acceleratorWorksWhenHidden` Boolean (optional) - default is `true`, and when `false` will prevent the accelerator from triggering the item if the item is not visible`. _macOS_
   * `visible` Boolean (optional) - If false, the menu item will be entirely hidden.
   * `checked` Boolean (optional) - Should only be specified for `checkbox` or `radio` type
     menu items.
@@ -48,6 +49,8 @@ See [`Menu`](menu.md) for examples.
     the placement of their containing group after the containing group of the item
     with the specified label.
 
+**Note:** `acceleratorWorksWhenHidden` is specified as being macOS-only because accelerators always work when items are hidden on Windows and Linux. The option is exposed to users to give them the option to turn it off, as this is possible in native macOS development. This property is only usable on macOS High Sierra 10.13 or newer.
+
 ### Roles
 
 Roles allow menu items to have predefined behaviors.

lib/browser/api/menu-item.js

@@ -36,6 +36,7 @@ const MenuItem = function (options) {
   this.overrideProperty('enabled', true)
   this.overrideProperty('visible', true)
   this.overrideProperty('checked', false)
+  this.overrideProperty('acceleratorWorksWhenHidden', true)
   this.overrideProperty('registerAccelerator', roles.shouldRegisterAccelerator(this.role))
 
   if (!MenuItem.types.includes(this.type)) {

lib/browser/api/menu.js

@@ -17,6 +17,7 @@ Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype)
 const delegate = {
   isCommandIdChecked: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].checked : undefined,
   isCommandIdEnabled: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].enabled : undefined,
+  shouldCommandIdWorkWhenHidden: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].acceleratorWorksWhenHidden : undefined,
   isCommandIdVisible: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].visible : undefined,
   getAcceleratorForCommandId: (menu, id, useDefaultAccelerator) => {
     const command = menu.commandsMap[id]