Merge pull request #2725 from matrix-org/travis/settings/pl-dropdowns

Convert PowerSelector to use mxField instead

res/css/_components.scss

@@ -85,6 +85,7 @@
 @import "./views/elements/_InlineSpinner.scss";
 @import "./views/elements/_ManageIntegsButton.scss";
 @import "./views/elements/_MemberEventListSummary.scss";
+@import "./views/elements/_PowerSelector.scss";
 @import "./views/elements/_ProgressBar.scss";
 @import "./views/elements/_ReplyThread.scss";
 @import "./views/elements/_ResizeHandle.scss";

res/css/views/elements/_PowerSelector.scss

@@ -0,0 +1,25 @@
+/*
+Copyright 2019 New Vector Ltd.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+.mx_PowerSelector {
+    width: 100%;
+}
+
+.mx_PowerSelector .mx_Field select,
+.mx_PowerSelector .mx_Field input {
+    width: 100%;
+    box-sizing: border-box;
+}

src/components/views/elements/PowerSelector.js

@@ -20,6 +20,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import * as Roles from '../../../Roles';
 import { _t } from '../../../languageHandler';
+import Field from "./Field";
 
 module.exports = React.createClass({
     displayName: 'PowerSelector',
@@ -32,19 +33,15 @@ module.exports = React.createClass({
         // Default user power level for the room
         usersDefault: PropTypes.number.isRequired,
 
-        // if true, the <select/> should be a 'controlled' form element and updated by React
-        // to reflect the current value, rather than left freeform.
-        // MemberInfo uses controlled; RoomSettings uses non-controlled.
-        //
-        // ignored if disabled is truthy. false by default.
-        controlled: PropTypes.bool,
-
         // should the user be able to change the value? false by default.
         disabled: PropTypes.bool,
         onChange: PropTypes.func,
 
         // Optional key to pass as the second argument to `onChange`
         powerLevelKey: PropTypes.string,
+
+        // The name to annotate the selector with
+        label: PropTypes.string,
     },
 
     getInitialState: function() {
@@ -52,6 +49,9 @@ module.exports = React.createClass({
             levelRoleMap: {},
             // List of power levels to show in the drop-down
             options: [],
+
+            customValue: this.props.value,
+            selectValue: 0,
         };
     },
 
@@ -77,61 +77,50 @@ module.exports = React.createClass({
             return l === undefined || l <= newProps.maxValue;
         });
 
+        const isCustom = levelRoleMap[newProps.value] === undefined;
+
         this.setState({
             levelRoleMap,
             options,
-            custom: levelRoleMap[newProps.value] === undefined,
+            custom: isCustom,
+            customLevel: newProps.value,
+            selectValue: isCustom ? "SELECT_VALUE_CUSTOM" : newProps.value,
         });
     },
 
     onSelectChange: function(event) {
-        this.setState({ custom: event.target.value === "SELECT_VALUE_CUSTOM" });
-        if (event.target.value !== "SELECT_VALUE_CUSTOM") {
+        const isCustom = event.target.value === "SELECT_VALUE_CUSTOM";
+        if (isCustom) {
+            this.setState({custom: true});
+        } else {
             this.props.onChange(event.target.value, this.props.powerLevelKey);
+            this.setState({selectValue: event.target.value});
         }
     },
 
+    onCustomChange: function(event) {
+        this.setState({customValue: event.target.value});
+    },
+
     onCustomBlur: function(event) {
-        this.props.onChange(parseInt(this.refs.custom.value), this.props.powerLevelKey);
+        this.props.onChange(parseInt(this.state.customValue), this.props.powerLevelKey);
     },
 
     onCustomKeyDown: function(event) {
-        if (event.key == "Enter") {
-            this.props.onChange(parseInt(this.refs.custom.value), this.props.powerLevelKey);
+        if (event.key === "Enter") {
+            this.props.onChange(parseInt(this.state.customValue), this.props.powerLevelKey);
         }
     },
 
     render: function() {
-        let customPicker;
+        let picker;
         if (this.state.custom) {
-            if (this.props.disabled) {
-                customPicker = <span>{ _t(
-                    "Custom of %(powerLevel)s",
-                    { powerLevel: this.props.value },
-                ) }</span>;
-            } else {
-                customPicker = <span> = <input
-                    ref="custom"
-                    type="text"
-                    size="3"
-                    defaultValue={this.props.value}
-                    onBlur={this.onCustomBlur}
-                    onKeyDown={this.onCustomKeyDown}
-                />
-                </span>;
-            }
-        }
-
-        let selectValue;
-        if (this.state.custom) {
-            selectValue = "SELECT_VALUE_CUSTOM";
-        } else {
-            selectValue = this.state.levelRoleMap[this.props.value] ?
-                this.props.value : "SELECT_VALUE_CUSTOM";
-        }
-        let select;
-        if (this.props.disabled) {
-            select = <span>{ this.state.levelRoleMap[selectValue] }</span>;
+            picker = (
+                <Field id={`powerSelector_custom_${this.props.powerLevelKey}`} type="number"
+                       label={this.props.label || _t("Power level")} max={this.props.maxValue}
+                       onBlur={this.onCustomBlur} onKeyDown={this.onCustomKeyDown} onChange={this.onCustomChange}
+                       value={this.state.customValue} disabled={this.props.disabled} />
+            );
         } else {
             // Each level must have a definition in this.state.levelRoleMap
             let options = this.state.options.map((level) => {
@@ -145,20 +134,19 @@ module.exports = React.createClass({
                 return <option value={op.value} key={op.value}>{ op.text }</option>;
             });
 
-            select =
-                <select ref="select"
-                        value={this.props.controlled ? selectValue : undefined}
-                        defaultValue={!this.props.controlled ? selectValue : undefined}
-                        onChange={this.onSelectChange}>
-                    { options }
-                </select>;
+            picker = (
+                <Field id={`powerSelector_notCustom_${this.props.powerLevelKey}`} element="select"
+                       label={this.props.label || _t("Power level")} onChange={this.onSelectChange}
+                       value={this.state.selectValue} disabled={this.props.disabled}>
+                    {options}
+                </Field>
+            );
         }
 
         return (
-            <span className="mx_PowerSelector">
-                { select }
-                { customPicker }
-            </span>
+            <div className="mx_PowerSelector">
+                { picker }
+            </div>
         );
     },
 });

src/components/views/rooms/MemberInfo.js

@@ -947,14 +947,12 @@ module.exports = withMatrixClient(React.createClass({
             const PowerSelector = sdk.getComponent('elements.PowerSelector');
             roomMemberDetails = <div>
                 <div className="mx_MemberInfo_profileField">
-                    { _t("Level:") } <b>
-                        <PowerSelector controlled={true}
-                            value={parseInt(this.props.member.powerLevel)}
-                            maxValue={this.state.can.modifyLevelMax}
-                            disabled={!this.state.can.modifyLevel}
-                            usersDefault={powerLevelUsersDefault}
-                            onChange={this.onPowerChange} />
-                    </b>
+                    <PowerSelector
+                        value={parseInt(this.props.member.powerLevel)}
+                        maxValue={this.state.can.modifyLevelMax}
+                        disabled={!this.state.can.modifyLevel}
+                        usersDefault={powerLevelUsersDefault}
+                        onChange={this.onPowerChange} />
                 </div>
                 <div className="mx_MemberInfo_profileField">
                     {presenceLabel}

src/components/views/settings/tabs/room/RolesRoomSettingsTab.js

@@ -24,14 +24,14 @@ import Modal from "../../../../../Modal";
 
 const plEventsToLabels = {
     // These will be translated for us later.
-    "m.room.avatar": _td("To change the room's avatar, you must be a"),
-    "m.room.name": _td("To change the room's name, you must be a"),
-    "m.room.canonical_alias": _td("To change the room's main address, you must be a"),
-    "m.room.history_visibility": _td("To change the room's history visibility, you must be a"),
-    "m.room.power_levels": _td("To change the permissions in the room, you must be a"),
-    "m.room.topic": _td("To change the topic, you must be a"),
+    "m.room.avatar": _td("Change room avatar"),
+    "m.room.name": _td("Change room name"),
+    "m.room.canonical_alias": _td("Change main address for the room"),
+    "m.room.history_visibility": _td("Change history visibility"),
+    "m.room.power_levels": _td("Change permissions"),
+    "m.room.topic": _td("Change topic"),
 
-    "im.vector.modular.widgets": _td("To modify widgets in the room, you must be a"),
+    "im.vector.modular.widgets": _td("Modify widgets"),
 };
 
 const plEventsToShow = {
@@ -158,35 +158,35 @@ export default class RolesRoomSettingsTab extends React.Component {
 
         const powerLevelDescriptors = {
             "users_default": {
-                desc: _t('The default role for new room members is'),
+                desc: _t('Default role'),
                 defaultValue: 0,
             },
             "events_default": {
-                desc: _t('To send messages, you must be a'),
+                desc: _t('Send messages'),
                 defaultValue: 0,
             },
             "invite": {
-                desc: _t('To invite users into the room, you must be a'),
+                desc: _t('Invite users'),
                 defaultValue: 50,
             },
             "state_default": {
-                desc: _t('To configure the room, you must be a'),
+                desc: _t('Change settings'),
                 defaultValue: 50,
             },
             "kick": {
-                desc: _t('To kick users, you must be a'),
+                desc: _t('Kick users'),
                 defaultValue: 50,
             },
             "ban": {
-                desc: _t('To ban users, you must be a'),
+                desc: _t('Ban users'),
                 defaultValue: 50,
             },
             "redact": {
-                desc: _t('To remove other users\' messages, you must be a'),
+                desc: _t('Remove messages'),
                 defaultValue: 50,
             },
             "notifications.room": {
-                desc: _t('To notify everyone in the room, you must be a'),
+                desc: _t('Notify everyone'),
                 defaultValue: 50,
             },
         };
@@ -217,20 +217,15 @@ export default class RolesRoomSettingsTab extends React.Component {
             const mutedUsers = [];
 
             Object.keys(userLevels).forEach(function(user) {
+                const canChange = userLevels[user] < currentUserLevel && canChangeLevels;
                 if (userLevels[user] > defaultUserLevel) { // privileged
-                    privilegedUsers.push(<li key={user}>
-                        { _t("%(user)s is a %(userRole)s", {
-                            user: user,
-                            userRole: <PowerSelector value={userLevels[user]} disabled={true} />,
-                        }) }
-                    </li>);
+                    privilegedUsers.push(
+                        <PowerSelector value={userLevels[user]} disabled={!canChange} label={user} key={user} />,
+                    );
                 } else if (userLevels[user] < defaultUserLevel) { // muted
-                    mutedUsers.push(<li key={user}>
-                        { _t("%(user)s is a %(userRole)s", {
-                            user: user,
-                            userRole: <PowerSelector value={userLevels[user]} disabled={true} />,
-                        }) }
-                    </li>);
+                    mutedUsers.push(
+                        <PowerSelector value={userLevels[user]} disabled={!canChange} label={user} key={user} />,
+                    );
                 }
             });
 
@@ -247,18 +242,14 @@ export default class RolesRoomSettingsTab extends React.Component {
                 privilegedUsersSection =
                     <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
                         <div className='mx_SettingsTab_subheading'>{ _t('Privileged Users') }</div>
-                        <ul>
-                            {privilegedUsers}
-                        </ul>
+                        {privilegedUsers}
                     </div>;
             }
             if (mutedUsers.length) {
                 mutedUsersSection =
                     <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
                         <div className='mx_SettingsTab_subheading'>{ _t('Muted Users') }</div>
-                        <ul>
-                            {mutedUsers}
-                        </ul>
+                        {mutedUsers}
                     </div>;
             }
         }
@@ -300,11 +291,10 @@ export default class RolesRoomSettingsTab extends React.Component {
 
             const value = parseIntWithDefault(currentObj, descriptor.defaultValue);
             return <div key={index} className="">
-                <span>{descriptor.desc}&nbsp;</span>
                 <PowerSelector
+                    label={descriptor.desc}
                     value={value}
                     usersDefault={defaultUserLevel}
-                    controlled={false}
                     disabled={!canChangeLevels || currentUserLevel < value}
                     powerLevelKey={key} // Will be sent as the second parameter to `onChange`
                     onChange={this._onPowerLevelsChanged}
@@ -317,18 +307,14 @@ export default class RolesRoomSettingsTab extends React.Component {
             if (label) {
                 label = _t(label);
             } else {
-                label = _t(
-                    "To send events of type <eventType/>, you must be a", {},
-                    { 'eventType': <code>{ eventType }</code> },
-                );
+                label = _t("Send %(eventType)s events", {eventType});
             }
             return (
                 <div className="" key={eventType}>
-                    <span>{label}&nbsp;</span>
                     <PowerSelector
+                        label={label}
                         value={eventsLevels[eventType]}
                         usersDefault={defaultUserLevel}
-                        controlled={false}
                         disabled={!canChangeLevels || currentUserLevel < eventsLevels[eventType]}
                         powerLevelKey={"event_levels_" + eventType}
                         onChange={this._onPowerLevelsChanged}
@@ -345,6 +331,7 @@ export default class RolesRoomSettingsTab extends React.Component {
                 {bannedUsersSection}
                 <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
                     <span className='mx_SettingsTab_subheading'>{_t("Permissions")}</span>
+                    <p>{_t('Select the roles required to change various parts of the room')}</p>
                     {powerSelectors}
                     {eventPowerSelectors}
                 </div>

src/i18n/strings/en_EN.json

@@ -586,32 +586,32 @@
     "Room Addresses": "Room Addresses",
     "Publish this room to the public in %(domain)s's room directory?": "Publish this room to the public in %(domain)s's room directory?",
     "URL Previews": "URL Previews",
-    "To change the room's avatar, you must be a": "To change the room's avatar, you must be a",
-    "To change the room's name, you must be a": "To change the room's name, you must be a",
-    "To change the room's main address, you must be a": "To change the room's main address, you must be a",
-    "To change the room's history visibility, you must be a": "To change the room's history visibility, you must be a",
-    "To change the permissions in the room, you must be a": "To change the permissions in the room, you must be a",
-    "To change the topic, you must be a": "To change the topic, you must be a",
-    "To modify widgets in the room, you must be a": "To modify widgets in the room, you must be a",
+    "Change room avatar": "Change room avatar",
+    "Change room name": "Change room name",
+    "Change main address for the room": "Change main address for the room",
+    "Change history visibility": "Change history visibility",
+    "Change permissions": "Change permissions",
+    "Change topic": "Change topic",
+    "Modify widgets": "Modify widgets",
     "Failed to unban": "Failed to unban",
     "Unban": "Unban",
     "Banned by %(displayName)s": "Banned by %(displayName)s",
-    "The default role for new room members is": "The default role for new room members is",
-    "To send messages, you must be a": "To send messages, you must be a",
-    "To invite users into the room, you must be a": "To invite users into the room, you must be a",
-    "To configure the room, you must be a": "To configure the room, you must be a",
-    "To kick users, you must be a": "To kick users, you must be a",
-    "To ban users, you must be a": "To ban users, you must be a",
-    "To remove other users' messages, you must be a": "To remove other users' messages, you must be a",
-    "To notify everyone in the room, you must be a": "To notify everyone in the room, you must be a",
+    "Default role": "Default role",
+    "Send messages": "Send messages",
+    "Invite users": "Invite users",
+    "Change settings": "Change settings",
+    "Kick users": "Kick users",
+    "Ban users": "Ban users",
+    "Remove messages": "Remove messages",
+    "Notify everyone": "Notify everyone",
     "No users have specific privileges in this room": "No users have specific privileges in this room",
-    "%(user)s is a %(userRole)s": "%(user)s is a %(userRole)s",
     "Privileged Users": "Privileged Users",
     "Muted Users": "Muted Users",
     "Banned users": "Banned users",
-    "To send events of type <eventType/>, you must be a": "To send events of type <eventType/>, you must be a",
+    "Send %(eventType)s events": "Send %(eventType)s events",
     "Roles & Permissions": "Roles & Permissions",
     "Permissions": "Permissions",
+    "Select the roles required to change various parts of the room": "Select the roles required to change various parts of the room",
     "Guests cannot join this room even if explicitly invited.": "Guests cannot join this room even if explicitly invited.",
     "Click here to fix": "Click here to fix",
     "To link to this room, please add an alias.": "To link to this room, please add an alias.",
@@ -685,7 +685,6 @@
     "Revoke Moderator": "Revoke Moderator",
     "Make Moderator": "Make Moderator",
     "Admin Tools": "Admin Tools",
-    "Level:": "Level:",
     "Close": "Close",
     "and %(count)s others...|other": "and %(count)s others...",
     "and %(count)s others...|one": "and one other...",
@@ -998,7 +997,7 @@
     "%(items)s and %(lastItem)s": "%(items)s and %(lastItem)s",
     "collapse": "collapse",
     "expand": "expand",
-    "Custom of %(powerLevel)s": "Custom of %(powerLevel)s",
+    "Power level": "Power level",
     "Custom level": "Custom level",
     "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.": "Unable to load event that was replied to, it either does not exist or you do not have permission to view it.",
     "<a>In reply to</a> <pill>": "<a>In reply to</a> <pill>",