From 28013f6ffa24632ebfa405ea3f1b3a1a12eff4c0 Mon Sep 17 00:00:00 2001 From: virtuacoplenny Date: Tue, 19 Dec 2017 15:11:54 -0800 Subject: [PATCH] ref(avatars): remove Avatar.js (#2289) * ref(avatars): remove Avatar.js - Rely on redux getting updated with new participant state and any calls to getAvatarURL passing in the redux participant state. This way the state within Avatar.js can be removed. - Clean up methods on UI.js. Because all state is in the store, separate methods for updating the avatar aren't as necessary. Instead centralize accessing of the avatar for components outside of redux and centralize the call to update avatars for non-react components. - Controversial: cache a participant's avatarURL on the participant state. Currently the participant's avatarURL that is generated without jwt (which sets the avatarURL directly) is not cached. Without cache, there can be many redundant calls to APP.API.notifyAvatarChanged. * Leverage middleware timing to diff avatars One alternative implementation is to leverage middleware's ability to intercept updates before and after redux has upated and then compare avatarURLs. * kill UI.getAvatarUrl * profile button sets its own avatar url (solves update timing) * remove calls to updating avatar outside of middleware * update UI.js doc * remove left over logic from initial implementation * try to move local user fallback into selector func * default to id 'local' in selector --- conference.js | 9 +- modules/UI/UI.js | 58 ++-------- modules/UI/avatar/Avatar.js | 109 ------------------ modules/UI/side_pannels/profile/Profile.js | 8 -- modules/UI/videolayout/LargeVideoManager.js | 8 +- modules/UI/videolayout/LocalVideo.js | 9 ++ modules/UI/videolayout/SmallVideo.js | 7 +- react/features/base/participants/functions.js | 28 ++++- .../features/base/participants/middleware.js | 41 ++++++- .../toolbox/components/ProfileButton.web.js | 21 +++- 10 files changed, 114 insertions(+), 184 deletions(-) delete mode 100644 modules/UI/avatar/Avatar.js diff --git a/conference.js b/conference.js index cb48ec56b0..081d2e094e 100644 --- a/conference.js +++ b/conference.js @@ -70,6 +70,7 @@ import { } from './react/features/base/media'; import { dominantSpeakerChanged, + getAvatarURLByParticipantId, getLocalParticipant, getParticipantById, localParticipantConnectionStatusChanged, @@ -2092,7 +2093,6 @@ export default { id: from, avatarURL: data.value })); - APP.UI.setUserAvatarUrl(from, data.value); }); room.addCommandListener(this.commands.defaults.AVATAR_ID, @@ -2102,7 +2102,6 @@ export default { id: from, avatarID: data.value })); - APP.UI.setUserAvatarID(from, data.value); }); APP.UI.addListener(UIEvents.NICKNAME_CHANGED, @@ -2414,7 +2413,8 @@ export default { formattedDisplayName: appendSuffix( displayName, interfaceConfig.DEFAULT_LOCAL_DISPLAY_NAME), - avatarURL: APP.UI.getAvatarUrl() + avatarURL: getAvatarURLByParticipantId( + APP.store.getState(), this._room.myUserId()) } ); APP.UI.markVideoInterrupted(false); @@ -2704,7 +2704,7 @@ export default { APP.store.dispatch(participantUpdated({ id: localId, local: true, - formattedEmail + email: formattedEmail })); APP.settings.setEmail(formattedEmail); @@ -2732,7 +2732,6 @@ export default { })); APP.settings.setAvatarUrl(url); - APP.UI.setUserAvatarUrl(id, url); sendData(commands.AVATAR_URL, url); }, diff --git a/modules/UI/UI.js b/modules/UI/UI.js index 400f66e236..62eabaf2e6 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -6,7 +6,6 @@ const UI = {}; import Chat from './side_pannels/chat/Chat'; import SidePanels from './side_pannels/SidePanels'; -import Avatar from './avatar/Avatar'; import SideContainerToggler from './side_pannels/SideContainerToggler'; import messageHandler from './util/MessageHandler'; import UIUtil from './util/UIUtil'; @@ -255,7 +254,7 @@ UI.setLocalRaisedHandStatus */ UI.initConference = function() { const { dispatch, getState } = APP.store; - const { avatarID, email, id, name } = getLocalParticipant(getState); + const { email, id, name } = getLocalParticipant(getState); // Update default button states before showing the toolbar // if local role changes buttons state will be again updated. @@ -272,8 +271,6 @@ UI.initConference = function() { // Make sure we configure our avatar id, before creating avatar for us if (email) { UI.setUserEmail(id, email); - } else { - UI.setUserAvatarID(id, avatarID); } dispatch(checkAutoEnableDesktopSharing()); @@ -789,65 +786,26 @@ UI.showToolbar = timeout => APP.store.dispatch(showToolbox(timeout)); // Used by torture. UI.dockToolbar = dock => APP.store.dispatch(dockToolbox(dock)); -/** - * Updates the avatar for participant. - * @param {string} id user id - * @param {string} avatarUrl the URL for the avatar - */ -function changeAvatar(id, avatarUrl) { - VideoLayout.changeUserAvatar(id, avatarUrl); - if (APP.conference.isLocalId(id)) { - Profile.changeAvatar(avatarUrl); - } -} - -/** - * Returns the avatar URL for a given user. - * - * @param {string} id - The id of the user. - * @returns {string} The avatar URL. - */ -UI.getAvatarUrl = function(id) { - return Avatar.getAvatarUrl(id); -}; - /** * Update user email. * @param {string} id user id * @param {string} email user email */ UI.setUserEmail = function(id, email) { - // update avatar - Avatar.setUserEmail(id, email); - - changeAvatar(id, Avatar.getAvatarUrl(id)); if (APP.conference.isLocalId(id)) { Profile.changeEmail(email); } }; /** - * Update user avtar id. - * @param {string} id user id - * @param {string} avatarId user's avatar id + * Updates the displayed avatar for participant. + * + * @param {string} id - User id whose avatar should be updated. + * @param {string} avatarURL - The URL to avatar image to display. + * @returns {void} */ -UI.setUserAvatarID = function(id, avatarId) { - // update avatar - Avatar.setUserAvatarID(id, avatarId); - - changeAvatar(id, Avatar.getAvatarUrl(id)); -}; - -/** - * Update user avatar URL. - * @param {string} id user id - * @param {string} url user avatar url - */ -UI.setUserAvatarUrl = function(id, url) { - // update avatar - Avatar.setUserAvatarUrl(id, url); - - changeAvatar(id, Avatar.getAvatarUrl(id)); +UI.refreshAvatarDisplay = function(id, avatarURL) { + VideoLayout.changeUserAvatar(id, avatarURL); }; /** diff --git a/modules/UI/avatar/Avatar.js b/modules/UI/avatar/Avatar.js deleted file mode 100644 index 0b164f309c..0000000000 --- a/modules/UI/avatar/Avatar.js +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Adorable Avatars service used at the end of this file is released under the - * terms of the MIT License. - * - * Copyright (c) 2014 Adorable IO LLC - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -/* global APP */ - -import { getAvatarURL } from '../../../react/features/base/participants'; - -const users = {}; - -export default { - /** - * Sets prop in users object. - * @param id {string} user id or undefined for the local user. - * @param prop {string} name of the prop - * @param val {string} value to be set - */ - _setUserProp(id, prop, val) { - // FIXME: Fixes the issue with not be able to return avatar for the - // local user when the conference has been left. Maybe there is beter - // way to solve it. - if (!id || APP.conference.isLocalId(id)) { - id = 'local';// eslint-disable-line no-param-reassign - } - if (!val || (users[id] && users[id][prop] === val)) { - return; - } - if (!users[id]) { - users[id] = {}; - } - users[id][prop] = val; - APP.API.notifyAvatarChanged( - id === 'local' ? APP.conference.getMyUserId() : id, - this.getAvatarUrl(id) - ); - }, - - /** - * Sets the user's avatar in the settings menu(if local user), contact list - * and thumbnail - * @param id id of the user - * @param email email or nickname to be used as a hash - */ - setUserEmail(id, email) { - this._setUserProp(id, 'email', email); - }, - - /** - * Sets the user's avatar in the settings menu(if local user), contact list - * and thumbnail - * @param id id of the user - * @param url the url for the avatar - */ - setUserAvatarUrl(id, url) { - this._setUserProp(id, 'avatarUrl', url); - }, - - /** - * Sets the user's avatar id. - * @param id id of the user - * @param avatarId an id to be used for the avatar - */ - setUserAvatarID(id, avatarId) { - this._setUserProp(id, 'avatarId', avatarId); - }, - - /** - * Returns the URL of the image for the avatar of a particular user, - * identified by its id. - * @param {string} userId user id - */ - getAvatarUrl(userId) { - let user; - - if (!userId || APP.conference.isLocalId(userId)) { - user = users.local; - // eslint-disable-next-line no-param-reassign - userId = APP.conference.getMyUserId(); - } else { - user = users[userId]; - } - - return getAvatarURL({ - avatarID: user ? user.avatarId : undefined, - avatarURL: user ? user.avatarUrl : undefined, - email: user ? user.email : undefined, - id: userId - }); - } -}; diff --git a/modules/UI/side_pannels/profile/Profile.js b/modules/UI/side_pannels/profile/Profile.js index b9b3bdd82e..2669b52940 100644 --- a/modules/UI/side_pannels/profile/Profile.js +++ b/modules/UI/side_pannels/profile/Profile.js @@ -142,14 +142,6 @@ export default { $('#setDisplayName').val(newDisplayName); }, - /** - * Change user avatar in the settings menu. - * @param {string} avatarUrl url of the new avatar - */ - changeAvatar(avatarUrl) { - $('#avatar').attr('src', avatarUrl); - }, - /** * Change the value of the field for the user email. * @param {string} email the new value that will be displayed in the field. diff --git a/modules/UI/videolayout/LargeVideoManager.js b/modules/UI/videolayout/LargeVideoManager.js index 97d0c3e458..9ddafa45ad 100644 --- a/modules/UI/videolayout/LargeVideoManager.js +++ b/modules/UI/videolayout/LargeVideoManager.js @@ -12,11 +12,12 @@ const logger = require('jitsi-meet-logger').getLogger(__filename); import { JitsiParticipantConnectionStatus } from '../../../react/features/base/lib-jitsi-meet'; +import { + getAvatarURLByParticipantId +} from '../../../react/features/base/participants'; import { updateKnownLargeVideoResolution } from '../../../react/features/large-video'; - -import Avatar from '../avatar/Avatar'; import { createDeferred } from '../../util/helpers'; import UIEvents from '../../../service/UI/UIEvents'; import UIUtil from '../util/UIUtil'; @@ -219,7 +220,8 @@ export default class LargeVideoManager { container.setStream(id, stream, videoType); // change the avatar url on large - this.updateAvatar(Avatar.getAvatarUrl(id)); + this.updateAvatar( + getAvatarURLByParticipantId(APP.store.getState(), id)); // If the user's connection is disrupted then the avatar will be // displayed in case we have no video image cached. That is if diff --git a/modules/UI/videolayout/LocalVideo.js b/modules/UI/videolayout/LocalVideo.js index 9fa4e1ad52..86f86b1463 100644 --- a/modules/UI/videolayout/LocalVideo.js +++ b/modules/UI/videolayout/LocalVideo.js @@ -7,6 +7,9 @@ import { Provider } from 'react-redux'; import { JitsiTrackEvents } from '../../../react/features/base/lib-jitsi-meet'; import { VideoTrack } from '../../../react/features/base/media'; +import { + getAvatarURLByParticipantId +} from '../../../react/features/base/participants'; /* eslint-enable no-unused-vars */ const logger = require('jitsi-meet-logger').getLogger(__filename); @@ -46,6 +49,12 @@ function LocalVideo(VideoLayout, emitter) { // Set default display name. this.setDisplayName(); + // Initialize the avatar display with an avatar url selected from the redux + // state. Redux stores the local user with a hardcoded participant id of + // 'local' if no id has been assigned yet. + this.avatarChanged( + getAvatarURLByParticipantId(APP.store.getState(), this.id)); + this.addAudioLevelIndicator(); this.updateIndicators(); diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index e20bb29988..2211cb18d7 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -11,7 +11,8 @@ import { i18next } from '../../../react/features/base/i18n'; import { AudioLevelIndicator } from '../../../react/features/audio-level-indicator'; import { - Avatar as AvatarDisplay + Avatar as AvatarDisplay, + getAvatarURLByParticipantId } from '../../../react/features/base/participants'; import { ConnectionIndicator @@ -28,7 +29,6 @@ import { const logger = require('jitsi-meet-logger').getLogger(__filename); -import Avatar from '../avatar/Avatar'; import UIUtil from '../util/UIUtil'; import UIEvents from '../../../service/UI/UIEvents'; @@ -590,7 +590,8 @@ SmallVideo.prototype.updateView = function() { if (!this.hasAvatar) { if (this.id) { // Init avatar - this.avatarChanged(Avatar.getAvatarUrl(this.id)); + this.avatarChanged( + getAvatarURLByParticipantId(APP.store.getState(), this.id)); } else { logger.error('Unable to init avatar - no id', this); diff --git a/react/features/base/participants/functions.js b/react/features/base/participants/functions.js index 3cf8ae1947..bf0a1fa08e 100644 --- a/react/features/base/participants/functions.js +++ b/react/features/base/participants/functions.js @@ -3,7 +3,10 @@ import md5 from 'js-md5'; import { toState } from '../redux'; -import { DEFAULT_AVATAR_RELATIVE_PATH } from './constants'; +import { + DEFAULT_AVATAR_RELATIVE_PATH, + LOCAL_PARTICIPANT_DEFAULT_ID +} from './constants'; declare var config: Object; declare var interfaceConfig: Object; @@ -74,6 +77,29 @@ export function getAvatarURL({ avatarID, avatarURL, email, id }: { return urlPrefix + md5.hex(key.trim().toLowerCase()) + urlSuffix; } +/** + * Returns the avatarURL for the participant associated with the passed in + * participant ID. + * + * @param {(Function|Object|Participant[])} stateful - The redux state + * features/base/participants, the (whole) redux state, or redux's + * {@code getState} function to be used to retrieve the state + * features/base/participants. + * @param {string} id - The ID of the participant to retrieve. + * @param {boolean} isLocal - An optional parameter indicating whether or not + * the partcipant id is for the local user. If true, a different logic flow is + * used find the local user, ignoring the id value as it can change through the + * beginning and end of a call. + * @returns {(string|undefined)} + */ +export function getAvatarURLByParticipantId( + stateful: Object | Function, + id: string = LOCAL_PARTICIPANT_DEFAULT_ID) { + const participant = getParticipantById(stateful, id); + + return participant && getAvatarURL(participant); +} + /** * Returns local participant from Redux state. * diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index 3f21945585..f9c9a8a807 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -12,10 +12,15 @@ import { localParticipantIdChanged } from './actions'; import { KICK_PARTICIPANT, MUTE_REMOTE_PARTICIPANT, - PARTICIPANT_DISPLAY_NAME_CHANGED + PARTICIPANT_DISPLAY_NAME_CHANGED, + PARTICIPANT_JOINED, + PARTICIPANT_UPDATED } from './actionTypes'; import { LOCAL_PARTICIPANT_DEFAULT_ID } from './constants'; -import { getLocalParticipant } from './functions'; +import { + getAvatarURLByParticipantId, + getLocalParticipant +} from './functions'; declare var APP: Object; @@ -59,6 +64,38 @@ MiddlewareRegistry.register(store => next => action => { break; } + + case PARTICIPANT_JOINED: + case PARTICIPANT_UPDATED: { + if (typeof APP !== 'undefined') { + const participant = action.participant; + const { id, local } = participant; + + const preUpdateAvatarURL + = getAvatarURLByParticipantId(store.getState(), id); + + // Allow the redux update to go through and compare the old avatar + // to the new avatar and emit out change events if necessary. + const result = next(action); + + const postUpdateAvatarURL + = getAvatarURLByParticipantId(store.getState(), id); + + if (preUpdateAvatarURL !== postUpdateAvatarURL) { + const currentKnownId = local + ? APP.conference.getMyUserId() : id; + + APP.UI.refreshAvatarDisplay( + currentKnownId, postUpdateAvatarURL); + APP.API.notifyAvatarChanged( + currentKnownId, postUpdateAvatarURL); + } + + return result; + } + + break; + } } return next(action); diff --git a/react/features/toolbox/components/ProfileButton.web.js b/react/features/toolbox/components/ProfileButton.web.js index c427c65bf7..a020328609 100644 --- a/react/features/toolbox/components/ProfileButton.web.js +++ b/react/features/toolbox/components/ProfileButton.web.js @@ -5,7 +5,10 @@ import React, { Component } from 'react'; import { connect } from 'react-redux'; import { TOOLBAR_PROFILE_TOGGLED, sendAnalyticsEvent } from '../../analytics'; -import { DEFAULT_AVATAR_RELATIVE_PATH } from '../../base/participants'; +import { + getAvatarURL, + getLocalParticipant +} from '../../base/participants'; import UIEvents from '../../../../service/UI/UIEvents'; import ToolbarButton from './ToolbarButton'; @@ -39,6 +42,11 @@ class ProfileButton extends Component<*> { * @static */ static propTypes = { + /** + * The redux representation of the local participant. + */ + _localParticipant: PropTypes.object, + /** * Whether the button support clicking or not. */ @@ -76,7 +84,12 @@ class ProfileButton extends Component<*> { * @returns {ReactElement} */ render() { - const { _unclickable, tooltipPosition, toggled } = this.props; + const { + _localParticipant, + _unclickable, + tooltipPosition, + toggled + } = this.props; const buttonConfiguration = { ...DEFAULT_BUTTON_CONFIGURATION, unclickable: _unclickable, @@ -90,7 +103,7 @@ class ProfileButton extends Component<*> { tooltipPosition = { tooltipPosition }> + src = { getAvatarURL(_localParticipant) } /> ); } @@ -115,11 +128,13 @@ class ProfileButton extends Component<*> { * @param {Object} state - The Redux state. * @private * @returns {{ + * _localParticipant: Object, * _unclickable: boolean * }} */ function _mapStateToProps(state) { return { + _localParticipant: getLocalParticipant(state), _unclickable: !state['features/base/jwt'].isGuest }; }