From 9910caf29f54c0463c12136ad9e30a7869cb25d7 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 20:38:14 -0600 Subject: [PATCH 1/6] docs: Fix some documentation. --- modules/UI/videolayout/SmallVideo.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index cd708adf5..c58f83f25 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -120,7 +120,10 @@ SmallVideo.prototype.setDeviceAvailabilityIcons = function (devices) { /** * Sets the type of the video displayed by this instance. - * @param videoType 'camera' or 'desktop' + * Note that this is a string without clearly defined or checked values, and + * it is NOT one of the strings defined in service/RTC/VideoType in + * lib-jitsi-meet. + * @param videoType 'camera' or 'desktop', or 'sharedvideo'. */ SmallVideo.prototype.setVideoType = function (videoType) { this.videoType = videoType; @@ -128,7 +131,10 @@ SmallVideo.prototype.setVideoType = function (videoType) { /** * Returns the type of the video displayed by this instance. - * @returns {String} 'camera', 'screen' or undefined. + * Note that this is a string without clearly defined or checked values, and + * it is NOT one of the strings defined in service/RTC/VideoType in + * lib-jitsi-meet. + * @returns {String} 'camera', 'screen', 'sharedvideo', or undefined. */ SmallVideo.prototype.getVideoType = function () { return this.videoType; From 0ca9389e4bd3f6b2eab8c6fe8f4fddd6d823e0b5 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 20:39:28 -0600 Subject: [PATCH 2/6] cleanup: Simplify code. --- modules/UI/videolayout/VideoLayout.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index a38a4b2ca..5458b76ff 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -406,13 +406,11 @@ var VideoLayout = { addRemoteVideoContainer (id, remoteVideo) { remoteVideos[id] = remoteVideo; - let videoType = VideoLayout.getRemoteVideoType(id); - if (!videoType) { + if (!remoteVideo.getVideoType()) { // make video type the default one (camera) // FIXME container type is not a video type - videoType = VIDEO_CONTAINER_TYPE; + remoteVideo.setVideoType(VIDEO_CONTAINER_TYPE); } - remoteVideo.setVideoType(videoType); // In case this is not currently in the last n we don't show it. if (localLastNCount && localLastNCount > 0 && From ac0ee771edb75aba781a92ab452d03a53e46188c Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 20:45:52 -0600 Subject: [PATCH 3/6] docs: Adds a FIXME. --- modules/UI/videolayout/VideoLayout.js | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index 5458b76ff..b2cc53320 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -621,6 +621,7 @@ var VideoLayout = { // since we don't want to switch to local video. // Update the large video if the video source is already available, // otherwise wait for the "videoactive.jingle" event. + // FIXME: there is no "videoactive.jingle" event. if (!pinnedId && remoteVideo.hasVideoStarted() && !this.getCurrentlyOnLargeContainer().stayOnStage()) { From c0c198098b4ec4f9aa47c56520ae7a85a85b5f39 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 20:47:43 -0600 Subject: [PATCH 4/6] style: Renames variables, adds a FIXME. --- modules/UI/videolayout/RemoteVideo.js | 5 ++--- modules/UI/videolayout/VideoLayout.js | 7 ++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/UI/videolayout/RemoteVideo.js b/modules/UI/videolayout/RemoteVideo.js index f7607cf7e..98f75bb7e 100644 --- a/modules/UI/videolayout/RemoteVideo.js +++ b/modules/UI/videolayout/RemoteVideo.js @@ -440,11 +440,10 @@ RemoteVideo.prototype.waitForPlayback = function (streamElement, stream) { var self = this; - // Register 'onplaying' listener to trigger 'videoactive' on VideoLayout - // when video playback starts + // Triggers when video playback starts var onPlayingHandler = function () { self.wasVideoPlayed = true; - self.VideoLayout.videoactive(streamElement, self.id); + self.VideoLayout.remoteVideoActive(streamElement, self.id); streamElement.onplaying = null; // Refresh to show the video self.updateView(); diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index b2cc53320..289277051 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -423,12 +423,13 @@ var VideoLayout = { remoteVideo.updateView(); }, - videoactive (videoelem, resourceJid) { + // FIXME: what does this do??? + remoteVideoActive(videoElement, resourceJid) { - console.info(resourceJid + " video is now active", videoelem); + console.info(resourceJid + " video is now active", videoElement); VideoLayout.resizeThumbnails( - false, false, function() {$(videoelem).show();}); + false, false, function() {$(videoElement).show();}); // Update the large video to the last added video only if there's no // current dominant, focused speaker or update it to From cf241effbf35c6dca44d7dc90de8f94da4d0d82f Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 21:23:33 -0600 Subject: [PATCH 5/6] style: Fixes indentation, spelling. --- conference.js | 4 ++-- modules/UI/toolbars/Toolbar.js | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/conference.js b/conference.js index 17bac3d26..29c674cfa 100644 --- a/conference.js +++ b/conference.js @@ -432,8 +432,8 @@ function disconnect() { } /** - * Set permanent ptoperties to analytics. - * NOTE: Has to be used after JitsiMeetJS.init. otherwise analytics will be + * Set permanent properties to analytics. + * NOTE: Has to be used after JitsiMeetJS.init. Otherwise analytics will be * null. */ function setAnalyticsPermanentProperties() { diff --git a/modules/UI/toolbars/Toolbar.js b/modules/UI/toolbars/Toolbar.js index 5e17d9be6..f7d4c4c5f 100644 --- a/modules/UI/toolbars/Toolbar.js +++ b/modules/UI/toolbars/Toolbar.js @@ -118,13 +118,11 @@ const buttonHandlers = { }); }, "toolbar_film_strip": function () { - JitsiMeetJS.analytics.sendEvent( - 'toolbar.filmstrip.toggled'); + JitsiMeetJS.analytics.sendEvent('toolbar.filmstrip.toggled'); emitter.emit(UIEvents.TOGGLE_FILM_STRIP); }, "toolbar_button_raisehand": function () { - JitsiMeetJS.analytics.sendEvent( - 'toolbar.raiseHand.clicked'); + JitsiMeetJS.analytics.sendEvent('toolbar.raiseHand.clicked'); APP.conference.maybeToggleRaisedHand(); } }; From 6740b9edf6e7a3fb95ebd8dca6967e0f6ef1f058 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Mon, 7 Nov 2016 21:36:10 -0600 Subject: [PATCH 6/6] feat: Logs pin/unpin events via analytics. --- conference.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/conference.js b/conference.js index 29c674cfa..c10274242 100644 --- a/conference.js +++ b/conference.js @@ -1437,11 +1437,21 @@ export default { }); APP.UI.addListener(UIEvents.PINNED_ENDPOINT, (smallVideo, isPinned) => { - var smallVideoId = smallVideo.getId(); + 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 - && !APP.conference.isLocalId(smallVideoId)) { + && !isLocal) { // When the library starts supporting multiple pins we would // pass the isPinned parameter together with the identifier,