ref(1-on-1): move remote video visibility to a selector (#1922)

* ref(1-on-1): move remote visibility to a selector

Derive whether or not remote videos should display using a selector
to look across different states. A selector was chosen over using
memoized selectors (reselect) or subscribers as a first step
approach, avoiding additional mutations caused by a subscriber
updating the filmstrip state and avoiding additional api overhead
introduced by reselect.

* rename selector

conference.js

@@ -67,7 +67,6 @@ import { getLocationContextRoot } from './react/features/base/util';
 import { statsEmitter } from './react/features/connection-indicator';
 import { showDesktopPicker } from  './react/features/desktop-picker';
 import { maybeOpenFeedbackDialog } from './react/features/feedback';
-import { setFilmstripRemoteVideosVisibility } from './react/features/filmstrip';
 import {
     mediaPermissionPromptVisibilityChanged,
     suspendDetected
@@ -1626,17 +1625,14 @@ export default {
 
             // check the roles for the new user and reflect them
             APP.UI.updateUserRole(user);
-
-            updateRemoteThumbnailsVisibility();
         });
+
         room.on(ConferenceEvents.USER_LEFT, (id, user) => {
             APP.store.dispatch(participantLeft(id, user));
             logger.log('USER %s LEFT', id, user);
             APP.API.notifyUserLeft(id);
             APP.UI.removeUser(id, user.getDisplayName());
             APP.UI.onSharedVideoStop(id);
-
-            updateRemoteThumbnailsVisibility();
         });
 
         room.on(ConferenceEvents.USER_STATUS_CHANGED, (id, status) => {
@@ -1780,10 +1776,6 @@ export default {
             APP.UI.addListener(
                 UIEvents.VIDEO_UNMUTING_WHILE_AUDIO_ONLY,
                 () => this._displayAudioOnlyTooltip('videoMute'));
-
-            APP.UI.addListener(
-                UIEvents.PINNED_ENDPOINT,
-                updateRemoteThumbnailsVisibility);
         }
 
         room.on(ConferenceEvents.CONNECTION_INTERRUPTED, () => {
@@ -2147,8 +2139,6 @@ export default {
                     }
                 });
             }
-
-            updateRemoteThumbnailsVisibility();
         });
         room.addCommandListener(
             this.commands.defaults.SHARED_VIDEO, ({value, attributes}, id) => {
@@ -2164,24 +2154,6 @@ export default {
                     APP.UI.onSharedVideoUpdate(id, value, attributes);
                 }
             });
-
-        function updateRemoteThumbnailsVisibility() {
-            const localUserId = APP.conference.getMyUserId();
-            const remoteParticipantsCount = room.getParticipantCount() - 1;
-
-            // Get the remote thumbnail count for cases where there are
-            // non-participants displaying video, such as with video sharing.
-            const remoteVideosCount = APP.UI.getRemoteVideosCount();
-
-            const shouldShowRemoteThumbnails = interfaceConfig.filmStripOnly
-                || (APP.UI.isPinned(localUserId) && remoteVideosCount)
-                || remoteVideosCount > 1
-                || remoteParticipantsCount !== remoteVideosCount;
-
-            APP.store.dispatch(
-                setFilmstripRemoteVideosVisibility(
-                    Boolean(shouldShowRemoteThumbnails)));
-        }
     },
     /**
     * Adds any room listener.

react/features/filmstrip/actionTypes.js

@@ -1,15 +1,3 @@
-/**
- * The type of action which signals to change the visibility of remote videos in
- * the filmstrip.
- *
- * {
- *     type: SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
- *     remoteVideosVisible: boolean
- * }
- */
-export const SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY
-    = Symbol('SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY');
-
 /**
  * The type of action sets the visibility of the entire filmstrip;
  *

react/features/filmstrip/actions.js

@@ -1,25 +1,7 @@
 import {
-    SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
     SET_FILMSTRIP_VISIBILITY
 } from './actionTypes';
 
-/**
- * Sets the visibility of remote videos in the filmstrip.
- *
- * @param {boolean} remoteVideosVisible - Whether or not remote videos in the
- * filmstrip should be visible.
- * @returns {{
- *     type: SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
- *     remoteVideosVisible: boolean
- * }}
- */
-export function setFilmstripRemoteVideosVisibility(remoteVideosVisible) {
-    return {
-        type: SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
-        remoteVideosVisible
-    };
-}
-
 /**
  * Sets if the entire filmstrip should be visible.
  *

react/features/filmstrip/components/Filmstrip.web.js

@@ -5,6 +5,8 @@ import { connect } from 'react-redux';
 
 import { Toolbox } from '../../toolbox';
 
+import { shouldRemoteVideosBeVisible } from '../functions';
+
 /**
  * Implements a React {@link Component} which represents the filmstrip on
  * Web/React.
@@ -89,11 +91,8 @@ class Filmstrip extends Component {
  * }}
  */
 function _mapStateToProps(state) {
-    const { remoteVideosVisible } = state['features/filmstrip'];
-    const { disable1On1Mode } = state['features/base/config'];
-
     return {
-        _remoteVideosVisible: Boolean(remoteVideosVisible || disable1On1Mode)
+        _remoteVideosVisible: shouldRemoteVideosBeVisible(state)
     };
 }
 

react/features/filmstrip/functions.js

@@ -0,0 +1,34 @@
+declare var interfaceConfig: Object;
+
+import {
+    getPinnedParticipant,
+    getLocalParticipant
+} from '../base/participants';
+
+/**
+ * A selector for determining whether or not remote video thumbnails should be
+ * displayed in the filmstrip.
+ *
+ * @param {Object} state - The full redux state.
+ * @returns {boolean} - True if remote video thumbnails should be displayed.
+ */
+export function shouldRemoteVideosBeVisible(state) {
+    const participants = state['features/base/participants'];
+
+    const shouldShowVideos
+        = state['features/base/config'].disable1On1Mode
+
+        || interfaceConfig.filmStripOnly
+
+        // This is not a 1-on-1 call.
+        || participants.length > 2
+
+        // There is another participant and the local participant is pinned.
+        || (participants.length > 1
+            && getLocalParticipant(state) === getPinnedParticipant(state))
+
+        // There is any non-person participant, like a shared video.
+        || participants.find(participant => participant.isBot);
+
+    return Boolean(shouldShowVideos);
+}

react/features/filmstrip/index.js

@@ -1,6 +1,7 @@
 export * from './actions';
 export * from './actionTypes';
 export * from './components';
+export * from './functions';
 
 import './middleware';
 import './reducer';

react/features/filmstrip/reducer.js

@@ -1,16 +1,9 @@
 import { ReducerRegistry } from '../base/redux';
 import {
-    SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
     SET_FILMSTRIP_VISIBILITY
 } from './actionTypes';
 
 const DEFAULT_STATE = {
-    /**
-     * By default start with remote videos hidden for 1-on-1 mode and rely on
-     * other logic to invoke an action to make them visible when needed.
-     */
-    remoteVideosVisible: false,
-
     visible: true
 };
 
@@ -18,11 +11,6 @@ ReducerRegistry.register(
     'features/filmstrip',
     (state = DEFAULT_STATE, action) => {
         switch (action.type) {
-        case SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY:
-            return {
-                ...state,
-                remoteVideosVisible: action.remoteVideosVisible
-            };
         case SET_FILMSTRIP_VISIBILITY:
             return {
                 ...state,

react/features/recording/components/RecordingLabel.web.js

@@ -2,6 +2,7 @@ import React, { Component } from 'react';
 import { connect } from 'react-redux';
 
 import { translate } from '../../base/i18n';
+import { shouldRemoteVideosBeVisible } from '../../filmstrip';
 
 /**
  * Implements a React {@link Component} which displays the current state of
@@ -140,7 +141,7 @@ class RecordingLabel extends Component {
  * }}
  */
 function _mapStateToProps(state) {
-    const { remoteVideosVisible, visible } = state['features/filmstrip'];
+    const { visible } = state['features/filmstrip'];
     const { labelDisplayConfiguration } = state['features/recording'];
 
     return {
@@ -164,7 +165,7 @@ function _mapStateToProps(state) {
          *
          * @type {boolean}
          */
-        _remoteVideosVisible: remoteVideosVisible
+        _remoteVideosVisible: shouldRemoteVideosBeVisible(state)
     };
 }
 

react/features/video-quality/components/VideoQualityLabel.web.js

@@ -3,6 +3,8 @@ import { connect } from 'react-redux';
 
 import { translate } from '../../base/i18n';
 import { Popover } from '../../base/popover';
+import { shouldRemoteVideosBeVisible } from '../../filmstrip';
+
 import { VideoQualityDialog } from './';
 
 import {
@@ -209,24 +211,15 @@ export class VideoQualityLabel extends Component {
  * }}
  */
 function _mapStateToProps(state) {
-    const {
-        audioOnly,
-        conference
-    } = state['features/base/conference'];
-    const { disable1On1Mode } = state['features/base/config'];
-    const {
-        remoteVideosVisible,
-        visible
-    } = state['features/filmstrip'];
-    const {
-        resolution
-    } = state['features/large-video'];
+    const { audioOnly, conference } = state['features/base/conference'];
+    const { visible } = state['features/filmstrip'];
+    const { resolution } = state['features/large-video'];
 
     return {
         _audioOnly: audioOnly,
         _conferenceStarted: Boolean(conference),
         _filmstripVisible: visible,
-        _remoteVideosVisible: Boolean(remoteVideosVisible || disable1On1Mode),
+        _remoteVideosVisible: shouldRemoteVideosBeVisible(state),
         _resolution: resolution
     };
 }