fix(context-menus) Fix participant context menus/toolbar overflow menu

- on ipads, long touch open dialog now opens the context menu to the left of the thumbnail as expected
- on ipads, now we close context menus on tap out
- fix case when participant context menu's height > tileview videos' height causing scroll on videos pane
- keep toolbox open while the overflow menu is shown
- keep remote participant video thumbnail in filmstrip visible even if toolbox is hidden, if context menu is opened
- Fix bug where toolbox could be completely disabled
This commit is contained in:
Horatiu Muresan 2021-09-14 10:43:52 +03:00 committed by GitHub
parent 1add438a1f
commit b801e0115d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 161 additions and 75 deletions

View File

@ -54,4 +54,13 @@ export const CONNECTION_WILL_CONNECT = 'CONNECTION_WILL_CONNECT';
*/
export const SET_LOCATION_URL = 'SET_LOCATION_URL';
/**
* The type of (redux) action which tells whether connection info should be displayed
* on context menu.
*
* {
* type: SHOW_CONNECTION_INFO,
* showConnectionInfo: boolean
* }
*/
export const SHOW_CONNECTION_INFO = 'SHOW_CONNECTION_INFO';

View File

@ -113,6 +113,12 @@ class Popover extends Component<Props, State> {
id: ''
};
/**
* Reference to the dialog container.
*/
_containerRef: Object;
/**
* Initializes a new {@code Popover} instance.
*
@ -130,8 +136,10 @@ class Popover extends Component<Props, State> {
this._onHideDialog = this._onHideDialog.bind(this);
this._onShowDialog = this._onShowDialog.bind(this);
this._onKeyPress = this._onKeyPress.bind(this);
this._containerRef = React.createRef();
this._onEscKey = this._onEscKey.bind(this);
this._onThumbClick = this._onThumbClick.bind(this);
this._onTouchStart = this._onTouchStart.bind(this);
}
/**
@ -141,7 +149,27 @@ class Popover extends Component<Props, State> {
* @public
*/
showDialog() {
this.setState({ showDialog: true });
this._onShowDialog();
}
/**
* Sets up a touch event listener to attach.
*
* @inheritdoc
* @returns {void}
*/
componentDidMount() {
window.addEventListener('touchstart', this._onTouchStart);
}
/**
* Removes the listener set up in the {@code componentDidMount} method.
*
* @inheritdoc
* @returns {void}
*/
componentWillUnmount() {
window.removeEventListener('touchstart', this._onTouchStart);
}
/**
@ -178,7 +206,8 @@ class Popover extends Component<Props, State> {
onClick = { this._onThumbClick }
onKeyPress = { this._onKeyPress }
onMouseEnter = { this._onShowDialog }
onMouseLeave = { this._onHideDialog }>
onMouseLeave = { this._onHideDialog }
ref = { this._containerRef }>
<InlineDialog
content = { this._renderContent() }
isOpen = { this.state.showDialog }
@ -189,6 +218,25 @@ class Popover extends Component<Props, State> {
);
}
_onTouchStart: (event: TouchEvent) => void;
/**
* Hide dialog on touch outside of the context menu.
*
* @param {TouchEvent} event - The touch event.
* @private
* @returns {void}
*/
_onTouchStart(event) {
if (this.state.showDialog
&& !this.props.overflowDrawer
&& this._containerRef
&& this._containerRef.current
&& !this._containerRef.current.contains(event.target)) {
this._onHideDialog();
}
}
_onHideDialog: () => void;
/**
@ -216,7 +264,7 @@ class Popover extends Component<Props, State> {
* @returns {void}
*/
_onShowDialog(event) {
event.stopPropagation();
event && event.stopPropagation();
if (!this.props.disablePopover) {
this.setState({ showDialog: true });

View File

@ -30,3 +30,15 @@ export const SET_ASPECT_RATIO = 'SET_ASPECT_RATIO';
* @public
*/
export const SET_REDUCED_UI = 'SET_REDUCED_UI';
/**
* The type of (redux) action which tells whether a local or remote participant
* context menu is open.
*
* {
* type: SET_CONTEXT_MENU_OPEN,
* showConnectionInfo: boolean
* }
*/
export const SET_CONTEXT_MENU_OPEN = 'SET_CONTEXT_MENU_OPEN';

View File

@ -7,7 +7,7 @@ import { CHAT_SIZE } from '../../chat/constants';
import { getParticipantsPaneOpen } from '../../participants-pane/functions';
import theme from '../../participants-pane/theme.json';
import { CLIENT_RESIZED, SET_ASPECT_RATIO, SET_REDUCED_UI } from './actionTypes';
import { CLIENT_RESIZED, SET_ASPECT_RATIO, SET_CONTEXT_MENU_OPEN, SET_REDUCED_UI } from './actionTypes';
import { ASPECT_RATIO_NARROW, ASPECT_RATIO_WIDE } from './constants';
/**
@ -110,3 +110,16 @@ export function setReducedUI(width: number, height: number): Function {
}
};
}
/**
* Sets whether the local or remote participant context menu is open.
*
* @param {boolean} isOpen - Whether local or remote context menu is open.
* @returns {Object}
*/
export function setParticipantContextMenuOpen(isOpen: boolean) {
return {
type: SET_CONTEXT_MENU_OPEN,
isOpen
};
}

View File

@ -2,7 +2,7 @@
import { ReducerRegistry, set } from '../redux';
import { CLIENT_RESIZED, SET_ASPECT_RATIO, SET_REDUCED_UI } from './actionTypes';
import { CLIENT_RESIZED, SET_ASPECT_RATIO, SET_CONTEXT_MENU_OPEN, SET_REDUCED_UI } from './actionTypes';
import { ASPECT_RATIO_NARROW } from './constants';
const {
@ -17,7 +17,8 @@ const DEFAULT_STATE = {
aspectRatio: ASPECT_RATIO_NARROW,
clientHeight: innerHeight,
clientWidth: innerWidth,
reducedUI: false
reducedUI: false,
contextMenuOpened: false
};
ReducerRegistry.register('features/base/responsive-ui', (state = DEFAULT_STATE, action) => {
@ -34,6 +35,9 @@ ReducerRegistry.register('features/base/responsive-ui', (state = DEFAULT_STATE,
case SET_REDUCED_UI:
return set(state, 'reducedUI', action.reducedUI);
case SET_CONTEXT_MENU_OPEN:
return set(state, 'contextMenuOpened', action.isOpen);
}
return state;

View File

@ -34,6 +34,11 @@ import ThumbnailWrapper from './ThumbnailWrapper';
declare var APP: Object;
declare var interfaceConfig: Object;
/**
* Fixes case in which context menu overflows and creates a scroll on the whole filmstrip videos pane.
*/
const TILEVIEW_VIDEO_PANES_STYLE = { overflow: 'visible' };
/**
* The type of the React {@code Component} props of {@link Filmstrip}.
*/
@ -386,6 +391,7 @@ class Filmstrip extends PureComponent <Props> {
overscanRowCount = { 1 }
rowCount = { _rows }
rowHeight = { _thumbnailHeight + TILE_VERTICAL_MARGIN }
style = { TILEVIEW_VIDEO_PANES_STYLE }
width = { _filmstripWidth }>
{
ThumbnailWrapper

View File

@ -70,9 +70,11 @@ export function shouldRemoteVideosBeVisible(state: Object) {
const participantCount = getParticipantCountWithFake(state);
let pinnedParticipant;
const { disable1On1Mode } = state['features/base/config'];
const { contextMenuOpened } = state['features/base/responsive-ui'];
return Boolean(
participantCount > 2
contextMenuOpened
|| participantCount > 2
// Always show the filmstrip when there is another participant to
// show and the local video is pinned, or the toolbar is displayed.

View File

@ -13,8 +13,7 @@ import {
clearToolboxTimeout,
setToolboxTimeout,
setToolboxTimeoutMS,
setToolboxVisible,
setToolboxEnabled
setToolboxVisible
} from './actions.native';
declare var interfaceConfig: Object;
@ -132,10 +131,13 @@ export function showToolbox(timeout: number = 0): Object {
alwaysVisible,
enabled,
timeoutMS,
visible
visible,
overflowDrawer
} = state['features/toolbox'];
const { contextMenuOpened } = state['features/base/responsive-ui'];
const contextMenuOpenedInTileview = isLayoutTileView(state) && contextMenuOpened && !overflowDrawer;
if (enabled && !visible) {
if (enabled && !visible && !contextMenuOpenedInTileview) {
dispatch(setToolboxVisible(true));
// If the Toolbox is always visible, there's no need for a timeout
@ -167,18 +169,20 @@ export function setOverflowDrawer(displayAsDrawer: boolean) {
};
}
/**
* Disables and hides the toolbox on demand when in tile view.
*
* @returns {void}
*/
export function disableToolboxOnTileView() {
export function hideToolboxOnTileView() {
return (dispatch: Dispatch<any>, getState: Function) => {
if (!isLayoutTileView(getState())) {
return;
}
const state = getState();
const { overflowDrawer } = state['features/toolbox'];
dispatch(setToolboxEnabled(false));
if (!overflowDrawer && isLayoutTileView(state)) {
dispatch(hideToolbox(true));
}
};
}

View File

@ -389,29 +389,27 @@ class Toolbox extends Component<Props, State> {
* @inheritdoc
*/
componentDidUpdate(prevProps) {
// Ensure the dialog is closed when the toolbox becomes hidden.
if (prevProps._overflowMenuVisible && !this.props._visible) {
this._onSetOverflowVisible(false);
}
const { _dialog, _reactionsEnabled, _participantCount, dispatch, t } = this.props;
if (prevProps._overflowMenuVisible
&& !prevProps._dialog
&& this.props._dialog) {
&& _dialog) {
this._onSetOverflowVisible(false);
this.props.dispatch(setToolbarHovered(false));
dispatch(setToolbarHovered(false));
}
if (!this.state.reactionsShortcutsRegistered
&& (prevProps._reactionsEnabled !== this.props._reactionsEnabled
|| prevProps._participantCount !== this.props._participantCount)) {
if (this.props._reactionsEnabled && this.props._participantCount > 1) {
&& (prevProps._reactionsEnabled !== _reactionsEnabled
|| prevProps._participantCount !== _participantCount)) {
if (_reactionsEnabled && _participantCount > 1) {
// eslint-disable-next-line react/no-did-update-set-state
this.setState({
reactionsShortcutsRegistered: true
});
const REACTION_SHORTCUTS = Object.keys(REACTIONS).map(key => {
const onShortcutSendReaction = () => {
this.props.dispatch(addReactionToBuffer(key));
dispatch(addReactionToBuffer(key));
sendAnalytics(createShortcutEvent(
`reaction.${key}`
));
@ -420,7 +418,7 @@ class Toolbox extends Component<Props, State> {
return {
character: REACTIONS[key].shortcutChar,
exec: onShortcutSendReaction,
helpDescription: this.props.t(`toolbar.reaction${key.charAt(0).toUpperCase()}${key.slice(1)}`),
helpDescription: t(`toolbar.reaction${key.charAt(0).toUpperCase()}${key.slice(1)}`),
altKey: true
};
});
@ -911,7 +909,9 @@ class Toolbox extends Component<Props, State> {
* @returns {void}
*/
_onMouseOut() {
this.props.dispatch(setToolbarHovered(false));
const { _overflowMenuVisible, dispatch } = this.props;
!_overflowMenuVisible && dispatch(setToolbarHovered(false));
}
_onMouseOver: () => void;
@ -939,6 +939,7 @@ class Toolbox extends Component<Props, State> {
*/
_onSetOverflowVisible(visible) {
this.props.dispatch(setOverflowMenuVisible(visible));
this.props.dispatch(setToolbarHovered(visible));
}
_onShortcutToggleChat: () => void;

View File

@ -1,6 +1,7 @@
// @flow
import React, { Component } from 'react';
import { batch } from 'react-redux';
import { isMobileBrowser } from '../../../base/environment/utils';
import { translate } from '../../../base/i18n';
@ -10,10 +11,10 @@ import {
} from '../../../base/participants';
import { Popover } from '../../../base/popover';
import { connect } from '../../../base/redux';
import { setParticipantContextMenuOpen } from '../../../base/responsive-ui/actions';
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 { hideToolboxOnTileView } from '../../../toolbox/actions';
import { getCurrentLayout, LAYOUTS } from '../../../video-layout';
import { renderConnectionStatus } from '../../actions.web';
@ -65,11 +66,6 @@ type Props = {
*/
_showLocalVideoFlipButton: boolean,
/**
* Whether the toolbox is enabled or not.
*/
_toolboxEnabled: boolean,
/**
* Invoked to obtain translated strings.
*/
@ -83,11 +79,6 @@ type Props = {
* @extends {Component}
*/
class LocalVideoMenuTriggerButton extends Component<Props> {
/**
* Preserve the intial toolbox state.
*/
initialToolboxEnabled: boolean;
/**
* Reference to the Popover instance.
*/
@ -103,7 +94,6 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
super(props);
this.popoverRef = React.createRef();
this.initialToolboxEnabled = true;
this._onPopoverClose = this._onPopoverClose.bind(this);
this._onPopoverOpen = this._onPopoverOpen.bind(this);
}
@ -129,8 +119,6 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
if (this.props.getRef) {
this.props.getRef(this);
}
this.initialToolboxEnabled = this.props._toolboxEnabled;
}
/**
@ -181,16 +169,17 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
overflowDrawer = { _overflowDrawer }
position = { _menuPosition }
ref = { this.popoverRef }>
{!isMobileBrowser() && (
{!_overflowDrawer && (
<span
className = 'popover-trigger local-video-menu-trigger'>
<Icon
{!isMobileBrowser() && <Icon
ariaLabel = { t('dialog.localUserControls') }
role = 'button'
size = '1em'
src = { IconMenuThumb }
tabIndex = { 0 }
title = { t('dialog.localUserControls') } />
}
</span>
)}
</Popover>
@ -206,7 +195,8 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverOpen() {
this.props.dispatch(disableToolboxOnTileView());
this.props.dispatch(setParticipantContextMenuOpen(true));
this.props.dispatch(hideToolboxOnTileView());
}
_onPopoverClose: () => void;
@ -217,8 +207,12 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverClose() {
this.props.dispatch(setToolboxEnabled(this.initialToolboxEnabled));
this.props.dispatch(renderConnectionStatus(false));
const { dispatch } = this.props;
batch(() => {
dispatch(setParticipantContextMenuOpen(false));
dispatch(renderConnectionStatus(false));
});
}
}
@ -258,8 +252,7 @@ function _mapStateToProps(state) {
_showLocalVideoFlipButton: !disableLocalVideoFlip && videoTrack?.videoType !== 'desktop',
_overflowDrawer: overflowDrawer,
_localParticipantId: localParticipant.id,
_showConnectionInfo: showConnectionInfo,
_toolboxEnabled: isToolboxEnabled(state)
_showConnectionInfo: showConnectionInfo
};
}

View File

@ -1,6 +1,7 @@
// @flow
import React, { Component } from 'react';
import { batch } from 'react-redux';
import ConnectionIndicatorContent from
'../../../../features/connection-indicator/components/web/ConnectionIndicatorContent';
@ -10,9 +11,9 @@ import { Icon, IconMenuThumb } from '../../../base/icons';
import { getLocalParticipant, getParticipantById, PARTICIPANT_ROLE } from '../../../base/participants';
import { Popover } from '../../../base/popover';
import { connect } from '../../../base/redux';
import { setParticipantContextMenuOpen } from '../../../base/responsive-ui/actions';
import { requestRemoteControl, stopController } from '../../../remote-control';
import { setToolboxEnabled, disableToolboxOnTileView } from '../../../toolbox/actions';
import { isToolboxEnabled } from '../../../toolbox/functions';
import { hideToolboxOnTileView } from '../../../toolbox/actions';
import { getCurrentLayout, LAYOUTS } from '../../../video-layout';
import { renderConnectionStatus } from '../../actions.web';
@ -78,11 +79,6 @@ type Props = {
*/
_remoteControlState: number,
/**
* Whether the toolbox is enabled or not.
*/
_toolboxEnabled: boolean,
/**
* The redux dispatch function.
*/
@ -133,11 +129,6 @@ type Props = {
* @extends {Component}
*/
class RemoteVideoMenuTriggerButton extends Component<Props> {
/**
* Preserve the intial toolbox state.
*/
initialToolboxEnabled: boolean;
/**
* Reference to the Popover instance.
*/
@ -153,7 +144,6 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
super(props);
this.popoverRef = React.createRef();
this.initialToolboxEnabled = true;
this._onPopoverClose = this._onPopoverClose.bind(this);
this._onPopoverOpen = this._onPopoverOpen.bind(this);
}
@ -179,8 +169,6 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
if (this.props.getRef) {
this.props.getRef(this);
}
this.initialToolboxEnabled = this.props._toolboxEnabled;
}
/**
@ -202,7 +190,7 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {ReactElement}
*/
render() {
const { _showConnectionInfo, _participantDisplayName, participantID } = this.props;
const { _overflowDrawer, _showConnectionInfo, _participantDisplayName, participantID } = this.props;
const content = _showConnectionInfo
? <ConnectionIndicatorContent participantId = { participantID } />
: this._renderRemoteVideoMenu();
@ -218,18 +206,19 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
content = { content }
onPopoverClose = { this._onPopoverClose }
onPopoverOpen = { this._onPopoverOpen }
overflowDrawer = { this.props._overflowDrawer }
overflowDrawer = { _overflowDrawer }
position = { this.props._menuPosition }
ref = { this.popoverRef }>
{!isMobileBrowser() && (
{!_overflowDrawer && (
<span className = 'popover-trigger remote-video-menu-trigger'>
<Icon
{!isMobileBrowser() && <Icon
ariaLabel = { this.props.t('dialog.remoteUserControls', { username }) }
role = 'button'
size = '1.4em'
src = { IconMenuThumb }
tabIndex = { 0 }
title = { this.props.t('dialog.remoteUserControls', { username }) } />
}
</span>
)}
</Popover>
@ -244,7 +233,8 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverOpen() {
this.props.dispatch(disableToolboxOnTileView());
this.props.dispatch(setParticipantContextMenuOpen(true));
this.props.dispatch(hideToolboxOnTileView());
}
_onPopoverClose: () => void;
@ -255,8 +245,12 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverClose() {
this.props.dispatch(setToolboxEnabled(this.initialToolboxEnabled));
this.props.dispatch(renderConnectionStatus(false));
const { dispatch } = this.props;
batch(() => {
dispatch(setParticipantContextMenuOpen(false));
dispatch(renderConnectionStatus(false));
});
}
/**
@ -349,6 +343,7 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
if (isMobileBrowser()) {
buttons.push(
<ConnectionStatusButton
key = 'conn-status'
participantId = { participantID } />
);
}
@ -435,8 +430,7 @@ function _mapStateToProps(state, ownProps) {
_overflowDrawer: overflowDrawer,
_participantDisplayName,
_disableGrantModerator: Boolean(disableGrantModerator),
_showConnectionInfo: showConnectionInfo,
_toolboxEnabled: isToolboxEnabled(state)
_showConnectionInfo: showConnectionInfo
};
}