fix(vertical-filmstrip): different label animations for filmstrip states (#1596)

* fix(vertical-filmstrip): different label animations for filmstrip states

Instead of one timing for sliding the video status label left and right,
have different timings depending on the filmstrip state. To facilitate
triggering the different animations, add more classes to the labels
that need to move that specify the filmstrip state.

- Faster transition if focusing on self-view with videos present so
  the label does not overlap videos transitioning from 0 opacity.
- Transition delay when de-focusing on self-view with videos present
  so videos have time to go away before the label moves over them.
- Maintain no movement if there are no videos, regardless of
  filmstrip toggle state.
- Different delays for when the filmstrip is being toggled visible
  and hidden if there are remote videos visible.

* SQUASH: remove remote videos count

* SQUASH: add docs to scss

conference.js

@@ -1844,11 +1844,12 @@ export default {
             const remoteVideosCount = APP.UI.getRemoteVideosCount();
 
             const shouldShowRemoteThumbnails = interfaceConfig.filmStripOnly
-                || APP.UI.isPinned(localUserId)
+                || (APP.UI.isPinned(localUserId) && remoteVideosCount)
                 || remoteVideosCount > 1
                 || remoteParticipantsCount !== remoteVideosCount;
 
-            APP.UI.setRemoteThumbnailsVisibility(shouldShowRemoteThumbnails);
+            APP.UI.setRemoteThumbnailsVisibility(
+                Boolean(shouldShowRemoteThumbnails));
         }
     },
     /**

css/_vertical_filmstrip_overrides.scss

@@ -88,13 +88,24 @@
     }
 
     /**
-     * For video labels that display on the top right to adjust its position as
-     * the filmstrip itself or filmstrip remote videos appear and disappear.
+     * These styles are for the video labels that display on the top right. The
+     * styles adjust the labels' positioning as the filmstrip itself or
+     * filmstrip's remote videos appear and disappear.
+     *
+     * The class with-filmstrip is for when the filmstrip is visible.
+     * The class without-filmstrip is for when the filmstrip has been toggled to
+     * be hidden.
+     * The class opening is for when the filmstrip is transitioning from hidden
+     * to visible.
+     * The class with-remote-videos is for when the filmstrip has remote videos
+     * displayed, as opposed to 1-on-1 mode where they might be hidden.
+     * The class without-remote-videos is for when the filmstrip is visible
+     * but it has no videos to display.
      */
     .video-state-indicator.moveToCorner {
-        transition: right 2s;
+        transition: right 0.5s;
 
-        &.with-filmstrip {
+        &.with-filmstrip.with-remote-videos {
             &#recordingLabel {
                 right: 200px;
             }
@@ -103,6 +114,20 @@
                 right: 150px;
             }
         }
+
+        &.with-filmstrip.without-remote-videos {
+            transition-delay: 0.5s;
+        }
+
+        &.with-filmstrip.with-remote-videos.opening {
+            transition: 0.9s;
+            transition-timing-function: ease-in-out;
+        }
+
+        &.without-filmstrip {
+            transition: 1.2s ease-in-out;
+            transition-delay: 0.1s;
+        }
     }
 
     /**

modules/UI/recording/Recording.js

@@ -196,20 +196,14 @@ function _showStopRecordingPrompt(recordingType) {
 
 /**
  * Moves the element given by {selector} to the top right corner of the screen.
+ * Set additional classes that can be used to style the selector relative to the
+ * state of the filmstrip.
+ *
  * @param selector the selector for the element to move
  * @param move {true} to move the element, {false} to move it back to its intial
  * position
  */
 function moveToCorner(selector, move) {
-    const {
-        remoteVideosCount,
-        remoteVideosVisible,
-        visible
-    } = APP.store.getState()['features/filmstrip'];
-    selector.toggleClass(
-        'with-filmstrip',
-        Boolean(remoteVideosCount && remoteVideosVisible && visible));
-
     let moveToCornerClass = "moveToCorner";
     let containsClass = selector.hasClass(moveToCornerClass);
 
@@ -217,6 +211,20 @@ function moveToCorner(selector, move) {
         selector.addClass(moveToCornerClass);
     else if (!move && containsClass)
         selector.removeClass(moveToCornerClass);
+
+    const {
+        remoteVideosVisible,
+        visible
+    } = APP.store.getState()['features/filmstrip'];
+    const filmstripWasHidden = selector.hasClass('without-filmstrip');
+    const filmstipIsOpening = filmstripWasHidden && visible;
+    selector.toggleClass('opening', filmstipIsOpening);
+
+    selector.toggleClass('with-filmstrip', visible);
+    selector.toggleClass('without-filmstrip', !visible);
+
+    selector.toggleClass('with-remote-videos', remoteVideosVisible);
+    selector.toggleClass('without-remote-videos', !remoteVideosVisible);
 }
 
 /**

modules/UI/videolayout/VideoLayout.js

@@ -1,10 +1,6 @@
 /* global APP, $, interfaceConfig */
 const logger = require("jitsi-meet-logger").getLogger(__filename);
 
-import {
-    setFilmstripRemoteVideosCount
-} from '../../../react/features/filmstrip';
-
 import Filmstrip from "./Filmstrip";
 import UIEvents from "../../../service/UI/UIEvents";
 import UIUtil from "../util/UIUtil";
@@ -555,8 +551,6 @@ var VideoLayout = {
                     onComplete();
             });
 
-        APP.store.dispatch(
-            setFilmstripRemoteVideosCount(this.getRemoteVideosCount()));
         return { localVideo, remoteVideo };
     },
 

react/features/filmstrip/actionTypes.js

@@ -1,24 +1,12 @@
 import { Symbol } from '../base/react';
 
-/**
- * The type of action which signals to change the count of known remote videos
- * displayed in the filmstrip.
- *
- * {
- *     type: SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
- *     remoteVideosCount: number
- * }
- */
-export const SET_FILMSTRIP_REMOTE_VIDEOS_COUNT
-    = Symbol('SET_FILMSTRIP_REMOTE_VIDEOS_COUNT');
-
 /**
  * The type of action which signals to change the visibility of remote videos in
  * the filmstrip.
  *
  * {
  *     type: SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
- *     removeVideosVisible: boolean
+ *     remoteVideosVisible: boolean
  * }
  */
 export const SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY

react/features/filmstrip/actions.js

@@ -1,5 +1,4 @@
 import {
-    SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
     SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
     SET_FILMSTRIP_VISIBILITY
 } from './actionTypes';
@@ -21,22 +20,6 @@ export function setFilmstripRemoteVideosVisibility(remoteVideosVisible) {
     };
 }
 
-/**
- * Sets how many remote videos are currently in the filmstrip.
- *
- * @param {number} remoteVideosCount - The number of remote videos.
- * @returns {{
- *     type: SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
- *     remoteVideosCount: number
- * }}
- */
-export function setFilmstripRemoteVideosCount(remoteVideosCount) {
-    return {
-        type: SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
-        remoteVideosCount
-    };
-}
-
 /**
  * Sets if the entire filmstrip should be visible.
  *

react/features/filmstrip/middleware.js

@@ -3,7 +3,6 @@ import UIEvents from '../../../service/UI/UIEvents';
 import { MiddlewareRegistry } from '../base/redux';
 
 import {
-    SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
     SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
     SET_FILMSTRIP_VISIBILITY
 } from './actionTypes';
@@ -15,7 +14,6 @@ MiddlewareRegistry.register(store => next => action => {
     const result = next(action);
 
     switch (action.type) {
-    case SET_FILMSTRIP_REMOTE_VIDEOS_COUNT:
     case SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY:
     case SET_FILMSTRIP_VISIBILITY: {
         if (typeof APP !== 'undefined') {

react/features/filmstrip/reducer.js

@@ -1,12 +1,10 @@
 import { ReducerRegistry } from '../base/redux';
 import {
-    SET_FILMSTRIP_REMOTE_VIDEOS_COUNT,
     SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY,
     SET_FILMSTRIP_VISIBILITY
 } from './actionTypes';
 
 const DEFAULT_STATE = {
-    remoteVideosCount: 0,
     remoteVideosVisible: true,
     visible: true
 };
@@ -15,11 +13,6 @@ ReducerRegistry.register(
     'features/filmstrip',
     (state = DEFAULT_STATE, action) => {
         switch (action.type) {
-        case SET_FILMSTRIP_REMOTE_VIDEOS_COUNT:
-            return {
-                ...state,
-                remoteVideosCount: action.remoteVideosCount
-            };
         case SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY:
             return {
                 ...state,

react/features/video-status-label/components/VideoStatusLabel.js

@@ -29,7 +29,8 @@ export class VideoStatusLabel extends Component {
         _conferenceStarted: React.PropTypes.bool,
 
         /**
-         * Whether or not the filmstrip is displayed with remote videos.
+         * Whether or not the filmstrip is displayed with remote videos. Used to
+         * determine display classes to set.
          */
         _filmstripVisible: React.PropTypes.bool,
 
@@ -38,6 +39,12 @@ export class VideoStatusLabel extends Component {
          */
         _largeVideoHD: React.PropTypes.bool,
 
+        /**
+         * Whether or note remote videos are visible in the filmstrip,
+         * regardless of count. Used to determine display classes to set.
+         */
+        _remoteVideosVisible: React.PropTypes.bool,
+
         /**
          * Invoked to request toggling of audio only mode.
          */
@@ -58,10 +65,32 @@ export class VideoStatusLabel extends Component {
     constructor(props) {
         super(props);
 
+        this.state = {
+            // Whether or not the filmstrip is transitioning from not visible
+            // to visible. Used to set a transition class for animation.
+            togglingToVisible: false
+        };
+
         // Bind event handler so it is only bound once for every instance.
         this._toggleAudioOnly = this._toggleAudioOnly.bind(this);
     }
 
+    /**
+     * Updates the state for whether or not the filmstrip is being toggled to
+     * display after having being hidden.
+     *
+     * @inheritdoc
+     * @param {Object} nextProps - The read-only props which this Component will
+     * receive.
+     * @returns {void}
+     */
+    componentWillReceiveProps(nextProps) {
+        this.setState({
+            togglingToVisible: nextProps._filmstripVisible
+                && !this.props._filmstripVisible
+        });
+    }
+
     /**
      * Implements React's {@link Component#render()}.
      *
@@ -73,6 +102,7 @@ export class VideoStatusLabel extends Component {
             _audioOnly,
             _conferenceStarted,
             _filmstripVisible,
+            _remoteVideosVisible,
             _largeVideoHD,
             t
         } = this.props;
@@ -93,10 +123,17 @@ export class VideoStatusLabel extends Component {
                 ? t('videoStatus.hd') : t('videoStatus.sd');
         }
 
-        const filmstripClassName
+        // Determine which classes should be set on the component. These classes
+        // will used to help with animations and setting position.
+        const baseClasses = 'video-state-indicator moveToCorner';
+        const filmstrip
             = _filmstripVisible ? 'with-filmstrip' : 'without-filmstrip';
+        const remoteVideosVisible = _remoteVideosVisible
+            ? 'with-remote-videos'
+            : 'without-remote-videos';
+        const opening = this.state.togglingToVisible ? 'opening' : '';
         const classNames
-            = `video-state-indicator moveToCorner ${filmstripClassName}`;
+            = `${baseClasses} ${filmstrip} ${remoteVideosVisible} ${opening}`;
 
         return (
             <div
@@ -159,7 +196,9 @@ export class VideoStatusLabel extends Component {
  * @returns {{
  *     _audioOnly: boolean,
  *     _conferenceStarted: boolean,
- *     _largeVideoHD: (boolean|undefined)
+ *     _filmstripVisible: true,
+ *     _largeVideoHD: (boolean|undefined),
+ *     _remoteVideosVisible: boolean
  * }}
  */
 function _mapStateToProps(state) {
@@ -169,7 +208,6 @@ function _mapStateToProps(state) {
         isLargeVideoHD
     } = state['features/base/conference'];
     const {
-        remoteVideosCount,
         remoteVideosVisible,
         visible
     } = state['features/filmstrip'];
@@ -177,9 +215,9 @@ function _mapStateToProps(state) {
     return {
         _audioOnly: audioOnly,
         _conferenceStarted: Boolean(conference),
-        _filmstripVisible:
-            Boolean(remoteVideosCount && remoteVideosVisible && visible),
-        _largeVideoHD: isLargeVideoHD
+        _filmstripVisible: visible,
+        _largeVideoHD: isLargeVideoHD,
+        _remoteVideosVisible: remoteVideosVisible
     };
 }