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 registering multiple shortcuts #15542

Merged
merged 7 commits into from Nov 7, 2018
@@ -5,6 +5,7 @@
#include "atom/browser/api/atom_api_global_shortcut.h"

#include <string>
#include <vector>

#include "atom/common/native_mate_converters/accelerator_converter.h"
#include "atom/common/native_mate_converters/callback.h"
@@ -38,6 +39,19 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
accelerator_callback_map_[accelerator].Run();
}

bool GlobalShortcut::RegisterAll(
const std::vector<ui::Accelerator>& accelerators,
const base::Closure& callback) {
for (auto& accelerator : accelerators) {
GlobalShortcutListener* listener = GlobalShortcutListener::GetInstance();
if (!listener->RegisterAccelerator(accelerator, this))
This conversation was marked as resolved by codebytere

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Nov 2, 2018

Member

If one fails, should we unregister the successful ones?

Otherwise we will be returning false as if it failed when in fact some were successfully registered

This comment has been minimized.

Copy link
@codebytere

codebytere Nov 2, 2018

Author Member

oh yes this is a good point 🧐

return false;

accelerator_callback_map_[accelerator] = callback;
}
return true;
}

bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
const base::Closure& callback) {
if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(accelerator,
@@ -77,6 +91,7 @@ void GlobalShortcut::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::FunctionTemplate> prototype) {
prototype->SetClassName(mate::StringToV8(isolate, "GlobalShortcut"));
mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
.SetMethod("registerAll", &GlobalShortcut::RegisterAll)
.SetMethod("register", &GlobalShortcut::Register)
.SetMethod("isRegistered", &GlobalShortcut::IsRegistered)
.SetMethod("unregister", &GlobalShortcut::Unregister)
@@ -7,6 +7,7 @@

#include <map>
#include <string>
#include <vector>

#include "atom/browser/api/trackable_object.h"
#include "base/callback.h"
@@ -33,6 +34,8 @@ class GlobalShortcut : public extensions::GlobalShortcutListener::Observer,
private:
typedef std::map<ui::Accelerator, base::Closure> AcceleratorCallbackMap;

bool RegisterAll(const std::vector<ui::Accelerator>& accelerators,
const base::Closure& callback);
bool Register(const ui::Accelerator& accelerator,
const base::Closure& callback);
bool IsRegistered(const ui::Accelerator& accelerator);
@@ -42,6 +42,17 @@ app.on('will-quit', () => {

The `globalShortcut` module has the following methods:

### `globalShortcut.registerAll(accelerator, callback)`
This conversation was marked as resolved by codebytere

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Nov 2, 2018

Member

s/accelerator/accelerators

This conversation was marked as resolved by codebytere

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Nov 2, 2018

Member

Just to be alphabetical can this be after globalShortcut.register(


* `accelerators` String[] - an array of [Accelerator](accelerator.md)s.
* `callback` Function

Registers a global shortcut of all `accelerator` items in `accelerators`. The `callback` is called when any of the registered shortcuts are pressed by the user.

When a given accelerator is already taken by other applications, this call will
silently fail. This behavior is intended by operating systems, since they don't
want applications to fight for global shortcuts.

### `globalShortcut.register(accelerator, callback)`

* `accelerator` [Accelerator](accelerator.md)
@@ -18,7 +18,7 @@ describe('globalShortcut module', () => {
globalShortcut.unregisterAll()
})

it('can register and unregister accelerators', () => {
it('can register and unregister single accelerators', () => {
const accelerator = 'CommandOrControl+A+B+C'

expect(globalShortcut.isRegistered(accelerator)).to.be.false()
@@ -33,4 +33,21 @@ describe('globalShortcut module', () => {
globalShortcut.unregisterAll()
expect(globalShortcut.isRegistered(accelerator)).to.be.false()
})

it('can register and unregister multiple accelerators', () => {
const accelerators = ['CommandOrControl+X', 'CommandOrControl+Y']

expect(globalShortcut.isRegistered(accelerators[0])).to.be.false()
expect(globalShortcut.isRegistered(accelerators[1])).to.be.false()

globalShortcut.registerAll(accelerators, () => {})

expect(globalShortcut.isRegistered(accelerators[0])).to.be.true()
expect(globalShortcut.isRegistered(accelerators[1])).to.be.true()

globalShortcut.unregisterAll()

expect(globalShortcut.isRegistered(accelerators[0])).to.be.false()
expect(globalShortcut.isRegistered(accelerators[1])).to.be.false()
})
})
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.