From 74ddae4a6ab5a2a3f5ea6a81543016e8e9e19f06 Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Mon, 31 Jul 2017 11:36:41 -0700 Subject: [PATCH 1/2] feat(device-errors): move device error dialogs to notifications - Create a notification component for displaying a toggle. - Create an action for showing the component if allowed by the local storage setting and for saving the setting to local storage. - Remove all notifications having a timeout by default so the device error notification must be dismissed manually. - Split the camera and mic error dialog into two separate notifications. --- conference.js | 14 +- lang/main.json | 4 + modules/UI/UI.js | 166 ++++++-------- modules/UI/util/MessageHandler.js | 2 +- modules/devices/mediaDeviceHelper.js | 13 +- package.json | 1 + react/features/notifications/actionTypes.js | 2 +- react/features/notifications/actions.js | 35 ++- .../NotificationWithToggle.native.js | 0 .../components/NotificationWithToggle.web.js | 210 ++++++++++++++++++ .../components/NotificationsContainer.web.js | 15 +- .../notifications/components/index.js | 1 + 12 files changed, 335 insertions(+), 128 deletions(-) create mode 100644 react/features/notifications/components/NotificationWithToggle.native.js create mode 100644 react/features/notifications/components/NotificationWithToggle.web.js diff --git a/conference.js b/conference.js index 508c6a36a..ef47664ec 100644 --- a/conference.js +++ b/conference.js @@ -625,13 +625,13 @@ export default { // If both requests for 'audio' + 'video' and 'audio' // only failed, we assume that there are some problems // with user's microphone and show corresponding dialog. - APP.UI.showDeviceErrorDialog( - audioOnlyError, videoOnlyError); + APP.UI.showMicErrorNotification(audioOnlyError); + APP.UI.showCameraErrorNotification(videoOnlyError); } else { // If request for 'audio' + 'video' failed, but request // for 'audio' only was OK, we assume that we had // problems with camera and show corresponding dialog. - APP.UI.showDeviceErrorDialog(null, audioAndVideoError); + APP.UI.showCameraErrorNotification(audioAndVideoError); } } @@ -770,7 +770,7 @@ export default { const maybeShowErrorDialog = (error) => { if (showUI) { - APP.UI.showDeviceErrorDialog(error, null); + APP.UI.showMicErrorNotification(error); } }; @@ -830,7 +830,7 @@ export default { const maybeShowErrorDialog = (error) => { if (showUI) { - APP.UI.showDeviceErrorDialog(null, error); + APP.UI.showCameraErrorNotification(error); } }; @@ -2028,7 +2028,7 @@ export default { APP.settings.setCameraDeviceId(cameraDeviceId, true); }) .catch((err) => { - APP.UI.showDeviceErrorDialog(null, err); + APP.UI.showCameraErrorNotification(err); }); } ); @@ -2049,7 +2049,7 @@ export default { APP.settings.setMicDeviceId(micDeviceId, true); }) .catch((err) => { - APP.UI.showDeviceErrorDialog(err, null); + APP.UI.showMicErrorNotification(err); }); } ); diff --git a/lang/main.json b/lang/main.json index a5e1447f2..c59901a53 100644 --- a/lang/main.json +++ b/lang/main.json @@ -474,5 +474,9 @@ "retry": "Try again", "support": "Support", "supportMsg": "If this keeps happening, reach out to" + }, + "deviceError": { + "cameraPermission": "Error obtaining camera permission", + "microphonePermission": "Error obtaining microphone permission" } } diff --git a/modules/UI/UI.js b/modules/UI/UI.js index 39e3fb3a0..ca1703f50 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -39,6 +39,9 @@ import { showDialOutButton, showToolbox } from '../../react/features/toolbox'; +import { + maybeShowNotificationWithDoNotDisplay +} from '../../react/features/notifications'; var EventEmitter = require("events"); UI.messageHandler = messageHandler; @@ -64,8 +67,6 @@ let sharedVideoManager; let followMeHandler; -let deviceErrorDialog; - const TrackErrors = JitsiMeetJS.errors.track; const JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP = { @@ -1189,115 +1190,74 @@ UI.showExtensionInlineInstallationDialog = function (callback) { }); }; +/** + * Shows a notifications about the passed in microphone error. + * + * @param {JitsiTrackError} micError - An error object related to using or + * acquiring an audio stream. + * @returns {void} + */ +UI.showMicErrorNotification = function (micError) { + if (!micError) { + return; + } + + const { message, name } = micError; + + const persistenceKey = `doNotShowErrorAgain-mic-${name}`; + + const micJitsiTrackErrorMsg + = JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.microphone[name]; + const micErrorMsg = micJitsiTrackErrorMsg + || JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.microphone[TrackErrors.GENERAL]; + const additionalMicErrorMsg = micJitsiTrackErrorMsg ? null : message; + + APP.store.dispatch(maybeShowNotificationWithDoNotDisplay( + persistenceKey, + { + additionalMessage: additionalMicErrorMsg, + messageKey: micErrorMsg, + showToggle: Boolean(micJitsiTrackErrorMsg), + subtitleKey: 'dialog.micErrorPresent', + titleKey: name === TrackErrors.PERMISSION_DENIED + ? 'deviceError.microphonePermission' : 'dialog.error', + toggleLabelKey: 'dialog.doNotShowWarningAgain' + })); +}; /** - * Shows dialog with combined information about camera and microphone errors. - * @param {JitsiTrackError} micError - * @param {JitsiTrackError} cameraError + * Shows a notifications about the passed in camera error. + * + * @param {JitsiTrackError} cameraError - An error object related to using or + * acquiring a video stream. + * @returns {void} */ -UI.showDeviceErrorDialog = function (micError, cameraError) { - let dontShowAgain = { - id: "doNotShowWarningAgain", - localStorageKey: "doNotShowErrorAgain", - textKey: "dialog.doNotShowWarningAgain" - }; - let isMicJitsiTrackErrorAndHasName = micError && micError.name && - micError instanceof JitsiMeetJS.errorTypes.JitsiTrackError; - let isCameraJitsiTrackErrorAndHasName = cameraError && cameraError.name && - cameraError instanceof JitsiMeetJS.errorTypes.JitsiTrackError; - let showDoNotShowWarning = false; - - if (micError && cameraError && isMicJitsiTrackErrorAndHasName && - isCameraJitsiTrackErrorAndHasName) { - showDoNotShowWarning = true; - } else if (micError && isMicJitsiTrackErrorAndHasName && !cameraError) { - showDoNotShowWarning = true; - } else if (cameraError && isCameraJitsiTrackErrorAndHasName && !micError) { - showDoNotShowWarning = true; +UI.showCameraErrorNotification = function (cameraError) { + if (!cameraError) { + return; } - if (micError) { - dontShowAgain.localStorageKey += "-mic-" + micError.name; - } + const { message, name } = cameraError; - if (cameraError) { - dontShowAgain.localStorageKey += "-camera-" + cameraError.name; - } + const persistenceKey = `doNotShowErrorAgain-camera-${name}`; - let cameraJitsiTrackErrorMsg = cameraError - ? JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.camera[cameraError.name] - : undefined; - let micJitsiTrackErrorMsg = micError - ? JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.microphone[micError.name] - : undefined; - let cameraErrorMsg = cameraError - ? cameraJitsiTrackErrorMsg || - JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.camera[TrackErrors.GENERAL] - : ""; - let micErrorMsg = micError - ? micJitsiTrackErrorMsg || - JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.microphone[TrackErrors.GENERAL] - : ""; - let additionalCameraErrorMsg = !cameraJitsiTrackErrorMsg && cameraError && - cameraError.message - ? `
${cameraError.message}
` - : ``; - let additionalMicErrorMsg = !micJitsiTrackErrorMsg && micError && - micError.message - ? `
${micError.message}
` - : ``; - let message = ''; + const cameraJitsiTrackErrorMsg = + JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.camera[name]; + const cameraErrorMsg = cameraJitsiTrackErrorMsg + || JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP.camera[TrackErrors.GENERAL]; + const additionalCameraErrorMsg = cameraJitsiTrackErrorMsg ? null : message; - if (micError) { - message = ` - ${message} -

-

- ${additionalMicErrorMsg}`; - } - - if (cameraError) { - message = ` - ${message} -

-

- ${additionalCameraErrorMsg}`; - } - - // To make sure we don't have multiple error dialogs open at the same time, - // we will just close the previous one if we are going to show a new one. - deviceErrorDialog && deviceErrorDialog.close(); - - deviceErrorDialog = messageHandler.openDialog( - getTitleKey(), - message, - false, - {Ok: true}, - function () {}, - null, - function () { - // Reset dialog reference to null to avoid memory leaks when - // user closed the dialog manually. - deviceErrorDialog = null; - }, - showDoNotShowWarning ? dontShowAgain : undefined - ); - - function getTitleKey() { - let title = "dialog.error"; - - if (micError && micError.name === TrackErrors.PERMISSION_DENIED) { - if (!cameraError - || cameraError.name === TrackErrors.PERMISSION_DENIED) { - title = "dialog.permissionDenied"; - } - } else if (cameraError - && cameraError.name === TrackErrors.PERMISSION_DENIED) { - title = "dialog.permissionDenied"; - } - - return title; - } + APP.store.dispatch(maybeShowNotificationWithDoNotDisplay( + persistenceKey, + { + additionalMessage: additionalCameraErrorMsg, + messageKey: cameraErrorMsg, + showToggle: Boolean(cameraJitsiTrackErrorMsg), + subtitleKey: 'dialog.cameraErrorPresent', + titleKey: name === TrackErrors.PERMISSION_DENIED + ? 'deviceError.cameraPermission' : 'dialog.error', + toggleLabelKey: 'dialog.doNotShowWarningAgain' + })); }; /** diff --git a/modules/UI/util/MessageHandler.js b/modules/UI/util/MessageHandler.js index 57ca94cec..7ff527d2c 100644 --- a/modules/UI/util/MessageHandler.js +++ b/modules/UI/util/MessageHandler.js @@ -455,7 +455,7 @@ var messageHandler = { * @param optional configurations for the notification (e.g. timeout) */ participantNotification: function(displayName, displayNameKey, cls, - messageKey, messageArguments, timeout) { + messageKey, messageArguments, timeout = 2500) { // If we're in ringing state we skip all notifications. if (!notificationsEnabled || APP.UI.isOverlayVisible()) { return; diff --git a/modules/devices/mediaDeviceHelper.js b/modules/devices/mediaDeviceHelper.js index cf03c107b..1c83bbef5 100644 --- a/modules/devices/mediaDeviceHelper.js +++ b/modules/devices/mediaDeviceHelper.js @@ -192,9 +192,12 @@ export default { createVideoTrack(false).then(([stream]) => stream) ])) .then(tracks => { - if (audioTrackError || videoTrackError) { - APP.UI.showDeviceErrorDialog( - audioTrackError, videoTrackError); + if (audioTrackError) { + APP.UI.showMicErrorNotification(audioTrackError); + } + + if (videoTrackError) { + APP.UI.showCameraErrorNotification(videoTrackError); } return tracks.filter(t => typeof t !== 'undefined'); @@ -215,7 +218,7 @@ export default { }) .catch(err => { audioTrackError = err; - showError && APP.UI.showDeviceErrorDialog(err, null); + showError && APP.UI.showMicErrorNotification(err); return []; }); } @@ -228,7 +231,7 @@ export default { }) .catch(err => { videoTrackError = err; - showError && APP.UI.showDeviceErrorDialog(null, err); + showError && APP.UI.showCameraErrorNotification(err); return []; }); } diff --git a/package.json b/package.json index 76929cc99..eb3645aa0 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "@atlaskit/multi-select": "6.2.0", "@atlaskit/spinner": "2.2.3", "@atlaskit/tabs": "2.0.0", + "@atlaskit/toggle": "2.0.4", "@atlassian/aui": "6.0.6", "async": "0.9.0", "autosize": "1.18.13", diff --git a/react/features/notifications/actionTypes.js b/react/features/notifications/actionTypes.js index 4b8593a6d..762a20aec 100644 --- a/react/features/notifications/actionTypes.js +++ b/react/features/notifications/actionTypes.js @@ -4,7 +4,7 @@ * * { * type: HIDE_NOTIFICATION, - * uid: string + * uid: number * } */ export const HIDE_NOTIFICATION = Symbol('HIDE_NOTIFICATION'); diff --git a/react/features/notifications/actions.js b/react/features/notifications/actions.js index ce56b432d..fc7c1a1ad 100644 --- a/react/features/notifications/actions.js +++ b/react/features/notifications/actions.js @@ -1,7 +1,10 @@ +import jitsiLocalStorage from '../../../modules/util/JitsiLocalStorage'; + import { HIDE_NOTIFICATION, SHOW_NOTIFICATION } from './actionTypes'; +import { NotificationWithToggle } from './components'; /** * Removes the notification with the passed in id. @@ -10,7 +13,7 @@ import { * removed. * @returns {{ * type: HIDE_NOTIFICATION, - * uid: string + * uid: number * }} */ export function hideNotification(uid) { @@ -45,3 +48,33 @@ export function showNotification(component, props = {}, timeout) { uid: window.Date.now() }; } + +/** + * Displays a notification unless the passed in persistenceKey value exists in + * local storage and has been set to "true". + * + * @param {string} persistenceKey - The local storage key to look up for whether + * or not the notification should display. + * @param {Object} props - The props needed to show the notification component. + * @returns {Function} + */ +export function maybeShowNotificationWithDoNotDisplay(persistenceKey, props) { + return dispatch => { + if (jitsiLocalStorage.getItem(persistenceKey) === 'true') { + return; + } + + const newProps = Object.assign({}, props, { + onToggleSubmit: isToggled => { + jitsiLocalStorage.setItem(persistenceKey, isToggled); + } + }); + + dispatch({ + type: SHOW_NOTIFICATION, + component: NotificationWithToggle, + props: newProps, + uid: window.Date.now() + }); + }; +} diff --git a/react/features/notifications/components/NotificationWithToggle.native.js b/react/features/notifications/components/NotificationWithToggle.native.js new file mode 100644 index 000000000..e69de29bb diff --git a/react/features/notifications/components/NotificationWithToggle.web.js b/react/features/notifications/components/NotificationWithToggle.web.js new file mode 100644 index 000000000..3112af279 --- /dev/null +++ b/react/features/notifications/components/NotificationWithToggle.web.js @@ -0,0 +1,210 @@ +import Flag from '@atlaskit/flag'; +import WarningIcon from '@atlaskit/icon/glyph/warning'; +import { ToggleStateless } from '@atlaskit/toggle'; +import React, { Component } from 'react'; + +import { translate } from '../../base/i18n'; + +/** + * React {@code Component} for displaying a notification with a toggle element. + * + * @extends Component + */ +class NotificationWithToggle extends Component { + /** + * {@code NotificationWithToggle} component's property types. + * + * @static + */ + static propTypes = { + /** + * Any additional text to display at the end of the notification message + * body. + */ + additionalMessage: React.PropTypes.string, + + /** + * Whether or not the dismiss button should be displayed. This is passed + * in by {@code FlagGroup}. + */ + isDismissAllowed: React.PropTypes.bool, + + /** + * The translation key to be used as the main body of the notification. + */ + messageKey: React.PropTypes.string, + + /** + * Callback invoked when the user clicks to dismiss the notification. + * This is passed in by {@code FlagGroup}. + */ + onDismissed: React.PropTypes.func, + + /** + * Optional callback to invoke when the notification is dismissed. The + * current value of the toggle element will be passed in. + */ + onToggleSubmit: React.PropTypes.func, + + /** + * Whether or not the toggle element should be displayed. + */ + showToggle: React.PropTypes.bool, + + /** + * Translation key for a message to display at the top of the + * notification body. + */ + subtitleKey: React.PropTypes.string, + + /** + * Invoked to obtain translated strings. + */ + t: React.PropTypes.func, + + /** + * The translation key to be used as the title of the notification. + */ + titleKey: React.PropTypes.string, + + /* + * The translation key to be used as a label describing what setting the + * toggle will change. + */ + toggleLabelKey: React.PropTypes.string, + + /** + * The unique identifier for the notification. Passed back by the + * {@code Flag} component in the onDismissed callback. + */ + uid: React.PropTypes.number + }; + + /** + * Initializes a new {@code NotificationWithToggle} instance. + * + * @param {Object} props - The read-only properties with which the new + * instance is to be initialized. + */ + constructor(props) { + super(props); + + this.state = { + /** + * Whether or not the toggle element is active/checked/selected. + * + * @type {boolean} + */ + isToggleChecked: false + }; + + // Bind event handlers so they are only bound once for every instance. + this._onDismissed = this._onDismissed.bind(this); + this._onToggleChange = this._onToggleChange.bind(this); + } + + /** + * Implements React's {@link Component#render()}. + * + * @inheritdoc + * @returns {ReactElement} + */ + render() { + const { + isDismissAllowed, + t, + titleKey, + uid + } = this.props; + + return ( + + ) } + id = { uid } + isDismissAllowed = { isDismissAllowed } + onDismissed = { this._onDismissed } + title = { t(titleKey) } /> + ); + } + + /** + * Calls back into {@code FlagGroup} to dismiss the notification. Optionally + * will execute a passed in onToggleSubmit callback with the current state + * of the toggle element. + * + * @private + * @returns {void} + */ + _onDismissed() { + const { onDismissed, onToggleSubmit, showToggle, uid } = this.props; + + if (showToggle && onToggleSubmit) { + onToggleSubmit(this.state.isToggleChecked); + } + + onDismissed(uid); + } + + /** + * Updates the current known state of the toggle selection. + * + * @param {Object} event - The DOM event from changing the toggle selection. + * @private + * @returns {void} + */ + _onToggleChange(event) { + this.setState({ + isToggleChecked: event.target.checked + }); + } + + /** + * Creates a React Element for displaying the notification message as well + * as a toggle. + * + * @private + * @returns {ReactElement} + */ + _renderDescription() { + const { + additionalMessage, + messageKey, + showToggle, + subtitleKey, + t, + toggleLabelKey + } = this.props; + + return ( +
+
{ t(subtitleKey) }
+ { messageKey ?
{ t(messageKey) }
: null } + { additionalMessage ?
{ additionalMessage }
+ : null } + { showToggle + ?
+ { t(toggleLabelKey) } + +
+ : null } +
+ ); + } +} + +export default translate(NotificationWithToggle); diff --git a/react/features/notifications/components/NotificationsContainer.web.js b/react/features/notifications/components/NotificationsContainer.web.js index 32447f00e..9c49dcaf6 100644 --- a/react/features/notifications/components/NotificationsContainer.web.js +++ b/react/features/notifications/components/NotificationsContainer.web.js @@ -4,14 +4,6 @@ import { connect } from 'react-redux'; import { hideNotification } from '../actions'; -/** - * The duration for which a notification should be displayed before being - * dismissed automatically. - * - * @type {number} - */ -const DEFAULT_NOTIFICATION_TIMEOUT = 2500; - /** * Implements a React {@link Component} which displays notifications and handles * automatic dismissmal after a notification is shown for a defined timeout @@ -74,8 +66,11 @@ class NotificationsContainer extends Component { const { timeout, uid } = notification; this._notificationDismissTimeout = setTimeout(() => { - this._onDismissed(uid); - }, timeout || DEFAULT_NOTIFICATION_TIMEOUT); + // Perform a no-op if a timeout is not specified. + if (Number.isInteger(timeout)) { + this._onDismissed(uid); + } + }, timeout); } } diff --git a/react/features/notifications/components/index.js b/react/features/notifications/components/index.js index 642f2f283..eb71c3bc7 100644 --- a/react/features/notifications/components/index.js +++ b/react/features/notifications/components/index.js @@ -1,2 +1,3 @@ export { default as Notification } from './Notification'; export { default as NotificationsContainer } from './NotificationsContainer'; +export { default as NotificationWithToggle } from './NotificationWithToggle'; From cd66a7fcb7057ba8f4678d4ed4add63924c1695c Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Wed, 2 Aug 2017 11:15:55 -0700 Subject: [PATCH 2/2] ref(notifications): bring hiding of notifications into redux --- modules/UI/UI.js | 29 +---- modules/UI/recording/Recording.js | 3 +- modules/UI/util/MessageHandler.js | 23 ---- .../conference/components/Conference.web.js | 2 +- react/features/notifications/actionTypes.js | 15 ++- react/features/notifications/actions.js | 17 +++ .../components/NotificationsContainer.web.js | 105 +++++++++++++----- react/features/notifications/reducer.js | 38 +++++-- .../overlay/components/OverlayContainer.js | 18 --- 9 files changed, 138 insertions(+), 112 deletions(-) diff --git a/modules/UI/UI.js b/modules/UI/UI.js index ca1703f50..1aa3f4648 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -40,7 +40,8 @@ import { showToolbox } from '../../react/features/toolbox'; import { - maybeShowNotificationWithDoNotDisplay + maybeShowNotificationWithDoNotDisplay, + setNotificationsEnabled } from '../../react/features/notifications'; var EventEmitter = require("events"); @@ -51,17 +52,6 @@ import FollowMe from "../FollowMe"; var eventEmitter = new EventEmitter(); UI.eventEmitter = eventEmitter; -/** - * Whether an overlay is visible or not. - * - * FIXME: This is temporary solution. Don't use this variable! - * Should be removed when all the code is move to react. - * - * @type {boolean} - * @public - */ -UI.overlayVisible = false; - let etherpadManager; let sharedVideoManager; @@ -335,7 +325,7 @@ UI.start = function () { $("body").addClass("filmstrip-only"); UI.showToolbar(); Filmstrip.setFilmstripOnly(); - messageHandler.enableNotifications(false); + APP.store.dispatch(setNotificationsEnabled(false)); JitsiPopover.enabled = false; } @@ -1307,19 +1297,6 @@ UI.onSharedVideoStop = function (id, attributes) { sharedVideoManager.onSharedVideoStop(id, attributes); }; -/** - * Indicates if any the "top" overlays are currently visible. The check includes - * the call/ring overlay, the suspended overlay, the GUM permissions overlay, - * and the page-reload overlay. - * - * @returns {*|boolean} {true} if an overlay is visible; {false}, otherwise - */ -UI.isOverlayVisible = function () { - return ( - this.overlayVisible - || APP.store.getState()['features/jwt'].callOverlayVisible); -}; - /** * Handles user's features changes. */ diff --git a/modules/UI/recording/Recording.js b/modules/UI/recording/Recording.js index 679e06d2a..78f99d216 100644 --- a/modules/UI/recording/Recording.js +++ b/modules/UI/recording/Recording.js @@ -22,6 +22,7 @@ import VideoLayout from '../videolayout/VideoLayout'; import Feedback from '../feedback/Feedback.js'; import { setToolboxEnabled } from '../../../react/features/toolbox'; +import { setNotificationsEnabled } from '../../../react/features/notifications'; /** * The dialog for user input. @@ -309,7 +310,7 @@ var Recording = { VideoLayout.setLocalVideoVisible(false); Feedback.enableFeedback(false); APP.store.dispatch(setToolboxEnabled(false)); - APP.UI.messageHandler.enableNotifications(false); + APP.store.dispatch(setNotificationsEnabled(false)); APP.UI.messageHandler.enablePopups(false); } diff --git a/modules/UI/util/MessageHandler.js b/modules/UI/util/MessageHandler.js index 7ff527d2c..0392d6f01 100644 --- a/modules/UI/util/MessageHandler.js +++ b/modules/UI/util/MessageHandler.js @@ -8,12 +8,6 @@ import { showNotification } from '../../../react/features/notifications'; -/** - * Flag for enable/disable of the notifications. - * @type {boolean} - */ -let notificationsEnabled = true; - /** * Flag for enabling/disabling popups. * @type {boolean} @@ -456,11 +450,6 @@ var messageHandler = { */ participantNotification: function(displayName, displayNameKey, cls, messageKey, messageArguments, timeout = 2500) { - // If we're in ringing state we skip all notifications. - if (!notificationsEnabled || APP.UI.isOverlayVisible()) { - return; - } - APP.store.dispatch( showNotification( Notification, @@ -485,22 +474,10 @@ var messageHandler = { * @returns {void} */ notify: function(titleKey, messageKey, messageArguments) { - - // If we're in ringing state we skip all notifications. - if(!notificationsEnabled || APP.UI.isOverlayVisible()) - return; - this.participantNotification( null, titleKey, null, messageKey, messageArguments); }, - /** - * Enables / disables notifications. - */ - enableNotifications: function (enable) { - notificationsEnabled = enable; - }, - enablePopups: function (enable) { popupEnabled = enable; }, diff --git a/react/features/conference/components/Conference.web.js b/react/features/conference/components/Conference.web.js index a4ead6213..55ba31cac 100644 --- a/react/features/conference/components/Conference.web.js +++ b/react/features/conference/components/Conference.web.js @@ -79,7 +79,7 @@ class Conference extends Component { { filmStripOnly ? null : } - { filmStripOnly ? null : } + {/* diff --git a/react/features/notifications/actionTypes.js b/react/features/notifications/actionTypes.js index 762a20aec..e9860c739 100644 --- a/react/features/notifications/actionTypes.js +++ b/react/features/notifications/actionTypes.js @@ -1,4 +1,4 @@ -/* +/** * The type of (redux) action which signals that a specific notification should * not be displayed anymore. * @@ -9,7 +9,7 @@ */ export const HIDE_NOTIFICATION = Symbol('HIDE_NOTIFICATION'); -/* +/** * The type of (redux) action which signals that a notification component should * be displayed. * @@ -22,3 +22,14 @@ export const HIDE_NOTIFICATION = Symbol('HIDE_NOTIFICATION'); * } */ export const SHOW_NOTIFICATION = Symbol('SHOW_NOTIFICATION'); + +/** + * The type of (redux) action which signals that notifications should not + * display. + * + * { + * type: SET_NOTIFICATIONS_ENABLED, + * enabled: Boolean + * } + */ +export const SET_NOTIFICATIONS_ENABLED = Symbol('SET_NOTIFICATIONS_ENABLED'); diff --git a/react/features/notifications/actions.js b/react/features/notifications/actions.js index fc7c1a1ad..bea4ee066 100644 --- a/react/features/notifications/actions.js +++ b/react/features/notifications/actions.js @@ -2,6 +2,7 @@ import jitsiLocalStorage from '../../../modules/util/JitsiLocalStorage'; import { HIDE_NOTIFICATION, + SET_NOTIFICATIONS_ENABLED, SHOW_NOTIFICATION } from './actionTypes'; import { NotificationWithToggle } from './components'; @@ -23,6 +24,22 @@ export function hideNotification(uid) { }; } +/** + * Stops notifications from being displayed. + * + * @param {boolean} enabled - Whether or not notifications should display. + * @returns {{ + * type: SET_NOTIFICATIONS_ENABLED, + * enabled: boolean + * }} + */ +export function setNotificationsEnabled(enabled) { + return { + type: SET_NOTIFICATIONS_ENABLED, + enabled + }; +} + /** * Queues a notification for display. * diff --git a/react/features/notifications/components/NotificationsContainer.web.js b/react/features/notifications/components/NotificationsContainer.web.js index 9c49dcaf6..2e7e1f37b 100644 --- a/react/features/notifications/components/NotificationsContainer.web.js +++ b/react/features/notifications/components/NotificationsContainer.web.js @@ -24,6 +24,12 @@ class NotificationsContainer extends Component { */ _notifications: React.PropTypes.array, + /** + * Whether or not notifications should be displayed at all. If not, + * notifications will be dismissed immediately. + */ + _showNotifications: React.PropTypes.bool, + /** * Invoked to update the redux store in order to remove notifications. */ @@ -59,18 +65,27 @@ class NotificationsContainer extends Component { * returns {void} */ componentDidUpdate() { - const { _notifications } = this.props; + const { _notifications, _showNotifications } = this.props; - if (_notifications.length && !this._notificationDismissTimeout) { + if (_notifications.length) { const notification = _notifications[0]; - const { timeout, uid } = notification; - this._notificationDismissTimeout = setTimeout(() => { - // Perform a no-op if a timeout is not specified. - if (Number.isInteger(timeout)) { - this._onDismissed(uid); - } - }, timeout); + if (!_showNotifications) { + this._onDismissed(notification.uid); + } else if (this._notificationDismissTimeout) { + + // No-op because there should already be a notification that + // is waiting for dismissal. + } else { + const { timeout, uid } = notification; + + this._notificationDismissTimeout = setTimeout(() => { + // Perform a no-op if a timeout is not specified. + if (Number.isInteger(timeout)) { + this._onDismissed(uid); + } + }, timeout); + } } } @@ -91,28 +106,9 @@ class NotificationsContainer extends Component { * @returns {ReactElement} */ render() { - const { _notifications } = this.props; - - const flags = _notifications.map(notification => { - const Notification = notification.component; - const { props, uid } = notification; - - // The id attribute is necessary as {@code FlagGroup} looks for - // either id or key to set a key on notifications, but accessing - // props.key will cause React to print an error. - return ( - - - ); - }); - return ( - { flags } + { this._renderFlags() } ); } @@ -131,6 +127,38 @@ class NotificationsContainer extends Component { this.props.dispatch(hideNotification(flagUid)); } + + /** + * Renders notifications to display as ReactElements. An empty array will + * be returned if notifications are disabled. + * + * @private + * @returns {ReactElement[]} + */ + _renderFlags() { + const { _notifications, _showNotifications } = this.props; + + if (!_showNotifications) { + return []; + } + + return _notifications.map(notification => { + const Notification = notification.component; + const { props, uid } = notification; + + // The id attribute is necessary as {@code FlagGroup} looks for + // either id or key to set a key on notifications, but accessing + // props.key will cause React to print an error. + return ( + + + ); + }); + } } /** @@ -144,8 +172,25 @@ class NotificationsContainer extends Component { * }} */ function _mapStateToProps(state) { + // TODO: Per existing behavior, notifications should not display when an + // overlay is visible. This logic for checking overlay display can likely be + // simplified. + const { + connectionEstablished, + haveToReload, + isMediaPermissionPromptVisible, + suspendDetected + } = state['features/overlay']; + const isAnyOverlayVisible = (connectionEstablished && haveToReload) + || isMediaPermissionPromptVisible + || suspendDetected + || state['features/jwt'].callOverlayVisible; + + const { enabled, notifications } = state['features/notifications']; + return { - _notifications: state['features/notifications'] + _notifications: notifications, + _showNotifications: enabled && !isAnyOverlayVisible }; } diff --git a/react/features/notifications/reducer.js b/react/features/notifications/reducer.js index a510806c6..105d4eb18 100644 --- a/react/features/notifications/reducer.js +++ b/react/features/notifications/reducer.js @@ -2,6 +2,7 @@ import { ReducerRegistry } from '../base/redux'; import { HIDE_NOTIFICATION, + SET_NOTIFICATIONS_ENABLED, SHOW_NOTIFICATION } from './actionTypes'; @@ -10,7 +11,10 @@ import { * * @type {array} */ -const DEFAULT_STATE = []; +const DEFAULT_STATE = { + enabled: true, + notifications: [] +}; /** * Reduces redux actions which affect the display of notifications. @@ -24,19 +28,31 @@ ReducerRegistry.register('features/notifications', (state = DEFAULT_STATE, action) => { switch (action.type) { case HIDE_NOTIFICATION: - return state.filter( - notification => notification.uid !== action.uid); + return { + ...state, + notifications: state.notifications.filter( + notification => notification.uid !== action.uid) + }; + + case SET_NOTIFICATIONS_ENABLED: + return { + ...state, + enabled: action.enabled + }; case SHOW_NOTIFICATION: - return [ + return { ...state, - { - component: action.component, - props: action.props, - timeout: action.timeout, - uid: action.uid - } - ]; + notifications: [ + ...state.notifications, + { + component: action.component, + props: action.props, + timeout: action.timeout, + uid: action.uid + } + ] + }; } return state; diff --git a/react/features/overlay/components/OverlayContainer.js b/react/features/overlay/components/OverlayContainer.js index c12638389..b9381da55 100644 --- a/react/features/overlay/components/OverlayContainer.js +++ b/react/features/overlay/components/OverlayContainer.js @@ -11,7 +11,6 @@ import UserMediaPermissionsFilmstripOnlyOverlay from './UserMediaPermissionsFilmstripOnlyOverlay'; import UserMediaPermissionsOverlay from './UserMediaPermissionsOverlay'; -declare var APP: Object; declare var interfaceConfig: Object; /** @@ -133,23 +132,6 @@ class OverlayContainer extends Component { }; } - /** - * React Component method that executes once component is updated. - * - * @inheritdoc - * @returns {void} - * @protected - */ - componentDidUpdate() { - if (typeof APP === 'object') { - APP.UI.overlayVisible - = (this.props._connectionEstablished - && this.props._haveToReload) - || this.props._suspendDetected - || this.props._isMediaPermissionPromptVisible; - } - } - /** * Implements React's {@link Component#render()}. *