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

Respectful keyboard shortcuts #5

Merged
merged 2 commits into from
Apr 28, 2018
Merged

Respectful keyboard shortcuts #5

merged 2 commits into from
Apr 28, 2018

Conversation

fallanic
Copy link
Member

@fallanic fallanic commented Apr 21, 2018

Ticket: https://github.com/Carglass/viz/projects/1#card-9340457

Note

This branch was originally for the slider dev, but took care of this more urgent matter first.
Will create a different branch / PR for the slider.

Changes

  • unregistering the keyboard shortcuts when the app is losing focus, so we can still code while our app is running 😅

How To Test

  • run the electron app, then open VSCode, or chrome, or Slack, or Atom (anything using Electron under the hood) and make sure you can still use the left and right arrows

Applicable Research Resources

@fallanic fallanic changed the title OPEN FOR VISIBILITY ONLY - slider Respectful keyboard shortcuts Apr 27, 2018
@fallanic fallanic requested a review from Carglass April 27, 2018 17:37
Copy link
Collaborator

@Carglass Carglass left a comment

Choose a reason for hiding this comment

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

ok

// registering / unregistering shortcuts when necessary
registerShortcuts();
win.on('focus', registerShortcuts);
win.on('blur', unregisterShortcuts);
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting, I would not have thought to do it this way ˆˆ'

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, you would think there are more obvious ways, and more documented too 😄

@fallanic fallanic merged commit c69598c into master Apr 28, 2018
@fallanic fallanic deleted the slider branch April 28, 2018 21:28
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