preserve ctrl-f for macOS for moving forward
#759
Conversation
src/utility/commonKeys.js
Outdated
|
|
||
| const commonKeys = { | ||
| // Persistent search box in Query Editor | ||
| [isMacOs ? "Cmd-F" : "Ctrl-F"]: "findPersistent", |
There was a problem hiding this comment.
The previous behavior is definitely clowny. However, I don't quite trust the codemirror regex, since it only returns true for mountain lion and higher. Let's keep the former way for now.
There was a problem hiding this comment.
Let's use navigator.platform - much less clowny :)
There was a problem hiding this comment.
this UA detection seems needless here. Just make two entries, one for Cmd-F and one for Ctrl-F, with the same value. Thats the usual codemirror keymap MO
src/utility/commonKeys.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| // keep consistent with codemirror: | |||
| // https://github.com/codemirror/CodeMirror/blob/ed8dfeb5e2/src/util/browser.js#L17 | |||
| const isMacOs = /Mac OS X 1\d\D([8-9]|\d\d)\D/.test(navigator.userAgent); | |||
There was a problem hiding this comment.
Rather than sniffing the user agent, let's just use the platform API:
| const isMacOs = /Mac OS X 1\d\D([8-9]|\d\d)\D/.test(navigator.userAgent); | |
| const isMacOs = window.navigator.platform === 'MacIntel'; |
I don't think we need to worry about 'Mac68K' and 'MacPPC' which are other valid options for Mac platforms.
src/utility/commonKeys.js
Outdated
|
|
||
| const commonKeys = { | ||
| // Persistent search box in Query Editor | ||
| [isMacOs ? "Cmd-F" : "Ctrl-F"]: "findPersistent", |
There was a problem hiding this comment.
this UA detection seems needless here. Just make two entries, one for Cmd-F and one for Ctrl-F, with the same value. Thats the usual codemirror keymap MO
|
ohhh i see. so if its mac, keep the default ctrl-f behavior? |
|
@acao Exactly; many Mac users (including, until recently, me) expect the standard Emacs-style bindings to work. It's safe to bind |
ctrl-ffor macOS for moving forward (Emacs style key binding)