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 support for CMD/Meta key in place of CTRL #88

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bencoder
Copy link

@bencoder bencoder commented May 7, 2020

I really struggle to get used to using control key on Mac when almost everything else uses the command key in its place.

It makes it more comfortable for me to use, but there might be something I've missed that makes this not a good idea. Happy for feedback!

@ojack
Copy link
Member

ojack commented May 7, 2020

Hi! I don't have a mac to try it on right now, but I remember there being a reason that I did it this way when I started developing hydra on a mac. (Can't remember now though!) Does the alt key still work fine with this pull request? I want to make sure we try on major operating systems and browsers before merging this.

@bencoder
Copy link
Author

bencoder commented May 7, 2020

Hi! Yes the alt key still works for me. On Chrome and Firefox.

Understand completely, I don't have a windows/linux machine right now so I can't test what effect it has there.

Potential issues that I see

  • Meta is the windows key on windows keyboards. Although Firefox at least doesn't treat it as meta: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/metaKey. I'm not sure what chrome does. It seems unusual for a non-system app to respond to the windows key
  • Limits potential key combo (no meta+ctrl at the same time)
  • 2 key combos to do the same thing feels a little wrong :)

Base automatically changed from master to main February 28, 2021 13:22
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