From 2b37a36c114f9ba389412b740aa244277dd0f53d Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Tue, 9 Jan 2018 14:18:56 -0800 Subject: [PATCH] Chore: Setup viewer notifications in the correct location (#563) --- src/lib/Notification.js | 7 ++++--- src/lib/PreviewUI.js | 28 +++++++++++++++++++--------- src/lib/__tests__/PreviewUI-test.js | 8 ++++---- src/lib/constants.js | 3 +++ 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/lib/Notification.js b/src/lib/Notification.js index 5fdb705b5..0ad740b52 100644 --- 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 index 1c2a1662e..a88660dad 100644 --- 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 index 2e0ec1047..da89c9f83 100644 --- 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 index 6fa574e32..e1a5efe4f 100644 --- 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';