From 032d347b4f68b9387c2a87d7e758e016d6c6f93b Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Mon, 9 Aug 2021 16:56:49 +0530
Subject: [PATCH] settings: Refactor frontend code for notification settings.

This is a prep commit for adding UI for realm-level default
of user settings. We refactor the code to use  classes
instead of ids such that we can use the common code for the
new settings.
---
 frontend_tests/puppeteer_tests/settings.ts    |  4 +--
 static/js/settings_notifications.js           | 33 +++++++++++--------
 static/styles/settings.css                    | 12 +++----
 .../settings/notification_settings.hbs        | 20 +++++------
 .../notification_settings_checkboxes.hbs      |  3 +-
 .../templates/settings/settings_checkbox.hbs  |  2 +-
 6 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/frontend_tests/puppeteer_tests/settings.ts b/frontend_tests/puppeteer_tests/settings.ts
index 28ec5ef93b..08284346bb 100644
--- a/frontend_tests/puppeteer_tests/settings.ts
+++ b/frontend_tests/puppeteer_tests/settings.ts
@@ -365,7 +365,7 @@ async function test_notifications_section(page: Page): Promise<void> {
     // At the beginning, "PMs, mentions, and alerts"(checkbox name=enable_sounds) audio will be on
     // and "Streams"(checkbox name=enable_stream_audible_notifications) audio will be off by default.
 
-    const notification_sound_enabled = "#notification_sound:enabled";
+    const notification_sound_enabled = ".notification_sound:enabled";
     await page.waitForSelector(notification_sound_enabled, {visible: true});
 
     await common.fill_form(page, ".notification-settings-form", {
@@ -382,7 +382,7 @@ async function test_notifications_section(page: Page): Promise<void> {
     all audio notifications. But this seems flaky in tests.
     TODO: Find the right fix and enable this.
 
-    const notification_sound_disabled = "#notification_sound:disabled";
+    const notification_sound_disabled = ".notification_sound:disabled";
     await page.waitForSelector(notification_sound_disabled);
     */
 }
diff --git a/static/js/settings_notifications.js b/static/js/settings_notifications.js
index 4c3ff38bdf..fff9d26b7b 100644
--- a/static/js/settings_notifications.js
+++ b/static/js/settings_notifications.js
@@ -51,28 +51,33 @@ function change_notification_setting(setting, value, status_element) {
 }
 
 function update_desktop_icon_count_display() {
-    $("#desktop_icon_count_display").val(user_settings.desktop_icon_count_display);
+    $("#user-notification-settings .desktop_icon_count_display").val(
+        user_settings.desktop_icon_count_display,
+    );
     unread_ui.update_unread_counts();
 }
 
 export function set_enable_digest_emails_visibility() {
+    const container = $("#user-notification-settings");
     if (page_params.realm_digest_emails_enabled) {
-        $("#enable_digest_emails_label").parent().show();
+        container.find(".enable_digest_emails_label").parent().show();
     } else {
-        $("#enable_digest_emails_label").parent().hide();
+        container.find(".enable_digest_emails_label").parent().hide();
     }
 }
 
 export function set_enable_marketing_emails_visibility() {
+    const container = $("#user-notification-settings");
     if (page_params.corporate_enabled) {
-        $("#enable_marketing_emails_label").parent().show();
+        container.find(".enable_marketing_emails_label").parent().show();
     } else {
-        $("#enable_marketing_emails_label").parent().hide();
+        container.find(".enable_marketing_emails_label").parent().hide();
     }
 }
 
 export function set_up() {
-    $("#notification-settings").on("change", "input, select", function (e) {
+    const container = $("#user-notification-settings");
+    $("#user-notification-settings").on("change", "input, select", function (e) {
         e.preventDefault();
         e.stopPropagation();
         const input_elem = $(e.currentTarget);
@@ -90,25 +95,25 @@ export function set_up() {
 
     update_desktop_icon_count_display();
 
-    $("#send_test_notification").on("click", () => {
+    container.find(".send_test_notification").on("click", () => {
         notifications.send_test_notification(
             $t({defaultMessage: "This is what a Zulip notification looks like."}),
         );
     });
 
-    $("#play_notification_sound").on("click", () => {
+    container.find(".play_notification_sound").on("click", () => {
         if (user_settings.notification_sound !== "none") {
             $("#notification-sound-audio")[0].play();
         }
     });
 
-    const notification_sound_dropdown = $("#notification_sound");
+    const notification_sound_dropdown = container.find(".notification_sound");
     notification_sound_dropdown.val(user_settings.notification_sound);
 
-    $("#enable_sounds, #enable_stream_audible_notifications").on("change", () => {
+    container.find(".enable_sounds, .enable_stream_audible_notifications").on("change", () => {
         if (
-            $("#enable_stream_audible_notifications").prop("checked") ||
-            $("#enable_sounds").prop("checked")
+            container.find(".enable_stream_audible_notifications").prop("checked") ||
+            container.find(".enable_sounds").prop("checked")
         ) {
             notification_sound_dropdown.prop("disabled", false);
             notification_sound_dropdown.parent().removeClass("control-label-disabled");
@@ -136,7 +141,9 @@ export function update_page() {
             continue;
         }
 
-        $(`#${CSS.escape(setting)}`).prop("checked", user_settings[setting]);
+        $("#user-notification-settings")
+            .find(`.${CSS.escape(setting)}`)
+            .prop("checked", user_settings[setting]);
     }
     rerender_ui();
 }
diff --git a/static/styles/settings.css b/static/styles/settings.css
index babcad771a..936d0500cc 100644
--- a/static/styles/settings.css
+++ b/static/styles/settings.css
@@ -128,8 +128,8 @@ h3 .fa-question-circle-o {
     padding: 0 20px;
 }
 
-#notification_sound,
-#play_notification_sound {
+.notification_sound,
+.play_notification_sound {
     display: inline;
     margin-right: 4px;
 
@@ -139,7 +139,7 @@ h3 .fa-question-circle-o {
     }
 }
 
-#notification_sound {
+.notification_sound {
     text-transform: capitalize;
 }
 
@@ -562,12 +562,12 @@ input[type="checkbox"] {
     margin-top: 4px;
 }
 
-#notification-settings {
+#user-notification-settings {
     .notification-reminder {
         text-align: left;
     }
 
-    #notification-table {
+    .notification-table {
         th,
         td {
             text-align: center;
@@ -1561,7 +1561,7 @@ body:not(.night-mode) #settings_page .custom_user_field .datepicker {
 }
 
 /* These have enough space for "Admins and full members" in German. */
-#desktop_icon_count_display,
+.desktop_icon_count_display,
 #id_realm_waiting_period_setting,
 #id_realm_create_stream_policy,
 #id_realm_invite_to_stream_policy,
diff --git a/static/templates/settings/notification_settings.hbs b/static/templates/settings/notification_settings.hbs
index ee08ccf74d..9f8195a09a 100644
--- a/static/templates/settings/notification_settings.hbs
+++ b/static/templates/settings/notification_settings.hbs
@@ -1,10 +1,10 @@
-<div id="notification-settings" class="settings-section" data-name="notifications">
+<div id="user-notification-settings" class="settings-section" data-name="notifications">
     <form class="notification-settings-form">
-        <div id="general_notifications" class="subsection-parent">
+        <div class="general_notifications subsection-parent">
             <h3 class="inline-block">{{t "Notification triggers" }}</h3>
-            <div class="alert-notification" id="general-notify-settings-status"></div>
+            <div class="alert-notification general-notify-settings-status"></div>
             <p>{{t "Configure how Zulip notifies you about new messages." }}</p>
-            <table id="notification-table" class="table table-condensed table-bordered wrapped-table">
+            <table class="notification-table table table-condensed table-bordered wrapped-table">
                 <thead>
                     <tr>
                         <th rowspan="2" width="30%"></th>
@@ -44,14 +44,14 @@
             </table>
         </div>
 
-        <div id="other_notifications" class="m-10 inline-block subsection-parent">
+        <div class="other_notifications m-10 inline-block subsection-parent">
 
             <h3 class="inline-block">{{t "Other notification settings" }}</h3>
-            <div class="alert-notification" id="other-notify-settings-status"></div>
+            <div class="alert-notification other-notify-settings-status"></div>
 
             <h5>{{t "Desktop" }}</h5>
 
-            <p><a id="send_test_notification">{{t "Send test notification" }}</a></p>
+            <p><a class="send_test_notification">{{t "Send test notification" }}</a></p>
 
             {{#each notification_settings.desktop_notification_settings}}
             {{> settings_checkbox
@@ -65,7 +65,7 @@
             </label>
 
             <div class="input-group {{#unless enable_sound_select}}control-label-disabled{{/unless}}">
-                <select name="notification_sound" id="notification_sound" data-setting-widget-type="string"
+                <select name="notification_sound" class="notification_sound" data-setting-widget-type="string"
                   {{#unless enable_sound_select}}
                   disabled
                   {{/unless}}>
@@ -74,14 +74,14 @@
                     <option value="{{ this }}">{{ this }}</option>
                     {{/each}}
                 </select>
-                <span id="play_notification_sound">
+                <span class="play_notification_sound">
                     <i class="fa fa-play-circle" aria-label="{{t 'Play sound' }}"></i>
                 </span>
             </div>
 
             <div class="input-group">
                 <label for="desktop_icon_count_display" class="dropdown-title">{{ settings_label.desktop_icon_count_display }}</label>
-                <select name="desktop_icon_count_display" id="desktop_icon_count_display" class="prop-element"
+                <select name="desktop_icon_count_display" class="desktop_icon_count_display prop-element"
                   data-setting-widget-type="number">
                     {{> dropdown_options_widget option_values=desktop_icon_count_display_values}}
                 </select>
diff --git a/static/templates/settings/notification_settings_checkboxes.hbs b/static/templates/settings/notification_settings_checkboxes.hbs
index 53b9a8b504..ac950dd16a 100644
--- a/static/templates/settings/notification_settings_checkboxes.hbs
+++ b/static/templates/settings/notification_settings_checkboxes.hbs
@@ -2,7 +2,8 @@
     <label class="checkbox">
         <input type="checkbox" name="{{setting_name}}" id="{{prefix}}{{setting_name}}"
           {{#if is_disabled}}disabled="disabled"{{/if}}
-          {{#if is_checked}}checked="checked"{{/if}} data-setting-widget-type="boolean"/>
+          {{#if is_checked}}checked="checked"{{/if}} data-setting-widget-type="boolean"
+          class="{{setting_name}}"/>
         <span></span>
     </label>
 </td>
diff --git a/static/templates/settings/settings_checkbox.hbs b/static/templates/settings/settings_checkbox.hbs
index 192d9834f3..e724da357f 100644
--- a/static/templates/settings/settings_checkbox.hbs
+++ b/static/templates/settings/settings_checkbox.hbs
@@ -5,7 +5,7 @@
           id="{{prefix}}{{setting_name}}" {{#if is_checked}}checked="checked"{{/if}} />
         <span></span>
     </label>
-    <label for="{{prefix}}{{setting_name}}" class="inline" id="{{prefix}}{{setting_name}}_label">
+    <label for="{{prefix}}{{setting_name}}" class="inline {{setting_name}}_label" id="{{prefix}}{{setting_name}}_label">
         {{label}}
         {{#if help_link}}
         {{> ../help_link_widget link=help_link }}