From df8eb36d0eb0ffe3cacbf4acc9f10dc768cac7d1 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Wed, 16 May 2018 16:49:03 -0500 Subject: [PATCH] Coding style: comments --- .../toolbox/components/AbstractToolboxItem.js | 6 +- .../toolbox/components/ToolboxItem.native.js | 33 +++-- .../invite/components/InviteButton.native.js | 3 +- .../toolbox/components/native/OverflowMenu.js | 22 +-- .../components/native/OverflowMenuButton.js | 9 +- .../toolbox/components/native/Toolbox.js | 135 ++++++++++-------- .../toolbox/components/native/styles.js | 6 + 7 files changed, 123 insertions(+), 91 deletions(-) diff --git a/react/features/base/toolbox/components/AbstractToolboxItem.js b/react/features/base/toolbox/components/AbstractToolboxItem.js index 4da613261..e044bd59b 100644 --- a/react/features/base/toolbox/components/AbstractToolboxItem.js +++ b/react/features/base/toolbox/components/AbstractToolboxItem.js @@ -10,7 +10,7 @@ export type Styles = { iconStyle: Object, /** - * Style for te item's label. + * Style for the item's label. */ labelStyle: Object, @@ -174,7 +174,9 @@ export default class AbstractToolboxItem

extends Component

{ } /** - * Handles rendering of the actual item. + * Renders this {@code AbstractToolboxItem} (if it is {@code visible}). To + * be implemented/overridden by extenders. The default implementation of + * {@code AbstractToolboxItem} does nothing. * * @protected * @returns {ReactElement} diff --git a/react/features/base/toolbox/components/ToolboxItem.native.js b/react/features/base/toolbox/components/ToolboxItem.native.js index 1262cfe33..ef3e67651 100644 --- a/react/features/base/toolbox/components/ToolboxItem.native.js +++ b/react/features/base/toolbox/components/ToolboxItem.native.js @@ -26,7 +26,7 @@ export default class ToolboxItem extends AbstractToolboxItem { } /** - * Helper function to render the {@code Icon} part of this item. + * Renders the {@code Icon} part of this {@code ToolboxItem}. * * @private * @returns {ReactElement} @@ -42,8 +42,9 @@ export default class ToolboxItem extends AbstractToolboxItem { } /** - * Handles rendering of the actual item. + * Renders this {@code ToolboxItem}. Invoked by {@link AbstractToolboxItem}. * + * @override * @protected * @returns {ReactElement} */ @@ -56,25 +57,29 @@ export default class ToolboxItem extends AbstractToolboxItem { styles } = this.props; - let children; + let children = this._renderIcon(); + + // XXX When using a wrapper View, apply the style to it instead of + // applying it to the TouchableHighlight. + let style = styles && styles.style; if (showLabel) { - // eslint-disable-next-line no-extra-parens - children = ( - - { this._renderIcon() } - + // XXX TouchableHighlight requires 1 child. If there's a need to + // show both the icon and the label, then these two need to be + // wrapped in a View. + children = ( // eslint-disable-line no-extra-parens + + { children } + { this.label } ); - } else { - children = this._renderIcon(); - } - // When using a wrapper view, apply the style to it instead of - // applying it to the TouchableHighlight. - const style = showLabel ? undefined : styles && styles.style; + // XXX As stated earlier, the style was applied to the wrapper View + // (above). + style = undefined; + } return ( { accessibilityLabel = 'Share room'; diff --git a/react/features/toolbox/components/native/OverflowMenu.js b/react/features/toolbox/components/native/OverflowMenu.js index 1f1d54892..f29354c56 100644 --- a/react/features/toolbox/components/native/OverflowMenu.js +++ b/react/features/toolbox/components/native/OverflowMenu.js @@ -3,16 +3,18 @@ import React, { Component } from 'react'; import { connect } from 'react-redux'; -import { hideDialog, BottomSheet } from '../../../base/dialog'; +import { BottomSheet, hideDialog } from '../../../base/dialog'; import { AudioRouteButton } from '../../../mobile/audio-mode'; import { PictureInPictureButton } from '../../../mobile/picture-in-picture'; import { RoomLockButton } from '../../../room-lock'; import AudioOnlyButton from './AudioOnlyButton'; +import { overflowMenuItemStyles } from './styles'; import ToggleCameraButton from './ToggleCameraButton'; -import { overflowMenuItemStyles } from './styles'; - +/** + * The type of the React {@code Component} props of {@link OverflowMenu}. + */ type Props = { /** @@ -22,12 +24,13 @@ type Props = { }; /** - * The exported React {@code Component}. We need a reference to the wrapped - * component in order to be able to hide it using the dialog hiding logic. + * The exported React {@code Component}. We need it to execute + * {@link hideDialog}. + * + * XXX It does not break our coding style rule to not utilize globals for state, + * because it is merely another name for {@code export}'s {@code default}. */ - -// eslint-disable-next-line prefer-const -let OverflowMenu_; +let OverflowMenu_; // eslint-disable-line prefer-const /** * Implements a React {@code Component} with some extra actions in addition to @@ -42,6 +45,7 @@ class OverflowMenu extends Component { constructor(props: Props) { super(props); + // Bind event handlers so they are only bound once per instance. this._onCancel = this._onCancel.bind(this); } @@ -76,7 +80,7 @@ class OverflowMenu extends Component { _onCancel: () => void; /** - * Hides the dialog. + * Hides this {@code OverflowMenu}. * * @private * @returns {void} diff --git a/react/features/toolbox/components/native/OverflowMenuButton.js b/react/features/toolbox/components/native/OverflowMenuButton.js index 985ec2abb..e398fc3a4 100644 --- a/react/features/toolbox/components/native/OverflowMenuButton.js +++ b/react/features/toolbox/components/native/OverflowMenuButton.js @@ -9,13 +9,16 @@ import type { AbstractButtonProps } from '../../../base/toolbox'; import OverflowMenu from './OverflowMenu'; +/** + * The type of the React {@code Component} props of {@link OverflowMenuButton}. + */ type Props = AbstractButtonProps & { /** * The redux {@code dispatch} function. */ dispatch: Function -} +}; /** * An implementation of a button for showing the {@code OverflowMenu}. @@ -26,9 +29,9 @@ class OverflowMenuButton extends AbstractButton { label = 'toolbar.moreActions'; /** - * Handles clicking / pressing the button. + * Handles clicking / pressing this {@code OverflowMenuButton}. * - * @private + * @protected * @returns {void} */ _handleClick() { diff --git a/react/features/toolbox/components/native/Toolbox.js b/react/features/toolbox/components/native/Toolbox.js index ed60956aa..8fe6876bf 100644 --- a/react/features/toolbox/components/native/Toolbox.js +++ b/react/features/toolbox/components/native/Toolbox.js @@ -13,24 +13,29 @@ import { InviteButton } from '../../../invite'; import AudioMuteButton from '../AudioMuteButton'; import HangupButton from '../HangupButton'; -import VideoMuteButton from '../VideoMuteButton'; - import OverflowMenuButton from './OverflowMenuButton'; import styles, { hangupButtonStyles, toolbarButtonStyles, toolbarToggledButtonStyles } from './styles'; +import VideoMuteButton from '../VideoMuteButton'; /** - * Number of buttons in the toolbar. + * The number of buttons to render in {@link Toolbox}. + * + * @private + * @type number */ -const NUM_TOOLBAR_BUTTONS = 4; +const _BUTTON_COUNT = 4; /** * Factor relating the hangup button and other toolbar buttons. + * + * @private + * @type number */ -const TOOLBAR_BUTTON_SIZE_FACTOR = 0.8; +const _BUTTON_SIZE_FACTOR = 0.8; /** * The type of {@link Toolbox}'s React {@code Component} props. @@ -75,48 +80,10 @@ class Toolbox extends Component { constructor(props: Props) { super(props); + // Bind event handlers so they are only bound once per instance. this._onLayout = this._onLayout.bind(this); } - _onLayout: (Object) => void; - - /** - * Handles the "on layout" View's event and stores the width as state. - * - * @param {Object} event - The "on layout" event object/structure passed - * by react-native. - * @private - * @returns {void} - */ - _onLayout({ nativeEvent: { layout: { width } } }) { - this.setState({ width }); - } - - /** - * Calculates how large our toolbar buttons can be, given the available - * width. In the future we might want to have a size threshold, and once - * it's passed a completely different style could be used, akin to the web. - * - * @private - * @returns {number} - */ - _calculateToolbarButtonSize() { - const width = this.state.width; - const hangupButtonSize = styles.hangupButton.width; - - let buttonSize - = (width - hangupButtonSize - - (NUM_TOOLBAR_BUTTONS * styles.toolbarButton.margin * 2)) - / NUM_TOOLBAR_BUTTONS; - - // Make sure it's an even number. - buttonSize = 2 * Math.round(buttonSize / 2); - - // The button should be at most 80% of the hangup button's size. - return Math.min( - buttonSize, hangupButtonSize * TOOLBAR_BUTTON_SIZE_FACTOR); - } - /** * Implements React's {@link Component#render()}. * @@ -129,31 +96,35 @@ class Toolbox extends Component { ? styles.toolboxNarrow : styles.toolboxWide; const { _visible } = this.props; - const buttonStyles = { - ...toolbarButtonStyles - }; - const toggledButtonStyles = { - ...toolbarToggledButtonStyles - }; + let buttonStyles = toolbarButtonStyles; + let toggledButtonStyles = toolbarToggledButtonStyles; - if (_visible && this.state.width) { - const buttonSize = this._calculateToolbarButtonSize(); - const extraStyle = { - borderRadius: buttonSize / 2, - height: buttonSize, - width: buttonSize - }; + if (_visible) { + const buttonSize = this._calculateButtonSize(); - buttonStyles.style = [ buttonStyles.style, extraStyle ]; - toggledButtonStyles.style - = [ toggledButtonStyles.style, extraStyle ]; + if (buttonSize > 0) { + const extraButtonStyle = { + borderRadius: buttonSize / 2, + height: buttonSize, + width: buttonSize + }; + + buttonStyles = { + ...buttonStyles, + style: [ buttonStyles.style, extraButtonStyle ] + }; + toggledButtonStyles = { + ...toggledButtonStyles, + style: [ toggledButtonStyles.style, extraButtonStyle ] + }; + } } return ( + visible = { _visible }> @@ -172,6 +143,47 @@ class Toolbox extends Component { ); } + + /** + * Calculates how large our toolbar buttons can be, given the available + * width. In the future we might want to have a size threshold, and once + * it's passed a completely different style could be used, akin to the web. + * + * @private + * @returns {number} + */ + _calculateButtonSize() { + const { width } = this.state; + const hangupButtonSize = styles.hangupButton.width; + + let buttonSize + = (width + - hangupButtonSize + - (_BUTTON_COUNT * styles.toolbarButton.margin * 2)) + / _BUTTON_COUNT; + + // Make sure it's an even number. + buttonSize = 2 * Math.round(buttonSize / 2); + + // The button should be at most 80% of the hangup button's size. + return Math.min( + buttonSize, + hangupButtonSize * _BUTTON_SIZE_FACTOR); + } + + _onLayout: (Object) => void; + + /** + * Handles the "on layout" View's event and stores the width as state. + * + * @param {Object} event - The "on layout" event object/structure passed + * by react-native. + * @private + * @returns {void} + */ + _onLayout({ nativeEvent: { layout: { width } } }) { + this.setState({ width }); + } } /** @@ -182,7 +194,6 @@ class Toolbox extends Component { * {@code Toolbox} props. * @private * @returns {{ - * _enabled: boolean, * _visible: boolean * }} */ diff --git a/react/features/toolbox/components/native/styles.js b/react/features/toolbox/components/native/styles.js index c52f95b23..5d1e0eaee 100644 --- a/react/features/toolbox/components/native/styles.js +++ b/react/features/toolbox/components/native/styles.js @@ -1,5 +1,9 @@ +// @flow + import { BoxModel, ColorPalette, createStyleSheet } from '../../../base/styles'; +// Toolbox, toolbar: + /** * The style of toolbar buttons. */ @@ -141,6 +145,8 @@ export const toolbarToggledButtonStyles = { style: styles.whiteToolbarButton }; +// Overflow menu: + /** * Styles for the {@code OverflowMenu} items. *