From d5b40280abdfbe9c7c3c5283f4f0f357cb34c038 Mon Sep 17 00:00:00 2001 From: virtuacoplenny Date: Wed, 24 May 2017 18:57:55 -0700 Subject: [PATCH] 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 | 5 +- css/_vertical_filmstrip_overrides.scss | 33 ++++++++++-- modules/UI/recording/Recording.js | 26 +++++---- modules/UI/videolayout/VideoLayout.js | 6 --- react/features/filmstrip/actionTypes.js | 14 +---- react/features/filmstrip/actions.js | 17 ------ react/features/filmstrip/middleware.js | 2 - react/features/filmstrip/reducer.js | 7 --- .../components/VideoStatusLabel.js | 54 ++++++++++++++++--- 9 files changed, 96 insertions(+), 68 deletions(-) diff --git a/conference.js b/conference.js index edea58036..e1046f800 100644 --- a/conference.js +++ b/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)); } }, /** diff --git a/css/_vertical_filmstrip_overrides.scss b/css/_vertical_filmstrip_overrides.scss index b58c3f6ef..5c2fcb6ee 100644 --- a/css/_vertical_filmstrip_overrides.scss +++ b/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; + } } /** diff --git a/modules/UI/recording/Recording.js b/modules/UI/recording/Recording.js index cde988eae..602c72574 100644 --- a/modules/UI/recording/Recording.js +++ b/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); } /** diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index b2bc72a30..b7127218e 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/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 }; }, diff --git a/react/features/filmstrip/actionTypes.js b/react/features/filmstrip/actionTypes.js index c39cc3cf5..14f7ea611 100644 --- a/react/features/filmstrip/actionTypes.js +++ b/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 diff --git a/react/features/filmstrip/actions.js b/react/features/filmstrip/actions.js index 76fc9a24c..7fbb0cc86 100644 --- a/react/features/filmstrip/actions.js +++ b/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. * diff --git a/react/features/filmstrip/middleware.js b/react/features/filmstrip/middleware.js index 560e5c8aa..cd848d4a0 100644 --- a/react/features/filmstrip/middleware.js +++ b/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') { diff --git a/react/features/filmstrip/reducer.js b/react/features/filmstrip/reducer.js index ae63692af..4615680b6 100644 --- a/react/features/filmstrip/reducer.js +++ b/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, diff --git a/react/features/video-status-label/components/VideoStatusLabel.js b/react/features/video-status-label/components/VideoStatusLabel.js index 6fd6a8e86..b0c5d4b65 100644 --- a/react/features/video-status-label/components/VideoStatusLabel.js +++ b/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 (