From e26ca5a547835cbcf9122a20e65246ccd6bb24bb Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Thu, 11 Jan 2024 09:23:11 +0530
Subject: [PATCH] stream-settings: Refactor code used to update url hash.

This commit refactors code to not try to update the hash multiple
times.

We now update the url hash only in the callback function for
toggler.goto and remove the call to setup_subscriptions_stream_hash
in open_edit_panel_for_row. This change works as the hash depends
on the section being opened in right panel, so it seems better to
handle the hash update while toggling to the correct section.

A couple of more changes we need to do to make this work correctly
are -

- We extract the code to call "toggler.goto" for the correct section
outside the update_toggler_for_sub to a new function
set_up_right_panel_section. This is changed as update_toggler_for_sub
returns early without doing anything if the url hash is not the one
we have when settings panel for a particular stream is opened and
now the hash is updated after that check as a callback from
toggler.goto.

- set_up_right_panel_section uses stream_edit_toggler.select_tab
to toggle to the correct section in right panel so we now need
to set stream_edit_toggler.select_tab in change_state.
---
 web/src/stream_edit.js         |  9 ++++-----
 web/src/stream_edit_toggler.ts |  4 ++++
 web/src/stream_settings_ui.js  |  3 ++-
 web/src/stream_ui_updates.js   | 13 +++++++++----
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/web/src/stream_edit.js b/web/src/stream_edit.js
index a12b2209c3..4605a8dbad 100644
--- a/web/src/stream_edit.js
+++ b/web/src/stream_edit.js
@@ -84,13 +84,12 @@ function get_sub_for_target(target) {
     return sub;
 }
 
-export function open_edit_panel_for_row(stream_row, right_side_tab) {
+export function open_edit_panel_for_row(stream_row) {
     const sub = get_sub_for_target(stream_row);
 
     $(".stream-row.active").removeClass("active");
     stream_settings_components.show_subs_pane.settings(sub);
     $(stream_row).addClass("active");
-    stream_edit_toggler.setup_subscriptions_stream_hash(sub, right_side_tab);
     setup_stream_settings(stream_row);
 }
 
@@ -258,7 +257,7 @@ export function show_settings_for(node) {
     scroll_util.get_content_element($("#stream_settings")).html(html);
 
     $("#stream_settings .tab-container").prepend(stream_edit_toggler.toggler.get());
-    stream_ui_updates.update_toggler_for_sub(sub);
+    stream_ui_updates.set_up_right_panel_section(sub);
 
     const $edit_container = stream_settings_containers.get_edit_container(sub);
 
@@ -564,7 +563,7 @@ export function initialize() {
         stream_settings_components.sub_or_unsub(sub, $stream_row);
 
         if (!sub.subscribed) {
-            open_edit_panel_for_row($stream_row, stream_edit_toggler.select_tab);
+            open_edit_panel_for_row($stream_row);
         }
         stream_ui_updates.update_regular_sub_settings(sub);
 
@@ -632,7 +631,7 @@ export function initialize() {
 
     $("#streams_overlay_container").on("click", ".stream-row", function (e) {
         if ($(e.target).closest(".check, .subscription_settings").length === 0) {
-            open_edit_panel_for_row(this, stream_edit_toggler.select_tab);
+            open_edit_panel_for_row(this);
         }
     });
 
diff --git a/web/src/stream_edit_toggler.ts b/web/src/stream_edit_toggler.ts
index 304d985b2e..62751ca9a9 100644
--- a/web/src/stream_edit_toggler.ts
+++ b/web/src/stream_edit_toggler.ts
@@ -10,6 +10,10 @@ import type {StreamSubscription} from "./sub_store";
 export let toggler: components.Toggle;
 export let select_tab = "personal";
 
+export function set_select_tab(right_side_tab: string): void {
+    select_tab = right_side_tab;
+}
+
 export function setup_subscriptions_stream_hash(
     sub: StreamSubscription,
     right_side_tab: string,
diff --git a/web/src/stream_settings_ui.js b/web/src/stream_settings_ui.js
index 9e33ec2315..9688a766d6 100644
--- a/web/src/stream_settings_ui.js
+++ b/web/src/stream_settings_ui.js
@@ -689,7 +689,7 @@ export function switch_to_stream_row(stream_id, right_side_tab) {
 
     scroll_util.scroll_element_into_container($stream_row, $container);
 
-    stream_edit.open_edit_panel_for_row($stream_row, right_side_tab);
+    stream_edit.open_edit_panel_for_row($stream_row);
     stream_edit_toggler.toggler.goto(right_side_tab);
 }
 
@@ -737,6 +737,7 @@ export function change_state(section, right_side_tab) {
             toggler.goto("subscribed");
         } else {
             show_right_section();
+            stream_edit_toggler.set_select_tab(right_side_tab);
             switch_to_stream_row(stream_id, right_side_tab);
         }
         return;
diff --git a/web/src/stream_ui_updates.js b/web/src/stream_ui_updates.js
index ea8018a8e9..2246f8815c 100644
--- a/web/src/stream_ui_updates.js
+++ b/web/src/stream_ui_updates.js
@@ -137,10 +137,7 @@ export function initialize_cant_subscribe_popover() {
     initialize_disable_btn_hint_popover($button_wrapper);
 }
 
-export function update_toggler_for_sub(sub) {
-    if (!hash_parser.is_editing_stream(sub.stream_id)) {
-        return;
-    }
+export function set_up_right_panel_section(sub) {
     if (sub.subscribed) {
         stream_edit_toggler.toggler.enable_tab("personal");
         stream_edit_toggler.toggler.goto(stream_edit_toggler.select_tab);
@@ -158,6 +155,14 @@ export function update_toggler_for_sub(sub) {
     enable_or_disable_subscribers_tab(sub);
 }
 
+export function update_toggler_for_sub(sub) {
+    if (!hash_parser.is_editing_stream(sub.stream_id)) {
+        return;
+    }
+
+    set_up_right_panel_section(sub);
+}
+
 export function enable_or_disable_subscribers_tab(sub) {
     if (!hash_parser.is_editing_stream(sub.stream_id)) {
         return;