From d7e112aaf0800d81e0ae360a0921c244ec0c78c6 Mon Sep 17 00:00:00 2001 From: virtuacoplenny Date: Tue, 26 Mar 2019 09:34:02 -0700 Subject: [PATCH] fix(display-name): do not default name to placeholder name (#4027) * ref(display-name): do not pass in display name The component gets the state itself from redux. * fix(display-name): do not default name to placeholder name The web display name component supports inline editing of the name. Problems can occur when the displayed name differs from the actual saved name, because participants without a display name, including the local user, have a different, default display name displayed. So when editing starts, the input field is populated with the default name. To workaround such while supporting fetching the display name using mapStateToProps, pass in both the name which should be shown and the name value saved in settings. * ref(display-name): rename methods --- modules/UI/shared_video/SharedVideoThumb.js | 11 +++---- modules/UI/videolayout/LocalVideo.js | 11 +++---- modules/UI/videolayout/RemoteVideo.js | 11 ++++--- modules/UI/videolayout/SmallVideo.js | 4 ++- modules/UI/videolayout/VideoLayout.js | 6 ++-- .../components/DisplayName.web.js | 30 ++++++++++++++----- 6 files changed, 45 insertions(+), 28 deletions(-) diff --git a/modules/UI/shared_video/SharedVideoThumb.js b/modules/UI/shared_video/SharedVideoThumb.js index 075018600..fe2360b18 100644 --- a/modules/UI/shared_video/SharedVideoThumb.js +++ b/modules/UI/shared_video/SharedVideoThumb.js @@ -19,7 +19,7 @@ export default function SharedVideoThumb(participant, videoType, VideoLayout) { this.bindHoverHandler(); SmallVideo.call(this, VideoLayout); this.isVideoMuted = true; - this.setDisplayName(participant.name); + this.updateDisplayName(); this.container.onclick = this._onContainerClick; this.container.ondblclick = this._onContainerDoubleClick; @@ -65,9 +65,11 @@ SharedVideoThumb.prototype.createContainer = function(spanId) { }; /** - * Sets the display name for the thumb. + * Triggers re-rendering of the display name using current instance state. + * + * @returns {void} */ -SharedVideoThumb.prototype.setDisplayName = function(displayName) { +SharedVideoThumb.prototype.updateDisplayName = function() { if (!this.container) { logger.warn(`Unable to set displayName - ${this.videoSpanId } does not exist`); @@ -75,8 +77,7 @@ SharedVideoThumb.prototype.setDisplayName = function(displayName) { return; } - this.updateDisplayName({ - displayName: displayName || '', + this._renderDisplayName({ elementID: `${this.videoSpanId}_name`, participantID: this.id }); diff --git a/modules/UI/videolayout/LocalVideo.js b/modules/UI/videolayout/LocalVideo.js index 764b3430b..69ecbea77 100644 --- a/modules/UI/videolayout/LocalVideo.js +++ b/modules/UI/videolayout/LocalVideo.js @@ -50,7 +50,7 @@ function LocalVideo(VideoLayout, emitter, streamEndedCallback) { SmallVideo.call(this, VideoLayout); // Set default display name. - this.setDisplayName(); + this.updateDisplayName(); // Initialize the avatar display with an avatar url selected from the redux // state. Redux stores the local user with a hardcoded participant id of @@ -87,9 +87,11 @@ LocalVideo.prototype.createContainer = function() { }; /** - * Sets the display name for the given video span id. + * Triggers re-rendering of the display name using current instance state. + * + * @returns {void} */ -LocalVideo.prototype.setDisplayName = function(displayName) { +LocalVideo.prototype.updateDisplayName = function() { if (!this.container) { logger.warn( `Unable to set displayName - ${this.videoSpanId @@ -98,9 +100,8 @@ LocalVideo.prototype.setDisplayName = function(displayName) { return; } - this.updateDisplayName({ + this._renderDisplayName({ allowEditing: APP.store.getState()['features/base/jwt'].isGuest, - displayName, displayNameSuffix: interfaceConfig.DEFAULT_LOCAL_DISPLAY_NAME, elementID: 'localDisplayName', participantID: this.id diff --git a/modules/UI/videolayout/RemoteVideo.js b/modules/UI/videolayout/RemoteVideo.js index 83bb5a312..f6f2d4e27 100644 --- a/modules/UI/videolayout/RemoteVideo.js +++ b/modules/UI/videolayout/RemoteVideo.js @@ -53,7 +53,7 @@ function RemoteVideo(user, VideoLayout, emitter) { ? 'left bottom' : 'top center'; this.addRemoteVideoContainer(); this.updateIndicators(); - this.setDisplayName(); + this.updateDisplayName(); this.bindHoverHandler(); this.flipX = false; this.isLocal = false; @@ -519,11 +519,11 @@ RemoteVideo.prototype.addRemoteStreamElement = function(stream) { }; /** - * Sets the display name for the given video span id. + * Triggers re-rendering of the display name using current instance state. * - * @param displayName the display name to set + * @returns {void} */ -RemoteVideo.prototype.setDisplayName = function(displayName) { +RemoteVideo.prototype.updateDisplayName = function() { if (!this.container) { logger.warn(`Unable to set displayName - ${this.videoSpanId } does not exist`); @@ -531,8 +531,7 @@ RemoteVideo.prototype.setDisplayName = function(displayName) { return; } - this.updateDisplayName({ - displayName: displayName || interfaceConfig.DEFAULT_REMOTE_DISPLAY_NAME, + this._renderDisplayName({ elementID: `${this.videoSpanId}_name`, participantID: this.id }); diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index 54bafd2fc..3b1823c0a 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -439,9 +439,11 @@ SmallVideo.prototype.$displayName = function() { * Creates or updates the participant's display name that is shown over the * video preview. * + * @param {Object} props - The React {@code Component} props to pass into the + * {@code DisplayName} component. * @returns {void} */ -SmallVideo.prototype.updateDisplayName = function(props) { +SmallVideo.prototype._renderDisplayName = function(props) { const displayNameContainer = this.container.querySelector('.displayNameContainer'); diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index fcbb50217..ab783d763 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -659,15 +659,15 @@ const VideoLayout = { /** * Display name changed. */ - onDisplayNameChanged(id, displayName, status) { + onDisplayNameChanged(id) { if (id === 'localVideoContainer' || APP.conference.isLocalId(id)) { - localVideoThumbnail.setDisplayName(displayName); + localVideoThumbnail.updateDisplayName(); } else { const remoteVideo = remoteVideos[id]; if (remoteVideo) { - remoteVideo.setDisplayName(displayName, status); + remoteVideo.updateDisplayName(); } } }, diff --git a/react/features/display-name/components/DisplayName.web.js b/react/features/display-name/components/DisplayName.web.js index 524e041fe..759b5941f 100644 --- a/react/features/display-name/components/DisplayName.web.js +++ b/react/features/display-name/components/DisplayName.web.js @@ -8,7 +8,8 @@ import { appendSuffix } from '../functions'; import { translate } from '../../base/i18n'; import { - getParticipantDisplayName + getParticipantDisplayName, + getParticipantById } from '../../base/participants'; import { updateSettings } from '../../base/settings'; @@ -18,9 +19,15 @@ import { updateSettings } from '../../base/settings'; type Props = { /** - * The participant's current display name. + * The participant's current display name which should be shown when in + * edit mode. Can be different from what is shown when not editing. */ - _displayName: string, + _configuredDisplayName: string, + + /** + * The participant's current display name which should be shown. + */ + _nameToDisplay: string, /** * Whether or not the display name should be editable on click. @@ -78,6 +85,10 @@ type State = { class DisplayName extends Component { _nameInput: ?HTMLInputElement; + static defaultProps = { + _configuredDisplayName: '' + }; + /** * Initializes a new {@code DisplayName} instance. * @@ -134,7 +145,7 @@ class DisplayName extends Component { */ render() { const { - _displayName, + _nameToDisplay, allowEditing, displayNameSuffix, elementID, @@ -163,7 +174,7 @@ class DisplayName extends Component { className = 'displayname' id = { elementID } onClick = { this._onStartEditing }> - { appendSuffix(_displayName, displayNameSuffix) } + { appendSuffix(_nameToDisplay, displayNameSuffix) } ); } @@ -212,7 +223,7 @@ class DisplayName extends Component { if (this.props.allowEditing) { this.setState({ isEditing: true, - editDisplayNameValue: this.props._displayName || '' + editDisplayNameValue: this.props._configuredDisplayName }); } } @@ -268,14 +279,17 @@ class DisplayName extends Component { * @param {Props} ownProps - The own props of the component. * @private * @returns {{ - * _displayName: string + * _configuredDisplayName: string, + * _nameToDisplay: string * }} */ function _mapStateToProps(state, ownProps) { const { participantID } = ownProps; + const participant = getParticipantById(state, participantID); return { - _displayName: getParticipantDisplayName( + _configuredDisplayName: participant && participant.name, + _nameToDisplay: getParticipantDisplayName( state, participantID) }; }