Chore: Setup viewer notifications in the correct location (#563)

box · Jan 9, 2018 · 2b37a36 · 2b37a36
1 parent 1f98c92
commit 2b37a36
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 16 deletions.
diff --git a/src/lib/Notification.js b/src/lib/Notification.js
@@ -1,4 +1,4 @@
-import { CLASS_HIDDEN } from './constants';
+import { CLASS_HIDDEN, CLASS_BOX_PREVIEW_NOTIFICATION, CLASS_BOX_PREVIEW_NOTIFICATION_WRAPPER } from './constants';
 
 const HIDE_TIMEOUT_MS = 5000; // 5s
 
@@ -13,7 +13,8 @@ class Notification {
         const uniqueLabel = `notification_${new Date().getTime()}_label`;
 
         this.notificationEl = document.createElement('div');
-        this.notificationEl.className = 'bp-notification bp-is-hidden';
+        this.notificationEl.classList.add(CLASS_BOX_PREVIEW_NOTIFICATION);
+        this.notificationEl.classList.add(CLASS_HIDDEN);
         this.notificationEl.addEventListener('click', this.clickHandler);
 
         // ARIA for accessibility
@@ -31,7 +32,7 @@ class Notification {
 
         // Append and position notification
         const notificationWrapperEl = document.createElement('div');
-        notificationWrapperEl.className = 'bp-notifications-wrapper';
+        notificationWrapperEl.classList.add(CLASS_BOX_PREVIEW_NOTIFICATION_WRAPPER);
         notificationWrapperEl.appendChild(this.notificationEl);
         containerEl.appendChild(notificationWrapperEl);
     }

diff --git a/src/lib/PreviewUI.js b/src/lib/PreviewUI.js
@@ -117,6 +117,9 @@ class PreviewUI {
         // Setup progress bar
         this.progressBar = new ProgressBar(this.container);
 
+        // Setup notification
+        this.notification = new Notification(this.contentContainer);
+
         // Setup loading indicator
         this.setupLoading();
 
@@ -256,17 +259,16 @@ class PreviewUI {
      * @return {void}
      */
     hideLoadingIndicator() {
-        if (this.contentContainer) {
-            this.contentContainer.classList.add(CLASS_PREVIEW_LOADED);
+        if (!this.contentContainer) {
+            return;
+        }
 
-            // Re-show the cralwer for the next preview since it is hidden in finishLoadingSetup() in BaseViewer.js
-            const crawler = this.contentContainer.querySelector(SELECTOR_BOX_PREVIEW_CRAWLER_WRAPPER);
-            if (crawler) {
-                crawler.classList.remove(CLASS_HIDDEN);
-            }
+        this.contentContainer.classList.add(CLASS_PREVIEW_LOADED);
 
-            // Setup viewer notification
-            this.notification = new Notification(this.contentContainer);
+        // Re-show the cralwer for the next preview since it is hidden in finishLoadingSetup() in BaseViewer.js
+        const crawler = this.contentContainer.querySelector(SELECTOR_BOX_PREVIEW_CRAWLER_WRAPPER);
+        if (crawler) {
+            crawler.classList.remove(CLASS_HIDDEN);
         }
     }
 
@@ -299,6 +301,10 @@ class PreviewUI {
      * @return {void}
      */
     showNotification(message, buttonText) {
+        if (!this.notification) {
+            return;
+        }
+
         this.notification.show(message, buttonText);
     }
 
@@ -309,6 +315,10 @@ class PreviewUI {
      * @return {void}
      */
     hideNotification() {
+        if (!this.notification) {
+            return;
+        }
+
         this.notification.hide();
     }
 

diff --git a/src/lib/__tests__/PreviewUI-test.js b/src/lib/__tests__/PreviewUI-test.js
@@ -57,6 +57,9 @@ describe('lib/PreviewUI', () => {
             // Check progress bar
             expect(resultEl).to.contain(constants.SELECTOR_BOX_PREVIEW_PROGRESS_BAR);
 
+            // Check notification
+            expect(resultEl).to.contain(constants.SELECTOR_BOX_PREVIEW_NOTIFICATION);
+
             // Check loading state
             const loadingWrapperEl = resultEl.querySelector(constants.SELECTOR_BOX_PREVIEW_LOADING_WRAPPER);
             expect(loadingWrapperEl).to.contain(constants.SELECTOR_BOX_PREVIEW_ICON);
@@ -187,13 +190,10 @@ describe('lib/PreviewUI', () => {
         });
 
         describe('hideLoadingIndicator()', () => {
-            it('should hide loading indicator and intializes the notification', () => {
+            it('should hide loading indicator', () => {
                 const contentContainerEl = containerEl.querySelector(constants.SELECTOR_BOX_PREVIEW);
                 ui.hideLoadingIndicator();
                 expect(contentContainerEl).to.have.class(constants.CLASS_PREVIEW_LOADED);
-
-                // Check that notification is initialized
-                expect(contentContainerEl).to.contain('.bp-notification');
             });
 
             it('should remove the hidden class from the crawler', () => {

diff --git a/src/lib/constants.js b/src/lib/constants.js
@@ -27,6 +27,8 @@ export const CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_DOCUMENT = 'bp-document-preload-w
 export const CLASS_BOX_PREVIEW_PRELOAD_WRAPPER_PRESENTATION = 'bp-presentation-preload-wrapper';
 export const CLASS_BOX_PREVIEW_PROGRESS_BAR = 'bp-progress-bar';
 export const CLASS_BOX_PREVIEW_PROGRESS_BAR_CONTAINER = 'bp-progress-bar-container';
+export const CLASS_BOX_PREVIEW_NOTIFICATION = 'bp-notification';
+export const CLASS_BOX_PREVIEW_NOTIFICATION_WRAPPER = 'bp-notifications-wrapper';
 export const CLASS_BOX_PREVIEW_TOGGLE_OVERLAY = 'bp-toggle-overlay';
 export const CLASS_BOX_PREVIEW_THEME_DARK = 'bp-theme-dark';
 export const CLASS_ELEM_KEYBOARD_FOCUS = 'bp-has-keyboard-focus';
@@ -60,6 +62,7 @@ export const SELECTOR_BOX_PREVIEW_LOADING_WRAPPER = `.${CLASS_BOX_PREVIEW_LOADIN
 export const SELECTOR_BOX_PREVIEW_LOGO_CUSTOM = `.${CLASS_BOX_PREVIEW_LOGO_CUSTOM}`;
 export const SELECTOR_BOX_PREVIEW_LOGO_DEFAULT = `.${CLASS_BOX_PREVIEW_LOGO_DEFAULT}`;
 export const SELECTOR_BOX_PREVIEW_PROGRESS_BAR = `.${CLASS_BOX_PREVIEW_PROGRESS_BAR}`;
+export const SELECTOR_BOX_PREVIEW_NOTIFICATION = `.${CLASS_BOX_PREVIEW_NOTIFICATION}`;
 
 export const PERMISSION_DOWNLOAD = 'can_download';
 export const PERMISSION_PREVIEW = 'can_preview';