Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow MenuItems to work optionally when hidden #16853

Merged
merged 2 commits into from
Feb 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions atom/browser/api/atom_api_menu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand All @@ -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,
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions atom/browser/api/atom_api_menu.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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_;
Expand Down
7 changes: 7 additions & 0 deletions atom/browser/mac/atom_application.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions atom/browser/ui/atom_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand Down
3 changes: 3 additions & 0 deletions atom/browser/ui/atom_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 7 additions & 2 deletions atom/browser/ui/cocoa/atom_menu_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -291,6 +292,11 @@ - (void)addItemToMenu:(NSMenu*)menu
[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()) {
Expand All @@ -307,8 +313,7 @@ - (void)addItemToMenu:(NSMenu*)menu
}

// 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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment removed as it's a remnant of dead code previously removed in #14939

// 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:))
Expand Down
3 changes: 3 additions & 0 deletions docs/api/menu-item.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_
codebytere marked this conversation as resolved.
Show resolved Hide resolved
* `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.
Expand All @@ -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.
Expand Down
1 change: 1 addition & 0 deletions lib/browser/api/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
1 change: 1 addition & 0 deletions lib/browser/api/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down