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

add CarbonLocalEventMonitor for NSMenu keyboard shortcut observation #28

Closed
wants to merge 1 commit into from

Conversation

honghaoz
Copy link

@honghaoz honghaoz commented Oct 11, 2020

Not sure why you mentioned the manual way in #1 but didn't include it in the repo, would like to set up a shortcut observation when the menu is open.

This PR:

A example usage is like:

    let localEventMonitor = CarbonLocalEventMonitor.monitor(for: .toggleUnicornMode) {
      // handle the shortcut
    }

    menu.rx.menuOpenState
      .subscribe(onNext: { isOpen in
        if isOpen {
          KeyboardShortcuts.disable(.toggleUnicornMode)
          localEventMonitor.start()
        } else {
          KeyboardShortcuts.enable(.toggleUnicornMode)
          localEventMonitor.stop()
        }
      })
      .disposed(by: disposeBag)

@sindresorhus
Copy link
Owner

It's easy enough to add the CarbonLocalEventMonitor. I could have done that myself too. The reason I didn't is that I would prefer a proper solution.

As I hinted at in the issue, the proper solution is automatic handling. I imagine we would provide a KeyboardShortcuts.isMenuOpen property, which the user could set to true/false when the menu is open/closed. And under the hood we would handle pausing the global keyboard shortcuts, activating the local monitor, etc.

We could also provide a NSMenu subclass which would handle calling KeyboardShortcuts.isMenuOpen automatically. We still need the property since not everyone can use a custom subclass.

Ideally, we would able to reuse https://github.com/sindresorhus/KeyboardShortcuts/blob/master/Sources/KeyboardShortcuts/CarbonKeyboardShortcuts.swift for local events too. I think it's just about what event specs we provide.

@sindresorhus
Copy link
Owner

@honghaoz Are you still interested in working on this?

@honghaoz
Copy link
Author

honghaoz commented Dec 2, 2020

Hi, @sindresorhus I'm not working on this one as I'm not an expert on Carbon APIs. I think we should close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants