From f1f46e0af53dab562e22a1146ccdc5aa8cdb752a Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Tue, 27 Jun 2017 15:56:55 -0700 Subject: [PATCH] feat(pinning): move web pinning logic into redux - Re-use the native redux pinning implementation for web - Remove pinning logic from conference.js - To the native pinning add a check for sharedVideo so youtube videos do not send a pin event - Add shared videos as a participant to enable pinning and so they can eventually get added to the filmstrip - Emit UIEvents.PINNED_ENDPOINT from middleware --- conference.js | 36 ++----------------- modules/FollowMe.js | 10 +++--- modules/UI/shared_video/SharedVideo.js | 13 +++++++ modules/UI/videolayout/VideoLayout.js | 24 ++++++------- react/features/base/conference/middleware.js | 35 +++++++++++++++--- react/features/base/participants/functions.js | 32 +++++++++++++++++ react/features/base/participants/reducer.js | 2 ++ react/features/jwt/middleware.js | 15 ++++---- 8 files changed, 104 insertions(+), 63 deletions(-) diff --git a/conference.js b/conference.js index dc99ba45c..4b95f9e43 100644 --- a/conference.js +++ b/conference.js @@ -83,8 +83,6 @@ let room; let connection; let localAudio, localVideo; -import {VIDEO_CONTAINER_TYPE} from "./modules/UI/videolayout/VideoContainer"; - /* * Logic to open a desktop picker put on the window global for * lib-jitsi-meet to detect and invoke @@ -1777,37 +1775,9 @@ export default { UIEvents.VIDEO_UNMUTING_WHILE_AUDIO_ONLY, () => this._displayAudioOnlyTooltip('videoMute')); - APP.UI.addListener(UIEvents.PINNED_ENDPOINT, - (smallVideo, isPinned) => { - let smallVideoId = smallVideo.getId(); - let isLocal = APP.conference.isLocalId(smallVideoId); - - let eventName - = (isPinned ? "pinned" : "unpinned") + "." + - (isLocal ? "local" : "remote"); - let participantCount = room.getParticipantCount(); - JitsiMeetJS.analytics.sendEvent( - eventName, - { value: participantCount }); - - // FIXME why VIDEO_CONTAINER_TYPE instead of checking if - // the participant is on the large video ? - if (smallVideo.getVideoType() === VIDEO_CONTAINER_TYPE - && !isLocal) { - - // When the library starts supporting multiple pins we - // would pass the isPinned parameter together with the - // identifier, but currently we send null to indicate that - // we unpin the last pinned. - try { - room.pinParticipant(isPinned ? smallVideoId : null); - } catch (e) { - reportError(e); - } - } - - updateRemoteThumbnailsVisibility(); - }); + APP.UI.addListener( + UIEvents.PINNED_ENDPOINT, + updateRemoteThumbnailsVisibility); } room.on(ConferenceEvents.CONNECTION_INTERRUPTED, () => { diff --git a/modules/FollowMe.js b/modules/FollowMe.js index cc039bc59..05197ba2e 100644 --- a/modules/FollowMe.js +++ b/modules/FollowMe.js @@ -153,7 +153,7 @@ class FollowMe { smallVideo = VideoLayout.getSmallVideo(pinnedId); } - this._nextOnStage(smallVideo, isPinned); + this._nextOnStage(smallVideo.getId(), isPinned); // check whether shared document is enabled/initialized if(this._UI.getSharedDocumentManager()) @@ -174,8 +174,8 @@ class FollowMe { this.filmstripEventHandler); var self = this; - this.pinnedEndpointEventHandler = function (smallVideo, isPinned) { - self._nextOnStage(smallVideo, isPinned); + this.pinnedEndpointEventHandler = function (videoId, isPinned) { + self._nextOnStage(videoId, isPinned); }; this._UI.addListener(UIEvents.PINNED_ENDPOINT, this.pinnedEndpointEventHandler); @@ -243,13 +243,13 @@ class FollowMe { * unpinned * @private */ - _nextOnStage (smallVideo, isPinned) { + _nextOnStage (videoId, isPinned) { if (!this._conference.isModerator) return; var nextOnStage = null; if(isPinned) - nextOnStage = smallVideo.getId(); + nextOnStage = videoId; this._local.nextOnStage = nextOnStage; } diff --git a/modules/UI/shared_video/SharedVideo.js b/modules/UI/shared_video/SharedVideo.js index e768a3ff2..4fb98bd28 100644 --- a/modules/UI/shared_video/SharedVideo.js +++ b/modules/UI/shared_video/SharedVideo.js @@ -10,6 +10,10 @@ import LargeContainer from '../videolayout/LargeContainer'; import SmallVideo from '../videolayout/SmallVideo'; import Filmstrip from '../videolayout/Filmstrip'; +import { + participantJoined, + participantLeft +} from '../../../react/features/base/participants'; import { dockToolbox, showToolbox } from '../../../react/features/toolbox'; export const SHARED_VIDEO_CONTAINER_TYPE = "sharedvideo"; @@ -267,6 +271,13 @@ export default class SharedVideoManager { VideoLayout.addLargeVideoContainer( SHARED_VIDEO_CONTAINER_TYPE, self.sharedVideo); + + APP.store.dispatch(participantJoined({ + id: self.url, + isBot: true, + name: player.getVideoData().title + })); + VideoLayout.handleVideoThumbClicked(self.url); // If we are sending the command and we are starting the player @@ -461,6 +472,8 @@ export default class SharedVideoManager { UIEvents.UPDATE_SHARED_VIDEO, null, 'removed'); }); + APP.store.dispatch(participantLeft(this.url)); + this.url = null; this.isSharedVideoShown = false; this.initialAttributes = null; diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index ec4090b22..ce4aa970f 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -1,6 +1,8 @@ /* global APP, $, interfaceConfig, JitsiMeetJS */ const logger = require("jitsi-meet-logger").getLogger(__filename); +import { pinParticipant } from '../../../react/features/base/participants'; + import Filmstrip from "./Filmstrip"; import UIEvents from "../../../service/UI/UIEvents"; import UIUtil from "../util/UIUtil"; @@ -20,6 +22,8 @@ var currentDominantSpeaker = null; var eventEmitter = null; +// TODO Remove this private reference to pinnedId once other components +// interested in its updates are moved to react/redux. /** * Currently focused video jid * @type {String} @@ -59,7 +63,7 @@ function onContactClicked (id) { // let the bridge adjust its lastN set for myjid and store // the pinned user in the lastNPickupId variable to be // picked up later by the lastN changed event handler. - eventEmitter.emit(UIEvents.PINNED_ENDPOINT, remoteVideo, true); + APP.store.dispatch(pinParticipant(remoteVideo.id)); } } } @@ -406,12 +410,6 @@ var VideoLayout = { var oldSmallVideo = VideoLayout.getSmallVideo(pinnedId); if (oldSmallVideo && !interfaceConfig.filmStripOnly) { oldSmallVideo.focus(false); - // as no pinned event will be sent for local video - // and we will unpin old one, lets signal it - // otherwise we will just send the new pinned one - if (smallVideo.isLocal) - eventEmitter.emit( - UIEvents.PINNED_ENDPOINT, oldSmallVideo, false); } } @@ -419,6 +417,9 @@ var VideoLayout = { if (pinnedId === id) { pinnedId = null; + + APP.store.dispatch(pinParticipant(null)); + // Enable the currently set dominant speaker. if (currentDominantSpeaker) { if(smallVideo && smallVideo.hasVideo()) { @@ -432,8 +433,6 @@ var VideoLayout = { this.updateLargeVideo(this.electLastVisibleVideo()); } - eventEmitter.emit(UIEvents.PINNED_ENDPOINT, smallVideo, false); - return; } @@ -442,10 +441,10 @@ var VideoLayout = { // Update focused/pinned interface. if (id) { - if (smallVideo && !interfaceConfig.filmStripOnly) + if (smallVideo && !interfaceConfig.filmStripOnly) { smallVideo.focus(true); - - eventEmitter.emit(UIEvents.PINNED_ENDPOINT, smallVideo, true); + APP.store.dispatch(pinParticipant(id)); + } } this.updateLargeVideo(id); @@ -823,6 +822,7 @@ var VideoLayout = { if (pinnedId === id) { logger.info("Focused video owner has left the conference"); pinnedId = null; + APP.store.dispatch(pinParticipant(null)); } if (currentDominantSpeaker === id) { diff --git a/react/features/base/conference/middleware.js b/react/features/base/conference/middleware.js index 3f01d5664..b629b65d0 100644 --- a/react/features/base/conference/middleware.js +++ b/react/features/base/conference/middleware.js @@ -2,10 +2,12 @@ import UIEvents from '../../../../service/UI/UIEvents'; import { CONNECTION_ESTABLISHED } from '../connection'; +import JitsiMeetJS from '../lib-jitsi-meet'; import { setVideoMuted, VIDEO_MUTISM_AUTHORITY } from '../media'; import { getLocalParticipant, getParticipantById, + getPinnedParticipant, PIN_PARTICIPANT } from '../participants'; import { MiddlewareRegistry } from '../redux'; @@ -168,27 +170,46 @@ function _conferenceJoined(store, next, action) { */ function _pinParticipant(store, next, action) { const state = store.getState(); + const { conference } = state['features/base/conference']; const participants = state['features/base/participants']; const id = action.participant.id; const participantById = getParticipantById(participants, id); let pin; - // The following condition prevents signaling to pin local participant. The - // logic is: + const shouldEmitToLegacyApp = typeof APP !== 'undefined'; + + if (shouldEmitToLegacyApp) { + const pinnedParticipant = getPinnedParticipant(participants); + const actionName = action.participant.id ? 'pinned' : 'unpinned'; + let videoType; + + if ((participantById && participantById.local) + || (!id && pinnedParticipant && pinnedParticipant.local)) { + videoType = 'local'; + } else { + videoType = 'remote'; + } + + JitsiMeetJS.analytics.sendEvent( + `${actionName}.${videoType}`, + { value: conference.getParticipantCount() }); + } + + // The following condition prevents signaling to pin local participant and + // shared videos. The logic is: // - If we have an ID, we check if the participant identified by that ID is - // local. + // local or a bot/fake participant (such as with shared video). // - If we don't have an ID (i.e. no participant identified by an ID), we // check for local participant. If she's currently pinned, then this // action will unpin her and that's why we won't signal here too. if (participantById) { - pin = !participantById.local; + pin = !participantById.local && !participantById.isBot; } else { const localParticipant = getLocalParticipant(participants); pin = !localParticipant || !localParticipant.pinned; } if (pin) { - const { conference } = state['features/base/conference']; try { conference.pinParticipant(id); @@ -197,6 +218,10 @@ function _pinParticipant(store, next, action) { } } + if (shouldEmitToLegacyApp) { + APP.UI.emitEvent(UIEvents.PINNED_ENDPOINT, id, Boolean(id)); + } + return next(action); } diff --git a/react/features/base/participants/functions.js b/react/features/base/participants/functions.js index 15d4495ea..ca0edfa3f 100644 --- a/react/features/base/participants/functions.js +++ b/react/features/base/participants/functions.js @@ -98,6 +98,38 @@ export function getParticipantById(stateOrGetState, id) { return participants.find(p => p.id === id); } +/** + * Returns a count of the known participants in the passed in redux state, + * excluding any fake participants. + * + * @param {(Function|Object|Participant[])} stateOrGetState - The redux state + * features/base/participants, the (whole) redux state, or redux's + * {@code getState} function to be used to retrieve the + * features/base/participants state. + * @returns {number} + */ +export function getParticipantCount(stateOrGetState) { + const participants = _getParticipants(stateOrGetState); + const realParticipants = participants.filter(p => !p.isBot); + + return realParticipants.length; +} + +/** + * Returns the participant which has its pinned state set to truthy. + * + * @param {(Function|Object|Participant[])} stateOrGetState - The redux state + * features/base/participants, the (whole) redux state, or redux's + * {@code getState} function to be used to retrieve the + * features/base/participants state. + * @returns {(Participant|undefined)} + */ +export function getPinnedParticipant(stateOrGetState) { + const participants = _getParticipants(stateOrGetState); + + return participants.find(p => p.pinned); +} + /** * Returns array of participants from Redux state. * diff --git a/react/features/base/participants/reducer.js b/react/features/base/participants/reducer.js index 4a0857772..99c6b8af9 100644 --- a/react/features/base/participants/reducer.js +++ b/react/features/base/participants/reducer.js @@ -73,6 +73,7 @@ function _participant(state, action) { connectionStatus, dominantSpeaker, email, + isBot, local, pinned, role @@ -108,6 +109,7 @@ function _participant(state, action) { dominantSpeaker: dominantSpeaker || false, email, id, + isBot, local: local || false, name, pinned: pinned || false, diff --git a/react/features/jwt/middleware.js b/react/features/jwt/middleware.js index 7b0ab5dd2..0bcc37d6d 100644 --- a/react/features/jwt/middleware.js +++ b/react/features/jwt/middleware.js @@ -9,7 +9,11 @@ import { import { SET_CONFIG } from '../base/config'; import { SET_LOCATION_URL } from '../base/connection'; import { LIB_INIT_ERROR } from '../base/lib-jitsi-meet'; -import { PARTICIPANT_JOINED } from '../base/participants'; +import { + getLocalParticipant, + getParticipantCount, + PARTICIPANT_JOINED +} from '../base/participants'; import { MiddlewareRegistry } from '../base/redux'; import { setCallOverlayVisible, setJWT } from './actions'; @@ -96,13 +100,8 @@ function _maybeSetCallOverlayVisible({ dispatch, getState }, next, action) { default: { // The CallOverlay it to no longer be displayed/visible as soon // as another participant joins. - const participants = state['features/base/participants']; - - callOverlayVisible - = Boolean( - participants - && participants.length === 1 - && participants[0].local); + callOverlayVisible = getParticipantCount(state) === 1 + && Boolean(getLocalParticipant(state)); // However, the CallDialog is not to be displayed/visible again // after all remote participants leave.