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

Changing hotkeys + fixing Windows compatibility #5

Merged
merged 1 commit into from
Jul 31, 2019

Conversation

Herrox
Copy link
Contributor

@Herrox Herrox commented Jan 24, 2019

Hello,

I found an issue using this plugin on Windows (an probably Linux): hotkeys are not working because they are declared with Command key. As explain on the following documentation https://github.com/electron/electron/blob/master/docs/api/accelerator.md#platform-notice, Command key is specific to MacOS and has no effect on both Windows and Linux.
So, I replace Command with CommandOrControl to make them work on all platforms.

I also had to change hotkeys for selectCurrentPane and toggleCurrentPane hotkeys otherwise it was the same shortcut. So, I totally redefined hotkeys with random keys (u, i,o and p) but I am totally open to better ones.

Waiting for your feedback.

Regards,
Herrox.

Changing hotkeys and fixing issue with Windows compatibility
@Herrox
Copy link
Contributor Author

Herrox commented Feb 14, 2019

@chabou :)

@Herrox
Copy link
Contributor Author

Herrox commented Mar 19, 2019

@chabou up :)

@Herrox
Copy link
Contributor Author

Herrox commented May 2, 2019

@chabou up up again, for this PR :)

@chabou
Copy link
Owner

chabou commented May 2, 2019

Sorry for the delay.
I was not very confortable to introduce a breaking change.
But I haven't a smart solution to fix this issue in a different way.

@Herrox
Copy link
Contributor Author

Herrox commented May 2, 2019

@chabou I agree breaking changes is sometimes not a good solution, but I haven't any other solution too and seems to be the better here. Or if someone has a better one...

Otherwise, do you know if this feature will/could be included in Hyper core? Maybe that's a better way to do it

@chabou
Copy link
Owner

chabou commented May 2, 2019

Ideally, Hyper-core could make easier for plugins to have different keymap per OS.
But for now, we can implement it on plugin space. But I think it doesn't worth the overhead.
Let's do this breaking change.

Users will restore their keymap in their config if needed.

I will merge your PR and make a major release ASAP.

Thank you for your help and your gentle reminder 🙏

@Herrox
Copy link
Contributor Author

Herrox commented May 2, 2019

Well noted for hyper-core.
Super, can't wait for that major release !

Pas de soucis pour le reminder ;-) Au plaisir d'avoir pu aider.

@chabou chabou merged commit 8e843d1 into chabou:master Jul 31, 2019
@chabou
Copy link
Owner

chabou commented Jul 31, 2019

Published as v2.0.0
I had to change your chosen hotkeys because cmd+alt+i opens chrome dev tools on macOS.

Merci again for your help @Herrox 🙏

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