From cebc2f5306fd20a9cde5fc9c66a5a4def71de65a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 Feb 2016 16:23:57 +0000 Subject: [PATCH 1/8] Put the room preview bar back for rooms that aren't peekable (since we always tried to peek, it would fail which would reject the promise and cause loadingTimeline to stay true forever). --- src/components/structures/RoomView.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 6dbff018ef..4373569fd4 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -163,10 +163,7 @@ module.exports = React.createClass({ console.log("Attempting to peek into room %s", this.props.roomId); - roomProm = MatrixClientPeg.get().peekInRoom(this.props.roomId).catch((err) => { - console.error("Failed to peek into room: %s", err); - throw err; - }).then((room) => { + roomProm = MatrixClientPeg.get().peekInRoom(this.props.roomId).then((room) => { this.setState({ room: room }); @@ -180,6 +177,11 @@ module.exports = React.createClass({ roomProm.then((room) => { this._calculatePeekRules(room); return this._initTimeline(this.props); + }).catch(() => { + // This is fine: the room just isn't peekable (we assume). + this.setState({ + timelineLoading: false, + }); }).done(); }, From 164c9b90314c246cf393d2386839933d8c6b5e24 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 3 Feb 2016 16:53:48 +0000 Subject: [PATCH 2/8] Check error to see if it's actually a failure to peek --- src/components/structures/RoomView.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 4373569fd4..0d00b9afe5 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -177,11 +177,15 @@ module.exports = React.createClass({ roomProm.then((room) => { this._calculatePeekRules(room); return this._initTimeline(this.props); - }).catch(() => { - // This is fine: the room just isn't peekable (we assume). - this.setState({ - timelineLoading: false, - }); + }).catch((err) => { + if (err.errcode == "M_GUEST_ACCESS_FORBIDDEN") { + // This is fine: the room just isn't peekable (we assume). + this.setState({ + timelineLoading: false, + }); + } else { + throw err; + } }).done(); }, From 5c430395ea8cffc0bd4820dd580f412c55cba6ff Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 10:50:21 +0000 Subject: [PATCH 3/8] init timeline when we get the room, as otherwise we never load it after joining --- src/components/structures/RoomView.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 0d00b9afe5..fa183cd00e 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -420,6 +420,7 @@ module.exports = React.createClass({ this.setState({ room: room }); + this._initTimeline(this.props); } }, From c9a3ad31ab4e8565d964034ae593ea27aa0c6fa2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 11:17:39 +0000 Subject: [PATCH 4/8] Comment error handling --- src/components/structures/RoomView.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index fa183cd00e..9df007e490 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -178,6 +178,9 @@ module.exports = React.createClass({ this._calculatePeekRules(room); return this._initTimeline(this.props); }).catch((err) => { + // This won't necessarily be a MatrixError, but we duck-type + // here and say if it's got an 'errcode' key with the right value, + // it means we can't peek. if (err.errcode == "M_GUEST_ACCESS_FORBIDDEN") { // This is fine: the room just isn't peekable (we assume). this.setState({ From 3c2c2b051b062ad027a0bb8446e18b90bbc7f188 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 11:29:00 +0000 Subject: [PATCH 5/8] Listen for Room and use this to init the timeline, not Room.name --- src/components/structures/RoomView.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 9df007e490..f77e2e226d 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -122,6 +122,7 @@ module.exports = React.createClass({ componentWillMount: function() { this.last_rr_sent_event_id = undefined; this.dispatcherRef = dis.register(this.onAction); + MatrixClientPeg.get().on("Room", this.onRoom); MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); MatrixClientPeg.get().on("Room.name", this.onRoomName); MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); @@ -418,12 +419,20 @@ module.exports = React.createClass({ } }, + onRoom: function(room) { + if (room.roomId == this.props.roomId) { + this.setState({ + room: room + }); + this._initTimeline(this.props).done(); + } + }, + onRoomName: function(room) { if (room.roomId == this.props.roomId) { this.setState({ room: room }); - this._initTimeline(this.props); } }, From 574560cc05db46b3dc08417411585c131c24a507 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 13:06:59 +0000 Subject: [PATCH 6/8] remove listener on unmount --- src/components/structures/RoomView.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index f77e2e226d..161818f8f1 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -274,6 +274,7 @@ module.exports = React.createClass({ } dis.unregister(this.dispatcherRef); if (MatrixClientPeg.get()) { + MatrixClientPeg.get().removeListener("Room", this.onRoom); MatrixClientPeg.get().removeListener("Room.timeline", this.onRoomTimeline); MatrixClientPeg.get().removeListener("Room.name", this.onRoomName); MatrixClientPeg.get().removeListener("Room.accountData", this.onRoomAccountData); From 891f4761a0b7d9f534548a8a8e52dc7f763ff486 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 13:21:42 +0000 Subject: [PATCH 7/8] Add comments and only set a room / init the timeline if we don't already have a room --- src/components/structures/RoomView.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 161818f8f1..4553e9053e 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -421,7 +421,12 @@ module.exports = React.createClass({ }, onRoom: function(room) { - if (room.roomId == this.props.roomId) { + // This event is fired when the room is 'stored' by the JS SDK, which + // means it's now a fully-fledged room object ready to be used, so + // set it in our state and start using it (ie. init the timeline) + // This will happen if we start off viewing a room we're not joined, + // then join it whilst RoomView is looking at that room. + if (room.roomId == this.props.roomId && !this.state.room) { this.setState({ room: room }); @@ -705,6 +710,10 @@ module.exports = React.createClass({ // NOT when it comes down /sync. If there is no room, we'll keep the // joining flag set until we see it. Likewise, if our state is not // "join" we'll keep this flag set until it comes down /sync. + + // We'll need to initialise the timeline when joining, but due to + // the above, we can't do it here: we do it in onRoom instead, + // once we have a useable room object. var room = MatrixClientPeg.get().getRoom(self.props.roomId); var me = MatrixClientPeg.get().credentials.userId; self.setState({ From b0da54533d040eeef3366854d8815a9d63067b24 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 4 Feb 2016 13:25:45 +0000 Subject: [PATCH 8/8] Don't do this check - it's not valid since we set the room in onRoomName --- src/components/structures/RoomView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 4553e9053e..ca564d3e11 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -426,7 +426,7 @@ module.exports = React.createClass({ // set it in our state and start using it (ie. init the timeline) // This will happen if we start off viewing a room we're not joined, // then join it whilst RoomView is looking at that room. - if (room.roomId == this.props.roomId && !this.state.room) { + if (room.roomId == this.props.roomId) { this.setState({ room: room });