From 3ebfb1de703d41e5129f62cb93dfa198cbf6bc0a Mon Sep 17 00:00:00 2001 From: Tudor-Ovidiu Avram Date: Thu, 13 May 2021 14:36:19 +0300 Subject: [PATCH] code review --- lang/main.json | 6 ++-- modules/API/API.js | 2 +- .../components/ParticipantView.native.js | 6 ++-- .../filmstrip/components/web/Thumbnail.js | 8 +---- .../components/AbstractSharedVideoDialog.js | 23 +------------ .../components/native/SharedVideoDialog.js | 34 +++++++++++++++++-- .../components/native/YoutubeLargeVideo.js | 19 ++++++----- .../components/web/SharedVideo.js | 4 +-- .../components/web/SharedVideoDialog.js | 22 +++++++++++- react/features/shared-video/constants.js | 7 +++- 10 files changed, 78 insertions(+), 53 deletions(-) diff --git a/lang/main.json b/lang/main.json index a27562087e..40f8ed61e1 100644 --- a/lang/main.json +++ b/lang/main.json @@ -294,7 +294,7 @@ "sessionRestarted": "Call restarted by the bridge", "Share": "Share", "shareVideoLinkError": "Please provide a correct youtube link.", - "shareVideoTitle": "Share a video", + "shareVideoTitle": "Share video", "shareYourScreen": "Share your screen", "shareYourScreenDisabled": "Screen sharing disabled.", "startLiveStreaming": "Start live stream", @@ -828,7 +828,7 @@ "security": "Security options", "Settings": "Settings", "shareaudio": "Share audio", - "sharedvideo": "Share a video", + "sharedvideo": "Share video", "shareRoom": "Invite someone", "shortcuts": "View shortcuts", "speakerStats": "Speaker stats", @@ -836,7 +836,7 @@ "startSubtitles": "Start subtitles", "stopScreenSharing": "Stop screen sharing", "stopSubtitles": "Stop subtitles", - "stopSharedVideo": "Stop YouTube video", + "stopSharedVideo": "Stop video", "talkWhileMutedPopup": "Trying to speak? You are muted.", "tileViewToggle": "Toggle tile view", "toggleCamera": "Toggle camera", diff --git a/modules/API/API.js b/modules/API/API.js index 218671b65b..f46cbdad89 100644 --- a/modules/API/API.js +++ b/modules/API/API.js @@ -272,7 +272,7 @@ function initCommands() { 'stop-share-video': () => { logger.debug('Share video command received'); - sendAnalytics(createApiEvent('share.video.start')); + sendAnalytics(createApiEvent('share.video.stop')); APP.store.dispatch(stopSharedVideo()); }, diff --git a/react/features/base/participants/components/ParticipantView.native.js b/react/features/base/participants/components/ParticipantView.native.js index 7910a39eb0..ffc94101d9 100644 --- a/react/features/base/participants/components/ParticipantView.native.js +++ b/react/features/base/participants/components/ParticipantView.native.js @@ -4,7 +4,6 @@ import React, { Component } from 'react'; import { Text, View } from 'react-native'; import { YoutubeLargeVideo } from '../../../shared-video/components'; -import { getYoutubeId } from '../../../shared-video/functions'; import { Avatar } from '../../avatar'; import { translate } from '../../i18n'; import { JitsiParticipantConnectionStatus } from '../../lib-jitsi-meet'; @@ -209,8 +208,7 @@ class ParticipantView extends Component { ? this.props.testHintId : `org.jitsi.meet.Participant#${this.props.participantId}`; - const youtubeId = getYoutubeId(this.props.participantId); - const renderYoutubeLargeVideo = _isFakeParticipant && !disableVideo && Boolean(youtubeId); + const renderYoutubeLargeVideo = _isFakeParticipant && !disableVideo; return ( { onPress = { renderYoutubeLargeVideo ? undefined : onPress } value = '' /> - { renderYoutubeLargeVideo && } + { renderYoutubeLargeVideo && } { !_isFakeParticipant && renderVideo && { * @returns {ReactElement} */ _renderFakeParticipant() { - const { _participant } = this.props; - const { id, avatarURL } = _participant; + const { _participant: { avatarURL } } = this.props; const styles = this._getStyles(); const containerClassName = this._getContainerClassName(); @@ -568,11 +567,6 @@ class Thumbnail extends Component { src = { avatarURL } /> ) : this._renderAvatar(styles.avatar)} -
- -
); } diff --git a/react/features/shared-video/components/AbstractSharedVideoDialog.js b/react/features/shared-video/components/AbstractSharedVideoDialog.js index 5c06d30502..ced2cd1470 100644 --- a/react/features/shared-video/components/AbstractSharedVideoDialog.js +++ b/react/features/shared-video/components/AbstractSharedVideoDialog.js @@ -29,6 +29,7 @@ export type Props = { * Implements an abstract class for {@code SharedVideoDialog}. */ export default class AbstractSharedVideoDialog extends Component < Props, S > { + /** * Instantiates a new component. * @@ -41,26 +42,4 @@ export default class AbstractSharedVideoDialog extends Component < Props, } _onSetVideoLink: string => boolean; - - /** - * Validates the entered video link by extracting the id and dispatches it. - * - * It returns a boolean to comply the Dialog behaviour: - * {@code true} - the dialog should be closed. - * {@code false} - the dialog should be left open. - * - * @param {string} link - The entered video link. - * @returns {boolean} - */ - _onSetVideoLink(link: string) { - if (!link || !link.trim()) { - return false; - } - - const { onPostSubmit } = this.props; - - onPostSubmit(link); - - return true; - } } diff --git a/react/features/shared-video/components/native/SharedVideoDialog.js b/react/features/shared-video/components/native/SharedVideoDialog.js index 351b9b01fb..c877062aed 100644 --- a/react/features/shared-video/components/native/SharedVideoDialog.js +++ b/react/features/shared-video/components/native/SharedVideoDialog.js @@ -4,13 +4,15 @@ import React from 'react'; import { InputDialog } from '../../../base/dialog'; import { connect } from '../../../base/redux'; -import { defaultSharedVideoLink } from '../../constants'; +import { defaultMobileSharedVideoLink } from '../../constants'; +import { getYoutubeId } from '../../functions'; import AbstractSharedVideoDialog from '../AbstractSharedVideoDialog'; /** * Implements a component to render a display name prompt. */ class SharedVideoDialog extends AbstractSharedVideoDialog<*> { + /** * Implements React's {@link Component#render()}. * @@ -22,12 +24,38 @@ class SharedVideoDialog extends AbstractSharedVideoDialog<*> { contentKey = 'dialog.shareVideoTitle' onSubmit = { this._onSetVideoLink } textInputProps = {{ - placeholder: defaultSharedVideoLink + placeholder: defaultMobileSharedVideoLink }} /> ); } - _onSetVideoLink: string => boolean; + /** + * Validates the entered video link by extracting the id and dispatches it. + * + * It returns a boolean to comply the Dialog behaviour: + * {@code true} - the dialog should be closed. + * {@code false} - the dialog should be left open. + * + * @param {string} link - The entered video link. + * @returns {boolean} + */ + _onSetVideoLink(link: string) { + if (!link || !link.trim()) { + return false; + } + + const videoId = getYoutubeId(link); + + if (videoId) { + const { onPostSubmit } = this.props; + + onPostSubmit && onPostSubmit(link); + + return true; + } + + return false; + } } export default connect()(SharedVideoDialog); diff --git a/react/features/shared-video/components/native/YoutubeLargeVideo.js b/react/features/shared-video/components/native/YoutubeLargeVideo.js index b3d46189cf..0096ccbafd 100644 --- a/react/features/shared-video/components/native/YoutubeLargeVideo.js +++ b/react/features/shared-video/components/native/YoutubeLargeVideo.js @@ -9,6 +9,7 @@ import { connect } from '../../../base/redux'; import { ASPECT_RATIO_WIDE } from '../../../base/responsive-ui'; import { setToolboxVisible } from '../../../toolbox/actions'; import { setSharedVideoStatus } from '../../actions.native'; +import { getYoutubeId } from '../../functions'; import styles from './styles'; @@ -100,11 +101,11 @@ type Props = { dispatch: Function, /** - * Youtube id of the video to be played. + * Youtube url of the video to be played. * * @private */ - youtubeId: string + youtubeUrl: string }; /** @@ -199,7 +200,7 @@ class YoutubeLargeVideo extends Component { _isPlaying, _playerHeight, _playerWidth, - youtubeId + youtubeUrl } = this.props; return ( @@ -220,7 +221,7 @@ class YoutubeLargeVideo extends Component { play = { _isPlaying } playbackRate = { 1 } ref = { this.playerRef } - videoId = { youtubeId } + videoId = { getYoutubeId(youtubeUrl) } volume = { 50 } webViewProps = {{ bounces: false, @@ -243,7 +244,7 @@ class YoutubeLargeVideo extends Component { _onReady() { if (this.props?._isOwner) { this.onVideoReady( - this.props.youtubeId, + this.props.youtubeUrl, this.playerRef.current && this.playerRef.current.getCurrentTime(), this.props._ownerId); } @@ -266,11 +267,11 @@ class YoutubeLargeVideo extends Component { _isStopped, _ownerId, _seek, - youtubeId + youtubeUrl } = this.props; if (shouldSetNewStatus(_isStopped, _isOwner, status, _isPlaying, time, _seek)) { - this.onVideoChangeEvent(youtubeId, status, time, _ownerId); + this.onVideoChangeEvent(youtubeUrl, status, time, _ownerId); } }); } @@ -282,10 +283,10 @@ class YoutubeLargeVideo extends Component { * @returns {void} */ saveRefTime() { - const { youtubeId, _status, _ownerId } = this.props; + const { youtubeUrl, _status, _ownerId } = this.props; this.playerRef.current && this.playerRef.current.getCurrentTime().then(time => { - this.onVideoChangeEvent(youtubeId, _status, time, _ownerId); + this.onVideoChangeEvent(youtubeUrl, _status, time, _ownerId); }); } diff --git a/react/features/shared-video/components/web/SharedVideo.js b/react/features/shared-video/components/web/SharedVideo.js index 353553254b..02f099a714 100644 --- a/react/features/shared-video/components/web/SharedVideo.js +++ b/react/features/shared-video/components/web/SharedVideo.js @@ -58,7 +58,7 @@ class SharedVideo extends Component { * width: number * }} */ - getDimmensions() { + getDimensions() { const { clientHeight, clientWidth } = this.props; let width; @@ -114,7 +114,7 @@ class SharedVideo extends Component {
+ style = { this.getDimensions() }> {this.getManager()}
); diff --git a/react/features/shared-video/components/web/SharedVideoDialog.js b/react/features/shared-video/components/web/SharedVideoDialog.js index 11a01345e5..13246cdd20 100644 --- a/react/features/shared-video/components/web/SharedVideoDialog.js +++ b/react/features/shared-video/components/web/SharedVideoDialog.js @@ -93,7 +93,27 @@ class SharedVideoDialog extends AbstractSharedVideoDialog<*> { ); } - _onSetVideoLink: string => boolean; + /** + * Validates the entered video link by extracting the id and dispatches it. + * + * It returns a boolean to comply the Dialog behaviour: + * {@code true} - the dialog should be closed. + * {@code false} - the dialog should be left open. + * + * @param {string} link - The entered video link. + * @returns {boolean} + */ + _onSetVideoLink(link: string) { + if (!link || !link.trim()) { + return false; + } + + const { onPostSubmit } = this.props; + + onPostSubmit(link); + + return true; + } _onChange: Object => void; } diff --git a/react/features/shared-video/constants.js b/react/features/shared-video/constants.js index dd61ee5db4..3e59e76adf 100644 --- a/react/features/shared-video/constants.js +++ b/react/features/shared-video/constants.js @@ -1,11 +1,16 @@ // @flow /** - * Example shared video link. + * Placeholder for web share video input. * @type {string} */ export const defaultSharedVideoLink = 'Youtube link or direct video link'; +/** + * Mobile example for a youtube video + */ +export const defaultMobileSharedVideoLink = 'https://youtu.be/TB7LlM4erx8'; + /** * Fixed name of the video player fake participant. * @type {string}