From bcbdaaa6ead88cdb97fd42690408974f34b86dd7 Mon Sep 17 00:00:00 2001 From: Ilya Daynatovich Date: Tue, 4 Apr 2017 14:53:10 +0300 Subject: [PATCH] Fix interface_config.js/interfaceConfig overriding It got broken while rewriting the Web toolbar in React Toolbox. There is a problem with the toolbars and how we construct the intialState of the buttons. The _getInitialState() in the toolbox reducer gets the list of buttons from interfaceConfig, but in fact interfaceConfig is meant to be overriden in several very important cases. One of the cases being the external API, which we use in several projects in production. --- react/features/toolbox/actionTypes.js | 12 +++++ react/features/toolbox/actions.native.js | 50 +++++++++++++------ .../toolbox/components/Toolbox.web.js | 35 ++++++++++++- react/features/toolbox/functions.js | 21 ++++++-- react/features/toolbox/reducer.js | 35 ++++++++++--- 5 files changed, 122 insertions(+), 31 deletions(-) diff --git a/react/features/toolbox/actionTypes.js b/react/features/toolbox/actionTypes.js index 54af2bed2..9586654ae 100644 --- a/react/features/toolbox/actionTypes.js +++ b/react/features/toolbox/actionTypes.js @@ -9,6 +9,18 @@ import { Symbol } from '../base/react'; */ export const CLEAR_TOOLBOX_TIMEOUT = Symbol('CLEAR_TOOLBOX_TIMEOUT'); +/** + * The type of the action which sets the default toolbar buttons of the Toolbox. + * + * { + * type: SET_DEFAULT_TOOLBOX_BUTTONS, + * primaryToolbarButtons: Map, + * secondaryToolbarButtons: Map + * } + */ +export const SET_DEFAULT_TOOLBOX_BUTTONS + = Symbol('SET_DEFAULT_TOOLBOX_BUTTONS'); + /** * The type of the action which sets the permanent visibility of the Toolbox. * diff --git a/react/features/toolbox/actions.native.js b/react/features/toolbox/actions.native.js index 828d3ff2c..aaef1f604 100644 --- a/react/features/toolbox/actions.native.js +++ b/react/features/toolbox/actions.native.js @@ -4,6 +4,7 @@ import type { Dispatch } from 'redux-thunk'; import { CLEAR_TOOLBOX_TIMEOUT, + SET_DEFAULT_TOOLBOX_BUTTONS, SET_TOOLBOX_ALWAYS_VISIBLE, SET_SUBJECT, SET_SUBJECT_SLIDE_IN, @@ -13,6 +14,7 @@ import { SET_TOOLBOX_TIMEOUT_MS, SET_TOOLBOX_VISIBLE } from './actionTypes'; +import { getDefaultToolboxButtons } from './functions'; /** * Event handler for local raise hand changed event. @@ -46,22 +48,6 @@ export function clearToolboxTimeout(): Object { }; } -/** - * Signals that always visible toolbars value should be changed. - * - * @param {boolean} alwaysVisible - Value to be set in redux store. - * @returns {{ - * type: SET_TOOLBOX_ALWAYS_VISIBLE, - * alwaysVisible: boolean - * }} - */ -export function setToolboxAlwaysVisible(alwaysVisible: boolean): Object { - return { - type: SET_TOOLBOX_ALWAYS_VISIBLE, - alwaysVisible - }; -} - /** * Enables/disables audio toolbar button. * @@ -83,6 +69,22 @@ export function setAudioIconEnabled(enabled: boolean = false): Function { }; } +/** + * Sets the default toolbar buttons of the Toolbox. + * + * @returns {{ + * type: SET_DEFAULT_TOOLBOX_BUTTONS, + * primaryToolbarButtons: Map, + * secondaryToolbarButtons: Map + * }} + */ +export function setDefaultToolboxButtons(): Object { + return { + type: SET_DEFAULT_TOOLBOX_BUTTONS, + ...getDefaultToolboxButtons() + }; +} + /** * Signals that value of conference subject should be changed. * @@ -148,6 +150,22 @@ export function setToolbarHovered(hovered: boolean): Object { }; } +/** + * Signals that always visible toolbars value should be changed. + * + * @param {boolean} alwaysVisible - Value to be set in redux store. + * @returns {{ + * type: SET_TOOLBOX_ALWAYS_VISIBLE, + * alwaysVisible: boolean + * }} + */ +export function setToolboxAlwaysVisible(alwaysVisible: boolean): Object { + return { + type: SET_TOOLBOX_ALWAYS_VISIBLE, + alwaysVisible + }; +} + /* eslint-disable flowtype/space-before-type-colon */ /** diff --git a/react/features/toolbox/components/Toolbox.web.js b/react/features/toolbox/components/Toolbox.web.js index 893df14f5..38073ac1d 100644 --- a/react/features/toolbox/components/Toolbox.web.js +++ b/react/features/toolbox/components/Toolbox.web.js @@ -5,7 +5,10 @@ import { connect } from 'react-redux'; import UIEvents from '../../../../service/UI/UIEvents'; -import { setToolboxAlwaysVisible } from '../actions'; +import { + setDefaultToolboxButtons, + setToolboxAlwaysVisible +} from '../actions'; import { abstractMapStateToProps, showCustomToolbarPopup @@ -28,6 +31,11 @@ class Toolbox extends Component { * @static */ static propTypes = { + /** + * Handler dispatching setting default buttons action. + */ + _setDefaultToolboxButtons: React.PropTypes.func, + /** * Handler dispatching reset always visible toolbox action. */ @@ -61,6 +69,19 @@ class Toolbox extends Component { APP.UI.addListener( UIEvents.SHOW_CUSTOM_TOOLBAR_BUTTON_POPUP, showCustomToolbarPopup); + + // FIXME The redux action SET_DEFAULT_TOOLBOX_BUTTONS and related source + // code such as the redux action creator setDefaultToolboxButtons and + // _setDefaultToolboxButtons were introduced to solve the following bug + // in the implementation of features/toolbar at the time of this + // writing: getDefaultToolboxButtons uses interfaceConfig which is not + // in the redux store at the time of this writing yet interfaceConfig is + // modified after getDefaultToolboxButtons is called. + // SET_DEFAULT_TOOLBOX_BUTTONS represents/implements an explicit delay + // of the invocation of getDefaultToolboxButtons until, heuristically, + // all existing changes to interfaceConfig have been applied already in + // our known execution paths. + this.props._setDefaultToolboxButtons(); } /** @@ -158,15 +179,25 @@ class Toolbox extends Component { * * @param {Function} dispatch - Redux action dispatcher. * @returns {{ + * _setDefaultToolboxButtons: Function, * _setToolboxAlwaysVisible: Function * }} * @private */ function _mapDispatchToProps(dispatch: Function): Object { return { + /** + * Dispatches a (redux) action to set the default toolbar buttons. + * + * @returns {Object} Dispatched action. + */ + _setDefaultToolboxButtons() { + dispatch(setDefaultToolboxButtons()); + }, /** - * Dispatches an action resetting always visible toolbox. + * Dispatches a (redux) action to reset the permanent visibility of + * the Toolbox. * * @returns {Object} Dispatched action. */ diff --git a/react/features/toolbox/functions.js b/react/features/toolbox/functions.js index cb05158c3..119ff5c04 100644 --- a/react/features/toolbox/functions.js +++ b/react/features/toolbox/functions.js @@ -112,14 +112,23 @@ export function abstractMapStateToProps(state: Object): Object { }; } +/* eslint-disable flowtype/space-before-type-colon */ + /** * Takes toolbar button props and maps them to HTML attributes to set. * * @param {Object} props - Props set to the React component. * @returns {MapOfAttributes} */ -export function getButtonAttributesByProps(props: Object): MapOfAttributes { - const classNames = [ ...props.classNames ]; +export function getButtonAttributesByProps(props: Object = {}) + : MapOfAttributes { + let classNames = props.classNames; + + if (classNames) { + // XXX Make sure to not modify props.classNames because that'd be bad + // practice. + classNames = [ ...classNames ]; + } props.toggled && classNames.push('toggled'); props.unclickable && classNames.push('unclickable'); @@ -142,13 +151,15 @@ export function getButtonAttributesByProps(props: Object): MapOfAttributes { return result; } +/* eslint-enable flowtype/space-before-type-colon */ + /** - * Returns object containing default buttons for the primary and secondary - * toolbars. + * Returns an object which contains the default buttons for the primary and + * secondary toolbars. * * @returns {Object} */ -export function getDefaultToolbarButtons(): Object { +export function getDefaultToolboxButtons(): Object { let toolbarButtons = { primaryToolbarButtons: new Map(), secondaryToolbarButtons: new Map() diff --git a/react/features/toolbox/reducer.js b/react/features/toolbox/reducer.js index 415e11da0..c53dd57df 100644 --- a/react/features/toolbox/reducer.js +++ b/react/features/toolbox/reducer.js @@ -4,6 +4,7 @@ import { ReducerRegistry } from '../base/redux'; import { CLEAR_TOOLBOX_TIMEOUT, + SET_DEFAULT_TOOLBOX_BUTTONS, SET_TOOLBOX_ALWAYS_VISIBLE, SET_SUBJECT, SET_SUBJECT_SLIDE_IN, @@ -13,7 +14,6 @@ import { SET_TOOLBOX_TIMEOUT_MS, SET_TOOLBOX_VISIBLE } from './actionTypes'; -import { getDefaultToolbarButtons } from './functions'; declare var interfaceConfig: Object; @@ -43,8 +43,6 @@ function _getInitialState() { } return { - ...getDefaultToolbarButtons(), - /** * The indicator which determines whether the Toolbox should always be * visible. @@ -61,6 +59,20 @@ function _getInitialState() { */ hovered: false, + /** + * A Map of the default buttons of the PrimaryToolbar. + * + * @type {Map} + */ + primaryToolbarButtons: new Map(), + + /** + * A Map of the default buttons of the SecondaryToolbar. + * + * @type {Map} + */ + secondaryToolbarButtons: new Map(), + /** * The text of the conference subject. * @@ -110,6 +122,16 @@ ReducerRegistry.register( timeoutID: undefined }; + case SET_DEFAULT_TOOLBOX_BUTTONS: { + const { primaryToolbarButtons, secondaryToolbarButtons } = action; + + return { + ...state, + primaryToolbarButtons, + secondaryToolbarButtons + }; + } + case SET_TOOLBOX_ALWAYS_VISIBLE: return { ...state, @@ -170,11 +192,8 @@ ReducerRegistry.register( * @private * @returns {Object} */ -function _setButton(state, { buttonName, button }): Object { - const { - primaryToolbarButtons, - secondaryToolbarButtons - } = state; +function _setButton(state, { button, buttonName }): Object { + const { primaryToolbarButtons, secondaryToolbarButtons } = state; let selectedButton = primaryToolbarButtons.get(buttonName); let place = 'primaryToolbarButtons';