From 88010fa26cf658fa9445d0a0c003a9a902266129 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Mon, 13 Nov 2017 11:57:34 +0000 Subject: [PATCH 1/2] Determine whether power level is custom once Roles have been determined Instead of potentially inspecting an empty {} before mounting. This fixes an issue where "Custom of N" would appear on the first mount of MemberInfo - part of https://github.com/vector-im/riot-web/issues/5107#issuecomment-331882294 --- .../views/elements/PowerSelector.js | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index a0aaa12ff1..50893850c1 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -20,9 +20,6 @@ import React from 'react'; import * as Roles from '../../../Roles'; import { _t } from '../../../languageHandler'; -let LEVEL_ROLE_MAP = {}; -const reverseRoles = {}; - module.exports = React.createClass({ displayName: 'PowerSelector', @@ -43,15 +40,23 @@ module.exports = React.createClass({ getInitialState: function() { return { - custom: (LEVEL_ROLE_MAP[this.props.value] === undefined), + levelRoleMap: {}, + reverseRoles: {}, }; }, componentWillMount: function() { - LEVEL_ROLE_MAP = Roles.levelRoleMap(); - Object.keys(LEVEL_ROLE_MAP).forEach(function(key) { - reverseRoles[LEVEL_ROLE_MAP[key]] = key; - }); + // This needs to be done now because levelRoleMap has translated strings + const levelRoleMap = Roles.levelRoleMap(); + const reverseRoles = {}; + Object.keys(levelRoleMap).forEach(function(key) { + reverseRoles[levelRoleMap[key]] = key; + }); + this.setState({ + levelRoleMap, + reverseRoles, + custom: levelRoleMap[this.props.value] === undefined, + }); }, onSelectChange: function(event) { @@ -74,7 +79,7 @@ module.exports = React.createClass({ getValue: function() { let value; if (this.refs.select) { - value = reverseRoles[this.refs.select.value]; + value = this.state.reverseRoles[this.refs.select.value]; if (this.refs.custom) { if (value === undefined) value = parseInt( this.refs.custom.value ); } @@ -98,17 +103,17 @@ module.exports = React.createClass({ if (this.state.custom) { selectValue = "Custom"; } else { - selectValue = LEVEL_ROLE_MAP[this.props.value] || "Custom"; + selectValue = this.state.levelRoleMap[this.props.value] || "Custom"; } let select; if (this.props.disabled) { select = { selectValue }; } else { - // Each level must have a definition in LEVEL_ROLE_MAP + // Each level must have a definition in this.state.levelRoleMap const levels = [0, 50, 100]; let options = levels.map((level) => { return { - value: LEVEL_ROLE_MAP[level], + value: this.state.levelRoleMap[level], // Give a userDefault (users_default in the power event) of 0 but // because level !== undefined, this should never be used. text: Roles.textualPowerLevel(level, 0), From d2ef6bffa88c954b222f467af67a09f2df00f19d Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 14 Nov 2017 12:02:37 +0000 Subject: [PATCH 2/2] Remove reverseRoles This variable seemed redundant in hindsight, it seemed better to remove it than to worry about where it went in the component. --- .../views/elements/PowerSelector.js | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/components/views/elements/PowerSelector.js b/src/components/views/elements/PowerSelector.js index 50893850c1..8dd848db00 100644 --- a/src/components/views/elements/PowerSelector.js +++ b/src/components/views/elements/PowerSelector.js @@ -41,27 +41,21 @@ module.exports = React.createClass({ getInitialState: function() { return { levelRoleMap: {}, - reverseRoles: {}, }; }, componentWillMount: function() { // This needs to be done now because levelRoleMap has translated strings const levelRoleMap = Roles.levelRoleMap(); - const reverseRoles = {}; - Object.keys(levelRoleMap).forEach(function(key) { - reverseRoles[levelRoleMap[key]] = key; - }); this.setState({ levelRoleMap, - reverseRoles, custom: levelRoleMap[this.props.value] === undefined, }); }, onSelectChange: function(event) { - this.setState({ custom: event.target.value === "Custom" }); - if (event.target.value !== "Custom") { + this.setState({ custom: event.target.value === "SELECT_VALUE_CUSTOM" }); + if (event.target.value !== "SELECT_VALUE_CUSTOM") { this.props.onChange(this.getValue()); } }, @@ -79,7 +73,7 @@ module.exports = React.createClass({ getValue: function() { let value; if (this.refs.select) { - value = this.state.reverseRoles[this.refs.select.value]; + value = this.refs.select.value; if (this.refs.custom) { if (value === undefined) value = parseInt( this.refs.custom.value ); } @@ -101,25 +95,26 @@ module.exports = React.createClass({ let selectValue; if (this.state.custom) { - selectValue = "Custom"; + selectValue = "SELECT_VALUE_CUSTOM"; } else { - selectValue = this.state.levelRoleMap[this.props.value] || "Custom"; + selectValue = this.state.levelRoleMap[selectValue] ? + this.props.value : "SELECT_VALUE_CUSTOM"; } let select; if (this.props.disabled) { - select = { selectValue }; + select = { this.state.levelRoleMap[selectValue] }; } else { // Each level must have a definition in this.state.levelRoleMap const levels = [0, 50, 100]; let options = levels.map((level) => { return { - value: this.state.levelRoleMap[level], + value: level, // Give a userDefault (users_default in the power event) of 0 but // because level !== undefined, this should never be used. text: Roles.textualPowerLevel(level, 0), }; }); - options.push({ value: "Custom", text: _t("Custom level") }); + options.push({ value: "SELECT_VALUE_CUSTOM", text: _t("Custom level") }); options = options.map((op) => { return ; });