From 74ddae4a6ab5a2a3f5ea6a81543016e8e9e19f06 Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Mon, 31 Jul 2017 11:36:41 -0700 Subject: [PATCH] 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';