From 8b29c38e62a83eec3085a1d65ba19fefd415e62d Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Thu, 2 Jul 2020 15:37:30 +0530 Subject: [PATCH] info_overlay: Make them overlays from quasi-overlays. We always use hashchange.go_to_location method now to open the info_overlay, this makes sure that the url hash are reliable and hotkeys don't get confused if an overlay is open or not. We don't want to change hash to "" (this also doesn't navigates us to 'All messages' view, hence the bug was not noticed.) on exit of info_overlay. --- frontend_tests/node_tests/hashchange.js | 18 +++--------------- frontend_tests/node_tests/hotkey.js | 2 +- static/js/click_handlers.js | 2 +- static/js/gear_menu.js | 2 +- static/js/hashchange.js | 23 +++++++++++++++++------ static/js/hotkey.js | 2 +- static/js/info_overlay.js | 2 +- 7 files changed, 25 insertions(+), 26 deletions(-) diff --git a/frontend_tests/node_tests/hashchange.js b/frontend_tests/node_tests/hashchange.js index 2b38bb682a..b0d4ef73ef 100644 --- a/frontend_tests/node_tests/hashchange.js +++ b/frontend_tests/node_tests/hashchange.js @@ -208,31 +208,19 @@ run_test("hash_interactions", () => { helper.clear_events(); $(window).trigger($.Event("hashchange", {})); - helper.assert_events([ - "overlays.close_for_hash_change", - "message_viewport.stop_auto_scrolling", - "info: keyboard-shortcuts", - ]); + helper.assert_events(["overlays.close_for_hash_change", "info: keyboard-shortcuts"]); window.location.hash = "#message-formatting/whatever"; helper.clear_events(); $(window).trigger($.Event("hashchange", {})); - helper.assert_events([ - "overlays.close_for_hash_change", - "message_viewport.stop_auto_scrolling", - "info: message-formatting", - ]); + helper.assert_events(["overlays.close_for_hash_change", "info: message-formatting"]); window.location.hash = "#search-operators/whatever"; helper.clear_events(); $(window).trigger($.Event("hashchange", {})); - helper.assert_events([ - "overlays.close_for_hash_change", - "message_viewport.stop_auto_scrolling", - "info: search-operators", - ]); + helper.assert_events(["overlays.close_for_hash_change", "info: search-operators"]); window.location.hash = "#drafts"; diff --git a/frontend_tests/node_tests/hotkey.js b/frontend_tests/node_tests/hotkey.js index 1b3434abc9..3ffaa68f39 100644 --- a/frontend_tests/node_tests/hotkey.js +++ b/frontend_tests/node_tests/hotkey.js @@ -262,7 +262,7 @@ run_test("basic_chars", () => { test_normal_typing(); overlays.is_active = return_false; - assert_mapping("?", "info_overlay.maybe_show_keyboard_shortcuts"); + assert_mapping("?", "hashchange.go_to_location"); assert_mapping("/", "search.initiate_search"); assert_mapping("w", "activity.initiate_search"); assert_mapping("q", "stream_list.initiate_search"); diff --git a/static/js/click_handlers.js b/static/js/click_handlers.js index a42f02059f..8a9c90ecf3 100644 --- a/static/js/click_handlers.js +++ b/static/js/click_handlers.js @@ -673,7 +673,7 @@ exports.initialize = function () { $("body").on("click", "[data-overlay-trigger]", function () { const target = $(this).attr("data-overlay-trigger"); - info_overlay.show(target); + hashchange.go_to_location(target); }); function handle_compose_click(e) { diff --git a/static/js/gear_menu.js b/static/js/gear_menu.js index a0c168db12..86b3a7dd51 100644 --- a/static/js/gear_menu.js +++ b/static/js/gear_menu.js @@ -64,7 +64,7 @@ The "info:" items use our info overlay system in static/js/info_overlay.js. They are dispatched using a click handler in static/js/click_handlers.js. The click handler uses "[data-overlay-trigger]" as -the selector and then calls info_overlay.show. +the selector and then calls hash_change.go_to_location. */ diff --git a/static/js/hashchange.js b/static/js/hashchange.js index 68e64fc23f..f82150fa30 100644 --- a/static/js/hashchange.js +++ b/static/js/hashchange.js @@ -69,6 +69,9 @@ function is_overlay_hash(hash) { "organization", "invite", "recent_topics", + "keyboard-shortcuts", + "message-formatting", + "search-operators", ]; const main_hash = hash_util.get_hash_category(hash); @@ -115,14 +118,8 @@ function do_hashchange_normal(from_reload) { activate_home_tab(); break; case "#keyboard-shortcuts": - info_overlay.show("keyboard-shortcuts"); - break; case "#message-formatting": - info_overlay.show("message-formatting"); - break; case "#search-operators": - info_overlay.show("search-operators"); - break; case "#drafts": case "#invite": case "#streams": @@ -223,6 +220,20 @@ function do_hashchange_overlay(old_hash) { recent_topics.launch(); return; } + if (base === "keyboard-shortcuts") { + info_overlay.show("keyboard-shortcuts"); + return; + } + + if (base === "message-formatting") { + info_overlay.show("message-formatting"); + return; + } + + if (base === "search-operators") { + info_overlay.show("search-operators"); + return; + } } function hashchanged(from_reload, e) { diff --git a/static/js/hotkey.js b/static/js/hotkey.js index 7c0528d5d7..112037d519 100644 --- a/static/js/hotkey.js +++ b/static/js/hotkey.js @@ -653,7 +653,7 @@ exports.process_hotkey = function (e, hotkey) { gear_menu.open(); return true; case "show_shortcuts": // Show keyboard shortcuts page - info_overlay.maybe_show_keyboard_shortcuts(); + hashchange.go_to_location("keyboard-shortcuts"); return true; case "stream_cycle_backward": narrow.stream_cycle_backward(); diff --git a/static/js/info_overlay.js b/static/js/info_overlay.js index f63881f29a..ad400711f9 100644 --- a/static/js/info_overlay.js +++ b/static/js/info_overlay.js @@ -57,7 +57,7 @@ exports.show = function (target) { name: "informationalOverlays", overlay, on_close() { - hashchange.changehash(""); + hashchange.exit_overlay(); }, }); }