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(macos): register media keys #72

Merged
merged 5 commits into from
Apr 23, 2024
Merged

Conversation

pewsheen
Copy link
Contributor

@pewsheen pewsheen commented Apr 18, 2024

  • Will pop up an Accessibility permission request when registering media hotkey.

@pewsheen pewsheen requested a review from amrbashir April 18, 2024 06:14
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

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

instead of requiring &mut self, let's use a RefCell and keep it as &self

Comment on lines 163 to 164
self.media_hotkeys.lock().unwrap().remove(&hotkey);
if self.media_hotkeys.lock().unwrap().is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

why lock twice, let's lock once and assign to a variable

@amrbashir amrbashir merged commit a6433ee into feat/media-keys Apr 23, 2024
9 checks passed
@amrbashir amrbashir deleted the feat/media-keys-mac branch April 23, 2024 12:19
amrbashir added a commit that referenced this pull request Apr 23, 2024
* feat: support registering media play/pause/stop/next/prev keys

closes #70

* feat(macos): register media keys (#72)

* feat(macos): register media keys

* chore: update documents

* organize codes

* fix: using RefCell for event_tap

* docs: remove mut from GlobalHotKeyManager::new()

* use mutex and stop watching media keys on drop

---------

Co-authored-by: Jason Tsai <[email protected]>
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.

2 participants