fix(Filmstrip): Prevent Toolbox from being shown indefinitely when hovering filmstrip

This commit is contained in:
Mihai-Andrei Uscat 2021-01-29 15:34:37 +02:00 committed by GitHub
parent f034f179ff
commit c370c05701
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 6 additions and 122 deletions

View File

@ -8,17 +8,6 @@
*/ */
export const SET_FILMSTRIP_ENABLED = 'SET_FILMSTRIP_ENABLED'; export const SET_FILMSTRIP_ENABLED = 'SET_FILMSTRIP_ENABLED';
/**
* The type of (redux) action which sets whether or not the filmstrip is being
* hovered with the cursor.
*
* {
* type: SET_FILMSTRIP_HOVERED,
* hovered: boolean
* }
*/
export const SET_FILMSTRIP_HOVERED = 'SET_FILMSTRIP_HOVERED';
/** /**
* The type of (redux) action which sets whether the filmstrip is visible. * The type of (redux) action which sets whether the filmstrip is visible.
* *

View File

@ -2,7 +2,6 @@
import { import {
SET_FILMSTRIP_ENABLED, SET_FILMSTRIP_ENABLED,
SET_FILMSTRIP_HOVERED,
SET_FILMSTRIP_VISIBLE, SET_FILMSTRIP_VISIBLE,
SET_TILE_VIEW_DIMENSIONS SET_TILE_VIEW_DIMENSIONS
} from './actionTypes'; } from './actionTypes';
@ -23,22 +22,6 @@ export function setFilmstripEnabled(enabled: boolean) {
}; };
} }
/**
* Sets whether the filmstrip is being hovered (over).
*
* @param {boolean} hovered - Whether the filmstrip is being hovered (over).
* @returns {{
* type: SET_FILMSTRIP_HOVERED,
* hovered: boolean
* }}
*/
export function setFilmstripHovered(hovered: boolean) {
return {
type: SET_FILMSTRIP_HOVERED,
hovered
};
}
/** /**
* Sets whether the filmstrip is visible. * Sets whether the filmstrip is visible.
* *

View File

@ -1,6 +1,5 @@
/* @flow */ /* @flow */
import _ from 'lodash';
import React, { Component } from 'react'; import React, { Component } from 'react';
import type { Dispatch } from 'redux'; import type { Dispatch } from 'redux';
@ -12,10 +11,9 @@ import {
import { translate } from '../../../base/i18n'; import { translate } from '../../../base/i18n';
import { Icon, IconMenuDown, IconMenuUp } from '../../../base/icons'; import { Icon, IconMenuDown, IconMenuUp } from '../../../base/icons';
import { connect } from '../../../base/redux'; import { connect } from '../../../base/redux';
import { dockToolbox } from '../../../toolbox/actions.web';
import { isButtonEnabled } from '../../../toolbox/functions.web'; import { isButtonEnabled } from '../../../toolbox/functions.web';
import { getCurrentLayout, LAYOUTS } from '../../../video-layout'; import { LAYOUTS, getCurrentLayout } from '../../../video-layout';
import { setFilmstripHovered, setFilmstripVisible } from '../../actions'; import { setFilmstripVisible } from '../../actions';
import { shouldRemoteVideosBeVisible } from '../../functions'; import { shouldRemoteVideosBeVisible } from '../../functions';
declare var APP: Object; declare var APP: Object;
@ -56,12 +54,6 @@ type Props = {
*/ */
_hideToolbar: boolean, _hideToolbar: boolean,
/**
* Whether or not remote videos are currently being hovered over. Hover
* handling is currently being handled detected outside of react.
*/
_hovered: boolean,
/** /**
* The number of rows in tile view. * The number of rows in tile view.
*/ */
@ -95,13 +87,6 @@ type Props = {
* @extends Component * @extends Component
*/ */
class Filmstrip extends Component <Props> { class Filmstrip extends Component <Props> {
_isHovered: boolean;
_notifyOfHoveredStateUpdate: Function;
_onMouseOut: Function;
_onMouseOver: Function;
/** /**
* Initializes a new {@code Filmstrip} instance. * Initializes a new {@code Filmstrip} instance.
@ -112,20 +97,7 @@ class Filmstrip extends Component <Props> {
constructor(props: Props) { constructor(props: Props) {
super(props); super(props);
// Debounce the method for dispatching the new filmstrip handled state
// so that it does not get called with each mouse movement event. This
// also works around an issue where mouseout and then a mouseover event
// is fired when hovering over remote thumbnails, which are not yet in
// react.
this._notifyOfHoveredStateUpdate = _.debounce(this._notifyOfHoveredStateUpdate, 100);
// Cache the current hovered state for _updateHoveredState to always
// send the last known hovered state.
this._isHovered = false;
// Bind event handlers so they are only bound once for every instance. // Bind event handlers so they are only bound once for every instance.
this._onMouseOut = this._onMouseOut.bind(this);
this._onMouseOver = this._onMouseOver.bind(this);
this._onShortcutToggleFilmstrip = this._onShortcutToggleFilmstrip.bind(this); this._onShortcutToggleFilmstrip = this._onShortcutToggleFilmstrip.bind(this);
this._onToolbarToggleFilmstrip = this._onToolbarToggleFilmstrip.bind(this); this._onToolbarToggleFilmstrip = this._onToolbarToggleFilmstrip.bind(this);
} }
@ -212,9 +184,7 @@ class Filmstrip extends Component <Props> {
id = 'remoteVideos'> id = 'remoteVideos'>
<div <div
className = 'filmstrip__videos' className = 'filmstrip__videos'
id = 'filmstripLocalVideo' id = 'filmstripLocalVideo'>
onMouseOut = { this._onMouseOut }
onMouseOver = { this._onMouseOver }>
<div id = 'filmstripLocalVideoThumbnail' /> <div id = 'filmstripLocalVideoThumbnail' />
</div> </div>
<div <div
@ -228,8 +198,6 @@ class Filmstrip extends Component <Props> {
<div <div
className = { remoteVideoContainerClassName } className = { remoteVideoContainerClassName }
id = 'filmstripRemoteVideosContainer' id = 'filmstripRemoteVideosContainer'
onMouseOut = { this._onMouseOut }
onMouseOver = { this._onMouseOver }
style = { filmstripRemoteVideosContainerStyle }> style = { filmstripRemoteVideosContainerStyle }>
<div id = 'localVideoTileViewContainer' /> <div id = 'localVideoTileViewContainer' />
</div> </div>
@ -249,44 +217,6 @@ class Filmstrip extends Component <Props> {
this.props.dispatch(setFilmstripVisible(!this.props._visible)); this.props.dispatch(setFilmstripVisible(!this.props._visible));
} }
/**
* If the current hover state does not match the known hover state in redux,
* dispatch an action to update the known hover state in redux.
*
* @private
* @returns {void}
*/
_notifyOfHoveredStateUpdate() {
if (this.props._hovered !== this._isHovered) {
this.props.dispatch(dockToolbox(this._isHovered));
this.props.dispatch(setFilmstripHovered(this._isHovered));
}
}
/**
* Updates the currently known mouseover state and attempt to dispatch an
* update of the known hover state in redux.
*
* @private
* @returns {void}
*/
_onMouseOut() {
this._isHovered = false;
this._notifyOfHoveredStateUpdate();
}
/**
* Updates the currently known mouseover state and attempt to dispatch an
* update of the known hover state in redux.
*
* @private
* @returns {void}
*/
_onMouseOver() {
this._isHovered = true;
this._notifyOfHoveredStateUpdate();
}
_onShortcutToggleFilmstrip: () => void; _onShortcutToggleFilmstrip: () => void;
/** /**
@ -358,7 +288,7 @@ class Filmstrip extends Component <Props> {
*/ */
function _mapStateToProps(state) { function _mapStateToProps(state) {
const { iAmSipGateway } = state['features/base/config']; const { iAmSipGateway } = state['features/base/config'];
const { hovered, visible } = state['features/filmstrip']; const { visible } = state['features/filmstrip'];
const reduceHeight const reduceHeight
= state['features/toolbox'].visible && interfaceConfig.TOOLBAR_BUTTONS.length; = state['features/toolbox'].visible && interfaceConfig.TOOLBAR_BUTTONS.length;
const remoteVideosVisible = shouldRemoteVideosBeVisible(state); const remoteVideosVisible = shouldRemoteVideosBeVisible(state);
@ -376,7 +306,6 @@ function _mapStateToProps(state) {
_filmstripWidth: filmstripWidth, _filmstripWidth: filmstripWidth,
_hideScrollbar: Boolean(iAmSipGateway), _hideScrollbar: Boolean(iAmSipGateway),
_hideToolbar: Boolean(iAmSipGateway), _hideToolbar: Boolean(iAmSipGateway),
_hovered: hovered,
_rows: gridDimensions.rows, _rows: gridDimensions.rows,
_videosClassName: videosClassName, _videosClassName: videosClassName,
_visible: visible _visible: visible

View File

@ -62,11 +62,9 @@ export function shouldRemoteVideosBeVisible(state: Object) {
participantCount > 2 participantCount > 2
// Always show the filmstrip when there is another participant to // Always show the filmstrip when there is another participant to
// show and the filmstrip is hovered, or local video is pinned, or // show and the local video is pinned, or the toolbar is displayed.
// the toolbar is displayed.
|| (participantCount > 1 || (participantCount > 1
&& (state['features/filmstrip'].hovered && (state['features/toolbox'].visible
|| state['features/toolbox'].visible
|| ((pinnedParticipant = getPinnedParticipant(state)) || ((pinnedParticipant = getPinnedParticipant(state))
&& pinnedParticipant.local))) && pinnedParticipant.local)))

View File

@ -4,7 +4,6 @@ import { ReducerRegistry } from '../base/redux';
import { import {
SET_FILMSTRIP_ENABLED, SET_FILMSTRIP_ENABLED,
SET_FILMSTRIP_HOVERED,
SET_FILMSTRIP_VISIBLE, SET_FILMSTRIP_VISIBLE,
SET_HORIZONTAL_VIEW_DIMENSIONS, SET_HORIZONTAL_VIEW_DIMENSIONS,
SET_TILE_VIEW_DIMENSIONS SET_TILE_VIEW_DIMENSIONS
@ -54,20 +53,6 @@ ReducerRegistry.register(
enabled: action.enabled enabled: action.enabled
}; };
case SET_FILMSTRIP_HOVERED:
return {
...state,
/**
* The indicator which determines whether the {@link Filmstrip}
* is being hovered (over).
*
* @public
* @type {boolean}
*/
hovered: action.hovered
};
case SET_FILMSTRIP_VISIBLE: case SET_FILMSTRIP_VISIBLE:
return { return {
...state, ...state,