From 73710e1cf07e846f4ee7c4a5a3f6c717cb07bcb2 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Mon, 25 Oct 2021 19:17:19 +0200 Subject: [PATCH] user_settings: Add option to disable escape key navigation to default view. Add `escape_navigates_to_default_view` as a bool setting in UserBaseSettings model and implement it as a checkbox that toggles the hotkey implementation of escape to the default view in the advanced user display settings. With /help/ documentation edits from Alya Abbott. Fixes #20043. --- frontend_tests/node_tests/dispatch.js | 12 +++- frontend_tests/node_tests/lib/events.js | 7 ++ static/js/hotkey.js | 11 ++- static/js/info_overlay.js | 5 ++ static/js/realm_user_settings_defaults.ts | 1 + static/js/server_events_dispatch.js | 5 ++ static/js/settings_config.ts | 1 + static/js/settings_display.js | 14 ++-- static/js/user_settings.ts | 1 + static/styles/settings.css | 6 +- static/templates/keyboard_shortcuts.hbs | 2 +- .../templates/settings/display_settings.hbs | 10 ++- templates/zerver/api/changelog.md | 9 ++- templates/zerver/help/change-default-view.md | 26 ------- .../zerver/help/configure-default-view.md | 68 +++++++++++++++++++ .../zerver/help/include/sidebar_index.md | 2 +- templates/zerver/help/keyboard-shortcuts.md | 6 +- version.py | 2 +- ...add_escnav_default_display_user_setting.py | 23 +++++++ zerver/models.py | 2 + zerver/openapi/zulip.yaml | 44 ++++++++++-- zerver/views/realm.py | 1 + zerver/views/user_settings.py | 1 + 23 files changed, 210 insertions(+), 49 deletions(-) delete mode 100644 templates/zerver/help/change-default-view.md create mode 100644 templates/zerver/help/configure-default-view.md create mode 100644 zerver/migrations/0369_add_escnav_default_display_user_setting.py diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index d57a12e086..baf57ad4c2 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -667,6 +667,16 @@ run_test("user_settings", ({override}) => { dispatch(event); assert_same(user_settings.left_side_userlist, true); + event = event_fixtures.user_settings__escape_navigates_to_default_view; + user_settings.escape_navigates_to_default_view = false; + let toggled = []; + $("#go-to-default-view-hotkey-help").toggleClass = (cls) => { + toggled.push(cls); + }; + dispatch(event); + assert_same(user_settings.escape_navigates_to_default_view, true); + assert_same(toggled, ["notdisplayed"]); + // We alias message_list.narrowed to message_lists.current // to make sure we get line coverage on re-rendering // the current message list. The actual code tests @@ -694,7 +704,7 @@ run_test("user_settings", ({override}) => { event = event_fixtures.user_settings__high_contrast_mode; user_settings.high_contrast_mode = false; - let toggled = []; + toggled = []; $("body").toggleClass = (cls) => { toggled.push(cls); }; diff --git a/frontend_tests/node_tests/lib/events.js b/frontend_tests/node_tests/lib/events.js index 177ad51451..dab3c5b25b 100644 --- a/frontend_tests/node_tests/lib/events.js +++ b/frontend_tests/node_tests/lib/events.js @@ -797,6 +797,13 @@ exports.fixtures = { value: true, }, + user_settings__escape_navigates_to_default_view: { + type: "user_settings", + op: "update", + property: "escape_navigates_to_default_view", + value: true, + }, + user_settings__fluid_layout_width: { type: "user_settings", op: "update", diff --git a/static/js/hotkey.js b/static/js/hotkey.js index b457f9d8b9..e0930f34f3 100644 --- a/static/js/hotkey.js +++ b/static/js/hotkey.js @@ -40,6 +40,7 @@ import * as stream_popover from "./stream_popover"; import * as stream_settings_ui from "./stream_settings_ui"; import * as topic_zoom from "./topic_zoom"; import * as ui from "./ui"; +import {user_settings} from "./user_settings"; function do_narrow_action(action) { action(message_lists.current.selected_id(), {trigger: "hotkey"}); @@ -329,8 +330,14 @@ export function process_escape_key(e) { return true; } - hashchange.set_hash_to_default_view(); - return true; + /* The Ctrl+[ hotkey navigates to the default view + * unconditionally; Esc's behavior depends on a setting. */ + if (user_settings.escape_navigates_to_default_view || e.which === 219) { + hashchange.set_hash_to_default_view(); + return true; + } + + return false; } function handle_popover_events(event_name) { diff --git a/static/js/info_overlay.js b/static/js/info_overlay.js index 8fea448e08..8b16fb38bb 100644 --- a/static/js/info_overlay.js +++ b/static/js/info_overlay.js @@ -13,6 +13,7 @@ import * as markdown from "./markdown"; import * as overlays from "./overlays"; import * as rendered_markdown from "./rendered_markdown"; import * as ui from "./ui"; +import {user_settings} from "./user_settings"; import * as util from "./util"; // Make it explicit that our toggler is undefined until @@ -210,6 +211,10 @@ export function set_up_toggler() { $(".informational-overlays .overlay-tabs").append(elem); + $("#go-to-default-view-hotkey-help").toggleClass( + "notdisplayed", + !user_settings.escape_navigates_to_default_view, + ); common.adjust_mac_shortcuts(".hotkeys_table .hotkey kbd"); common.adjust_mac_shortcuts("#markdown-instructions kbd"); } diff --git a/static/js/realm_user_settings_defaults.ts b/static/js/realm_user_settings_defaults.ts index 8ecdb9b0c8..cd2a1104fd 100644 --- a/static/js/realm_user_settings_defaults.ts +++ b/static/js/realm_user_settings_defaults.ts @@ -21,6 +21,7 @@ export type RealmDefaultSettingsType = { enable_stream_email_notifications: boolean; enable_stream_push_notifications: boolean; enter_sends: boolean; + escape_navigates_to_default_view: boolean; fluid_layout_width: boolean; high_contrast_mode: boolean; left_side_userlist: boolean; diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index 54ffd4eb24..b6996e73fc 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -576,6 +576,7 @@ export function dispatch_normal_event(event) { "demote_inactive_streams", "dense_mode", "emojiset", + "escape_navigates_to_default_view", "fluid_layout_width", "high_contrast_mode", "left_side_userlist", @@ -673,6 +674,10 @@ export function dispatch_normal_event(event) { $("#user_presence_enabled").prop("checked", user_settings.presence_enabled); break; } + if (event.property === "escape_navigates_to_default_view") { + $("#go-to-default-view-hotkey-help").toggleClass("notdisplayed", !event.value); + break; + } settings_display.update_page(settings_display.user_settings_panel); break; } diff --git a/static/js/settings_config.ts b/static/js/settings_config.ts index 78228b57ee..94c6b325bc 100644 --- a/static/js/settings_config.ts +++ b/static/js/settings_config.ts @@ -434,6 +434,7 @@ export const display_settings_labels = { defaultMessage: "Convert emoticons before sending (:) becomes 😃)", }), ), + escape_navigates_to_default_view: $t({defaultMessage: "Escape key navigates to default view"}), }; export const notification_settings_labels = { diff --git a/static/js/settings_display.js b/static/js/settings_display.js index 46f5f2630b..c3a2a43e3f 100644 --- a/static/js/settings_display.js +++ b/static/js/settings_display.js @@ -47,16 +47,14 @@ export function set_up(settings_panel) { container.find(".advanced-settings-status").hide(); + // Select current values for enum/select type fields. For boolean + // fields, the current value is set automatically in the template. container.find(".setting_demote_inactive_streams").val(settings_object.demote_inactive_streams); - container.find(".setting_color_scheme").val(settings_object.color_scheme); - container.find(".setting_default_view").val(settings_object.default_view); - container .find(".setting_twenty_four_hour_time") .val(JSON.stringify(settings_object.twenty_four_hour_time)); - container .find(`.setting_emojiset_choice[value="${CSS.escape(settings_object.emojiset)}"]`) .prop("checked", true); @@ -186,9 +184,15 @@ export function update_page(settings_panel) { const container = $(settings_panel.container); const settings_object = settings_panel.settings_object; + // Boolean fields container.find(".left_side_userlist").prop("checked", settings_object.left_side_userlist); - container.find(".default_language_name").text(default_language_name); container.find(".translate_emoticons").prop("checked", settings_object.translate_emoticons); + container + .find(".escape_navigates_to_default_view") + .prop("checked", settings_object.escape_navigates_to_default_view); + + // Enum/select fields + container.find(".default_language_name").text(default_language_name); container .find(".setting_twenty_four_hour_time") .val(JSON.stringify(settings_object.twenty_four_hour_time)); diff --git a/static/js/user_settings.ts b/static/js/user_settings.ts index 88ce89c4f5..bb6bbff0fc 100644 --- a/static/js/user_settings.ts +++ b/static/js/user_settings.ts @@ -21,6 +21,7 @@ export type UserSettingsType = { enable_stream_email_notifications: boolean; enable_stream_push_notifications: boolean; enter_sends: boolean; + escape_navigates_to_default_view: boolean; fluid_layout_width: boolean; high_contrast_mode: boolean; left_side_userlist: boolean; diff --git a/static/styles/settings.css b/static/styles/settings.css index 11c0e6b9b3..dd40764540 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -430,8 +430,10 @@ td .button { } .input-group { - .thinner { - margin: 10px 0; + /* Class to use when the following input-group is related and should + appear just after this element. */ + &.thinner { + margin: 0; } label.checkbox + label { diff --git a/static/templates/keyboard_shortcuts.hbs b/static/templates/keyboard_shortcuts.hbs index bf523487bc..32cfe2bfc7 100644 --- a/static/templates/keyboard_shortcuts.hbs +++ b/static/templates/keyboard_shortcuts.hbs @@ -54,7 +54,7 @@ {{t 'Go to default view' }} - Esc or Ctrl + [ + Ctrl + [ or Esc diff --git a/static/templates/settings/display_settings.hbs b/static/templates/settings/display_settings.hbs index 8025a49b43..1c594b61f0 100644 --- a/static/templates/settings/display_settings.hbs +++ b/static/templates/settings/display_settings.hbs @@ -80,15 +80,21 @@ {{> settings_save_discard_widget section_name="advanced-settings" show_only_indicator=(not for_realm_settings) }} -
+
+ {{> settings_checkbox + setting_name="escape_navigates_to_default_view" + is_checked=settings_object.escape_navigates_to_default_view + label=settings_label.escape_navigates_to_default_view + prefix=prefix}} +