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: add registerAccelerator flag to allow menu items to optionally skip accelerator registration (backport: 4-0-x) #15840

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
8 changes: 8 additions & 0 deletions atom/browser/api/atom_api_menu.cc
Expand Up @@ -41,6 +41,8 @@ void Menu::AfterInit(v8::Isolate* isolate) {
delegate.Get("isCommandIdEnabled", &is_enabled_);
delegate.Get("isCommandIdVisible", &is_visible_);
delegate.Get("getAcceleratorForCommandId", &get_accelerator_);
delegate.Get("shouldRegisterAcceleratorForCommandId",
&should_register_accelerator_);
delegate.Get("executeCommand", &execute_command_);
delegate.Get("menuWillShow", &menu_will_show_);
}
Expand Down Expand Up @@ -74,6 +76,12 @@ bool Menu::GetAcceleratorForCommandIdWithParams(
return mate::ConvertFromV8(isolate(), val, accelerator);
}

bool Menu::ShouldRegisterAcceleratorForCommandId(int command_id) const {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
return should_register_accelerator_.Run(GetWrapper(), command_id);
}

void Menu::ExecuteCommand(int command_id, int flags) {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
Expand Down
2 changes: 2 additions & 0 deletions atom/browser/api/atom_api_menu.h
Expand Up @@ -51,6 +51,7 @@ class Menu : public mate::TrackableObject<Menu>,
int command_id,
bool use_default_accelerator,
ui::Accelerator* accelerator) const override;
bool ShouldRegisterAcceleratorForCommandId(int command_id) const override;
void ExecuteCommand(int command_id, int event_flags) override;
void MenuWillShow(ui::SimpleMenuModel* source) override;

Expand Down Expand Up @@ -102,6 +103,7 @@ class Menu : public mate::TrackableObject<Menu>,
base::Callback<bool(v8::Local<v8::Value>, int)> is_visible_;
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_;
base::Callback<void(v8::Local<v8::Value>, v8::Local<v8::Value>, int)>
execute_command_;
base::Callback<void(v8::Local<v8::Value>)> menu_will_show_;
Expand Down
8 changes: 5 additions & 3 deletions atom/browser/ui/accelerator_util.cc
Expand Up @@ -77,9 +77,11 @@ void GenerateAcceleratorTable(AcceleratorTable* table,
GenerateAcceleratorTable(table, submodel);
} else {
ui::Accelerator accelerator;
if (model->GetAcceleratorAtWithParams(i, true, &accelerator)) {
MenuItem item = {i, model};
(*table)[accelerator] = item;
if (model->ShouldRegisterAcceleratorAt(i)) {
if (model->GetAcceleratorAtWithParams(i, true, &accelerator)) {
MenuItem item = {i, model};
(*table)[accelerator] = item;
}
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions atom/browser/ui/atom_menu_model.cc
Expand Up @@ -43,6 +43,14 @@ bool AtomMenuModel::GetAcceleratorAtWithParams(
return false;
}

bool AtomMenuModel::ShouldRegisterAcceleratorAt(int index) const {
if (delegate_) {
return delegate_->ShouldRegisterAcceleratorForCommandId(
GetCommandIdAt(index));
}
return true;
}

void AtomMenuModel::MenuWillClose() {
ui::SimpleMenuModel::MenuWillClose();
for (Observer& observer : observers_) {
Expand Down
4 changes: 4 additions & 0 deletions atom/browser/ui/atom_menu_model.h
Expand Up @@ -23,6 +23,9 @@ class AtomMenuModel : public ui::SimpleMenuModel {
bool use_default_accelerator,
ui::Accelerator* accelerator) const = 0;

virtual bool ShouldRegisterAcceleratorForCommandId(
int command_id) const = 0;

private:
// ui::SimpleMenuModel::Delegate:
bool GetAcceleratorForCommandId(
Expand Down Expand Up @@ -52,6 +55,7 @@ class AtomMenuModel : public ui::SimpleMenuModel {
bool GetAcceleratorAtWithParams(int index,
bool use_default_accelerator,
ui::Accelerator* accelerator) const;
bool ShouldRegisterAcceleratorAt(int index) const;

// ui::SimpleMenuModel:
void MenuWillClose() override;
Expand Down
2 changes: 2 additions & 0 deletions docs/api/menu-item.md
Expand Up @@ -27,6 +27,8 @@ See [`Menu`](menu.md) for examples.
* `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.
* `registerAccelerator` Boolean (optional) - If false, the accelerator won't be registered
with the system, but it will still be displayed. Defaults to true.
* `submenu` (MenuItemConstructorOptions[] | [Menu](menu.md)) (optional) - 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`](menu.md) then it will be automatically converted to one using
Expand Down
16 changes: 12 additions & 4 deletions lib/browser/api/menu-item-roles.js
Expand Up @@ -16,12 +16,14 @@ const roles = {
copy: {
label: 'Copy',
accelerator: 'CommandOrControl+C',
webContentsMethod: 'copy'
webContentsMethod: 'copy',
registerAccelerator: false
},
cut: {
label: 'Cut',
accelerator: 'CommandOrControl+X',
webContentsMethod: 'cut'
webContentsMethod: 'cut',
registerAccelerator: false
},
delete: {
label: 'Delete',
Expand Down Expand Up @@ -59,12 +61,14 @@ const roles = {
paste: {
label: 'Paste',
accelerator: 'CommandOrControl+V',
webContentsMethod: 'paste'
webContentsMethod: 'paste',
registerAccelerator: false
},
pasteandmatchstyle: {
label: 'Paste and Match Style',
accelerator: 'Shift+CommandOrControl+V',
webContentsMethod: 'pasteAndMatchStyle'
webContentsMethod: 'pasteAndMatchStyle',
registerAccelerator: false
},
quit: {
get label () {
Expand Down Expand Up @@ -243,6 +247,10 @@ exports.getDefaultAccelerator = (role) => {
if (roles.hasOwnProperty(role)) return roles[role].accelerator
}

exports.shouldRegisterAccelerator = (role) => {
return roles.hasOwnProperty(role) ? roles[role].registerAccelerator : true
}

exports.getDefaultSubmenu = (role) => {
if (!roles.hasOwnProperty(role)) return

Expand Down
1 change: 1 addition & 0 deletions lib/browser/api/menu-item.js
Expand Up @@ -36,6 +36,7 @@ const MenuItem = function (options) {
this.overrideProperty('enabled', true)
this.overrideProperty('visible', true)
this.overrideProperty('checked', false)
this.overrideProperty('registerAccelerator', roles.shouldRegisterAccelerator(this.role))

if (!MenuItem.types.includes(this.type)) {
throw new Error(`Unknown menu item type: ${this.type}`)
Expand Down
1 change: 1 addition & 0 deletions lib/browser/api/menu.js
Expand Up @@ -24,6 +24,7 @@ const delegate = {
if (command.accelerator != null) return command.accelerator
if (useDefaultAccelerator) return command.getDefaultRoleAccelerator()
},
shouldRegisterAcceleratorForCommandId: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].registerAccelerator : undefined,
executeCommand: (menu, event, id) => {
const command = menu.commandsMap[id]
if (!command) return
Expand Down