From 2ffef3bddaf421c500045369b7d362c0274c8751 Mon Sep 17 00:00:00 2001 From: yanas Date: Thu, 6 Apr 2017 17:40:10 -0500 Subject: [PATCH] Fixes toolbar tooltip positioning --- .../toolbox/components/PrimaryToolbar.web.js | 31 ++++++---- .../components/SecondaryToolbar.web.js | 60 +++++++++---------- .../toolbox/components/Toolbar.web.js | 30 +++++----- .../toolbox/components/ToolbarButton.web.js | 13 ++-- 4 files changed, 72 insertions(+), 62 deletions(-) diff --git a/react/features/toolbox/components/PrimaryToolbar.web.js b/react/features/toolbox/components/PrimaryToolbar.web.js index 5345e1764..62996d072 100644 --- a/react/features/toolbox/components/PrimaryToolbar.web.js +++ b/react/features/toolbox/components/PrimaryToolbar.web.js @@ -6,8 +6,8 @@ import { connect } from 'react-redux'; import UIEvents from '../../../../service/UI/UIEvents'; import { showDesktopSharingButton, toggleFullScreen } from '../actions'; -import Toolbar from './Toolbar'; import { getToolbarClassNames } from '../functions'; +import Toolbar from './Toolbar'; declare var APP: Object; declare var interfaceConfig: Object; @@ -19,8 +19,6 @@ declare var interfaceConfig: Object; * @extends Component */ class PrimaryToolbar extends Component { - state: Object; - static propTypes = { /** * Handler for toggling fullscreen mode. @@ -43,6 +41,8 @@ class PrimaryToolbar extends Component { _visible: React.PropTypes.bool }; + state: Object; + /** * Constructs instance of primary toolbar React component. * @@ -68,10 +68,12 @@ class PrimaryToolbar extends Component { */ fullscreen: { onMount: () => - APP.UI.addListener(UIEvents.FULLSCREEN_TOGGLED, + APP.UI.addListener( + UIEvents.FULLSCREEN_TOGGLED, this.props._onFullScreenToggled), onUnmount: () => - APP.UI.removeListener(UIEvents.FULLSCREEN_TOGGLED, + APP.UI.removeListener( + UIEvents.FULLSCREEN_TOGGLED, this.props._onFullScreenToggled) } }; @@ -105,21 +107,24 @@ class PrimaryToolbar extends Component { const { _primaryToolbarButtons } = this.props; // The number of buttons to show in the toolbar isn't fixed, it depends - // on availability of features and configuration parameters, so if we - // don't have anything to render we exit here. + // on the availability of features and configuration parameters. So + // there may be nothing to render. if (_primaryToolbarButtons.size === 0) { return null; } const { buttonHandlers, splitterIndex } = this.state; const { primaryToolbarClassName } = getToolbarClassNames(this.props); + const tooltipPosition + = interfaceConfig.filmStripOnly ? 'left' : 'bottom'; return ( + toolbarButtons = { _primaryToolbarButtons } + tooltipPosition = { tooltipPosition } /> ); } } @@ -129,7 +134,7 @@ class PrimaryToolbar extends Component { * * @param {Function} dispatch - Redux action dispatcher. * @returns {{ - * _onShowDesktopSharingButton: Function + * _onShowDesktopSharingButton: Function * }} * @private */ @@ -162,8 +167,8 @@ function _mapDispatchToProps(dispatch: Function): Object { * * @param {Object} state - Snapshot of Redux store. * @returns {{ - * _primaryToolbarButtons: Map, - * _visible: boolean + * _primaryToolbarButtons: Map, + * _visible: boolean * }} * @private */ @@ -177,7 +182,7 @@ function _mapStateToProps(state: Object): Object { /** * Default toolbar buttons for primary toolbar. * - * @protected + * @private * @type {Map} */ _primaryToolbarButtons: primaryToolbarButtons, @@ -185,7 +190,7 @@ function _mapStateToProps(state: Object): Object { /** * Shows whether toolbox is visible. * - * @protected + * @private * @type {boolean} */ _visible: visible diff --git a/react/features/toolbox/components/SecondaryToolbar.web.js b/react/features/toolbox/components/SecondaryToolbar.web.js index 134d425a4..2bef38ebf 100644 --- a/react/features/toolbox/components/SecondaryToolbar.web.js +++ b/react/features/toolbox/components/SecondaryToolbar.web.js @@ -12,8 +12,8 @@ import { showRecordingButton, toggleSideToolbarContainer } from '../actions'; -import Toolbar from './Toolbar'; import { getToolbarClassNames } from '../functions'; +import Toolbar from './Toolbar'; declare var APP: Object; declare var config: Object; @@ -79,10 +79,9 @@ class SecondaryToolbar extends Component { * @type {Object} */ profile: { - onMount: () => { + onMount: () => APP.tokenData.isGuest - || this.props._onSetProfileButtonUnclickable(true); - } + || this.props._onSetProfileButtonUnclickable(true) }, /** @@ -91,14 +90,14 @@ class SecondaryToolbar extends Component { * @type {button} */ raisehand: { - onMount: () => { - APP.UI.addListener(UIEvents.LOCAL_RAISE_HAND_CHANGED, - this.props._onLocalRaiseHandChanged); - }, - onUnmount: () => { - APP.UI.removeListener(UIEvents.LOCAL_RAISE_HAND_CHANGED, - this.props._onLocalRaiseHandChanged); - } + onMount: () => + APP.UI.addListener( + UIEvents.LOCAL_RAISE_HAND_CHANGED, + this.props._onLocalRaiseHandChanged), + onUnmount: () => + APP.UI.removeListener( + UIEvents.LOCAL_RAISE_HAND_CHANGED, + this.props._onLocalRaiseHandChanged) }, /** @@ -107,11 +106,9 @@ class SecondaryToolbar extends Component { * @type {Object} */ recording: { - onMount: () => { - if (config.enableRecording) { - this.props._onShowRecordingButton(); - } - } + onMount: () => + config.enableRecording + && this.props._onShowRecordingButton() } }; @@ -131,7 +128,8 @@ class SecondaryToolbar extends Component { * @returns {void} */ componentDidMount(): void { - APP.UI.addListener(UIEvents.SIDE_TOOLBAR_CONTAINER_TOGGLED, + APP.UI.addListener( + UIEvents.SIDE_TOOLBAR_CONTAINER_TOGGLED, this.props._onSideToolbarContainerToggled); } @@ -141,7 +139,8 @@ class SecondaryToolbar extends Component { * @returns {void} */ componentWillUnmount(): void { - APP.UI.removeListener(UIEvents.SIDE_TOOLBAR_CONTAINER_TOGGLED, + APP.UI.removeListener( + UIEvents.SIDE_TOOLBAR_CONTAINER_TOGGLED, this.props._onSideToolbarContainerToggled); } @@ -154,8 +153,8 @@ class SecondaryToolbar extends Component { const { _secondaryToolbarButtons } = this.props; // The number of buttons to show in the toolbar isn't fixed, it depends - // on availability of features and configuration parameters, so if we - // don't have anything to render we exit here. + // on the availability of features and configuration parameters. So + // there may be nothing to render. if (_secondaryToolbarButtons.size === 0) { return null; } @@ -167,7 +166,8 @@ class SecondaryToolbar extends Component { + toolbarButtons = { _secondaryToolbarButtons } + tooltipPosition = { 'right' }> ); @@ -179,10 +179,10 @@ class SecondaryToolbar extends Component { * * @param {Function} dispatch - Redux action dispatcher. * @returns {{ - * _onLocalRaiseHandChanged: Function, - * _onSetProfileButtonUnclickable: Function, - * _onShowRecordingButton: Function, - * _onSideToolbarContainerToggled + * _onLocalRaiseHandChanged: Function, + * _onSetProfileButtonUnclickable: Function, + * _onShowRecordingButton: Function, + * _onSideToolbarContainerToggled * }} * @private */ @@ -237,8 +237,8 @@ function _mapDispatchToProps(dispatch: Function): Object { * * @param {Object} state - Snapshot of Redux store. * @returns {{ - * _secondaryToolbarButtons: Map, - * _visible: boolean + * _secondaryToolbarButtons: Map, + * _visible: boolean * }} * @private */ @@ -252,7 +252,7 @@ function _mapStateToProps(state: Object): Object { /** * Default toolbar buttons for secondary toolbar. * - * @protected + * @private * @type {Map} */ _secondaryToolbarButtons: secondaryToolbarButtons, @@ -260,7 +260,7 @@ function _mapStateToProps(state: Object): Object { /** * Shows whether toolbar is visible. * - * @protected + * @private * @type {boolean} */ _visible: visible diff --git a/react/features/toolbox/components/Toolbar.web.js b/react/features/toolbox/components/Toolbar.web.js index cea9da7fd..38f3cd777 100644 --- a/react/features/toolbox/components/Toolbar.web.js +++ b/react/features/toolbox/components/Toolbar.web.js @@ -60,7 +60,13 @@ class Toolbar extends Component { /** * Map with toolbar buttons. */ - toolbarButtons: React.PropTypes.instanceOf(Map) + toolbarButtons: React.PropTypes.instanceOf(Map), + + /** + * Indicates the position of the tooltip. + */ + tooltipPosition: + React.PropTypes.oneOf([ 'bottom', 'left', 'right', 'top' ]) }; /** @@ -73,7 +79,7 @@ class Toolbar extends Component { this._setButtonHandlers(); - // Bind methods to save the context + // Bind callbacks to preverse this. this._renderToolbarButton = this._renderToolbarButton.bind(this); } @@ -115,7 +121,7 @@ class Toolbar extends Component { _renderToolbarButton(acc: Array<*>, keyValuePair: Array<*>, index: number): Array> { const [ key, button ] = keyValuePair; - const { splitterIndex } = this.props; + const { splitterIndex, tooltipPosition } = this.props; if (splitterIndex && index === splitterIndex) { const splitter = ; @@ -131,7 +137,8 @@ class Toolbar extends Component { key = { key } onClick = { onClick } onMount = { onMount } - onUnmount = { onUnmount } /> + onUnmount = { onUnmount } + tooltipPosition = { tooltipPosition } /> ); return acc; @@ -149,16 +156,11 @@ class Toolbar extends Component { toolbarButtons } = this.props; - // Only a few buttons have custom button handlers defined, so this - // list may be undefined or empty depending on the buttons we're - // rendering. - // TODO: merge the buttonHandlers and onClick properties and come up - // with a consistent event handling property. - if (!buttonHandlers) { - return; - } - - Object.keys(buttonHandlers).forEach(key => { + // Only a few buttons have buttonHandlers defined, so it may be + // undefined or empty depending on the buttons rendered. + // TODO Merge the buttonHandlers and onClick properties and come up with + // a consistent event handling property. + buttonHandlers && Object.keys(buttonHandlers).forEach(key => { let button = toolbarButtons.get(key); if (button) { diff --git a/react/features/toolbox/components/ToolbarButton.web.js b/react/features/toolbox/components/ToolbarButton.web.js index a0773519b..a44d9ce34 100644 --- a/react/features/toolbox/components/ToolbarButton.web.js +++ b/react/features/toolbox/components/ToolbarButton.web.js @@ -49,7 +49,13 @@ class ToolbarButton extends AbstractToolbarButton { /** * Translation helper function. */ - t: React.PropTypes.func + t: React.PropTypes.func, + + /** + * Indicates the position of the tooltip. + */ + tooltipPosition: + React.PropTypes.oneOf([ 'bottom', 'left', 'right', 'top' ]) }; /** @@ -199,13 +205,10 @@ class ToolbarButton extends AbstractToolbarButton { * @returns {void} */ _setShortcutAndTooltip(): void { - const { button } = this.props; + const { button, tooltipPosition } = this.props; const name = button.buttonName; if (UIUtil.isButtonEnabled(name)) { - const tooltipPosition - = interfaceConfig.MAIN_TOOLBAR_BUTTONS.indexOf(name) > -1 - ? 'bottom' : 'right'; if (!button.unclickable) { UIUtil.setTooltip(this.button,