From e74f7bde22c469ca90316f7bd69a0ba5233f8bff Mon Sep 17 00:00:00 2001 From: Michael Kuhn Date: Mon, 11 Mar 2019 11:29:10 +0100 Subject: [PATCH] Refactor hotkeys to use keypress instead of keydown keydown returns the "raw" key in event.which. Depending on the keyboard layout, this may not be what is wanted. For example, on a German keyboard, Shift+7 has to be pressed to get a slash. However, event.which will be 55, which corresponds to "7". In the keypress event, however, event.which will be 47, which corresponds to "/". Sadly, several important keys (such as escape and the arrow keys) do not trigger a keypress event. Therefore, they have to be handled using a keydown event. This change refactors the hotkey support to make use of keypress events whenever possible. This will make hotkeys work regardless of the user's keyboard layout. Escape and arrow keys are still handled via keydown events. There should be only one change in behavior: I could not make Ctrl+/ work and therefore rebound the help dialog to "?". --- include/functions.php | 49 +++++++++++++------------------ js/AppBase.js | 22 ++++++++------ js/Feeds.js | 1 + js/prefs.js | 1 + js/tt-rss.js | 4 +++ plugins/googlereaderkeys/init.php | 12 ++++---- plugins/hotkeys_noscroll/init.php | 4 +-- 7 files changed, 47 insertions(+), 46 deletions(-) diff --git a/include/functions.php b/include/functions.php index 68844013a..b824aa4c9 100755 --- a/include/functions.php +++ b/include/functions.php @@ -1222,40 +1222,36 @@ function get_hotkeys_map() { $hotkeys = array( - // "navigation" => array( "k" => "next_feed", "j" => "prev_feed", "n" => "next_article", "p" => "prev_article", - "(38)|up" => "prev_article", - "(40)|down" => "next_article", - // "^(38)|Ctrl-up" => "prev_article_noscroll", - // "^(40)|Ctrl-down" => "next_article_noscroll", - "(191)|/" => "search_dialog", - // "article" => array( + "(38)|Up" => "prev_article", + "(40)|Down" => "next_article", + "*(38)|Shift+Up" => "article_scroll_up", + "*(40)|Shift+Down" => "article_scroll_down", + "^(38)|Ctrl+Up" => "prev_article_noscroll", + "^(40)|Ctrl+Down" => "next_article_noscroll", + "/" => "search_dialog", "s" => "toggle_mark", - "*s" => "toggle_publ", + "S" => "toggle_publ", "u" => "toggle_unread", - "*t" => "edit_tags", + "T" => "edit_tags", "o" => "open_in_new_window", "c p" => "catchup_below", "c n" => "catchup_above", - "*n" => "article_scroll_down", - "*p" => "article_scroll_up", - "*(38)|Shift+up" => "article_scroll_up", - "*(40)|Shift+down" => "article_scroll_down", - "a *w" => "toggle_widescreen", + "N" => "article_scroll_down", + "P" => "article_scroll_up", + "a W" => "toggle_widescreen", "a e" => "toggle_embed_original", "e" => "email_article", "a q" => "close_article", - // "article_selection" => array( "a a" => "select_all", "a u" => "select_unread", - "a *u" => "select_marked", + "a U" => "select_marked", "a p" => "select_published", "a i" => "select_invert", "a n" => "select_none", - // "feed" => array( "f r" => "feed_refresh", "f a" => "feed_unhide_read", "f s" => "feed_subscribe", @@ -1263,31 +1259,26 @@ "f q" => "feed_catchup", "f x" => "feed_reverse", "f g" => "feed_toggle_vgroup", - "f *d" => "feed_debug_update", - "f *g" => "feed_debug_viewfeed", - "f *c" => "toggle_combined_mode", + "f D" => "feed_debug_update", + "f G" => "feed_debug_viewfeed", + "f C" => "toggle_combined_mode", "f c" => "toggle_cdm_expanded", - "*q" => "catchup_all", + "Q" => "catchup_all", "x" => "cat_toggle_collapse", - // "goto" => array( "g a" => "goto_all", "g f" => "goto_fresh", "g s" => "goto_marked", "g p" => "goto_published", "g t" => "goto_tagcloud", - "g *p" => "goto_prefs", - // "other" => array( + "g P" => "goto_prefs", "r" => "select_article_cursor", "c l" => "create_label", "c f" => "create_filter", "c s" => "collapse_sidebar", - "a *n" => "toggle_night_mode", - "^(191)|Ctrl+/" => "help_dialog", + "a N" => "toggle_night_mode", + "?" => "help_dialog", ); - $hotkeys["^(38)|Ctrl-up"] = "prev_article_noscroll"; - $hotkeys["^(40)|Ctrl-down"] = "next_article_noscroll"; - foreach (PluginHost::getInstance()->get_hooks(PluginHost::HOOK_HOTKEY_MAP) as $plugin) { $hotkeys = $plugin->hook_hotkey_map($hotkeys); } diff --git a/js/AppBase.js b/js/AppBase.js index 121b7aa85..a5e20b8f9 100644 --- a/js/AppBase.js +++ b/js/AppBase.js @@ -60,14 +60,12 @@ define(["dojo/_base/declare"], function (declare) { const hotkeys_map = App.getInitParam("hotkeys"); const keycode = event.which; - const keychar = String.fromCharCode(keycode).toLowerCase(); + const keychar = String.fromCharCode(keycode); if (keycode == 27) { // escape and drop prefix this.hotkey_prefix = false; } - if (keycode == 16 || keycode == 17) return; // ignore lone shift / ctrl - if (!this.hotkey_prefix && hotkeys_map[0].indexOf(keychar) != -1) { this.hotkey_prefix = keychar; @@ -87,13 +85,19 @@ define(["dojo/_base/declare"], function (declare) { Element.hide("cmdline"); - let hotkey_name = keychar.search(/[a-zA-Z0-9]/) != -1 ? keychar : "(" + keycode + ")"; + let hotkey_name = ""; - // ensure ^*char notation - if (event.shiftKey) hotkey_name = "*" + hotkey_name; - if (event.ctrlKey) hotkey_name = "^" + hotkey_name; - if (event.altKey) hotkey_name = "+" + hotkey_name; - if (event.metaKey) hotkey_name = "%" + hotkey_name; + if (event.type == "keydown") { + hotkey_name = "(" + keycode + ")"; + + // ensure ^*char notation + if (event.shiftKey) hotkey_name = "*" + hotkey_name; + if (event.ctrlKey) hotkey_name = "^" + hotkey_name; + if (event.altKey) hotkey_name = "+" + hotkey_name; + if (event.metaKey) hotkey_name = "%" + hotkey_name; + } else { + hotkey_name = keychar ? keychar : "(" + keycode + ")"; + } const hotkey_full = this.hotkey_prefix ? this.hotkey_prefix + " " + hotkey_name : hotkey_name; this.hotkey_prefix = false; diff --git a/js/Feeds.js b/js/Feeds.js index 401d669a7..ba63aac47 100644 --- a/js/Feeds.js +++ b/js/Feeds.js @@ -196,6 +196,7 @@ define(["dojo/_base/declare"], function (declare) { App.setLoadingProgress(50); document.onkeydown = (event) => { return App.hotkeyHandler(event) }; + document.onkeypress = (event) => { return App.hotkeyHandler(event) }; window.onresize = () => { Headlines.scrollHandler(); } if (!this.getActive()) { diff --git a/js/prefs.js b/js/prefs.js index b4ac9976e..ae6286330 100755 --- a/js/prefs.js +++ b/js/prefs.js @@ -78,6 +78,7 @@ require(["dojo/_base/kernel", this.enableCsrfSupport(); document.onkeydown = (event) => { return App.hotkeyHandler(event) }; + document.onkeypress = (event) => { return App.hotkeyHandler(event) }; App.setLoadingProgress(50); Notify.close(); diff --git a/js/tt-rss.js b/js/tt-rss.js index 99b44549b..a46fc17e4 100644 --- a/js/tt-rss.js +++ b/js/tt-rss.js @@ -206,6 +206,10 @@ require(["dojo/_base/kernel", hotkeyHandler(event) { if (event.target.nodeName == "INPUT" || event.target.nodeName == "TEXTAREA") return; + // Arrow buttons and escape are not reported via keypress, handle them via keydown. + // escape = 27, left = 37, up = 38, right = 39, down = 40 + if (event.type == "keydown" && event.which != 27 && (event.which < 37 || event.which > 40)) return; + const action_name = App.keyeventToAction(event); if (action_name) { diff --git a/plugins/googlereaderkeys/init.php b/plugins/googlereaderkeys/init.php index ca8b88516..4fb57ec94 100644 --- a/plugins/googlereaderkeys/init.php +++ b/plugins/googlereaderkeys/init.php @@ -24,11 +24,11 @@ class GoogleReaderKeys extends Plugin { $hotkeys["r"] = "feed_refresh"; $hotkeys["m"] = "toggle_unread"; $hotkeys["o"] = "toggle_expand"; - $hotkeys["(13)|enter"] = "toggle_expand"; - $hotkeys["*(191)|?"] = "help_dialog"; - $hotkeys["(32)|space"] = "next_article"; - $hotkeys["(38)|up"] = "article_scroll_up"; - $hotkeys["(40)|down"] = "article_scroll_down"; + $hotkeys["\r|Enter"] = "toggle_expand"; + $hotkeys["?"] = "help_dialog"; + $hotkeys[" |Space"] = "next_article"; + $hotkeys["(38)|Up"] = "article_scroll_up"; + $hotkeys["(40)|Down"] = "article_scroll_down"; return $hotkeys; } @@ -37,4 +37,4 @@ class GoogleReaderKeys extends Plugin { return 2; } -} \ No newline at end of file +} diff --git a/plugins/hotkeys_noscroll/init.php b/plugins/hotkeys_noscroll/init.php index 18f5aedab..2038997f5 100644 --- a/plugins/hotkeys_noscroll/init.php +++ b/plugins/hotkeys_noscroll/init.php @@ -16,8 +16,8 @@ class Hotkeys_Noscroll extends Plugin { function hook_hotkey_map($hotkeys) { - $hotkeys["(40)|down"] = "next_article_noscroll"; - $hotkeys["(38)|up"] = "prev_article_noscroll"; + $hotkeys["(40)|Down"] = "next_article_noscroll"; + $hotkeys["(38)|Up"] = "prev_article_noscroll"; $hotkeys["n"] = "next_article_noscroll"; $hotkeys["p"] = "prev_article_noscroll";