From 642fa8d6f8c8f64e007db8945769013104240464 Mon Sep 17 00:00:00 2001 From: yanas Date: Tue, 6 Dec 2016 17:05:32 -0600 Subject: [PATCH] Fixes some issues pointed out from hristoterezov --- modules/UI/UI.js | 2 +- modules/UI/overlay/Overlay.js | 23 +-- .../UI/reload_overlay/PageReloadOverlay.js | 144 +++++++++--------- 3 files changed, 90 insertions(+), 79 deletions(-) diff --git a/modules/UI/UI.js b/modules/UI/UI.js index fdac5ffd1..30fbcf58e 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -17,7 +17,7 @@ import Recording from "./recording/Recording"; import GumPermissionsOverlay from './gum_overlay/UserMediaPermissionsGuidanceOverlay'; -import PageReloadOverlay from './reload_overlay/PageReloadOverlay'; +import * as PageReloadOverlay from './reload_overlay/PageReloadOverlay'; import SuspendedOverlay from './suspended_overlay/SuspendedOverlay'; import VideoLayout from "./videolayout/VideoLayout"; import FilmStrip from "./videolayout/FilmStrip"; diff --git a/modules/UI/overlay/Overlay.js b/modules/UI/overlay/Overlay.js index 37d9a8749..e4559e0b9 100644 --- a/modules/UI/overlay/Overlay.js +++ b/modules/UI/overlay/Overlay.js @@ -15,13 +15,20 @@ export default class Overlay{ * @type {jQuery} */ this.$overlay = null; + + /** + * Indicates if this overlay should use the light look & feel or the + * standard one. + * @type {boolean} + */ + this.isLightOverlay = false; } /** * Template method which should be used by subclasses to provide the overlay * content. The contents provided by this method are later subject to * the translation using {@link APP.translation.translateElement}. * @return {string} HTML representation of the overlay dialog contents. - * @private + * @protected */ _buildOverlayContent() { return ''; @@ -31,8 +38,9 @@ export default class Overlay{ * * @param isLightOverlay indicates that this will be a light overlay look * and feel. + * @private */ - buildOverlayHtml(isLightOverlay) { + _buildOverlayHtml(isLightOverlay) { let overlayContent = this._buildOverlayContent(); @@ -59,21 +67,18 @@ export default class Overlay{ /** * Template method called just after the overlay is displayed for the first * time. - * @private + * @protected */ _onShow() { // To be overridden by subclasses. } /** - * Shows the overlay dialog adn attaches the underlying HTML representation + * Shows the overlay dialog and attaches the underlying HTML representation * to the DOM. - * - * @param isLightOverlay indicates that this will be a light overlay look - * and feel. */ - show(isLightOverlay) { + show() { - !this.$overlay && this.buildOverlayHtml(isLightOverlay); + !this.$overlay && this._buildOverlayHtml(this.isLightOverlay); if (!this.isVisible()) { this.$overlay.appendTo('body'); diff --git a/modules/UI/reload_overlay/PageReloadOverlay.js b/modules/UI/reload_overlay/PageReloadOverlay.js index 224b90869..aec4c5d2b 100644 --- a/modules/UI/reload_overlay/PageReloadOverlay.js +++ b/modules/UI/reload_overlay/PageReloadOverlay.js @@ -1,7 +1,7 @@ /* global $, APP, AJS */ const logger = require("jitsi-meet-logger").getLogger(__filename); -import Overlay from '../overlay/Overlay'; +import Overlay from "../overlay/Overlay"; /** * An overlay dialog which is shown before the conference is reloaded. Shows @@ -12,13 +12,14 @@ class PageReloadOverlayImpl extends Overlay{ * Creates new PageReloadOverlayImpl * @param {number} timeoutSeconds how long the overlay dialog will be * displayed, before the conference will be reloaded. - * @param {boolean} isDisconnect indicates if this reload screen is created - * to indicate a disconnect - * @param {boolean} isNetworkFailure true indicates that it's - * caused by network related failure or false when it's - * the infrastructure. + * @param {string} title the title of the overlay message + * @param {string} message the message of the overlay + * @param {string} buttonHtml the button html or an empty string if there's + * no button + * @param {boolean} isLightOverlay indicates if the overlay should be a + * light overlay or a standard one */ - constructor(timeoutSeconds, isNetworkFailure) { + constructor(timeoutSeconds, title, message, buttonHtml, isLightOverlay) { super(); /** * Conference reload counter in seconds. @@ -31,12 +32,10 @@ class PageReloadOverlayImpl extends Overlay{ */ this.timeout = timeoutSeconds; - /** - * Indicates that a network related failure is the reason for the - * reload. - * @type {boolean} - */ - this.isNetworkFailure = isNetworkFailure; + this.title = title; + this.message = message; + this.buttonHtml = buttonHtml; + this.isLightOverlay = isLightOverlay; } /** * Constructs overlay body with the warning message and count down towards @@ -44,27 +43,10 @@ class PageReloadOverlayImpl extends Overlay{ * @override */ _buildOverlayContent() { - let title = (this.isNetworkFailure) - ? "dialog.conferenceDisconnectTitle" - : "dialog.conferenceReloadTitle"; - let message = (this.isNetworkFailure) - ? "dialog.conferenceDisconnectMsg" - : "dialog.conferenceReloadMsg"; - - let button = (this.isNetworkFailure) - ? `` - : ""; - - $(document).on('click', '#reconnectNow', () => { - APP.ConferenceUrl.reload(); - }); - return `
- -
- ${button} + ${this.buttonHtml}
`; } @@ -97,6 +79,9 @@ class PageReloadOverlayImpl extends Overlay{ * @override */ _onShow() { + $("#reconnectNow").click(() => { + APP.ConferenceUrl.reload(); + }); // Initialize displays this.updateDisplay(); @@ -128,42 +113,63 @@ class PageReloadOverlayImpl extends Overlay{ */ let overlay; -export default { - /** - * Checks whether the page reload overlay has been displayed. - * @return {boolean} true if the page reload overlay is currently - * visible or false otherwise. - */ - isVisible() { +/** + * Checks whether the page reload overlay has been displayed. + * @return {boolean} true if the page reload overlay is currently + * visible or false otherwise. + */ +export function isVisible() { return overlay && overlay.isVisible(); - }, - /** - * Shows the page reload overlay which will do the conference reload after - * the given amount of time. - * - * @param {number} timeoutSeconds how many seconds before the conference - * reload will happen. - * @param {boolean} isNetworkFailure true indicates that it's - * caused by network related failure or false when it's - * the infrastructure. - * @param {string} reason a label string identifying the reason for the page - * reload which will be included in details of the log event - */ - show(timeoutSeconds, isNetworkFailure, reason) { +} - if (!overlay) { - overlay - = new PageReloadOverlayImpl(timeoutSeconds, isNetworkFailure); - } - // Log the page reload event - if (!this.isVisible()) { - // FIXME (CallStats - issue) this event will not make it to - // the CallStats, because the log queue is not flushed, before - // "fabric terminated" is sent to the backed - APP.conference.logEvent( - 'page.reload', undefined /* value */, reason /* label */); - } - // If it's a network failure we enable the light overlay. - overlay.show(isNetworkFailure); +/** + * Shows the page reload overlay which will do the conference reload after + * the given amount of time. + * + * @param {number} timeoutSeconds how many seconds before the conference + * reload will happen. + * @param {boolean} isNetworkFailure true indicates that it's + * caused by network related failure or false when it's + * the infrastructure. + * @param {string} reason a label string identifying the reason for the page + * reload which will be included in details of the log event + */ +export function show(timeoutSeconds, isNetworkFailure, reason) { + let title; + let message; + let buttonHtml; + let isLightOverlay; + + if (isNetworkFailure) { + title = "dialog.conferenceDisconnectTitle"; + message = "dialog.conferenceDisconnectMsg"; + buttonHtml + = ``; + isLightOverlay = true; } -}; + else { + title = "dialog.conferenceReloadTitle"; + message = "dialog.conferenceReloadMsg"; + buttonHtml = ""; + isLightOverlay = false; + } + + if (!overlay) { + overlay = new PageReloadOverlayImpl(timeoutSeconds, + title, + message, + buttonHtml, + isLightOverlay); + } + // Log the page reload event + if (!this.isVisible()) { + // FIXME (CallStats - issue) this event will not make it to + // the CallStats, because the log queue is not flushed, before + // "fabric terminated" is sent to the backed + APP.conference.logEvent( + 'page.reload', undefined /* value */, reason /* label */); + } + overlay.show(); +}