From 266a504981c0d36013a8ee774a277c352baaa468 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 10 Nov 2017 10:49:14 +0000 Subject: [PATCH 1/3] Use the same MatrixClient for the duration of GroupView lifecycle Using the pegged matrix client would lead to trying to call removeListener on `null` when GroupView is unmounted during logout. --- src/components/structures/GroupView.js | 27 +++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index caa5e7cb01..1b5ebb6b36 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -435,14 +435,15 @@ export default React.createClass({ }, componentWillMount: function() { + this._matrixClient = MatrixClientPeg.get(); + this._matrixClient.on("Group.myMembership", this._onGroupMyMembership); + this._changeAvatarComponent = null; this._initGroupStore(this.props.groupId, true); - - MatrixClientPeg.get().on("Group.myMembership", this._onGroupMyMembership); }, componentWillUnmount: function() { - MatrixClientPeg.get().removeListener("Group.myMembership", this._onGroupMyMembership); + this._matrixClient.removeListener("Group.myMembership", this._onGroupMyMembership); this._groupStore.removeAllListeners(); }, @@ -464,11 +465,11 @@ export default React.createClass({ }, _initGroupStore: function(groupId, firstInit) { - const group = MatrixClientPeg.get().getGroup(groupId); + const group = this._matrixClient.getGroup(groupId); if (group && group.inviter && group.inviter.userId) { this._fetchInviterProfile(group.inviter.userId); } - this._groupStore = GroupStoreCache.getGroupStore(MatrixClientPeg.get(), groupId); + this._groupStore = GroupStoreCache.getGroupStore(this._matrixClient, groupId); this._groupStore.registerListener(() => { const summary = this._groupStore.getSummary(); if (summary.profile) { @@ -486,7 +487,7 @@ export default React.createClass({ groupRooms: this._groupStore.getGroupRooms(), groupRoomsLoading: !this._groupStore.isStateReady(GroupStore.STATE_KEY.GroupRooms), isUserMember: this._groupStore.getGroupMembers().some( - (m) => m.userId === MatrixClientPeg.get().credentials.userId, + (m) => m.userId === this._matrixClient.credentials.userId, ), error: null, }); @@ -506,7 +507,7 @@ export default React.createClass({ this.setState({ inviterProfileBusy: true, }); - MatrixClientPeg.get().getProfileInfo(userId).then((resp) => { + this._matrixClient.getProfileInfo(userId).then((resp) => { this.setState({ inviterProfile: { avatarUrl: resp.avatar_url, @@ -571,7 +572,7 @@ export default React.createClass({ if (!file) return; this.setState({uploadingAvatar: true}); - MatrixClientPeg.get().uploadContent(file).then((url) => { + this._matrixClient.uploadContent(file).then((url) => { const newProfileForm = Object.assign(this.state.profileForm, { avatar_url: url }); this.setState({ uploadingAvatar: false, @@ -591,7 +592,7 @@ export default React.createClass({ _onSaveClick: function() { this.setState({saving: true}); const savePromise = this.state.isUserPrivileged ? - MatrixClientPeg.get().setGroupProfile(this.props.groupId, this.state.profileForm) : + this._matrixClient.setGroupProfile(this.props.groupId, this.state.profileForm) : Promise.resolve(); savePromise.then((result) => { this.setState({ @@ -630,7 +631,7 @@ export default React.createClass({ _onRejectInviteClick: function() { this.setState({membershipBusy: true}); - MatrixClientPeg.get().leaveGroup(this.props.groupId).then(() => { + this._matrixClient.leaveGroup(this.props.groupId).then(() => { // don't reset membershipBusy here: wait for the membership change to come down the sync }).catch((e) => { this.setState({membershipBusy: false}); @@ -653,7 +654,7 @@ export default React.createClass({ if (!confirmed) return; this.setState({membershipBusy: true}); - MatrixClientPeg.get().leaveGroup(this.props.groupId).then(() => { + this._matrixClient.leaveGroup(this.props.groupId).then(() => { // don't reset membershipBusy here: wait for the membership change to come down the sync }).catch((e) => { this.setState({membershipBusy: false}); @@ -829,7 +830,7 @@ export default React.createClass({ const Spinner = sdk.getComponent("elements.Spinner"); const BaseAvatar = sdk.getComponent("avatars.BaseAvatar"); - const group = MatrixClientPeg.get().getGroup(this.props.groupId); + const group = this._matrixClient.getGroup(this.props.groupId); if (!group) return null; if (group.myMembership === 'invite') { @@ -839,7 +840,7 @@ export default React.createClass({ ; } const httpInviterAvatar = this.state.inviterProfile ? - MatrixClientPeg.get().mxcUrlToHttp( + this._matrixClient.mxcUrlToHttp( this.state.inviterProfile.avatarUrl, 36, 36, ) : null; From f4428267a15dcdf691dc4852a50b2245b0808bad Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 10 Nov 2017 10:51:28 +0000 Subject: [PATCH 2/3] Refactor onboarding redirect, add groups to the onboarding starters --- src/components/structures/MatrixChat.js | 63 +++++++++++-------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index e8ca8e82fc..1ca1992d75 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -74,6 +74,17 @@ const VIEWS = { LOGGED_IN: 6, }; +// Actions that are redirected through the onboarding process prior to being +// re-dispatched. NOTE: some actions are non-trivial and would require +// re-factoring to be included in this list in future. +const ONBOARDING_FLOW_STARTERS = [ + 'view_user_settings', + 'view_create_chat', + 'view_create_room', + 'view_my_groups', + 'view_group', +]; + module.exports = React.createClass({ // we export this so that the integration tests can use it :-S statics: { @@ -374,6 +385,22 @@ module.exports = React.createClass({ const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); + // Start the onboarding process for certain actions + if (MatrixClientPeg.get().isGuest() && + ONBOARDING_FLOW_STARTERS.includes(payload.action) + ) { + // This will cause `payload` to be dispatched later, once a + // sync has reached the "prepared" state. Setting a matrix ID + // will cause a full login and sync and finally the deferred + // action will be dispatched. + dis.dispatch({ + action: 'do_after_sync_prepared', + deferred_action: payload, + }); + dis.dispatch({action: 'view_set_mxid'}); + return; + } + switch (payload.action) { case 'logout': Lifecycle.logout(); @@ -463,16 +490,6 @@ module.exports = React.createClass({ this._viewIndexedRoom(payload.roomIndex); break; case 'view_user_settings': - if (MatrixClientPeg.get().isGuest()) { - dis.dispatch({ - action: 'do_after_sync_prepared', - deferred_action: { - action: 'view_user_settings', - }, - }); - dis.dispatch({action: 'view_set_mxid'}); - break; - } this._setPage(PageTypes.UserSettings); this.notifyNewScreen('settings'); break; @@ -509,7 +526,7 @@ module.exports = React.createClass({ this._chatCreateOrReuse(payload.user_id, payload.go_home_on_cancel); break; case 'view_create_chat': - this._createChat(); + showStartChatInviteDialog(); break; case 'view_invite': showRoomInviteDialog(payload.roomId); @@ -750,31 +767,7 @@ module.exports = React.createClass({ }).close; }, - _createChat: function() { - if (MatrixClientPeg.get().isGuest()) { - dis.dispatch({ - action: 'do_after_sync_prepared', - deferred_action: { - action: 'view_create_chat', - }, - }); - dis.dispatch({action: 'view_set_mxid'}); - return; - } - showStartChatInviteDialog(); - }, - _createRoom: function() { - if (MatrixClientPeg.get().isGuest()) { - dis.dispatch({ - action: 'do_after_sync_prepared', - deferred_action: { - action: 'view_create_room', - }, - }); - dis.dispatch({action: 'view_set_mxid'}); - return; - } const CreateRoomDialog = sdk.getComponent('dialogs.CreateRoomDialog'); Modal.createTrackedDialog('Create Room', '', CreateRoomDialog, { onFinished: (shouldCreate, name, noFederate) => { From 0d174ffe9a55114388d35d3db33b192525bd26c8 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 10 Nov 2017 11:13:52 +0000 Subject: [PATCH 3/3] Fix tests --- src/components/structures/MatrixChat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 1ca1992d75..c201d139c5 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -386,7 +386,7 @@ module.exports = React.createClass({ const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); // Start the onboarding process for certain actions - if (MatrixClientPeg.get().isGuest() && + if (MatrixClientPeg.get() && MatrixClientPeg.get().isGuest() && ONBOARDING_FLOW_STARTERS.includes(payload.action) ) { // This will cause `payload` to be dispatched later, once a