From eb72f77d0a12b94d68652e5a74d09526999d2437 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Tue, 16 Mar 2021 06:22:28 +0000 Subject: [PATCH] settings: Allow switching between user and org settings manually. Since the base hash for org settings and user settings are different (organization and settings), the hashchange module gets confused that we are going from one overlay to other. A reproducer for this flow is to visit the organization "Bots" page, click on your own profile as the owner of a bot, and then click "Edit profile" from there. So, we fix this by making an exception for this particular case in the module. Fixes part of #18011. --- frontend_tests/node_tests/browser_history.js | 18 ++++++++++++++++++ static/js/browser_history.js | 6 ++++++ static/js/hashchange.js | 14 ++++++++++++++ static/js/settings_panel_menu.js | 4 +++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/frontend_tests/node_tests/browser_history.js b/frontend_tests/node_tests/browser_history.js index 58919564da..ff399d6f24 100644 --- a/frontend_tests/node_tests/browser_history.js +++ b/frontend_tests/node_tests/browser_history.js @@ -3,6 +3,7 @@ const {strict: assert} = require("assert"); const {set_global, zrequire} = require("../zjsunit/namespace"); +const {make_stub} = require("../zjsunit/stub"); const {run_test} = require("../zjsunit/test"); const blueslip = require("../zjsunit/zblueslip"); @@ -49,3 +50,20 @@ test("error for bad hashes", () => { blueslip.expect("error", "programming error: prefix hashes with #: bogus"); browser_history.update(hash); }); + +test("update internal hash if required", (override) => { + const hash = "#test/hash"; + const stub = make_stub(); + override(browser_history, "update", stub.f); + browser_history.update_hash_internally_if_required(hash); + assert.equal(stub.num_calls, 1); + + location.hash = "#test/hash"; + // update with same hash + browser_history.update_hash_internally_if_required(hash); + // but no update was made since the + // hash was already updated. + // Evident by no increase in number of + // calls to stub. + assert.equal(stub.num_calls, 1); +}); diff --git a/static/js/browser_history.js b/static/js/browser_history.js index 18f94c5fa2..6b75d50017 100644 --- a/static/js/browser_history.js +++ b/static/js/browser_history.js @@ -66,3 +66,9 @@ export function go_to_location(hash) { // function to run. window.location.hash = hash; } + +export function update_hash_internally_if_required(hash) { + if (window.location.hash !== hash) { + update(hash); + } +} diff --git a/static/js/hashchange.js b/static/js/hashchange.js index 30d30fe44b..fa551fd04b 100644 --- a/static/js/hashchange.js +++ b/static/js/hashchange.js @@ -18,6 +18,7 @@ import * as recent_topics from "./recent_topics"; import * as search from "./search"; import * as settings from "./settings"; import * as settings_panel_menu from "./settings_panel_menu"; +import * as settings_toggle from "./settings_toggle"; import * as subs from "./subs"; import * as top_left_corner from "./top_left_corner"; import * as ui_util from "./ui_util"; @@ -211,6 +212,19 @@ function do_hashchange_overlay(old_hash) { return; } + // This is a special case when user clicks on a URL that makes the overlay switch + // from org settings to user settings or user edits the URL to switch between them. + const settings_hashes = new Set(["settings", "organization"]); + // Ensure that we are just switching between user and org settings and the settings + // overlay is open. + const is_hashchange_internal = + settings_hashes.has(base) && settings_hashes.has(old_base) && overlays.settings_open(); + if (is_hashchange_internal) { + settings_toggle.highlight_toggle(base); + settings_panel_menu.normal_settings.activate_section_or_default(section); + return; + } + // It's not super likely that an overlay is already open, // but you can jump from /settings to /streams by using // the browser's history menu or hand-editing the URL or diff --git a/static/js/settings_panel_menu.js b/static/js/settings_panel_menu.js index 74c70d25b3..d21f2385ac 100644 --- a/static/js/settings_panel_menu.js +++ b/static/js/settings_panel_menu.js @@ -114,7 +114,9 @@ export class SettingsPanelMenu { this.curr_li.addClass("active"); const settings_section_hash = "#" + this.hash_prefix + section; - browser_history.update(settings_section_hash); + + // It could be that the hash has already been set. + browser_history.update_hash_internally_if_required(settings_section_hash); $(".settings-section").removeClass("show");