Styling to match the other tabs a bit closer

Introduces a new component to reduce code duplication

src/components/views/elements/LabelledToggleSwitch.js

@@ -0,0 +1,42 @@
+/*
+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.
+*/
+
+import React from 'react';
+import PropTypes from "prop-types";
+import ToggleSwitch from "./ToggleSwitch";
+
+export default class LabelledToggleSwitch extends React.Component {
+    static propTypes = {
+        // The value for the toggle switch
+        value: PropTypes.bool.isRequired,
+
+        // The function to call when the value changes
+        onChange: PropTypes.func.isRequired,
+
+        // The translated label for the switch
+        label: PropTypes.string.isRequired,
+    };
+
+    render() {
+        // This is a minimal version of a SettingsFlag
+        return (
+            <div className="mx_SettingsFlag">
+                <span className="mx_SettingsFlag_label">{this.props.label}</span>
+                <ToggleSwitch checked={this.props.value} onChange={this.props.onChange} />
+            </div>
+        );
+    }
+}

src/components/views/settings/Notifications.js

@@ -29,6 +29,7 @@ import {
     ContentRules,
 } from '../../../notifications';
 import * as SdkConfig from "../../../SdkConfig";
+import LabelledToggleSwitch from "../elements/LabelledToggleSwitch";
 
 // TODO: this "view" component still has far too much application logic in it,
 // which should be factored out to other files.
@@ -71,17 +72,6 @@ module.exports = React.createClass({
         ERROR: "ERROR", // There was an error
     },
 
-    propTypes: {
-        // The array of threepids from the JS SDK (required for email notifications)
-        threepids: React.PropTypes.array.isRequired,
-    },
-
-    getDefaultProps: function() {
-        return {
-            threepids: [],
-        };
-    },
-
     getInitialState: function() {
         return {
             phase: this.phases.LOADING,
@@ -93,6 +83,7 @@ module.exports = React.createClass({
             },
             externalPushRules: [], // Push rules (except content rule) that have been defined outside Vector UI
             externalContentRules: [], // Keyword push rules that have been defined outside Vector UI
+            threepids: [], // used for email notifications
         };
     },
 
@@ -100,42 +91,42 @@ module.exports = React.createClass({
         this._refreshFromServer();
     },
 
-    onEnableNotificationsChange: function(event) {
+    onEnableNotificationsChange: function(checked) {
         const self = this;
         this.setState({
             phase: this.phases.LOADING,
         });
 
-        MatrixClientPeg.get().setPushRuleEnabled('global', self.state.masterPushRule.kind, self.state.masterPushRule.rule_id, !event.target.checked).done(function() {
+        MatrixClientPeg.get().setPushRuleEnabled('global', self.state.masterPushRule.kind, self.state.masterPushRule.rule_id, !checked).done(function() {
            self._refreshFromServer();
         });
     },
 
-    onEnableDesktopNotificationsChange: function(event) {
+    onEnableDesktopNotificationsChange: function(checked) {
         SettingsStore.setValue(
             "notificationsEnabled", null,
             SettingLevel.DEVICE,
-            event.target.checked,
+            checked,
         ).finally(() => {
             this.forceUpdate();
         });
     },
 
-    onEnableDesktopNotificationBodyChange: function(event) {
+    onEnableDesktopNotificationBodyChange: function(checked) {
         SettingsStore.setValue(
             "notificationBodyEnabled", null,
             SettingLevel.DEVICE,
-            event.target.checked,
+            checked,
         ).finally(() => {
             this.forceUpdate();
         });
     },
 
-    onEnableAudioNotificationsChange: function(event) {
+    onEnableAudioNotificationsChange: function(checked) {
         SettingsStore.setValue(
             "audioNotificationsEnabled", null,
             SettingLevel.DEVICE,
-            event.target.checked,
+            checked,
         ).finally(() => {
             this.forceUpdate();
         });
@@ -433,7 +424,6 @@ module.exports = React.createClass({
     // Check if any legacy im.vector rules need to be ported to the new API
     // for overriding the actions of default rules.
     _portRulesToNewAPI: function(rulesets) {
-        const self = this;
         const needsUpdate = [];
         const cli = MatrixClientPeg.get();
 
@@ -632,6 +622,8 @@ module.exports = React.createClass({
                 externalPushRules: self.state.externalPushRules,
             });
         }).done();
+
+        MatrixClientPeg.get().getThreePids().then((r) => this.setState({threepids: r.threepids}));
     },
 
     _updatePushRuleActions: function(rule, actions, enabled) {
@@ -690,27 +682,25 @@ module.exports = React.createClass({
         return rows;
     },
 
+    hasEmailPusher: function(pushers, address) {
+        if (pushers === undefined) {
+            return false;
+        }
+        for (let i = 0; i < pushers.length; ++i) {
+            if (pushers[i].kind === 'email' && pushers[i].pushkey === address) {
+                return true;
+            }
+        }
+        return false;
+    },
+
     emailNotificationsRow: function(address, label) {
-        return (<div className="mx_UserNotifSettings_tableRow">
+        return <LabelledToggleSwitch value={this.hasEmailPusher(this.state.pushers, address)}
-            <div className="mx_UserNotifSettings_inputCell">
+                                     onChange={this.onEnableEmailNotificationsChange.bind(this, address)}
-                <input id="enableEmailNotifications_{address}"
+                                     label={label} />;
-                    ref="enableEmailNotifications_{address}"
-                    type="checkbox"
-                    checked={ UserSettingsStore.hasEmailPusher(this.state.pushers, address) }
-                    onChange={ this.onEnableEmailNotificationsChange.bind(this, address) }
-                />
-            </div>
-            <div className="mx_UserNotifSettings_labelCell">
-                <label htmlFor="enableEmailNotifications_{address}">
-                    {label}
-                </label>
-            </div>
-        </div>);
     },
 
     render: function() {
-        const self = this;
-
         let spinner;
         if (this.state.phase === this.phases.LOADING) {
             const Loader = sdk.getComponent("elements.Spinner");
@@ -719,23 +709,9 @@ module.exports = React.createClass({
 
         let masterPushRuleDiv;
         if (this.state.masterPushRule) {
-            masterPushRuleDiv = (
+            masterPushRuleDiv = <LabelledToggleSwitch value={!this.state.masterPushRule.enabled}
-                <div className="mx_UserNotifSettings_tableRow">
+                                                      onChange={this.onEnableNotificationsChange}
-                    <div className="mx_UserNotifSettings_inputCell">
+                                                      label={_t('Enable notifications for this account')}/>;
-                        <input id="enableNotifications"
-                            ref="enableNotifications"
-                            type="checkbox"
-                            checked={ !this.state.masterPushRule.enabled }
-                            onChange={ this.onEnableNotificationsChange }
-                        />
-                    </div>
-                    <div className="mx_UserNotifSettings_labelCell">
-                        <label htmlFor="enableNotifications">
-                            { _t('Enable notifications for this account') }
-                        </label>
-                    </div>
-                </div>
-            );
         }
 
         // When enabled, the master rule inhibits all existing rules
@@ -746,17 +722,17 @@ module.exports = React.createClass({
                     {masterPushRuleDiv}
 
                     <div className="mx_UserSettings_notifTable">
-                        { _t('All notifications are currently disabled for all targets.') }.
+                        { _t('All notifications are currently disabled for all targets.') }
                     </div>
                 </div>
             );
         }
 
-        const emailThreepids = this.props.threepids.filter((tp) => tp.medium === "email");
+        const emailThreepids = this.state.threepids.filter((tp) => tp.medium === "email");
         let emailNotificationsRow;
         if (emailThreepids.length === 0) {
             emailNotificationsRow = <div>
-                { _t('Add an email address above to configure email notifications') }
+                { _t('Add an email address to configure email notifications') }
             </div>;
         } else {
             // This only supports the first email address in your profile for now
@@ -787,7 +763,7 @@ module.exports = React.createClass({
         let devicesSection;
         if (this.state.pushers === undefined) {
             devicesSection = <div className="error">{ _t('Unable to fetch notification target list') }</div>;
-        } else if (this.state.pushers.length == 0) {
+        } else if (this.state.pushers.length === 0) {
             devicesSection = null;
         } else {
             // TODO: It would be great to be able to delete pushers from here too,
@@ -835,50 +811,17 @@ module.exports = React.createClass({
 
                     { spinner }
 
-                    <div className="mx_UserNotifSettings_tableRow">
+                    <LabelledToggleSwitch value={SettingsStore.getValue("notificationsEnabled")}
-                        <div className="mx_UserNotifSettings_inputCell">
+                                          onChange={this.onEnableDesktopNotificationsChange}
-                            <input id="enableDesktopNotifications"
+                                          label={_t('Enable desktop notifications')} />
-                                ref="enableDesktopNotifications"
-                                type="checkbox"
-                                checked={ SettingsStore.getValue("notificationsEnabled") }
-                                onChange={ this.onEnableDesktopNotificationsChange } />
-                        </div>
-                        <div className="mx_UserNotifSettings_labelCell">
-                            <label htmlFor="enableDesktopNotifications">
-                                { _t('Enable desktop notifications') }
-                            </label>
-                        </div>
-                    </div>
 
-                    <div className="mx_UserNotifSettings_tableRow">
+                    <LabelledToggleSwitch value={SettingsStore.getValue("notificationBodyEnabled")}
-                        <div className="mx_UserNotifSettings_inputCell">
+                                          onChange={this.onEnableDesktopNotificationBodyChange}
-                            <input id="enableDesktopNotificationBody"
+                                          label={_t('Show message in desktop notification')} />
-                                ref="enableDesktopNotificationBody"
-                                type="checkbox"
-                                checked={ SettingsStore.getValue("notificationBodyEnabled") }
-                                onChange={ this.onEnableDesktopNotificationBodyChange } />
-                        </div>
-                        <div className="mx_UserNotifSettings_labelCell">
-                            <label htmlFor="enableDesktopNotificationBody">
-                                { _t('Show message in desktop notification') }
-                            </label>
-                        </div>
-                    </div>
 
-                    <div className="mx_UserNotifSettings_tableRow">
+                    <LabelledToggleSwitch value={SettingsStore.getValue("audioNotificationsEnabled")}
-                        <div className="mx_UserNotifSettings_inputCell">
+                                          onChange={this.onEnableAudioNotificationsChange}
-                            <input id="enableDesktopAudioNotifications"
+                                          label={_t('Enable audible notifications in web client')} />
-                                ref="enableDesktopAudioNotifications"
-                                type="checkbox"
-                                checked={ SettingsStore.getValue("audioNotificationsEnabled") }
-                                onChange={ this.onEnableAudioNotificationsChange } />
-                        </div>
-                        <div className="mx_UserNotifSettings_labelCell">
-                            <label htmlFor="enableDesktopAudioNotifications">
-                                { _t('Enable audible notifications in web client') }
-                            </label>
-                        </div>
-                    </div>
 
                     { emailNotificationsRow }
 

src/components/views/settings/tabs/LabsSettingsTab.js

@@ -18,8 +18,8 @@ import React from 'react';
 import {_t} from "../../../../languageHandler";
 import PropTypes from "prop-types";
 import SettingsStore from "../../../../settings/SettingsStore";
-import ToggleSwitch from "../../elements/ToggleSwitch";
 import MatrixClientPeg from "../../../../MatrixClientPeg";
+import LabelledToggleSwitch from "../../elements/LabelledToggleSwitch";
 const Modal = require("../../../../Modal");
 const sdk = require("../../../../index");
 
@@ -65,15 +65,9 @@ export class LabsSettingToggle extends React.Component {
     };
 
     render() {
-        // This is a minimal version of a SettingsFlag
         const label = _t(SettingsStore.getDisplayName(this.props.featureId));
         const value = SettingsStore.isFeatureEnabled(this.props.featureId);
-        return (
+        return <LabelledToggleSwitch value={value} label={label} onChange={this._onChange} />
-            <div className="mx_SettingsFlag">
-                <span className="mx_SettingsFlag_label">{label}</span>
-                <ToggleSwitch checked={value} onChange={this._onChange} />
-            </div>
-        );
     }
 }
 

src/components/views/settings/tabs/NotificationSettingsTab.js

@@ -16,20 +16,11 @@ limitations under the License.
 
 import React from 'react';
 import {_t} from "../../../../languageHandler";
-import MatrixClientPeg from "../../../../MatrixClientPeg";
 const sdk = require("../../../../index");
 
 export default class NotificationSettingsTab extends React.Component {
     constructor() {
         super();
-
-        this.state = {
-            threepids: [],
-        };
-    }
-
-    async componentWillMount(): void {
-        MatrixClientPeg.get().getThreePids().then(r => this.setState({threepids: r.threepids}));
     }
 
     render() {
@@ -38,7 +29,7 @@ export default class NotificationSettingsTab extends React.Component {
             <div className="mx_SettingsTab">
                 <div className="mx_SettingsTab_heading">{_t("Notifications")}</div>
                 <div className="mx_SettingsTab_section">
-                    <Notifications threepids={this.state.threepids} />
+                    <Notifications />
                 </div>
             </div>
         );