From 56b12bd969b972496d9d9a7bfb6c94cad682f119 Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Tue, 23 May 2017 11:58:07 -0700 Subject: [PATCH] fix(vertical-filmstrip): move video status labels back to top right The video status labels, which include recording and hd status, have been moved back to the top left while in vertical filmstrip mode. The following had to be done: - Remove styling to move the labels to the bottom left - For VideoStatusLabel, move filmstrip remote video count, toggle state, and 1:1 state into redux. - Use middleware to emit out to the Recording label when the filmstrip changes. - Create an empty Filmstrip file for web and identify the existing Filmstrip component as native. --- css/_vertical_filmstrip_overrides.scss | 54 ++++--------------- modules/UI/recording/Recording.js | 13 +++++ modules/UI/videolayout/Filmstrip.js | 9 ++++ modules/UI/videolayout/VideoLayout.js | 7 +++ .../conference/components/Conference.web.js | 1 + react/features/filmstrip/actionTypes.js | 35 ++++++++++++ react/features/filmstrip/actions.js | 54 +++++++++++++++++++ .../{Filmstrip.js => Filmstrip.native.js} | 0 .../filmstrip/components/Filmstrip.web.js | 0 react/features/filmstrip/index.js | 5 ++ react/features/filmstrip/middleware.js | 30 +++++++++++ react/features/filmstrip/reducer.js | 36 +++++++++++++ .../components/VideoStatusLabel.js | 27 +++++++++- service/UI/UIEvents.js | 6 +++ 14 files changed, 232 insertions(+), 45 deletions(-) create mode 100644 react/features/filmstrip/actionTypes.js create mode 100644 react/features/filmstrip/actions.js rename react/features/filmstrip/components/{Filmstrip.js => Filmstrip.native.js} (100%) create mode 100644 react/features/filmstrip/components/Filmstrip.web.js create mode 100644 react/features/filmstrip/middleware.js create mode 100644 react/features/filmstrip/reducer.js diff --git a/css/_vertical_filmstrip_overrides.scss b/css/_vertical_filmstrip_overrides.scss index c7269f3b2..be256b0b7 100644 --- a/css/_vertical_filmstrip_overrides.scss +++ b/css/_vertical_filmstrip_overrides.scss @@ -87,53 +87,21 @@ } } + /** + * For video labels that display on the top right to adjust its position as + * the filmstrip itself or filmstrip remote videos appear and disappear. + */ .video-state-indicator { - bottom: 30px; - left: 30px; - right: auto; - top: auto; + transition: right 2s; - /** - * Move the label to the bottom left of the screen - */ - &#videoResolutionLabel { - left: 60px; - z-index: $poweredByZ; - - /** - * Open the menu above the label. - */ - .video-state-indicator-menu { - bottom: calc(100% - 15px); - left: -10px; - - /** - * Create padding for mouse travel on hover. - */ - padding-bottom: 20px; - right: auto; - top: auto; - - .video-state-indicator-menu-options { - margin: 0; - - /** - * The menu arrow should point down - */ - &::after { - border-color: $popoverBg transparent transparent; - bottom: -10px; - left: 15px; - right: auto; - top: auto; - } - } + &.with-filmstrip { + &#recordingLabel { + right: 200px; } - } - &#recordingLabel { - left: 110px; - z-index: $poweredByZ; + &#videoResolutionLabel { + right: 150px; + } } } diff --git a/modules/UI/recording/Recording.js b/modules/UI/recording/Recording.js index 5046b9cfd..cde988eae 100644 --- a/modules/UI/recording/Recording.js +++ b/modules/UI/recording/Recording.js @@ -201,6 +201,15 @@ function _showStopRecordingPrompt(recordingType) { * 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); @@ -295,6 +304,10 @@ var Recording = { APP.UI.messageHandler.enableNotifications(false); APP.UI.messageHandler.enablePopups(false); } + + this.eventEmitter.addListener(UIEvents.UPDATED_FILMSTRIP_DISPLAY, () =>{ + this._updateStatusLabel(); + }); }, /** diff --git a/modules/UI/videolayout/Filmstrip.js b/modules/UI/videolayout/Filmstrip.js index 172b8d09a..f60bbf83e 100644 --- a/modules/UI/videolayout/Filmstrip.js +++ b/modules/UI/videolayout/Filmstrip.js @@ -1,5 +1,10 @@ /* global $, APP, config, JitsiMeetJS, interfaceConfig */ +import { + setFilmstripRemoteVideosVisibility, + setFilmstripVisibility +} from '../../../react/features/filmstrip'; + import UIEvents from "../../../service/UI/UIEvents"; import UIUtil from "../util/UIUtil"; @@ -43,6 +48,7 @@ const Filmstrip = { return; } + APP.store.dispatch(setFilmstripRemoteVideosVisibility(shouldShow)); this.filmstripRemoteVideos.toggleClass('hide-videos', !shouldShow); }, @@ -172,11 +178,14 @@ const Filmstrip = { // Emit/fire UIEvents.TOGGLED_FILMSTRIP. const eventEmitter = this.eventEmitter; + const isFilmstripVisible = this.isFilmstripVisible(); + if (eventEmitter) { eventEmitter.emit( UIEvents.TOGGLED_FILMSTRIP, this.isFilmstripVisible()); } + APP.store.dispatch(setFilmstripVisibility(isFilmstripVisible)); }, /** diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index ef95f9915..b2bc72a30 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -1,6 +1,10 @@ /* 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"; @@ -550,6 +554,9 @@ var VideoLayout = { if (onComplete && typeof onComplete === "function") onComplete(); }); + + APP.store.dispatch( + setFilmstripRemoteVideosCount(this.getRemoteVideosCount())); return { localVideo, remoteVideo }; }, diff --git a/react/features/conference/components/Conference.web.js b/react/features/conference/components/Conference.web.js index 716e22906..b3aee0b07 100644 --- a/react/features/conference/components/Conference.web.js +++ b/react/features/conference/components/Conference.web.js @@ -10,6 +10,7 @@ import { OverlayContainer } from '../../overlay'; import { Toolbox } from '../../toolbox'; import { HideNotificationBarStyle } from '../../unsupported-browser'; import { VideoStatusLabel } from '../../video-status-label'; +import '../../filmstrip'; declare var $: Function; declare var APP: Object; diff --git a/react/features/filmstrip/actionTypes.js b/react/features/filmstrip/actionTypes.js new file mode 100644 index 000000000..c39cc3cf5 --- /dev/null +++ b/react/features/filmstrip/actionTypes.js @@ -0,0 +1,35 @@ +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 + * } + */ +export const SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY + = Symbol('SET_FILMSTRIP_REMOTE_VIDEOS_VISIBLITY'); + +/** + * The type of action sets the visibility of the entire filmstrip; + * + * { + * type: SET_FILMSTRIP_VISIBILITY, + * visible: boolean + * } + */ +export const SET_FILMSTRIP_VISIBILITY = Symbol('SET_FILMSTRIP_VISIBILITY'); diff --git a/react/features/filmstrip/actions.js b/react/features/filmstrip/actions.js new file mode 100644 index 000000000..76fc9a24c --- /dev/null +++ b/react/features/filmstrip/actions.js @@ -0,0 +1,54 @@ +import { + SET_FILMSTRIP_REMOTE_VIDEOS_COUNT, + 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 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. + * + * @param {boolean} visible - Whether not the filmstrip is visible. + * @returns {{ + * type: SET_FILMSTRIP_VISIBILITY, + * visible: boolean + * }} + */ +export function setFilmstripVisibility(visible) { + return { + type: SET_FILMSTRIP_VISIBILITY, + visible + }; +} diff --git a/react/features/filmstrip/components/Filmstrip.js b/react/features/filmstrip/components/Filmstrip.native.js similarity index 100% rename from react/features/filmstrip/components/Filmstrip.js rename to react/features/filmstrip/components/Filmstrip.native.js diff --git a/react/features/filmstrip/components/Filmstrip.web.js b/react/features/filmstrip/components/Filmstrip.web.js new file mode 100644 index 000000000..e69de29bb diff --git a/react/features/filmstrip/index.js b/react/features/filmstrip/index.js index 07635cbbc..a29aa08e0 100644 --- a/react/features/filmstrip/index.js +++ b/react/features/filmstrip/index.js @@ -1 +1,6 @@ +export * from './actions'; +export * from './actionTypes'; export * from './components'; + +import './middleware'; +import './reducer'; diff --git a/react/features/filmstrip/middleware.js b/react/features/filmstrip/middleware.js new file mode 100644 index 000000000..560e5c8aa --- /dev/null +++ b/react/features/filmstrip/middleware.js @@ -0,0 +1,30 @@ +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'; + +declare var APP: Object; + +// eslint-disable-next-line no-unused-vars +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') { + APP.UI.emitEvent(UIEvents.UPDATED_FILMSTRIP_DISPLAY); + + } + break; + } + } + + return result; +}); diff --git a/react/features/filmstrip/reducer.js b/react/features/filmstrip/reducer.js new file mode 100644 index 000000000..ae63692af --- /dev/null +++ b/react/features/filmstrip/reducer.js @@ -0,0 +1,36 @@ +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 +}; + +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, + remoteVideosVisible: action.remoteVideosVisible + }; + case SET_FILMSTRIP_VISIBILITY: + return { + ...state, + visible: action.visible + }; + } + + return state; + }); diff --git a/react/features/video-status-label/components/VideoStatusLabel.js b/react/features/video-status-label/components/VideoStatusLabel.js index cc4370bce..6fd6a8e86 100644 --- a/react/features/video-status-label/components/VideoStatusLabel.js +++ b/react/features/video-status-label/components/VideoStatusLabel.js @@ -28,6 +28,11 @@ export class VideoStatusLabel extends Component { */ _conferenceStarted: React.PropTypes.bool, + /** + * Whether or not the filmstrip is displayed with remote videos. + */ + _filmstripVisible: React.PropTypes.bool, + /** * Whether or not a high-definition large video is displayed. */ @@ -64,7 +69,13 @@ export class VideoStatusLabel extends Component { * @returns {ReactElement} */ render() { - const { _audioOnly, _conferenceStarted, _largeVideoHD, t } = this.props; + const { + _audioOnly, + _conferenceStarted, + _filmstripVisible, + _largeVideoHD, + t + } = this.props; // FIXME The _conferenceStarted check is used to be defensive against // toggling audio only mode while there is no conference and hides the @@ -82,9 +93,14 @@ export class VideoStatusLabel extends Component { ? t('videoStatus.hd') : t('videoStatus.sd'); } + const filmstripClassName + = _filmstripVisible ? 'with-filmstrip' : 'without-filmstrip'; + const classNames + = `video-state-indicator moveToCorner ${filmstripClassName}`; + return (
{ displayedLabel } { this._renderVideonMenu() } @@ -152,10 +168,17 @@ function _mapStateToProps(state) { conference, isLargeVideoHD } = state['features/base/conference']; + const { + remoteVideosCount, + remoteVideosVisible, + visible + } = state['features/filmstrip']; return { _audioOnly: audioOnly, _conferenceStarted: Boolean(conference), + _filmstripVisible: + Boolean(remoteVideosCount && remoteVideosVisible && visible), _largeVideoHD: isLargeVideoHD }; } diff --git a/service/UI/UIEvents.js b/service/UI/UIEvents.js index 6674e6800..d4d445120 100644 --- a/service/UI/UIEvents.js +++ b/service/UI/UIEvents.js @@ -65,6 +65,12 @@ export default { * @see {TOGGLE_FILMSTRIP} */ TOGGLED_FILMSTRIP: "UI.toggled_filmstrip", + + /** + * Notifies that the filmstrip has updated its appearance, such as by + * toggling or removing videos or adding videos. + */ + UPDATED_FILMSTRIP_DISPLAY: "UI.updated_filmstrip_display", TOGGLE_SCREENSHARING: "UI.toggle_screensharing", TOGGLED_SHARED_DOCUMENT: "UI.toggled_shared_document", CONTACT_CLICKED: "UI.contact_clicked",