From 0f213f13ffb1973b22bddfed2158039d85bc918e Mon Sep 17 00:00:00 2001 From: sayamsamal Date: Mon, 12 Sep 2022 01:39:18 +0530 Subject: [PATCH] tooltips: Add support for modifier key conversion for mac-syle keyboards. We scan a tooltip for any required windows-to-mac hotkey conversions from the list of attributes supplied to the hotkey_hints helper. If we find any, we add/modify the hotkyes in the hotkey hints list to match the mac-style key combinations and then return back the modified list of hotkey hints to be displayed in the tooltip. We also rename the "adjust_mac_shortcuts" function, used for the keyboard shortcuts menu and help center documnets, to "adjust_mac_kbd_tags" to avoid any ambiguity with the adjust_mac_tooltip_keys funtion which is used for tooltip hotkeys. --- frontend_tests/node_tests/common.js | 96 +++++++++++++++++++++++++++-- frontend_tests/zjsunit/index.js | 3 + static/js/common.ts | 50 ++++++++++----- static/js/info_overlay.js | 4 +- static/js/popover_menus.js | 2 +- static/js/portico/help.js | 2 +- static/js/templates.js | 2 + static/js/ui_init.js | 2 +- 8 files changed, 137 insertions(+), 24 deletions(-) diff --git a/frontend_tests/node_tests/common.js b/frontend_tests/node_tests/common.js index b6d1f63b42..54929b418f 100644 --- a/frontend_tests/node_tests/common.js +++ b/frontend_tests/node_tests/common.js @@ -80,16 +80,16 @@ run_test("copy_data_attribute_value", ({override}) => { assert.ok(faded_out); }); -run_test("adjust_mac_shortcuts non-mac", ({override}) => { +run_test("adjust_mac_kbd_tags non-mac", ({override}) => { override(navigator, "platform", "Windows"); - // The adjust_mac_shortcuts has a really simple guard + // The adjust_mac_kbd_tags has a really simple guard // at the top, and we just test the early-return behavior // by trying to pass it garbage. - common.adjust_mac_shortcuts("selector-that-does-not-exist"); + common.adjust_mac_kbd_tags("selector-that-does-not-exist"); }); -run_test("adjust_mac_shortcuts mac", ({override}) => { +run_test("adjust_mac_kbd_tags mac", ({override}) => { const keys_to_test_mac = new Map([ ["Backspace", "Delete"], ["Enter", "Return"], @@ -134,7 +134,7 @@ run_test("adjust_mac_shortcuts mac", ({override}) => { $.create(".markdown kbd", {children}); - common.adjust_mac_shortcuts(".markdown kbd"); + common.adjust_mac_kbd_tags(".markdown kbd"); for (const test_item of test_items) { assert.equal(test_item.$stub.text(), test_item.mac_key); @@ -142,6 +142,92 @@ run_test("adjust_mac_shortcuts mac", ({override}) => { } }); +run_test("adjust_mac_tooltip_keys non-mac", ({override}) => { + override(navigator, "platform", "Windows"); + + // The adjust_mac_tooltip_keys has a really simple guard + // at the top, and we just test the early-return behavior + // by trying to pass it garbage. + common.adjust_mac_tooltip_keys("not-an-array"); +}); + +// Test default values of adjust_mac_tooltip_keys +// Expected values +run_test("adjust_mac_tooltip_keys mac expected", ({override}) => { + const keys_to_test_mac = new Map([ + [["Backspace"], ["Delete"]], + [["Enter"], ["Return"]], + [["Home"], ["Fn", "←"]], + [["End"], ["Fn", "→"]], + [["PgUp"], ["Fn", "↑"]], + [["PgDn"], ["Fn", "↓"]], + [["Ctrl"], ["⌘"]], + [["Alt"], ["⌘"]], + ]); + + override(navigator, "platform", "MacIntel"); + + const test_items = []; + + for (const [old_key, mac_key] of keys_to_test_mac) { + const test_item = {}; + common.adjust_mac_tooltip_keys(old_key); + + test_item.mac_key = mac_key; + test_item.adjusted_key = old_key; + test_items.push(test_item); + } + + for (const test_item of test_items) { + assert.deepStrictEqual(test_item.mac_key, test_item.adjusted_key); + } +}); + +// Test non-default values of adjust_mac_tooltip_keys +// Random values +run_test("adjust_mac_tooltip_keys mac random", ({override}) => { + const keys_to_test_mac = new Map([ + [ + ["Ctrl", "["], + ["⌘", "["], + ], + [ + ["Ctrl", "K"], + ["⌘", "K"], + ], + [ + ["Shift", "G"], + ["Shift", "G"], + ], + [["Space"], ["Space"]], + [ + ["Alt", "←"], + ["⌘", "←"], + ], + [ + ["Alt", "→"], + ["⌘", "→"], + ], + ]); + + override(navigator, "platform", "MacIntel"); + + const test_items = []; + + for (const [old_key, mac_key] of keys_to_test_mac) { + const test_item = {}; + common.adjust_mac_tooltip_keys(old_key); + + test_item.mac_key = mac_key; + test_item.adjusted_key = old_key; + test_items.push(test_item); + } + + for (const test_item of test_items) { + assert.deepStrictEqual(test_item.mac_key, test_item.adjusted_key); + } +}); + run_test("show password", () => { const password_selector = "#id_password ~ .password_visibility_toggle"; diff --git a/frontend_tests/zjsunit/index.js b/frontend_tests/zjsunit/index.js index 7e32c57351..3da14f055f 100644 --- a/frontend_tests/zjsunit/index.js +++ b/frontend_tests/zjsunit/index.js @@ -20,6 +20,9 @@ process.env.NODE_ENV = "test"; const dom = new JSDOM("", {url: "http://zulip.zulipdev.com/"}); global.DOMParser = dom.window.DOMParser; +global.navigator = { + userAgent: "node.js", +}; require("@babel/register")({ extensions: [".es6", ".es", ".jsx", ".js", ".mjs", ".ts"], diff --git a/static/js/common.ts b/static/js/common.ts index 0229e5ae58..b595513594 100644 --- a/static/js/common.ts +++ b/static/js/common.ts @@ -44,28 +44,30 @@ export function copy_data_attribute_value($elem: JQuery, key: string): void { $elem.fadeIn(1000); } +const keys_map = new Map([ + ["Backspace", "Delete"], + ["Enter", "Return"], + ["Home", "←"], + ["End", "→"], + ["PgUp", "↑"], + ["PgDn", "↓"], + ["Ctrl", "⌘"], + ["Alt", "⌘"], +]); + +const fn_shortcuts = new Set(["Home", "End", "PgUp", "PgDn"]); + export function has_mac_keyboard(): boolean { return /mac/i.test(navigator.platform); } -export function adjust_mac_shortcuts(kbd_elem_class: string): void { +// We convert the tags used for keyboard shortcuts to mac equivalent +// key combinations, when we detect that the user is using a mac-style keyboard. +export function adjust_mac_kbd_tags(kbd_elem_class: string): void { if (!has_mac_keyboard()) { return; } - const keys_map = new Map([ - ["Backspace", "Delete"], - ["Enter", "Return"], - ["Home", "←"], - ["End", "→"], - ["PgUp", "↑"], - ["PgDn", "↓"], - ["Ctrl", "⌘"], - ["Alt", "⌘"], - ]); - - const fn_shortcuts = new Set(["Home", "End", "PgUp", "PgDn"]); - $(kbd_elem_class).each(function () { let key_text = $(this).text(); @@ -83,6 +85,26 @@ export function adjust_mac_shortcuts(kbd_elem_class: string): void { }); } +// We convert the hotkey hints used in the tooltips to mac equivalent +// key combinations, when we detect that the user is using a mac-style keyboard. +export function adjust_mac_tooltip_keys(hotkeys: string[]): void { + if (!has_mac_keyboard()) { + return; + } + + for (const [index, hotkey] of hotkeys.entries()) { + const replace_key = keys_map.get(hotkey); + + if (replace_key !== undefined) { + hotkeys[index] = replace_key; + } + + if (fn_shortcuts.has(hotkey)) { + hotkeys.unshift("Fn"); + } + } +} + // See https://zulip.readthedocs.io/en/latest/development/authentication.html#password-form-implementation // for design details on this feature. function set_password_toggle_label( diff --git a/static/js/info_overlay.js b/static/js/info_overlay.js index 13a09682b7..c2540b9dd2 100644 --- a/static/js/info_overlay.js +++ b/static/js/info_overlay.js @@ -250,8 +250,8 @@ export function set_up_toggler() { "notdisplayed", !user_settings.escape_navigates_to_default_view, ); - common.adjust_mac_shortcuts(".hotkeys_table .hotkey kbd"); - common.adjust_mac_shortcuts("#markdown-instructions kbd"); + common.adjust_mac_kbd_tags(".hotkeys_table .hotkey kbd"); + common.adjust_mac_kbd_tags("#markdown-instructions kbd"); } export function show(target) { diff --git a/static/js/popover_menus.js b/static/js/popover_menus.js index 788b5d0728..f8107cbd9b 100644 --- a/static/js/popover_menus.js +++ b/static/js/popover_menus.js @@ -236,7 +236,7 @@ export function initialize() { compose_enter_sends_popover_displayed = true; }, onMount(instance) { - common.adjust_mac_shortcuts(".enter_sends_choices kbd"); + common.adjust_mac_kbd_tags(".enter_sends_choices kbd"); $(instance.popper).one("click", ".enter_sends_choice", (e) => { let selected_behaviour = $(e.currentTarget) diff --git a/static/js/portico/help.js b/static/js/portico/help.js index fa6e5ecf0a..a907afad54 100644 --- a/static/js/portico/help.js +++ b/static/js/portico/help.js @@ -58,7 +58,7 @@ function render_code_sections() { highlight_current_article(); - common.adjust_mac_shortcuts(".markdown kbd"); + common.adjust_mac_kbd_tags(".markdown kbd"); $("table").each(function () { $(this).addClass("table table-striped"); diff --git a/static/js/templates.js b/static/js/templates.js index 390bbe89c7..c59a62997a 100644 --- a/static/js/templates.js +++ b/static/js/templates.js @@ -1,5 +1,6 @@ import Handlebars from "handlebars/runtime"; +import * as common from "./common"; import {default_html_elements, intl} from "./i18n"; import * as util from "./util"; @@ -108,6 +109,7 @@ Handlebars.registerHelper("numberFormat", (number) => number.toLocaleString()); Handlebars.registerHelper("hotkey_hints", (...hotkeys) => { hotkeys.pop(); // Handlebars options let hotkey_hints = ""; + common.adjust_mac_tooltip_keys(hotkeys); for (const hotkey of hotkeys) { hotkey_hints += `${hotkey}`; } diff --git a/static/js/ui_init.js b/static/js/ui_init.js index 7bad4296ea..3a479e21cb 100644 --- a/static/js/ui_init.js +++ b/static/js/ui_init.js @@ -229,7 +229,7 @@ function initialize_compose_box() { }), ); $(`.enter_sends_${user_settings.enter_sends}`).show(); - common.adjust_mac_shortcuts(".enter_sends kbd"); + common.adjust_mac_kbd_tags(".enter_sends kbd"); } function initialize_message_feed_errors() {