From 900aea88a4d6ac349aefed6a7ff91ff3dba3215a Mon Sep 17 00:00:00 2001
From: Tim Abbott <tabbott@zulipchat.com>
Date: Thu, 26 Mar 2020 12:49:28 -0700
Subject: [PATCH] panels: Restructure to actually make sense.

The original implementation of panels.js was just for notifications,
and ended up running a bunch of notifications-specific code, including
registration click handlers and some localstorage-related
notifications logic, every time a panel was supposed to be opened.

This refactoring makes the panels library make sense -- we now
initialize all click handlers in the initialize() method, and do the
notifications check in a single, coherent place scoped to notifications.
---
 static/js/panels.js | 61 +++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/static/js/panels.js b/static/js/panels.js
index df514807a5..f7f951a744 100644
--- a/static/js/panels.js
+++ b/static/js/panels.js
@@ -17,24 +17,16 @@ const get_step = function ($process) {
     return $process.find("[data-step]").filter(":visible").data("step");
 };
 
-exports.initialize = function () {
-    // if email has not been set up and the user is the admin, display a warning
-    // to tell them to set up an email server.
-    if (page_params.insecure_desktop_app) {
-        exports.open($("[data-process='insecure-desktop-app']"));
-    } else if (page_params.warn_no_email === true && page_params.is_admin) {
-        exports.open($("[data-process='email-server']"));
-    } else {
-        exports.open($("[data-process='notifications']"));
+function should_show_notifications(ls) {
+    // if the user said to never show banner on this computer again, it will
+    // be stored as `true` so we want to negate that.
+    if (localstorage.supported()) {
+        if (ls.get("dontAskForNotifications") === true) {
+            return false;
+        }
     }
-};
 
-exports.open = function ($process) {
-    const ls = localstorage();
-
-    $("[data-process]").hide();
-
-    let should_show_notifications =
+    return (
         // notifications *basically* don't work on any mobile platforms, so don't
         // event show the banners. This prevents trying to access things that
         // don't exist like `Notification.permission`.
@@ -43,26 +35,22 @@ exports.open = function ($process) {
         !notifications.granted_desktop_notifications_permission() &&
         // if permission is allowed to be requested (e.g. not in "denied" state).
         notifications.permission_state() !== "denied"
-    ;
+    );
+}
 
-    if (localstorage.supported()) {
-        // if the user said to never show banner on this computer again, it will
-        // be stored as `true` so we want to negate that.
-        should_show_notifications = should_show_notifications && !ls.get("dontAskForNotifications");
-    }
-
-    if (should_show_notifications) {
-        $process.show();
-        resize_app();
-    }
-
-    // if it is not the notifications prompt, show the error if it has been
-    // initialized here.
-    if ($process.is(":not([data-process='notifications'])")) {
-        $process.show();
-        resize_app();
+exports.initialize = function () {
+    const ls = localstorage();
+    if (page_params.insecure_desktop_app) {
+        exports.open($("[data-process='insecure-desktop-app']"));
+    } else if (page_params.warn_no_email === true && page_params.is_admin) {
+        // if email has not been set up and the user is the admin,
+        // display a warning to tell them to set up an email server.
+        exports.open($("[data-process='email-server']"));
+    } else if (should_show_notifications(ls)) {
+        exports.open($("[data-process='notifications']"));
     }
 
+    // Configure click handlers.
     $(".request-desktop-notifications").on("click", function (e) {
         e.preventDefault();
         $(this).closest(".alert").hide();
@@ -78,6 +66,7 @@ exports.open = function ($process) {
 
     $("#panels").on("click", ".alert .close, .alert .exit", function (e) {
         e.stopPropagation();
+        const $process = $(e.target).closest("[data-process]");
         if (get_step($process) === 1 && $process.data("process") === "notifications") {
             show_step($process, 2);
         } else {
@@ -87,4 +76,10 @@ exports.open = function ($process) {
     });
 };
 
+exports.open = function ($process) {
+    $("[data-process]").hide();
+    $process.show();
+    resize_app();
+};
+
 window.panels = exports;