From 535bd81d61bdeea4ee0d09c641a2366368cb06c3 Mon Sep 17 00:00:00 2001 From: Horatiu Muresan <39557534+horymury@users.noreply.github.com> Date: Fri, 10 Sep 2021 15:17:57 +0300 Subject: [PATCH] fix(context-menu) Hide toolbars when participant context menu opened (#9842) - hide toolbars only when in tile view - fix community issue: https://github.com/jitsi/jitsi-meet/issues/9818 --- .../base/popover/components/Popover.web.js | 69 +++++-------------- react/features/toolbox/actions.web.js | 21 +++++- react/features/toolbox/functions.web.js | 18 +++-- .../web/LocalVideoMenuTriggerButton.js | 32 ++++++++- .../web/RemoteVideoMenuTriggerButton.js | 32 ++++++++- 5 files changed, 113 insertions(+), 59 deletions(-) diff --git a/react/features/base/popover/components/Popover.web.js b/react/features/base/popover/components/Popover.web.js index cb517d1aa..e7a011410 100644 --- a/react/features/base/popover/components/Popover.web.js +++ b/react/features/base/popover/components/Popover.web.js @@ -4,7 +4,6 @@ import InlineDialog from '@atlaskit/inline-dialog'; import React, { Component } from 'react'; import { Drawer, DrawerPortal } from '../../../toolbox/components/web'; -import { isMobileBrowser } from '../../environment/utils'; /** * A map of dialog positions, relative to trigger, to css classes used to @@ -114,11 +113,6 @@ class Popover extends Component { id: '' }; - /** - * Reference to the Popover that is meant to open as a drawer. - */ - _drawerContainerRef: Object; - /** * Initializes a new {@code Popover} instance. * @@ -136,8 +130,8 @@ class Popover extends Component { this._onHideDialog = this._onHideDialog.bind(this); this._onShowDialog = this._onShowDialog.bind(this); this._onKeyPress = this._onKeyPress.bind(this); - this._drawerContainerRef = React.createRef(); this._onEscKey = this._onEscKey.bind(this); + this._onThumbClick = this._onThumbClick.bind(this); } /** @@ -150,50 +144,6 @@ class Popover extends Component { this.setState({ showDialog: true }); } - /** - * Sets up an event listener to open a drawer when clicking, rather than entering the - * overflow area. - * - * TODO: This should be done by setting an {@code onClick} handler on the div, but for some - * reason that doesn't seem to work whatsoever. - * - * @inheritdoc - * @returns {void} - */ - componentDidMount() { - if (this._drawerContainerRef && this._drawerContainerRef.current && !isMobileBrowser()) { - this._drawerContainerRef.current.addEventListener('click', this._onShowDialog); - } - } - - /** - * Removes the listener set up in the {@code componentDidMount} method. - * - * @inheritdoc - * @returns {void} - */ - componentWillUnmount() { - if (this._drawerContainerRef && this._drawerContainerRef.current) { - this._drawerContainerRef.current.removeEventListener('click', this._onShowDialog); - } - } - - /** - * Implements React Component's componentDidUpdate. - * - * @inheritdoc - */ - componentDidUpdate(prevProps: Props) { - if (prevProps.overflowDrawer !== this.props.overflowDrawer) { - // Make sure the listeners are set up when resizing the screen past the drawer threshold. - if (this.props.overflowDrawer) { - this.componentDidMount(); - } else { - this.componentWillUnmount(); - } - } - } - /** * Implements React's {@link Component#render()}. * @@ -208,7 +158,7 @@ class Popover extends Component {
+ onClick = { this._onShowDialog }> { children } {
@@ -275,6 +226,20 @@ class Popover extends Component { } } + _onThumbClick: (Object) => void; + + /** + * Prevents switching from tile view to stage view on accidentally clicking + * the popover thumbs. + * + * @param {Object} event - The mouse event or the keypress event to intercept. + * @private + * @returns {void} + */ + _onThumbClick(event) { + event.stopPropagation(); + } + _onKeyPress: (Object) => void; /** diff --git a/react/features/toolbox/actions.web.js b/react/features/toolbox/actions.web.js index b9210742d..54fc35040 100644 --- a/react/features/toolbox/actions.web.js +++ b/react/features/toolbox/actions.web.js @@ -2,6 +2,8 @@ import type { Dispatch } from 'redux'; +import { isLayoutTileView } from '../video-layout'; + import { FULL_SCREEN_CHANGED, SET_FULL_SCREEN, @@ -11,7 +13,8 @@ import { clearToolboxTimeout, setToolboxTimeout, setToolboxTimeoutMS, - setToolboxVisible + setToolboxVisible, + setToolboxEnabled } from './actions.native'; declare var interfaceConfig: Object; @@ -163,3 +166,19 @@ export function setOverflowDrawer(displayAsDrawer: boolean) { displayAsDrawer }; } + +/** + * Disables and hides the toolbox on demand when in tile view. + * + * @returns {void} + */ +export function disableToolboxOnTileView() { + return (dispatch: Dispatch, getState: Function) => { + if (!isLayoutTileView(getState())) { + return; + } + + dispatch(setToolboxEnabled(false)); + dispatch(hideToolbox(true)); + }; +} diff --git a/react/features/toolbox/functions.web.js b/react/features/toolbox/functions.web.js index 271afa9c7..23b5961c1 100644 --- a/react/features/toolbox/functions.web.js +++ b/react/features/toolbox/functions.web.js @@ -32,7 +32,7 @@ export function isButtonEnabled(name: string, state: Object) { /** * Indicates if the toolbox is visible or not. * - * @param {string} state - The state from the Redux store. + * @param {Object} state - The state from the Redux store. * @returns {boolean} - True to indicate that the toolbox is visible, false - * otherwise. */ @@ -52,7 +52,7 @@ export function isToolboxVisible(state: Object) { /** * Indicates if the audio settings button is disabled or not. * - * @param {string} state - The state from the Redux store. + * @param {Object} state - The state from the Redux store. * @returns {boolean} */ export function isAudioSettingsButtonDisabled(state: Object) { @@ -64,7 +64,7 @@ export function isAudioSettingsButtonDisabled(state: Object) { /** * Indicates if the video settings button is disabled or not. * - * @param {string} state - The state from the Redux store. + * @param {Object} state - The state from the Redux store. * @returns {boolean} */ export function isVideoSettingsButtonDisabled(state: Object) { @@ -74,7 +74,7 @@ export function isVideoSettingsButtonDisabled(state: Object) { /** * Indicates if the video mute button is disabled or not. * - * @param {string} state - The state from the Redux store. + * @param {Object} state - The state from the Redux store. * @returns {boolean} */ export function isVideoMuteButtonDisabled(state: Object) { @@ -91,3 +91,13 @@ export function isVideoMuteButtonDisabled(state: Object) { export function showOverflowDrawer(state: Object) { return state['features/toolbox'].overflowDrawer; } + +/** + * Indicates whether the toolbox is enabled or not. + * + * @param {Object} state - The state from the Redux store. + * @returns {boolean} + */ +export function isToolboxEnabled(state: Object) { + return state['features/toolbox'].enabled; +} diff --git a/react/features/video-menu/components/web/LocalVideoMenuTriggerButton.js b/react/features/video-menu/components/web/LocalVideoMenuTriggerButton.js index 9f42db5b1..b00cd09ed 100644 --- a/react/features/video-menu/components/web/LocalVideoMenuTriggerButton.js +++ b/react/features/video-menu/components/web/LocalVideoMenuTriggerButton.js @@ -12,6 +12,8 @@ import { Popover } from '../../../base/popover'; import { connect } from '../../../base/redux'; import { getLocalVideoTrack } from '../../../base/tracks'; import ConnectionIndicatorContent from '../../../connection-indicator/components/web/ConnectionIndicatorContent'; +import { setToolboxEnabled, disableToolboxOnTileView } from '../../../toolbox/actions'; +import { isToolboxEnabled } from '../../../toolbox/functions'; import { getCurrentLayout, LAYOUTS } from '../../../video-layout'; import { renderConnectionStatus } from '../../actions.web'; @@ -63,6 +65,11 @@ type Props = { */ _showLocalVideoFlipButton: boolean, + /** + * Whether the toolbox is enabled or not. + */ + _toolboxEnabled: boolean, + /** * Invoked to obtain translated strings. */ @@ -76,6 +83,11 @@ type Props = { * @extends {Component} */ class LocalVideoMenuTriggerButton extends Component { + /** + * Preserve the intial toolbox state. + */ + initialToolboxEnabled: boolean; + /** * Reference to the Popover instance. */ @@ -91,7 +103,9 @@ class LocalVideoMenuTriggerButton extends Component { super(props); this.popoverRef = React.createRef(); + this.initialToolboxEnabled = true; this._onPopoverClose = this._onPopoverClose.bind(this); + this._onPopoverOpen = this._onPopoverOpen.bind(this); } /** @@ -115,6 +129,8 @@ class LocalVideoMenuTriggerButton extends Component { if (this.props.getRef) { this.props.getRef(this); } + + this.initialToolboxEnabled = this.props._toolboxEnabled; } /** @@ -161,6 +177,7 @@ class LocalVideoMenuTriggerButton extends Component { ? @@ -181,6 +198,17 @@ class LocalVideoMenuTriggerButton extends Component { ); } + _onPopoverOpen: () => void; + + /** + * Disable and hide toolbox while context menu is open. + * + * @returns {void} + */ + _onPopoverOpen() { + this.props.dispatch(disableToolboxOnTileView()); + } + _onPopoverClose: () => void; /** @@ -189,6 +217,7 @@ class LocalVideoMenuTriggerButton extends Component { * @returns {void} */ _onPopoverClose() { + this.props.dispatch(setToolboxEnabled(this.initialToolboxEnabled)); this.props.dispatch(renderConnectionStatus(false)); } } @@ -229,7 +258,8 @@ function _mapStateToProps(state) { _showLocalVideoFlipButton: !disableLocalVideoFlip && videoTrack?.videoType !== 'desktop', _overflowDrawer: overflowDrawer, _localParticipantId: localParticipant.id, - _showConnectionInfo: showConnectionInfo + _showConnectionInfo: showConnectionInfo, + _toolboxEnabled: isToolboxEnabled(state) }; } diff --git a/react/features/video-menu/components/web/RemoteVideoMenuTriggerButton.js b/react/features/video-menu/components/web/RemoteVideoMenuTriggerButton.js index 72d5e59a1..7872331b8 100644 --- a/react/features/video-menu/components/web/RemoteVideoMenuTriggerButton.js +++ b/react/features/video-menu/components/web/RemoteVideoMenuTriggerButton.js @@ -11,6 +11,8 @@ import { getLocalParticipant, getParticipantById, PARTICIPANT_ROLE } from '../.. import { Popover } from '../../../base/popover'; import { connect } from '../../../base/redux'; import { requestRemoteControl, stopController } from '../../../remote-control'; +import { setToolboxEnabled, disableToolboxOnTileView } from '../../../toolbox/actions'; +import { isToolboxEnabled } from '../../../toolbox/functions'; import { getCurrentLayout, LAYOUTS } from '../../../video-layout'; import { renderConnectionStatus } from '../../actions.web'; @@ -76,6 +78,11 @@ type Props = { */ _remoteControlState: number, + /** + * Whether the toolbox is enabled or not. + */ + _toolboxEnabled: boolean, + /** * The redux dispatch function. */ @@ -126,6 +133,11 @@ type Props = { * @extends {Component} */ class RemoteVideoMenuTriggerButton extends Component { + /** + * Preserve the intial toolbox state. + */ + initialToolboxEnabled: boolean; + /** * Reference to the Popover instance. */ @@ -141,7 +153,9 @@ class RemoteVideoMenuTriggerButton extends Component { super(props); this.popoverRef = React.createRef(); + this.initialToolboxEnabled = true; this._onPopoverClose = this._onPopoverClose.bind(this); + this._onPopoverOpen = this._onPopoverOpen.bind(this); } /** @@ -165,6 +179,8 @@ class RemoteVideoMenuTriggerButton extends Component { if (this.props.getRef) { this.props.getRef(this); } + + this.initialToolboxEnabled = this.props._toolboxEnabled; } /** @@ -201,6 +217,7 @@ class RemoteVideoMenuTriggerButton extends Component { @@ -219,6 +236,17 @@ class RemoteVideoMenuTriggerButton extends Component { ); } + _onPopoverOpen: () => void; + + /** + * Disable and hide toolbox while context menu is open. + * + * @returns {void} + */ + _onPopoverOpen() { + this.props.dispatch(disableToolboxOnTileView()); + } + _onPopoverClose: () => void; /** @@ -227,6 +255,7 @@ class RemoteVideoMenuTriggerButton extends Component { * @returns {void} */ _onPopoverClose() { + this.props.dispatch(setToolboxEnabled(this.initialToolboxEnabled)); this.props.dispatch(renderConnectionStatus(false)); } @@ -406,7 +435,8 @@ function _mapStateToProps(state, ownProps) { _overflowDrawer: overflowDrawer, _participantDisplayName, _disableGrantModerator: Boolean(disableGrantModerator), - _showConnectionInfo: showConnectionInfo + _showConnectionInfo: showConnectionInfo, + _toolboxEnabled: isToolboxEnabled(state) }; }