From e997f60656872766e03755a111b3e22f960c6d49 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 5 Jan 2021 11:37:11 +0100
Subject: [PATCH] Only display a notification on the `sidebarToggle`-button,
 and not the individual view-buttons (PR 7959 follow-up)

The whole purpose of showing a notification on the `sidebarToggle` button, when the sidebar is closed, was to give users *some* kind of indication that the PDF document contains outline/attachments/layers without having to manually open the sidebar to check.
However, in the implementation in PR 7959, I also added notifications for each view-buttons in the sidebar. Looking back at this, I've always questioned the value of the last part, since the view-buttons already have a `disabled`-state which shows if they're available or not. Hence we're actually, in a sense, duplicating notifications for the outline/attachments/layers-buttons without adding (in my opinion) all that much overall value.

All-in-all, I'm thus proposing that we only display the notification on the `sidebarToggle`-button itself, since that should really be sufficient here, which also allows us to simplify the relevant code a fair bit.
---
 web/pdf_sidebar.js | 79 +++++++++-------------------------------------
 1 file changed, 15 insertions(+), 64 deletions(-)

diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js
index 94679aa4d..f18b87a36 100644
--- a/web/pdf_sidebar.js
+++ b/web/pdf_sidebar.js
@@ -25,8 +25,6 @@ const UI_NOTIFICATION_CLASS = "pdfSidebarNotification";
  * @property {PDFThumbnailViewer} pdfThumbnailViewer - The thumbnail viewer.
  * @property {EventBus} eventBus - The application event bus.
  * @property {IL10n} l10n - The localization service.
- * @property {boolean} [disableNotification] - Disable the notification for
- *   documents containing outline/attachments. The default value is `false`.
  */
 
 /**
@@ -69,7 +67,6 @@ class PDFSidebar {
     pdfThumbnailViewer,
     eventBus,
     l10n = NullL10n,
-    disableNotification = false,
   }) {
     this.isOpen = false;
     this.active = SidebarView.THUMBS;
@@ -103,7 +100,6 @@ class PDFSidebar {
 
     this.eventBus = eventBus;
     this.l10n = l10n;
-    this._disableNotification = disableNotification;
 
     this._addEventListeners();
   }
@@ -111,7 +107,7 @@ class PDFSidebar {
   reset() {
     this.isInitialViewSet = false;
 
-    this._hideUINotification(null);
+    this._hideUINotification(/* reset = */ true);
     this.switchView(SidebarView.THUMBS);
 
     this.outlineButton.disabled = false;
@@ -259,7 +255,6 @@ class PDFSidebar {
     if (isViewChanged) {
       this._dispatchEvent();
     }
-    this._hideUINotification(this.active);
     return isViewChanged;
   }
 
@@ -278,7 +273,7 @@ class PDFSidebar {
     this._forceRendering();
     this._dispatchEvent();
 
-    this._hideUINotification(this.active);
+    this._hideUINotification();
   }
 
   close() {
@@ -347,11 +342,7 @@ class PDFSidebar {
   /**
    * @private
    */
-  _showUINotification(view) {
-    if (this._disableNotification) {
-      return;
-    }
-
+  _showUINotification() {
     this.l10n
       .get(
         "toggle_sidebar_notification2.title",
@@ -366,66 +357,26 @@ class PDFSidebar {
       // Only show the notification on the `toggleButton` if the sidebar is
       // currently closed, to avoid unnecessarily bothering the user.
       this.toggleButton.classList.add(UI_NOTIFICATION_CLASS);
-    } else if (view === this.active) {
-      // If the sidebar is currently open *and* the `view` is visible, do not
-      // bother the user with a notification on the corresponding button.
-      return;
-    }
-
-    switch (view) {
-      case SidebarView.OUTLINE:
-        this.outlineButton.classList.add(UI_NOTIFICATION_CLASS);
-        break;
-      case SidebarView.ATTACHMENTS:
-        this.attachmentsButton.classList.add(UI_NOTIFICATION_CLASS);
-        break;
-      case SidebarView.LAYERS:
-        this.layersButton.classList.add(UI_NOTIFICATION_CLASS);
-        break;
     }
   }
 
   /**
    * @private
    */
-  _hideUINotification(view) {
-    if (this._disableNotification) {
-      return;
+  _hideUINotification(reset = false) {
+    if (this.isOpen || reset) {
+      // Only hide the notification on the `toggleButton` if the sidebar is
+      // currently open, or when the current PDF document is being closed.
+      this.toggleButton.classList.remove(UI_NOTIFICATION_CLASS);
     }
 
-    const removeNotification = sidebarView => {
-      switch (sidebarView) {
-        case SidebarView.OUTLINE:
-          this.outlineButton.classList.remove(UI_NOTIFICATION_CLASS);
-          break;
-        case SidebarView.ATTACHMENTS:
-          this.attachmentsButton.classList.remove(UI_NOTIFICATION_CLASS);
-          break;
-        case SidebarView.LAYERS:
-          this.layersButton.classList.remove(UI_NOTIFICATION_CLASS);
-          break;
-      }
-    };
-
-    if (!this.isOpen && view !== null) {
-      // Only hide the notifications when the sidebar is currently open,
-      // or when it is being reset (i.e. `view === null`).
-      return;
+    if (reset) {
+      this.l10n
+        .get("toggle_sidebar.title", null, "Toggle Sidebar")
+        .then(msg => {
+          this.toggleButton.title = msg;
+        });
     }
-    this.toggleButton.classList.remove(UI_NOTIFICATION_CLASS);
-
-    if (view !== null) {
-      removeNotification(view);
-      return;
-    }
-    // Remove all sidebar notifications on reset.
-    for (view in SidebarView) {
-      removeNotification(SidebarView[view]);
-    }
-
-    this.l10n.get("toggle_sidebar.title", null, "Toggle Sidebar").then(msg => {
-      this.toggleButton.title = msg;
-    });
   }
 
   /**
@@ -475,7 +426,7 @@ class PDFSidebar {
       button.disabled = !count;
 
       if (count) {
-        this._showUINotification(view);
+        this._showUINotification();
       } else if (this.active === view) {
         // If the `view` was opened by the user during document load,
         // switch away from it if it turns out to be empty.