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

Introducing tooltip helper for Vim mode on MacOS #3476

Merged
merged 3 commits into from Jun 17, 2021
Merged

Introducing tooltip helper for Vim mode on MacOS #3476

merged 3 commits into from Jun 17, 2021

Conversation

MrSnix
Copy link
Contributor

@MrSnix MrSnix commented Jun 15, 2021

Summary:
This line has been introduced to make the Vim mode correctly work on macOS.
Contrary to other platforms, if you preventDefault() a keypress event, no repeat events are fired for that key as it is being held down. This snippet, reintroduce the event firing disabling the Apple behaviour. The flag applies only at app level (just for Insomnia), it won't influence other applications.

When set to false, the custom apple behaviour is disabled and works as expected by code-mirror.

systemPreferences.setUserDefault('ApplePressAndHoldEnabled', 'boolean', false);

Jun-15-2021 19-47-04

I'm holding j and k in this gif

Important:
I have put this one-liner near the BrowserWindow object because I feel like it's more appropriate, it is part of the main Electron interface. As always, you can commit to this branch and put it somewhere else if you have a better and more appropriate place. Lastly, you must wrap the statement inside an if(isMac()) to prevent it from running against Windows or Linux systems.

It closes #3448

@MrSnix
Copy link
Contributor Author

MrSnix commented Jun 15, 2021

@dimitropoulos
I have applied the requested changes.
As always, let me know.
I'm open to any feedback :D

I apologize if I missed something.

Copy link
Contributor

@dimitropoulos dimitropoulos left a comment

Choose a reason for hiding this comment

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

I can't test it since I'm on linux, but the code looks good to me! thanks!

@DMarby
Copy link
Contributor

DMarby commented Jun 15, 2021

@MrSnix Thank you for fixing this, this is awesome!

@reynolek @MrSnix @develohpanda @dimitropoulos I don't have a strong opinion, but do you think it might make sense/be possible to only conditionally set the system preference if vim mode is enabled (and perhaps use removeUserDefault when vim mode is disabled to fallback to the system global setting)?

By default in maCOS if you hold eg a, rather then a repeating itself a bunch of times, you get a popup menu for special characters:
image

With this change as it is currently, I think we'd lose that behaviour within the app even when vim mode is disabled, which I think would be good to avoid if possible.

@reynolek
Copy link
Contributor

Ah good call -- It'd be even better if it was limited to non-insert mode with vim bindings enabled (although I dont know if thats possible).

@MrSnix
Copy link
Contributor Author

MrSnix commented Jun 15, 2021

Hi @DMarby, thanks for your feedback :D
I did some test about switching at runtime the flag but it is not working as I'd expect.

I will investigate and try to let you know more tomorrow.

@develohpanda
Copy link
Contributor

develohpanda commented Jun 15, 2021

Do you think it might make sense/be possible to only conditionally set the system preference if vim mode is enabled (and perhaps use removeUserDefault when vim mode is disabled to fallback to the system global setting)?

  1. It feels like this should be an option which is given to the user, because even with Vim mode enabled, some users may want to keep using the special-characters instead of press-and-hold for navigation. We're adding one feature but taking another away.
    1. If we provide an option, what should this default to, depending on the OS level setting?
  2. On looking at how other IDEs handle this, the official vim plugin for VS Code does not provide an option for VS Code to internally override it (remember, VS Code is also electron), but rather specifies in it's README. IntelliJ also does not provide any official settings for it, as several SO answers suggest.

With the above reasons in mind, leaning especially on VS Code's/IntelliJ's approach, I suggest we don't programmatically disable this option, rather add an info icon with a link to documentation which describes to disable this option using the command:

defaults write com.insomnia.app ApplePressAndHoldEnabled -bool false 

image

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Following on from the reasons outlined in #3476 (comment), I suggest we only link to documentation and have users change the setting themselves.

I have added documentation here: https://support.insomnia.rest/article/203-key-maps

Documentation links in the app are defined here for use throughout the app.

@reynolek
Copy link
Contributor

So in my testing, I disabled the value. In Insomnia press and works. If I switch over to iMessages or my browsers, press and hold still brings up other accented letters.

But other apps like Discord it no longer works (is discord electron? I always forget if they transitioned away or not).

@develohpanda
Copy link
Contributor

But other apps like Discord it no longer works (is discord electron? I always forget if they transitioned away or not).

@reynolek which command did you run?

In my testing, I ran defaults write com.insomnia.app ApplePressAndHoldEnabled -bool false, restarted both Insomnia and Discord - I see repeating characters in Insomnia and the special characters in Discord.

@reynolek
Copy link
Contributor

reynolek commented Jun 16, 2021

Ahh you know what, i did it globally 👍

EDIT: To be more explicit, I ran defaults write -g ApplePressAndHoldEnabled -bool false

@MrSnix MrSnix changed the title Disabling ApplePressAndHoldEnabled to fix the Vim mode on Insomnia Introducing tooltip helper for Vim mode on MacOS Jun 16, 2021
@MrSnix
Copy link
Contributor Author

MrSnix commented Jun 16, 2021

As requested, I applied the changes.

Jun-16-2021 21-24-43

Feel free to modify the text content if there is a better alternative.
As always, I'm open to any feedback :D

@MrSnix MrSnix requested a review from develohpanda June 16, 2021 19:22
Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Made a slight wording adjustment, but LGTM! 🎉

image

@dimitropoulos dimitropoulos merged commit aeabbfd into Kong:develop Jun 17, 2021
@MrSnix MrSnix deleted the bug/keyboard-vim-fix branch June 17, 2021 14:13
develohpanda added a commit that referenced this pull request Jun 22, 2021
* Introducing tooltip helper for Vim mode on MacOS

* Fix article link and update tooltip text

Co-authored-by: Opender Singh <opender.singh@konghq.com>
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.

Press and Hold not working on Vim keymap (on MacOS)
5 participants