From c265ec9571b8804302348c336a6962ecc5d1700b Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Sep 2017 15:07:09 +0100 Subject: [PATCH 1/4] Fix RoomView stuck in 'accept invite' state After accepting a 3pid invite. Rather than clear the joining flag when the join request completes, leave it so the RoomView can see that we're expecting the user to be joined in the various stages that might go through (waiting for join request, waiting for room object, waiting for 'joined' member event). The problem in this case was that we had to wait a bit for the last one, and there was no bit of state to represent it. This hopefully also makes the logic somewhat simpler. Fixes https://github.com/vector-im/riot-web/issues/5041 --- src/components/structures/RoomView.js | 28 +++++-------------- src/stores/RoomViewStore.js | 39 +++++++++++++++++---------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 2a6cf0aee4..e0332b1b19 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -123,6 +123,9 @@ module.exports = React.createClass({ // store the error here. roomLoadError: null, + // Have we sent a request to join the room that we're waiting to complete? + joining: false, + // this is true if we are fully scrolled-down, and are looking at // the end of the live timeline. It has the effect of hiding the // 'scroll to bottom' knob, among a couple of other things. @@ -185,10 +188,6 @@ module.exports = React.createClass({ shouldPeek: RoomViewStore.shouldPeek(), }; - // finished joining, start waiting for a room and show a spinner. See onRoom. - newState.waitingForRoom = this.state.joining && !newState.joining && - !RoomViewStore.getJoinError(); - // Temporary logging to diagnose https://github.com/vector-im/riot-web/issues/4307 console.log( 'RVS update:', @@ -197,7 +196,6 @@ module.exports = React.createClass({ 'loading?', newState.roomLoading, 'joining?', newState.joining, 'initial?', initial, - 'waiting?', newState.waitingForRoom, 'shouldPeek?', newState.shouldPeek, ); @@ -650,7 +648,6 @@ module.exports = React.createClass({ } this.setState({ room: room, - waitingForRoom: false, }, () => { this._onRoomLoaded(room); }); @@ -706,14 +703,7 @@ module.exports = React.createClass({ onRoomMemberMembership: function(ev, member, oldMembership) { if (member.userId == MatrixClientPeg.get().credentials.userId) { - - if (member.membership === 'join') { - this.setState({ - waitingForRoom: false, - }); - } else { - this.forceUpdate(); - } + this.forceUpdate(); } }, @@ -1463,10 +1453,6 @@ module.exports = React.createClass({ const Loader = sdk.getComponent("elements.Spinner"); const TimelinePanel = sdk.getComponent("structures.TimelinePanel"); - // Whether the preview bar spinner should be shown. We do this when joining or - // when waiting for a room to be returned by js-sdk when joining - const previewBarSpinner = this.state.joining || this.state.waitingForRoom; - if (!this.state.room) { if (this.state.roomLoading || this.state.peekLoading) { return ( @@ -1500,7 +1486,7 @@ module.exports = React.createClass({ onRejectClick={ this.onRejectThreepidInviteButtonClicked } canPreview={ false } error={ this.state.roomLoadError } roomAlias={roomAlias} - spinner={previewBarSpinner} + spinner={this.state.joining} inviterName={inviterName} invitedEmail={invitedEmail} room={this.state.room} @@ -1543,7 +1529,7 @@ module.exports = React.createClass({ onRejectClick={ this.onRejectButtonClicked } inviterName={ inviterName } canPreview={ false } - spinner={previewBarSpinner} + spinner={this.state.joining} room={this.state.room} /> @@ -1618,7 +1604,7 @@ module.exports = React.createClass({ { - dis.dispatch({ - action: 'joined_room', - }); + // We don't actually need to do anything here: we do *not* + // clear the 'joining' flag because the Room object and/or + // our 'joined' member event may not have come down the sync + // stream yet, and that's the point at which we'd consider + // the user joined to the room. }, (err) => { dis.dispatch({ action: 'join_room_error', @@ -202,12 +201,6 @@ class RoomViewStore extends Store { }); } - _joinedRoom(payload) { - this._setState({ - joining: false, - }); - } - _joinRoomError(payload) { this._setState({ joining: false, @@ -249,7 +242,25 @@ class RoomViewStore extends Store { return this._state.roomLoadError; } - // Whether we're joining the currently viewed room + // True if we're expecting the user to be joined to the room currently being + // viewed. Note that this is left true after the join request has finished, + // since we should still consider a join to be in progress until the room + // & member events come down the sync. + // + // This flag remains true after the room has been sucessfully joined, + // (this store doesn't listen for the appropriate member events) + // so you should always consider the room to be joined if the user's + // member events says they are joined. + // ie. The correct logic is: + // if (room && myMember.membership == 'joined') { + // // user is joined to the room + // } else { + // if (RoomViewStore.isJoining()) { + // // show spinner + // } else { + // // show join prompt + // } + // } isJoining() { return this._state.joining; } From 4d8eadad4f68584d39898bf8d0ba1d24de02f5e6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 19 Sep 2017 10:21:20 +0100 Subject: [PATCH 4/4] Clarify comment --- src/stores/RoomViewStore.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/stores/RoomViewStore.js b/src/stores/RoomViewStore.js index bd16c7a1d5..795345242e 100644 --- a/src/stores/RoomViewStore.js +++ b/src/stores/RoomViewStore.js @@ -249,11 +249,15 @@ class RoomViewStore extends Store { // // This flag remains true after the room has been sucessfully joined, // (this store doesn't listen for the appropriate member events) - // so you should always consider the room to be joined if the user's - // member events says they are joined. + // so you should always observe the joined state from the member event + // if a room object is present. // ie. The correct logic is: - // if (room && myMember.membership == 'joined') { - // // user is joined to the room + // if (room) { + // if (myMember.membership == 'joined') { + // // user is joined to the room + // } else { + // // Not joined + // } // } else { // if (RoomViewStore.isJoining()) { // // show spinner