From 1af5018597742465ed3703759f52af44271126cb Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 23 Dec 2015 11:47:56 +0000 Subject: [PATCH] General code cleanup / tweaks / fixes - Swap Phases enum to be using string literals - Swap roomId prop on UserSettings for a more sane onUserSettingsClose and make MatrixChat responsible for swapping the room. - s/then/done/ when terminating Promise chains to avoid subtle errors. - Rejig render() of UserSettings so we don't need to indent quite so much. --- src/UserSettingsStore.js | 21 +- src/components/structures/MatrixChat.js | 18 +- src/components/structures/UserSettings.js | 297 +++++++++++----------- 3 files changed, 170 insertions(+), 166 deletions(-) diff --git a/src/UserSettingsStore.js b/src/UserSettingsStore.js index 50ee03a433..bac85ab325 100644 --- a/src/UserSettingsStore.js +++ b/src/UserSettingsStore.js @@ -17,19 +17,10 @@ limitations under the License. 'use strict'; var MatrixClientPeg = require("./MatrixClientPeg"); -var sdk = require('./index'); - -// XXX: should we be doing something here to use this as a singleton rather than -// class methods? +var Notifier = require("./Notifier"); module.exports = { - // we add these wrappers to the js-sdk here in case we want to do react-specific - // dispatches or similar in future, and to give us a place to clearly separate - // business logic specific from the 'thin' react component and parent wiring component - // which actually handles the UI. - // XXX: I'm not convinced this abstraction is worth it though. - loadProfileInfo: function() { var cli = MatrixClientPeg.get(); return cli.getProfileInfo(cli.credentials.userId); @@ -48,13 +39,10 @@ module.exports = { }, getEnableNotifications: function() { - var Notifier = sdk.getComponent('organisms.Notifier'); return Notifier.isEnabled(); }, setEnableNotifications: function(enable) { - var Notifier = sdk.getComponent('organisms.Notifier'); - var self = this; if (!Notifier.supportsDesktopNotifications()) { return; } @@ -70,11 +58,6 @@ module.exports = { password: old_password }; - this.setState({ - phase: this.Phases.Uploading, - errorString: '', - }) - return cli.setPassword(authDict, new_password); }, -} +}; diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 7148848af1..9273e0e03d 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -628,6 +628,22 @@ module.exports = React.createClass({ this.showScreen("settings"); }, + onUserSettingsClose: function() { + // XXX: use browser history instead to find the previous room? + if (this.state.currentRoom) { + dis.dispatch({ + action: 'view_room', + room_id: this.state.currentRoom, + }); + } + else { + dis.dispatch({ + action: 'view_indexed_room', + roomIndex: 0, + }); + } + }, + render: function() { var LeftPanel = sdk.getComponent('structures.LeftPanel'); var RoomView = sdk.getComponent('structures.RoomView'); @@ -660,7 +676,7 @@ module.exports = React.createClass({ right_panel = break; case this.PageTypes.UserSettings: - page_element = + page_element = right_panel = break; case this.PageTypes.CreateRoom: diff --git a/src/components/structures/UserSettings.js b/src/components/structures/UserSettings.js index 9a62149023..1401bee337 100644 --- a/src/components/structures/UserSettings.js +++ b/src/components/structures/UserSettings.js @@ -25,10 +25,14 @@ var UserSettingsStore = require('../../UserSettingsStore'); module.exports = React.createClass({ displayName: 'UserSettings', - Phases: { - Loading: "loading", - Saving: "saving", - Display: "display", + propTypes: { + onClose: React.PropTypes.func + }, + + getDefaultProps: function() { + return { + onClose: function() {} + }; }, getInitialState: function() { @@ -37,38 +41,34 @@ module.exports = React.createClass({ displayName: null, threePids: [], clientVersion: version, - phase: this.Phases.Loading, + phase: "UserSettings.LOADING", // LOADING, DISPLAY, SAVING }; }, componentWillMount: function() { var self = this; - var profilePromise = UserSettingsStore.loadProfileInfo(); - var threepidPromise = UserSettingsStore.loadThreePids(); + q.all([ + UserSettingsStore.loadProfileInfo(), UserSettingsStore.loadThreePids() + ]).done(function(resps) { + self.setState({ + avatarUrl: resps[0].avatar_url, + displayName: resps[0].displayname, + threepids: resps[1].threepids, + phase: "UserSettings.DISPLAY", + }); - q.all([profilePromise, threepidPromise]).then( - function(resps) { - self.setState({ - avatarUrl: resps[0].avatar_url, - displayName: resps[0].displayname, - threepids: resps[1].threepids, - phase: self.Phases.Display, - }); - - // keep a copy of the original state in order to track changes - self.setState({ - originalState: self.state - }); - }, - function(error) { - var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - Modal.createDialog(ErrorDialog, { - title: "Can't load user settings", - description: error.toString() - }); - } - ); + // keep a copy of the original state in order to track changes + self.setState({ + originalState: self.state + }); + }, function(error) { + var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); + Modal.createDialog(ErrorDialog, { + title: "Can't load user settings", + description: error.toString() + }); + }); }, componentDidMount: function() { @@ -95,51 +95,33 @@ module.exports = React.createClass({ } if (this.state.originalState.threepids.length !== this.state.threepids.length || - this.state.originalState.threepids.every(function(element, index) { + this.state.originalState.threepids.every((element, index) => { return element === this.state.threepids[index]; - })) - { - savePromises.push( UserSettingsStore.saveThreePids(this.state.threepids) ); + })) { + savePromises.push( + UserSettingsStore.saveThreePids(this.state.threepids) + ); } self.setState({ - phase: self.Phases.Saving, + phase: "UserSettings.SAVING", }); - q.all(savePromises).then( - function(resps) { - self.setState({ - phase: self.Phases.Display, - }); - self.onClose(); - }, - function(error) { - self.setState({ - phase: self.Phases.Display, - }); - var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - Modal.createDialog(ErrorDialog, { - title: "Can't save user settings", - description: error.toString() - }); - } - ); - }, - - onClose: function(ev) { - // XXX: use browser history instead to find the previous room? - if (this.props.roomId) { - dis.dispatch({ - action: 'view_room', - room_id: this.props.roomId, + q.all(savePromises).done(function(resps) { + self.setState({ + phase: "UserSettings.DISPLAY", }); - } - else { - dis.dispatch({ - action: 'view_indexed_room', - roomIndex: 0, + self.props.onClose(); + }, function(error) { + self.setState({ + phase: "UserSettings.DISPLAY", }); - } + var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); + Modal.createDialog(ErrorDialog, { + title: "Can't save user settings", + description: error.toString() + }); + }); }, onAction: function(payload) { @@ -170,7 +152,9 @@ module.exports = React.createClass({ onLogoutClicked: function(ev) { var LogoutPrompt = sdk.getComponent('dialogs.LogoutPrompt'); - this.logoutModal = Modal.createDialog(LogoutPrompt, {onCancel: this.onLogoutPromptCancel}); + this.logoutModal = Modal.createDialog( + LogoutPrompt, {onCancel: this.onLogoutPromptCancel} + ); }, onLogoutPromptCancel: function() { @@ -193,100 +177,121 @@ module.exports = React.createClass({ var Loader = sdk.getComponent("elements.Spinner"); var saving; switch (this.state.phase) { - case this.Phases.Loading: + case "UserSettings.LOADING": return - case this.Phases.Saving: + case "UserSettings.SAVING": saving = - case this.Phases.Display: - var RoomHeader = sdk.getComponent('rooms.RoomHeader'); - return ( -
- + // intentional fall through + case "UserSettings.DISPLAY": + break; // quit the switch to return the common state + default: + throw new Error("Unknown state.phase => " + this.state.phase); + } + // can only get here if phase is UserSettings.DISPLAY + var RoomHeader = sdk.getComponent('rooms.RoomHeader'); + return ( +
+ -

Profile

+

Profile

-
-
-
-
- -
-
- -
-
- - {this.state.threepids.map(function(val) { - var id = "email-" + val.address; - return ( -
-
- -
-
- -
-
- ); - })} - -
-
- -
-
- -
-
-
-
- -
-
- -
-
- -
- -
-
+
+
+
+
+ +
+
+
-
-
Log out
-
- -

Notifications

- -
-
-
-
- + {this.state.threepids.map(function(val, pidIndex) { + var id = "email-" + val.address; + return ( +
+
+
-
- +
+
+ ); + })} + +
+
+ +
+
+ +
+
+
+
+ +
+
+
-

Advanced

+
-
-
- Version {this.state.clientVersion} -
-
- -
-
{ saving }
-
Save and close
+
+
- ); - } +
+ +
+
+ Log out +
+
+ +

Notifications

+ +
+
+
+
+ +
+
+ +
+
+
+
+ +

Advanced

+ +
+
+ Version {this.state.clientVersion} +
+
+ +
+
{ saving }
+
+ Save and close +
+
+
+ ); } });