From 11a38fce487e3230eda5eb73375b1605a70b4ccf Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Jun 2016 11:37:04 +0100 Subject: [PATCH 1/6] Fix peeking Sorts out more of the room joining mess. currentRoom which held the room ID is now more appropriately called currentRoomId. RoomView will now take a roomID or alias as before but will now look up the room ID as required if given the alias. Also, now look up the alias every time you click on it so it's never stale, rather than looking in your current rooms for a room that thinks it has that ID. --- src/components/structures/MatrixChat.js | 37 ++++++++------------ src/components/structures/RoomView.js | 46 +++++++++++++++++++------ 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index b70c89e2d8..01aa9dbda6 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -409,7 +409,7 @@ module.exports = React.createClass({ ); var roomIndex = -1; for (var i = 0; i < allRooms.length; ++i) { - if (allRooms[i].roomId == this.state.currentRoom) { + if (allRooms[i].roomId == this.state.currentRoomId) { roomIndex = i; break; } @@ -506,20 +506,10 @@ module.exports = React.createClass({ page_type: this.PageTypes.RoomView, thirdPartyInvite: thirdPartyInvite, roomOobData: oob_data, + newState.currentRoomAlias: roomAlias, + newState.currentRoomId: roomId, }; - // If an alias has been provided, we use that and only that, - // since otherwise we'll prefer to pass in an ID to RoomView - // but if we're not in the room, we should join by alias rather - // than ID. - if (roomAlias) { - newState.currentRoomAlias = roomAlias; - newState.currentRoom = null; - } else { - newState.currentRoomAlias = null; - newState.currentRoom = roomId; - } - // if we aren't given an explicit event id, look for one in the // scrollStateMap. if (!eventId) { @@ -612,13 +602,13 @@ module.exports = React.createClass({ dis.dispatch(self.starting_room_alias_payload); delete self.starting_room_alias_payload; } else if (!self.state.page_type) { - if (!self.state.currentRoom) { + if (!self.state.currentRoomId) { var firstRoom = null; if (cli.getRooms() && cli.getRooms().length) { firstRoom = RoomListSorter.mostRecentActivityFirst( cli.getRooms() )[0].roomId; - self.setState({ready: true, currentRoom: firstRoom, page_type: self.PageTypes.RoomView}); + self.setState({ready: true, currentRoomId: firstRoom, page_type: self.PageTypes.RoomView}); } else { self.setState({ready: true, page_type: self.PageTypes.RoomDirectory}); } @@ -628,8 +618,8 @@ module.exports = React.createClass({ // we notifyNewScreen now because now the room will actually be displayed, // and (mostly) now we can get the correct alias. - var presentedId = self.state.currentRoom; - var room = MatrixClientPeg.get().getRoom(self.state.currentRoom); + var presentedId = self.state.currentRoomId; + var room = MatrixClientPeg.get().getRoom(self.state.currentRoomId); if (room) { var theAlias = MatrixTools.getCanonicalAliasForRoom(room); if (theAlias) presentedId = theAlias; @@ -979,10 +969,10 @@ module.exports = React.createClass({ onUserSettingsClose: function() { // XXX: use browser history instead to find the previous room? // or maintain a this.state.pageHistory in _setPage()? - if (this.state.currentRoom) { + if (this.state.currentRoomId) { dis.dispatch({ action: 'view_room', - room_id: this.state.currentRoom, + room_id: this.state.currentRoomId, }); } else { @@ -1022,17 +1012,18 @@ module.exports = React.createClass({ page_element = ( ); - right_panel = + right_panel = break; case this.PageTypes.UserSettings: page_element = @@ -1068,7 +1059,7 @@ module.exports = React.createClass({
{topBar}
- +
{page_element}
diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 9fc335236c..d840630760 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -55,7 +55,10 @@ module.exports = React.createClass({ propTypes: { ConferenceHandler: React.PropTypes.any, - // the ID for this room (or, if we don't know it, an alias for it) + // Either a room ID or room alias for the room to display. + // If the room is being displayed as a result of the user clicking + // on a room alias, the alias should be supplied. Otherwise, a room + // ID should be supplied. roomAddress: React.PropTypes.string.isRequired, // An object representing a third party invite to join this room @@ -95,15 +98,14 @@ module.exports = React.createClass({ getInitialState: function() { var room; + var room_id; if (this.props.roomAddress[0] == '!') { - room = MatrixClientPeg.get().getRoom(this.props.roomAddress); - } else { - room = MatrixTools.getRoomForAlias( - MatrixClientPeg.get().getRooms(), this.props.roomAddress - ); + room_id = this.props.roomAddress; + room = MatrixClientPeg.get().getRoom(room_id); } return { room: room, + roomId: room_id, roomLoading: !room, editingRoomSettings: false, uploadingRoomSettings: false, @@ -143,6 +145,27 @@ module.exports = React.createClass({ } }); + if (this.props.roomAddress[0] == '#') { + // we always look up the alias from the directory server: + // we want the room that the given alias is pointing to + // right now. We may have joined that alias before but there's + // no guarantee the alias hasn't subsequently been remapped. + MatrixClientPeg.get().getRoomIdForAlias(this.props.roomAddress).done((result) => { + this.setState({ + roomId: result.room_id, + roomLoading: false, + }, this.updatePeeking); + }, (err) => { + this.setState({ + roomLoading: false, + }); + }); + } + + this.updatePeeking(); + }, + + updatePeeking: function() { // if this is an unknown room then we're in one of three states: // - This is a room we can peek into (search engine) (we can /peek) // - This is a room we can publicly join or were invited to. (we can /join) @@ -150,10 +173,13 @@ module.exports = React.createClass({ // We can't try to /join because this may implicitly accept invites (!) // We can /peek though. If it fails then we present the join UI. If it // succeeds then great, show the preview (but we still may be able to /join!). - if (!this.state.room) { - console.log("Attempting to peek into room %s", this.props.roomAddress); + // Note that peeking works by room ID and room ID only, as opposed to joining + // which must be by alias or invite wherever possible (peeking currently does + // not work over federation). + if (!this.state.room && this.state.roomId) { + console.log("Attempting to peek into room %s", this.state.roomId); - MatrixClientPeg.get().peekInRoom(this.props.roomAddress).then((room) => { + MatrixClientPeg.get().peekInRoom(this.state.roomId).then((room) => { this.setState({ room: room, roomLoading: false, @@ -172,7 +198,7 @@ module.exports = React.createClass({ throw err; } }).done(); - } else { + } else if (this.state.room) { MatrixClientPeg.get().stopPeeking(); this._onRoomLoaded(this.state.room); } From aaefdf19c578266ccfd44f5470ac7835475148b3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Jun 2016 11:57:07 +0100 Subject: [PATCH 2/6] Fix MatrixChat syntax fail --- src/components/structures/MatrixChat.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 01aa9dbda6..5dd460d74a 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -506,8 +506,8 @@ module.exports = React.createClass({ page_type: this.PageTypes.RoomView, thirdPartyInvite: thirdPartyInvite, roomOobData: oob_data, - newState.currentRoomAlias: roomAlias, - newState.currentRoomId: roomId, + currentRoomAlias: roomAlias, + currentRoomId: roomId, }; // if we aren't given an explicit event id, look for one in the @@ -1012,8 +1012,7 @@ module.exports = React.createClass({ page_element = ( Date: Tue, 14 Jun 2016 12:56:37 +0100 Subject: [PATCH 3/6] Fix member list vanishing Add a callback to RoomView that it can give the room ID to once it's resolved it, since this lookup is now the responsibility of the roomview and only the roomview. The view_room action now has either an alias or an ID, not both. Also fix RoomView to load the room properly and not try to peek when it shouldn't. --- src/components/structures/MatrixChat.js | 23 +++++++++++++++++------ src/components/structures/RoomView.js | 12 +++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 5dd460d74a..494792d902 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -392,10 +392,10 @@ module.exports = React.createClass({ }); break; case 'view_room': - // Takes both room ID and room alias: if switching to a room the client is already - // know to be in (eg. user clicks on a room in the recents panel), supply only the - // ID. If the user is clicking on a room in the context of the alias being presented - // to them, supply the room alias and optionally the room ID. + // Takes either a room ID or room alias: if switching to a room the client is already + // known to be in (eg. user clicks on a room in the recents panel), supply the ID + // If the user is clicking on a room in the context of the alias being presented + // to them, supply the room alias. If both are supplied, the room ID will be ignored. this._viewRoom( payload.room_id, payload.room_alias, payload.show_settings, payload.event_id, payload.third_party_invite, payload.oob_data @@ -507,9 +507,12 @@ module.exports = React.createClass({ thirdPartyInvite: thirdPartyInvite, roomOobData: oob_data, currentRoomAlias: roomAlias, - currentRoomId: roomId, }; + if (!roomAlias) { + newState.currentRoomId = roomId; + } + // if we aren't given an explicit event id, look for one in the // scrollStateMap. if (!eventId) { @@ -982,6 +985,13 @@ module.exports = React.createClass({ } }, + onRoomIdResolved: function(room_id) { + // It's the RoomView's resposibility to look up room aliases, but we need the + // ID to pass into things like the Member List, so the Room View tells us when + // its done that resolution so we can display things that take a room ID. + this.setState({currentRoomId: room_id}); + }, + render: function() { var LeftPanel = sdk.getComponent('structures.LeftPanel'); var RoomView = sdk.getComponent('structures.RoomView'); @@ -1013,12 +1023,13 @@ module.exports = React.createClass({ ); diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index d840630760..4a3d3e7a31 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -61,6 +61,11 @@ module.exports = React.createClass({ // ID should be supplied. roomAddress: React.PropTypes.string.isRequired, + // If a room alias is passed to roomAddress, a function can be + // provided here that will be called with the ID of the room + // once it has been resolved. + onRoomIdResolved: React.PropTypes.func, + // An object representing a third party invite to join this room // Fields: // * inviteSignUrl (string) The URL used to join this room from an email invite @@ -151,9 +156,14 @@ module.exports = React.createClass({ // right now. We may have joined that alias before but there's // no guarantee the alias hasn't subsequently been remapped. MatrixClientPeg.get().getRoomIdForAlias(this.props.roomAddress).done((result) => { + if (this.props.onRoomIdResolved) { + this.props.onRoomIdResolved(result.room_id); + } + var room = MatrixClientPeg.get().getRoom(result.room_id); this.setState({ + room: room, roomId: result.room_id, - roomLoading: false, + roomLoading: !room, }, this.updatePeeking); }, (err) => { this.setState({ From 0ef5cc891e7f35b0c2035eb2a9a8ed41268067db Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Jun 2016 14:10:49 +0100 Subject: [PATCH 4/6] Add currentRoomId / Alias to getInitialState with docs --- src/components/structures/MatrixChat.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 494792d902..9899f66cc5 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -64,6 +64,13 @@ module.exports = React.createClass({ getInitialState: function() { var s = { + // If we are viewing a room by alias, this contains the alias + currentRoomAlias: null, + + // The ID of the room we're viewing. This is either populated directly + // in the case where we view a room by ID or by RoomView when it resolves + // what ID an alias points at. + currentRoomId: null, logged_in: !!(MatrixClientPeg.get() && MatrixClientPeg.get().credentials), collapse_lhs: false, collapse_rhs: false, From a95d8b5ed6f320a77e2ae743b7bcf56548d65aa9 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Jun 2016 14:38:45 +0100 Subject: [PATCH 5/6] Move init logic into componentWillMount to simplify getInitialState --- src/components/structures/RoomView.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 4a3d3e7a31..2ff7ed1cbd 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -102,23 +102,17 @@ module.exports = React.createClass({ }, getInitialState: function() { - var room; - var room_id; - if (this.props.roomAddress[0] == '!') { - room_id = this.props.roomAddress; - room = MatrixClientPeg.get().getRoom(room_id); - } return { - room: room, - roomId: room_id, - roomLoading: !room, + room: null, + roomId: null, + roomLoading: true, editingRoomSettings: false, uploadingRoomSettings: false, numUnreadMessages: 0, draggingFile: false, searching: false, searchResults: null, - hasUnsentMessages: this._hasUnsentMessages(room), + hasUnsentMessages: false, callState: null, guestsCanJoin: false, canPeek: false, @@ -164,15 +158,22 @@ module.exports = React.createClass({ room: room, roomId: result.room_id, roomLoading: !room, + hasUnsentMessages: this._hasUnsentMessages(room), }, this.updatePeeking); }, (err) => { this.setState({ roomLoading: false, }); }); + } else { + var room = MatrixClientPeg.get().getRoom(this.props.roomAddress); + this.setState({ + roomId: this.props.roomAddress, + room: room, + roomLoading: !room, + hasUnsentMessages: this._hasUnsentMessages(room), + }, this.updatePeeking); } - - this.updatePeeking(); }, updatePeeking: function() { From 40b1b99c38b83c2a3323b335e955e7bab9934927 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 14 Jun 2016 14:40:03 +0100 Subject: [PATCH 6/6] underscore prefix internal method --- src/components/structures/RoomView.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 2ff7ed1cbd..e1b4c00175 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -159,7 +159,7 @@ module.exports = React.createClass({ roomId: result.room_id, roomLoading: !room, hasUnsentMessages: this._hasUnsentMessages(room), - }, this.updatePeeking); + }, this._updatePeeking); }, (err) => { this.setState({ roomLoading: false, @@ -172,11 +172,11 @@ module.exports = React.createClass({ room: room, roomLoading: !room, hasUnsentMessages: this._hasUnsentMessages(room), - }, this.updatePeeking); + }, this._updatePeeking); } }, - updatePeeking: function() { + _updatePeeking: function() { // if this is an unknown room then we're in one of three states: // - This is a room we can peek into (search engine) (we can /peek) // - This is a room we can publicly join or were invited to. (we can /join)