fix(Drawer): Close drawer on item click

Clicking on an item when the popup drawer is displayed would keep it open.
Now clicking on any item should automatically close the drawer.

Popup was also refactored and no longer uses refs.
This commit is contained in:
Vlad Piersec 2021-11-01 11:39:19 +02:00
parent fc69c61d04
commit a31d176074
12 changed files with 247 additions and 183 deletions

View File

@ -11,12 +11,6 @@
{
@extend %connection-info;
/**
* Apply negative margin to reduce the appearance of padding in AtlasKit
* InlineDialog.
*/
margin: -15px;
> table {
white-space: nowrap;
@extend %connection-info;

View File

@ -46,3 +46,7 @@
padding: 16px 24px;
z-index: $popoverZ;
}
.padded-content {
padding: 4px 8px;
}

View File

@ -5,6 +5,7 @@
.popupmenu {
background-color: $menuBG;
border-radius: 3px;
list-style-type: none;
min-width: 150px;
text-align: left;
padding: 0px;
@ -38,6 +39,11 @@
}
}
&__list {
margin: 0;
padding: 0;
}
&__text {
display: inline-block;
margin-left: 8px;

View File

@ -1,9 +1,10 @@
/* @flow */
import clsx from 'clsx';
import React, { Component } from 'react';
import { Drawer, JitsiPortal, DialogPortal } from '../../../toolbox/components/web';
import { isMobileBrowser } from '../../environment/utils';
import { connect } from '../../redux';
import { getContextMenuStyle } from '../functions.web';
/**
@ -57,7 +58,17 @@ type Props = {
* From which side of the dialog trigger the dialog should display. The
* value will be passed to {@code InlineDialog}.
*/
position: string
position: string,
/**
* Whether the content show have some padding.
*/
paddedContent: ?boolean,
/**
* Whether the popover is visible or not.
*/
visible: boolean
};
/**
@ -68,12 +79,7 @@ type State = {
/**
* The style to apply to the context menu in order to position it correctly.
*/
contextMenuStyle: Object,
/**
* Whether or not the {@code InlineDialog} should be displayed.
*/
showDialog: boolean
contextMenuStyle: Object
};
/**
@ -110,7 +116,6 @@ class Popover extends Component<Props, State> {
super(props);
this.state = {
showDialog: false,
contextMenuStyle: null
};
@ -127,16 +132,6 @@ class Popover extends Component<Props, State> {
this._getCustomDialogStyle = this._getCustomDialogStyle.bind(this);
}
/**
* Public method for triggering showing the context menu dialog.
*
* @returns {void}
* @public
*/
showDialog() {
this._onShowDialog();
}
/**
* Sets up a touch event listener to attach.
*
@ -164,7 +159,7 @@ class Popover extends Component<Props, State> {
* @returns {ReactElement}
*/
render() {
const { children, className, content, id, overflowDrawer } = this.props;
const { children, className, content, id, overflowDrawer, visible } = this.props;
if (overflowDrawer) {
return (
@ -175,7 +170,7 @@ class Popover extends Component<Props, State> {
{ children }
<JitsiPortal>
<Drawer
isOpen = { this.state.showDialog }
isOpen = { visible }
onClose = { this._onHideDialog }>
{ content }
</Drawer>
@ -193,7 +188,7 @@ class Popover extends Component<Props, State> {
onMouseEnter = { this._onShowDialog }
onMouseLeave = { this._onHideDialog }
ref = { this._containerRef }>
{ this.state.showDialog && (
{ visible && (
<DialogPortal
getRef = { this._setContextMenuRef }
setSize = { this._setContextMenuStyle }
@ -244,7 +239,7 @@ class Popover extends Component<Props, State> {
* @returns {void}
*/
_onTouchStart(event) {
if (this.state.showDialog
if (this.props.visible
&& !this.props.overflowDrawer
&& this._contextMenuRef
&& this._contextMenuRef.contains
@ -263,7 +258,6 @@ class Popover extends Component<Props, State> {
*/
_onHideDialog() {
this.setState({
showDialog: false,
contextMenuStyle: null
});
@ -284,12 +278,9 @@ class Popover extends Component<Props, State> {
*/
_onShowDialog(event) {
event && event.stopPropagation();
if (!this.props.disablePopover) {
this.setState({ showDialog: true });
if (this.props.onPopoverOpen) {
this.props.onPopoverOpen();
}
if (!this.props.disablePopover) {
this.props.onPopoverOpen();
}
}
@ -319,7 +310,7 @@ class Popover extends Component<Props, State> {
_onKeyPress(e) {
if (e.key === ' ' || e.key === 'Enter') {
e.preventDefault();
if (this.state.showDialog) {
if (this.props.visible) {
this._onHideDialog();
} else {
this._onShowDialog(e);
@ -340,7 +331,7 @@ class Popover extends Component<Props, State> {
if (e.key === 'Escape') {
e.preventDefault();
e.stopPropagation();
if (this.state.showDialog) {
if (this.props.visible) {
this._onHideDialog();
}
}
@ -373,11 +364,15 @@ class Popover extends Component<Props, State> {
* @returns {ReactElement}
*/
_renderContent() {
const { content } = this.props;
const { content, paddedContent } = this.props;
const className = clsx(
'popover popupmenu',
paddedContent && 'padded-content'
);
return (
<div
className = 'popover popupmenu'
className = { className }
onKeyDown = { this._onEscKey }>
{ content }
{!isMobileBrowser() && (
@ -392,4 +387,18 @@ class Popover extends Component<Props, State> {
}
}
export default Popover;
/**
* Maps (parts of) the Redux state to the associated {@code Popover}'s props.
*
* @param {Object} state - The Redux state.
* @param {Object} ownProps - The own props of the component.
* @private
* @returns {Props}
*/
function _mapStateToProps(state) {
return {
overflowDrawer: state['features/toolbox'].overflowDrawer
};
}
export default connect(_mapStateToProps)(Popover);

View File

@ -109,13 +109,21 @@ type Props = AbstractProps & {
t: Function,
};
type State = AbstractState & {
/**
* Whether popover is ivisible or not.
*/
popoverVisible: boolean
}
/**
* Implements a React {@link Component} which displays the current connection
* quality percentage and has a popover to show more detailed connection stats.
*
* @extends {Component}
*/
class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractState> {
class ConnectionIndicator extends AbstractConnectionIndicator<Props, State> {
/**
* Initializes a new {@code ConnectionIndicator} instance.
*
@ -126,10 +134,12 @@ class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractSta
super(props);
this.state = {
autoHideTimeout: undefined,
showIndicator: false,
stats: {}
stats: {},
popoverVisible: false
};
this._onShowPopover = this._onShowPopover.bind(this);
this._onHidePopover = this._onHidePopover.bind(this);
}
/**
@ -139,6 +149,7 @@ class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractSta
* @returns {ReactElement}
*/
render() {
const { iconSize, enableStatsDisplay, participantId, statsPopoverPosition } = this.props;
const visibilityClass = this._getVisibilityClass();
const rootClassNames = `indicator-container ${visibilityClass}`;
@ -151,13 +162,19 @@ class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractSta
className = { rootClassNames }
content = { <ConnectionIndicatorContent
inheritedStats = { this.state.stats }
participantId = { this.props.participantId } /> }
disablePopover = { !this.props.enableStatsDisplay }
position = { this.props.statsPopoverPosition }>
participantId = { participantId } /> }
disablePopover = { !enableStatsDisplay }
id = 'participant-connection-indicator'
noPaddingContent = { true }
onPopoverClose = { this._onHidePopover }
onPopoverOpen = { this._onShowPopover }
paddedContent = { true }
position = { statsPopoverPosition }
visible = { this.state.popoverVisible }>
<div className = 'popover-trigger'>
<div
className = { indicatorContainerClassNames }
style = {{ fontSize: this.props.iconSize }}>
style = {{ fontSize: iconSize }}>
<div className = 'connection indicatoricon'>
{ this._renderIcon() }
</div>
@ -222,6 +239,18 @@ class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractSta
? 'show-connection-indicator' : 'hide-connection-indicator';
}
_onHidePopover: () => void;
/**
* Hides popover.
*
* @private
* @returns {void}
*/
_onHidePopover() {
this.setState({ popoverVisible: false });
}
/**
* Creates a ReactElement for displaying an icon that represents the current
* connection quality.
@ -282,6 +311,18 @@ class ConnectionIndicator extends AbstractConnectionIndicator<Props, AbstractSta
</span>
];
}
_onShowPopover: () => void;
/**
* Shows popover.
*
* @private
* @returns {void}
*/
_onShowPopover() {
this.setState({ popoverVisible: true });
}
}
/**

View File

@ -66,7 +66,12 @@ export type State = {|
/**
* Indicates whether the thumbnail is hovered or not.
*/
isHovered: boolean
isHovered: boolean,
/**
* Whether popover is visible or not.
*/
popoverVisible: boolean
|};
/**
@ -277,18 +282,19 @@ class Thumbnail extends Component<Props, State> {
audioLevel: 0,
canPlayEventReceived: false,
isHovered: false,
displayMode: DISPLAY_VIDEO
displayMode: DISPLAY_VIDEO,
popoverVisible: false
};
this.state = {
...state,
displayMode: computeDisplayMode(Thumbnail.getDisplayModeInput(props, state))
displayMode: computeDisplayMode(Thumbnail.getDisplayModeInput(props, state)),
popoverVisible: false
};
this.timeoutHandle = null;
this.videoMenuTriggerRef = null;
this._clearDoubleClickTimeout = this._clearDoubleClickTimeout.bind(this);
this._setInstance = this._setInstance.bind(this);
this._updateAudioLevel = this._updateAudioLevel.bind(this);
this._onCanPlay = this._onCanPlay.bind(this);
this._onClick = this._onClick.bind(this);
@ -299,7 +305,8 @@ class Thumbnail extends Component<Props, State> {
this._onTouchStart = this._onTouchStart.bind(this);
this._onTouchEnd = this._onTouchEnd.bind(this);
this._onTouchMove = this._onTouchMove.bind(this);
this._showPopupMenu = this._showPopupMenu.bind(this);
this._showPopover = this._showPopover.bind(this);
this._hidePopover = this._hidePopover.bind(this);
}
/**
@ -503,6 +510,34 @@ class Thumbnail extends Component<Props, State> {
});
}
_showPopover: () => void;
/**
* Shows popover.
*
* @private
* @returns {void}
*/
_showPopover() {
this.setState({
popoverVisible: true
});
}
_hidePopover: () => void;
/**
* Hides popover.
*
* @private
* @returns {void}
*/
_hidePopover() {
this.setState({
popoverVisible: false
});
}
/**
* Returns an object with the styles for thumbnail.
*
@ -583,19 +618,6 @@ class Thumbnail extends Component<Props, State> {
this.setState({ isHovered: false });
}
_showPopupMenu: () => void;
/**
* Triggers showing the popover context menu.
*
* @returns {void}
*/
_showPopupMenu() {
if (this.videoMenuTriggerRef) {
this.videoMenuTriggerRef.showContextMenu();
}
}
_onTouchStart: () => void;
/**
@ -604,7 +626,7 @@ class Thumbnail extends Component<Props, State> {
* @returns {void}
*/
_onTouchStart() {
this.timeoutHandle = setTimeout(this._showPopupMenu, SHOW_TOOLBAR_CONTEXT_MENU_AFTER);
this.timeoutHandle = setTimeout(this._showPopover, SHOW_TOOLBAR_CONTEXT_MENU_AFTER);
if (this._firstTap) {
this._clearDoubleClickTimeout();
@ -860,7 +882,9 @@ class Thumbnail extends Component<Props, State> {
</span>
<span className = 'localvideomenu'>
<LocalVideoMenuTriggerButton
getRef = { this._setInstance } />
hidePopover = { this._hidePopover }
popoverVisible = { this.state.popoverVisible }
showPopover = { this._showPopover } />
</span>
</span>
@ -906,19 +930,6 @@ class Thumbnail extends Component<Props, State> {
dispatch(updateLastTrackVideoMediaEvent(jitsiVideoTrack, event.type));
}
_setInstance: Object => void;
/**
* Stores the local or remote video menu button instance in a variable.
*
* @param {Object} instance - The local or remote video menu trigger instance.
*
* @returns {void}
*/
_setInstance(instance) {
this.videoMenuTriggerRef = instance;
}
/**
* Renders a remote participant's 'thumbnail.
*
@ -1005,10 +1016,12 @@ class Thumbnail extends Component<Props, State> {
</span>
<span className = 'remotevideomenu'>
<RemoteVideoMenuTriggerButton
getRef = { this._setInstance }
hidePopover = { this._hidePopover }
initialVolumeValue = { _volume }
onVolumeChange = { onVolumeChange }
participantID = { id } />
participantID = { id }
popoverVisible = { this.state.popoverVisible }
showPopover = { this._showPopover } />
</span>
</span>
);

View File

@ -308,10 +308,9 @@ class MeetingParticipantContextMenu extends Component<Props, State> {
* @returns {void}
*/
_onSendPrivateMessage() {
const { closeDrawer, dispatch, overflowDrawer } = this.props;
const { dispatch } = this.props;
dispatch(openChatById(this._getCurrentParticipantId()));
overflowDrawer && closeDrawer();
}
_position: () => void;
@ -444,7 +443,7 @@ class MeetingParticipantContextMenu extends Component<Props, State> {
) : (
<>
{_isLocalModerator && (
<ContextMenuItemGroup>
<ContextMenuItemGroup onClick = { closeDrawer }>
<>
{overflowDrawer && (_isAudioForceMuted || _isVideoForceMuted)
&& <ContextMenuItem onClick = { this._onAskToUnmute }>
@ -481,7 +480,7 @@ class MeetingParticipantContextMenu extends Component<Props, State> {
</ContextMenuItemGroup>
)}
<ContextMenuItemGroup>
<ContextMenuItemGroup onClick = { closeDrawer }>
{
_isLocalModerator && (
<>

View File

@ -1,5 +1,5 @@
// @flow
import { batch } from 'react-redux';
import type { Dispatch } from 'redux';
import {
@ -11,6 +11,7 @@ import { translate } from '../../base/i18n';
import { IconTileView } from '../../base/icons';
import { connect } from '../../base/redux';
import { AbstractButton, type AbstractButtonProps } from '../../base/toolbox/components';
import { setOverflowMenuVisible } from '../../toolbox/actions';
import { setTileView } from '../actions';
import { shouldDisplayTileView } from '../functions';
import logger from '../logger';
@ -68,7 +69,11 @@ class TileViewButton<P: Props> extends AbstractButton<P, *> {
}));
logger.debug(`Tile view ${value ? 'enable' : 'disable'}`);
dispatch(setTileView(value));
batch(() => {
dispatch(setTileView(value));
dispatch(setOverflowMenuVisible(false));
});
}
/**

View File

@ -23,6 +23,11 @@ type Props = {
*/
dispatch: Function,
/**
* Click handler executed aside from the main action.
*/
onClick?: Function,
/**
* Invoked to obtain translated strings.
*/
@ -77,8 +82,9 @@ class FlipLocalVideoButton extends PureComponent<Props> {
* @returns {void}
*/
_onClick() {
const { _localFlipX, dispatch } = this.props;
const { _localFlipX, dispatch, onClick } = this.props;
onClick && onClick();
dispatch(updateSettings({
localFlipX: !_localFlipX
}));

View File

@ -38,6 +38,21 @@ type Props = {
*/
getRef: Function,
/**
* Hides popover.
*/
hidePopover: Function,
/**
* Whether the popover is visible or not.
*/
popoverVisible: boolean,
/**
* Shows popover.
*/
showPopover: Function,
/**
* The id of the local participant.
*/
@ -78,10 +93,6 @@ type Props = {
* @extends {Component}
*/
class LocalVideoMenuTriggerButton extends Component<Props> {
/**
* Reference to the Popover instance.
*/
popoverRef: Object;
/**
* Initializes a new LocalVideoMenuTriggerButton instance.
@ -92,45 +103,10 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
constructor(props: Props) {
super(props);
this.popoverRef = React.createRef();
this._onPopoverClose = this._onPopoverClose.bind(this);
this._onPopoverOpen = this._onPopoverOpen.bind(this);
}
/**
* Triggers showing the popover's context menu.
*
* @returns {void}
*/
showContextMenu() {
if (this.popoverRef && this.popoverRef.current) {
this.popoverRef.current.showDialog();
}
}
/**
* Calls the ref(instance) getter.
*
* @inheritdoc
* @returns {void}
*/
componentDidMount() {
if (this.props.getRef) {
this.props.getRef(this);
}
}
/**
* Calls the ref(instance) getter.
*
* @inheritdoc
* @returns {void}
*/
componentWillUnmount() {
if (this.props.getRef) {
this.props.getRef(null);
}
}
/**
* Implements React's {@link Component#render()}.
@ -145,6 +121,8 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
_showConnectionInfo,
_overflowDrawer,
_showLocalVideoFlipButton,
hidePopover,
popoverVisible,
t
} = this.props;
@ -152,7 +130,7 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
? <ConnectionIndicatorContent participantId = { _localParticipantId } />
: (
<VideoMenu id = 'localVideoMenu'>
<FlipLocalVideoButton />
<FlipLocalVideoButton onClick = { hidePopover } />
{ isMobileBrowser()
&& <ConnectionStatusButton participantId = { _localParticipantId } />
}
@ -163,11 +141,12 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
isMobileBrowser() || _showLocalVideoFlipButton
? <Popover
content = { content }
id = 'local-video-menu-trigger'
onPopoverClose = { this._onPopoverClose }
onPopoverOpen = { this._onPopoverOpen }
overflowDrawer = { _overflowDrawer }
position = { _menuPosition }
ref = { this.popoverRef }>
visible = { popoverVisible }>
{!_overflowDrawer && (
<span
className = 'popover-trigger local-video-menu-trigger'>
@ -194,7 +173,10 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverOpen() {
this.props.dispatch(setParticipantContextMenuOpen(true));
const { dispatch, showPopover } = this.props;
showPopover();
dispatch(setParticipantContextMenuOpen(true));
}
_onPopoverClose: () => void;
@ -205,8 +187,9 @@ class LocalVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverClose() {
const { dispatch } = this.props;
const { hidePopover, dispatch } = this.props;
hidePopover();
batch(() => {
dispatch(setParticipantContextMenuOpen(false));
dispatch(renderConnectionStatus(false));

View File

@ -1,5 +1,6 @@
// @flow
/* eslint-disable react/jsx-handler-names */
import React, { Component } from 'react';
import { batch } from 'react-redux';
@ -41,6 +42,21 @@ declare var $: Object;
*/
type Props = {
/**
* Hides popover.
*/
hidePopover: Function,
/**
* Whether the popover is visible or not.
*/
popoverVisible: boolean,
/**
* Shows popover.
*/
showPopover: Function,
/**
* Whether or not to display the kick button.
*/
@ -128,10 +144,6 @@ type Props = {
* @extends {Component}
*/
class RemoteVideoMenuTriggerButton extends Component<Props> {
/**
* Reference to the Popover instance.
*/
popoverRef: Object;
/**
* Initializes a new RemoteVideoMenuTriggerButton instance.
@ -142,46 +154,10 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
constructor(props: Props) {
super(props);
this.popoverRef = React.createRef();
this._onPopoverClose = this._onPopoverClose.bind(this);
this._onPopoverOpen = this._onPopoverOpen.bind(this);
}
/**
* Triggers showing the popover's context menu.
*
* @returns {void}
*/
showContextMenu() {
if (this.popoverRef && this.popoverRef.current) {
this.popoverRef.current.showDialog();
}
}
/**
* Calls the ref(instance) getter.
*
* @inheritdoc
* @returns {void}
*/
componentDidMount() {
if (this.props.getRef) {
this.props.getRef(this);
}
}
/**
* Calls the ref(instance) getter.
*
* @inheritdoc
* @returns {void}
*/
componentWillUnmount() {
if (this.props.getRef) {
this.props.getRef(null);
}
}
/**
* Implements React's {@link Component#render()}.
*
@ -189,7 +165,13 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {ReactElement}
*/
render() {
const { _overflowDrawer, _showConnectionInfo, _participantDisplayName, participantID } = this.props;
const {
_overflowDrawer,
_showConnectionInfo,
_participantDisplayName,
participantID,
popoverVisible
} = this.props;
const content = _showConnectionInfo
? <ConnectionIndicatorContent participantId = { participantID } />
: this._renderRemoteVideoMenu();
@ -203,11 +185,11 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
return (
<Popover
content = { content }
id = 'remote-video-menu-trigger'
onPopoverClose = { this._onPopoverClose }
onPopoverOpen = { this._onPopoverOpen }
overflowDrawer = { _overflowDrawer }
position = { this.props._menuPosition }
ref = { this.popoverRef }>
visible = { popoverVisible }>
{!_overflowDrawer && (
<span className = 'popover-trigger remote-video-menu-trigger'>
{!isMobileBrowser() && <Icon
@ -232,7 +214,10 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverOpen() {
this.props.dispatch(setParticipantContextMenuOpen(true));
const { dispatch, showPopover } = this.props;
showPopover();
dispatch(setParticipantContextMenuOpen(true));
}
_onPopoverClose: () => void;
@ -243,8 +228,9 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
* @returns {void}
*/
_onPopoverClose() {
const { dispatch } = this.props;
const { dispatch, hidePopover } = this.props;
hidePopover();
batch(() => {
dispatch(setParticipantContextMenuOpen(false));
dispatch(renderConnectionStatus(false));
@ -271,6 +257,7 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
participantID
} = this.props;
const actions = [];
const buttons = [];
const showVolumeSlider = !isIosMobileBrowser()
&& onVolumeChange
@ -343,7 +330,7 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
);
if (isMobileBrowser()) {
buttons.push(
actions.push(
<ConnectionStatusButton
key = 'conn-status'
participantId = { participantID } />
@ -351,7 +338,7 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
}
if (showVolumeSlider) {
buttons.push(
actions.push(
<VolumeSlider
initialValue = { initialVolumeValue }
key = 'volume-slider'
@ -359,10 +346,27 @@ class RemoteVideoMenuTriggerButton extends Component<Props> {
);
}
if (buttons.length > 0) {
if (buttons.length > 0 || actions.length > 0) {
return (
<VideoMenu id = { participantID }>
{ buttons }
<>
{ buttons.length > 0
&& <li onClick = { this.props.hidePopover }>
<ul className = 'popupmenu__list'>
{ buttons }
</ul>
</li>
}
</>
<>
{ actions.length > 0
&& <li>
<ul className = 'popupmenu__list'>
{actions}
</ul>
</li>
}
</>
</VideoMenu>
);
}
@ -437,3 +441,4 @@ function _mapStateToProps(state, ownProps) {
}
export default translate(connect(_mapStateToProps)(RemoteVideoMenuTriggerButton));
/* eslint-enable react/jsx-handler-names */

View File

@ -61,8 +61,7 @@ export default class VideoMenuButton extends Component<Props> {
/**
* KeyPress handler for accessibility.
*
* @param {Object} e - The key event to handle.
*
* @param {Object} e - The synthetic event.
* @returns {void}
*/
_onKeyPress(e) {