From a4f4bb9e726d7732470aea05c4df0228a4a0636a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 27 Jan 2016 07:54:15 +0000 Subject: [PATCH 1/6] Convert RoomView to using a TimelineWindow Instead of using the Room's active timeline directly, use a TimelineWindow. This shouldn't (yet) have much effect, beyond maybe making scrollback after a gappy sync slightly more efficient. For now, I have disabled the 'restoreScrollState' functionality. This will be reinstated once I land the link-to-event code. --- src/MatrixClientPeg.js | 3 +- src/components/structures/MatrixChat.js | 3 +- src/components/structures/RoomView.js | 245 ++++++++++-------------- 3 files changed, 104 insertions(+), 147 deletions(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index dbb3dbf83e..bd8aa31a6f 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -39,7 +39,8 @@ function createClient(hs_url, is_url, user_id, access_token, guestAccess) { baseUrl: hs_url, idBaseUrl: is_url, accessToken: access_token, - userId: user_id + userId: user_id, + timelineSupport: true, }; if (localStorage) { diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index f02eb30299..f8db14b854 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -463,10 +463,11 @@ module.exports = React.createClass({ newState.ready = true; } this.setState(newState); + /* if (this.scrollStateMap[roomId]) { var scrollState = this.scrollStateMap[roomId]; this.refs.roomView.restoreScrollState(scrollState); - } + }*/ if (this.refs.roomView && showSettings) { this.refs.roomView.showSettings(true); } diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index e2ed8231ef..020f8fa876 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -26,6 +26,7 @@ var ReactDOM = require("react-dom"); var q = require("q"); var classNames = require("classnames"); var Matrix = require("matrix-js-sdk"); +var EventTimeline = Matrix.EventTimeline; var MatrixClientPeg = require("../../MatrixClientPeg"); var ContentMessages = require("../../ContentMessages"); @@ -44,6 +45,7 @@ var Tinter = require("../../Tinter"); var PAGINATE_SIZE = 20; var INITIAL_SIZE = 20; var SEND_READ_RECEIPT_DELAY = 2000; +var TIMELINE_CAP = 1000; // the most events to show in a timeline var DEBUG_SCROLL = false; @@ -70,7 +72,9 @@ module.exports = React.createClass({ var room = this.props.roomId ? MatrixClientPeg.get().getRoom(this.props.roomId) : null; return { room: room, - messageCap: INITIAL_SIZE, + events: [], + canBackPaginate: true, + paginating: room != null, editingRoomSettings: false, uploadingRoomSettings: false, numUnreadMessages: 0, @@ -80,7 +84,7 @@ module.exports = React.createClass({ syncState: MatrixClientPeg.get().getSyncState(), hasUnsentMessages: this._hasUnsentMessages(room), callState: null, - autoPeekDone: false, // track whether our autoPeek (if any) has completed) + timelineLoaded: false, // track whether our room timeline has loaded guestsCanJoin: false, canPeek: false, readMarkerEventId: room ? room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId) : null, @@ -92,7 +96,6 @@ module.exports = React.createClass({ componentWillMount: function() { this.last_rr_sent_event_id = undefined; this.dispatcherRef = dis.register(this.onAction); - MatrixClientPeg.get().on("Room", this.onNewRoom); MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); MatrixClientPeg.get().on("Room.name", this.onRoomName); MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); @@ -110,6 +113,15 @@ module.exports = React.createClass({ this.forceUpdate(); } }); + + + // to make the timeline load work correctly, build up a chain of promises which + // take us through the necessary steps. + + // First of all, we may need to load the room. Construct a promise + // which resolves to the Room object. + var roomProm; + // 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) @@ -118,22 +130,43 @@ module.exports = React.createClass({ // 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) { - if (this.props.autoPeek) { - console.log("Attempting to peek into room %s", this.props.roomId); - MatrixClientPeg.get().peekInRoom(this.props.roomId).catch((err) => { - console.error("Failed to peek into room: %s", err); - }).finally(() => { - // we don't need to do anything - JS SDK will emit Room events - // which will update the UI. - this.setState({ - autoPeekDone: true - }); - }); + if (!this.props.autoPeek) { + console.log("No room loaded, and autopeek disabled"); + return; } + + 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) => { + this.setState({ + room: room + }); + return room; + }); + } else { + roomProm = q(this.state.room); } - else { - this._calculatePeekRules(this.state.room); - } + + // Next, load the timeline. + roomProm.then((room) => { + this._calculatePeekRules(room); + this._timelineWindow = new Matrix.TimelineWindow( + MatrixClientPeg.get(), room, + {windowLimit: TIMELINE_CAP}); + + return this._timelineWindow.load(undefined, + INITIAL_SIZE); + }).then(() => { + debuglog("RoomView: timeline loaded"); + this._onTimelineUpdated(true); + }).finally(() => { + this.setState({ + timelineLoaded: true + }); + }).done(); }, componentWillUnmount: function() { @@ -156,7 +189,6 @@ module.exports = React.createClass({ } dis.unregister(this.dispatcherRef); if (MatrixClientPeg.get()) { - MatrixClientPeg.get().removeListener("Room", this.onNewRoom); MatrixClientPeg.get().removeListener("Room.timeline", this.onRoomTimeline); MatrixClientPeg.get().removeListener("Room.name", this.onRoomName); MatrixClientPeg.get().removeListener("Room.accountData", this.onRoomAccountData); @@ -248,46 +280,36 @@ module.exports = React.createClass({ /*componentWillReceiveProps: function(props) { },*/ - onRoomTimeline: function(ev, room, toStartOfTimeline) { + onRoomTimeline: function(ev, room, toStartOfTimeline, removed, data) { if (this.unmounted) return; - // ignore anything that comes in whilst paginating: we get one - // event for each new matrix event so this would cause a huge - // number of UI updates. Just update the UI when the paginate - // call returns. - if (this.state.paginating) return; + // ignore events for other rooms + if (room.roomId != this.props.roomId) return; + + // ignore anything but real-time updates at the end of the room: + // updates from pagination will happen when the paginate completes. + if (toStartOfTimeline || !data || !data.liveEvent) return; // no point handling anything while we're waiting for the join to finish: // we'll only be showing a spinner. if (this.state.joining) return; - if (room.roomId != this.props.roomId) return; - var currentUnread = this.state.numUnreadMessages; - if (!toStartOfTimeline && - (ev.getSender() !== MatrixClientPeg.get().credentials.userId)) { + if (ev.getSender() !== MatrixClientPeg.get().credentials.userId) { // update unread count when scrolled up if (!this.state.searchResults && this.refs.messagePanel && this.refs.messagePanel.isAtBottom()) { - currentUnread = 0; + // no change } else { - currentUnread += 1; + this.setState((state, props) => { + return {numUnreadMessages: state.numUnreadMessages + 1}; + }); } } - this.setState({ - room: MatrixClientPeg.get().getRoom(this.props.roomId), - numUnreadMessages: currentUnread - }); - }, - - onNewRoom: function(room) { - if (room.roomId == this.props.roomId) { - this.setState({ - room: room - }); + // tell the messagepanel to go paginate itself + if (this.refs.messagePanel) { + this.refs.messagePanel.checkFillState(); } - - this._calculatePeekRules(room); }, _calculatePeekRules: function(room) { @@ -349,14 +371,14 @@ module.exports = React.createClass({ // if the event after the one referenced in the read receipt if sent by us, do nothing since // this is a temporary period before the synthesized receipt for our own message arrives var readMarkerGhostEventIndex; - for (var i = 0; i < room.timeline.length; ++i) { - if (room.timeline[i].getId() == readMarkerGhostEventId) { + for (var i = 0; i < this.state.events.length; ++i) { + if (this.state.events[i].getId() == readMarkerGhostEventId) { readMarkerGhostEventIndex = i; break; } } - if (readMarkerGhostEventIndex + 1 < room.timeline.length) { - var nextEvent = room.timeline[readMarkerGhostEventIndex + 1]; + if (readMarkerGhostEventIndex + 1 < this.state.events.length) { + var nextEvent = this.state.events[readMarkerGhostEventIndex + 1]; if (nextEvent.sender && nextEvent.sender.userId == MatrixClientPeg.get().credentials.userId) { readMarkerGhostEventId = undefined; } @@ -504,17 +526,21 @@ module.exports = React.createClass({ } }, - _paginateCompleted: function() { - debuglog("paginate complete"); - - // we might have switched rooms since the paginate started - just bin + _onTimelineUpdated: function(gotResults) { + // we might have switched rooms since the load started - just bin // the results if so. if (this.unmounted) return; this.setState({ - room: MatrixClientPeg.get().getRoom(this.props.roomId), paginating: false, }); + + if (gotResults) { + this.setState({ + events: this._timelineWindow.getEvents(), + canBackPaginate: this._timelineWindow.canPaginate(EventTimeline.BACKWARDS), + }); + } }, onSearchResultsFillRequest: function(backwards) { @@ -534,30 +560,19 @@ module.exports = React.createClass({ // set off a pagination request. onMessageListFillRequest: function(backwards) { - if (!backwards) + var dir = backwards ? EventTimeline.BACKWARDS : EventTimeline.FORWARDS; + if(!this._timelineWindow.canPaginate(dir)) { + debuglog("RoomView: can't paginate at this time; backwards:"+backwards); return q(false); - - // Either wind back the message cap (if there are enough events in the - // timeline to do so), or fire off a pagination request. - - if (this.state.messageCap < this.state.room.timeline.length) { - var cap = Math.min(this.state.messageCap + PAGINATE_SIZE, this.state.room.timeline.length); - debuglog("winding back message cap to", cap); - this.setState({messageCap: cap}); - return q(true); - } else if(this.state.room.oldState.paginationToken) { - var cap = this.state.messageCap + PAGINATE_SIZE; - debuglog("starting paginate to cap", cap); - this.setState({messageCap: cap, paginating: true}); - return MatrixClientPeg.get().scrollback(this.state.room, PAGINATE_SIZE). - finally(this._paginateCompleted).then(true); } - }, + this.setState({paginating: true}); - // return true if there's more messages in the backlog which we aren't displaying - _canPaginate: function() { - return (this.state.messageCap < this.state.room.timeline.length) || - this.state.room.oldState.paginationToken; + debuglog("RoomView: Initiating paginate; backwards:"+backwards); + return this._timelineWindow.paginate(dir, PAGINATE_SIZE).then((r) => { + debuglog("RoomView: paginate complete backwards:"+backwards+"; success:"+r); + this._onTimelineUpdated(r); + return r; + }); }, onResendAllClick: function() { @@ -829,11 +844,10 @@ module.exports = React.createClass({ var EventTile = sdk.getComponent('rooms.EventTile'); var prevEvent = null; // the last event we showed - var startIdx = Math.max(0, this.state.room.timeline.length - this.state.messageCap); - var readMarkerIndex; var ghostIndex; - for (var i = startIdx; i < this.state.room.timeline.length; i++) { - var mxEv = this.state.room.timeline[i]; + var readMarkerIndex; + for (var i = 0; i < this.state.events.length; i++) { + var mxEv = this.state.events[i]; if (!EventTile.haveTileForEvent(mxEv)) { continue; @@ -879,7 +893,7 @@ module.exports = React.createClass({ // do we need a date separator since the last event? var ts1 = mxEv.getTs(); - if ((prevEvent == null && !this._canPaginate()) || + if ((prevEvent == null && !this.state.canBackPaginate) || (prevEvent != null && new Date(prevEvent.getTs()).toDateString() !== new Date(ts1).toDateString())) { var dateSeparator =
    • ; @@ -888,7 +902,7 @@ module.exports = React.createClass({ } var last = false; - if (i == this.state.room.timeline.length - 1) { + if (i == this.state.events.length - 1) { // XXX: we might not show a tile for the last event. last = true; } @@ -1171,8 +1185,8 @@ module.exports = React.createClass({ }, _indexForEventId(evId) { - for (var i = 0; i < this.state.room.timeline.length; ++i) { - if (evId == this.state.room.timeline[i].getId()) { + for (var i = 0; i < this.state.events.length; ++i) { + if (evId == this.state.events[i].getId()) { return i; } } @@ -1187,7 +1201,7 @@ module.exports = React.createClass({ var lastReadEventIndex = this._getLastDisplayedEventIndexIgnoringOwn(); if (lastReadEventIndex === null) return; - var lastReadEvent = this.state.room.timeline[lastReadEventIndex]; + var lastReadEvent = this.state.events[lastReadEventIndex]; // we also remember the last read receipt we sent to avoid spamming the same one at the server repeatedly if (lastReadEventIndex > currentReadUpToEventIndex && this.last_rr_sent_event_id != lastReadEvent.getId()) { @@ -1206,8 +1220,8 @@ module.exports = React.createClass({ if (messageWrapper === undefined) return null; var wrapperRect = ReactDOM.findDOMNode(messageWrapper).getBoundingClientRect(); - for (var i = this.state.room.timeline.length-1; i >= 0; --i) { - var ev = this.state.room.timeline[i]; + for (var i = this.state.events.length-1; i >= 0; --i) { + var ev = this.state.events[i]; if (ev.sender && ev.sender.userId == MatrixClientPeg.get().credentials.userId) { continue; @@ -1326,46 +1340,6 @@ module.exports = React.createClass({ messagePanel.scrollToBottom(); }, - // scroll the event view to put the given event at the bottom. - // - // pixel_offset gives the number of pixels between the bottom of the event - // and the bottom of the container. - scrollToEvent: function(eventId, pixelOffset) { - var messagePanel = this.refs.messagePanel; - if (!messagePanel) return; - - var idx = this._indexForEventId(eventId); - if (idx === null) { - // we don't seem to have this event in our timeline. Presumably - // it's fallen out of scrollback. We ought to backfill until we - // find it, but we'd have to be careful we didn't backfill forever - // looking for a non-existent event. - // - // for now, just scroll to the top of the buffer. - console.log("Refusing to scroll to unknown event "+eventId); - messagePanel.scrollToTop(); - return; - } - - // we might need to roll back the messagecap (to generate tiles for - // older messages). This just means telling getEventTiles to create - // tiles for events we already have in our timeline (we already know - // the event in question is in our timeline, so we shouldn't need to - // backfill). - // - // we actually wind back slightly further than the event in question, - // because we want the event to be at the *bottom* of the container. - // Don't roll it back past the timeline we have, though. - var minCap = this.state.room.timeline.length - Math.min(idx - INITIAL_SIZE, 0); - if (minCap > this.state.messageCap) { - this.setState({messageCap: minCap}); - } - - // the scrollTokens on our DOM nodes are the event IDs, so we can pass - // eventId directly into _scrollToToken. - messagePanel.scrollToToken(eventId, pixelOffset); - }, - // get the current scroll position of the room, so that it can be // restored when we switch back to it getScrollState: function() { @@ -1375,25 +1349,6 @@ module.exports = React.createClass({ return messagePanel.getScrollState(); }, - restoreScrollState: function(scrollState) { - var messagePanel = this.refs.messagePanel; - if (!messagePanel) return null; - - if(scrollState.atBottom) { - // we were at the bottom before. Ideally we'd scroll to the - // 'read-up-to' mark here. - messagePanel.scrollToBottom(); - - } else if (scrollState.lastDisplayedScrollToken) { - // we might need to backfill, so we call scrollToEvent rather than - // scrollToToken here. The scrollTokens on our DOM nodes are the - // event IDs, so lastDisplayedScrollToken will be the event ID we need, - // and we can pass it directly into scrollToEvent. - this.scrollToEvent(scrollState.lastDisplayedScrollToken, - scrollState.pixelOffset); - } - }, - onResize: function(e) { // It seems flexbox doesn't give us a way to constrain the auxPanel height to have // a minimum of the height of the video element, whilst also capping it from pushing out the page @@ -1486,9 +1441,9 @@ module.exports = React.createClass({ var TintableSvg = sdk.getComponent("elements.TintableSvg"); var RoomPreviewBar = sdk.getComponent("rooms.RoomPreviewBar"); - if (!this.state.room) { + if (!this._timelineWindow) { if (this.props.roomId) { - if (this.props.autoPeek && !this.state.autoPeekDone) { + if (!this.state.timelineLoaded) { var Loader = sdk.getComponent("elements.Spinner"); return (
    From f0cf5c0affd06ba28171ad7ef7add0a372b20672 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Feb 2016 12:35:41 +0000 Subject: [PATCH 2/6] Update onRoomTimeline comment --- src/components/structures/RoomView.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 020f8fa876..6cc90bfc7d 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -306,7 +306,11 @@ module.exports = React.createClass({ } } - // tell the messagepanel to go paginate itself + // tell the messagepanel to go paginate itself. This in turn will cause + // onMessageListFillRequest to be called, which will call + // _onTimelineUpdated, which will update the state with the new event - + // so there is no need update the state here. + // if (this.refs.messagePanel) { this.refs.messagePanel.checkFillState(); } From e6c93530e257c9a0e7cbe08248ec19343d56dffa Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 1 Feb 2016 14:19:20 +0000 Subject: [PATCH 3/6] Strip trailing slashes on HS/IS URLs on register/login --- src/components/views/login/ServerConfig.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/login/ServerConfig.js b/src/components/views/login/ServerConfig.js index 741f0ebc69..fe80a0d61b 100644 --- a/src/components/views/login/ServerConfig.js +++ b/src/components/views/login/ServerConfig.js @@ -58,7 +58,7 @@ module.exports = React.createClass({ onHomeserverChanged: function(ev) { this.setState({hs_url: ev.target.value}, function() { this._hsTimeoutId = this._waitThenInvoke(this._hsTimeoutId, function() { - this.props.onHsUrlChanged(this.state.hs_url); + this.props.onHsUrlChanged(this.state.hs_url.replace(/\/$/, "")); }); }); }, @@ -66,7 +66,7 @@ module.exports = React.createClass({ onIdentityServerChanged: function(ev) { this.setState({is_url: ev.target.value}, function() { this._isTimeoutId = this._waitThenInvoke(this._isTimeoutId, function() { - this.props.onIsUrlChanged(this.state.is_url); + this.props.onIsUrlChanged(this.state.is_url.replace(/\/$/, "")); }); }); }, From d9e13780b8ae0ae84d21fdb9835fca9f8f8ab005 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 1 Feb 2016 16:31:12 +0000 Subject: [PATCH 4/6] Implement direct-to-event linking. This adds support for links to particular event ids: add / to the URL for a room. This commit also ensures that we scroll to the 'read marker' when switching to a room which has no previous scroll state, as well as preventing that marker from going past the middle of the screen. This also reinstates the preservation of scroll state when switching rooms, which was disabled previously. --- src/components/structures/MatrixChat.js | 63 +++++-- src/components/structures/RoomView.js | 231 +++++++++++++++++++---- src/components/structures/ScrollPanel.js | 149 ++++++++++----- src/components/views/rooms/EventTile.js | 4 + 4 files changed, 354 insertions(+), 93 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index f8db14b854..9d9e7758b6 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -313,7 +313,7 @@ module.exports = React.createClass({ // by default we autoPeek rooms, unless we were called explicitly with // autoPeek=false by something like RoomDirectory who has already peeked this.setState({ autoPeek : payload.auto_peek === false ? false : true }); - this._viewRoom(payload.room_id, payload.show_settings); + this._viewRoom(payload.room_id, payload.show_settings, payload.event_id); break; case 'view_prev_room': roomIndexDelta = -1; @@ -348,7 +348,8 @@ module.exports = React.createClass({ if (foundRoom) { dis.dispatch({ action: 'view_room', - room_id: foundRoom.roomId + room_id: foundRoom.roomId, + event_id: payload.event_id, }); return; } @@ -357,7 +358,8 @@ module.exports = React.createClass({ function(result) { dis.dispatch({ action: 'view_room', - room_id: result.room_id + room_id: result.room_id, + event_id: payload.event_id, }); }); break; @@ -429,15 +431,34 @@ module.exports = React.createClass({ }); }, - _viewRoom: function(roomId, showSettings) { + // switch view to the given room + // + // eventId is optional and will cause a switch to the context of that + // particular event. + _viewRoom: function(roomId, showSettings, eventId) { // before we switch room, record the scroll state of the current room this._updateScrollMap(); this.focusComposer = true; + var newState = { currentRoom: roomId, + initialEventId: eventId, + highlightedEventId: eventId, + initialEventPixelOffset: undefined, page_type: this.PageTypes.RoomView, }; + + // if we aren't given an explicit event id, look for one in the + // scrollStateMap. + if (!eventId) { + var scrollState = this.scrollStateMap[roomId]; + if (scrollState) { + newState.initialEventId = scrollState.focussedEvent; + newState.initialEventPixelOffset = scrollState.pixelOffset; + } + } + if (this.sdkReady) { // if the SDK is not ready yet, remember what room // we're supposed to be on but don't notify about @@ -459,15 +480,14 @@ module.exports = React.createClass({ Tinter.tint(color_scheme.primary_color, color_scheme.secondary_color); } + if (eventId) { + presentedId += "/"+eventId; + } this.notifyNewScreen('room/'+presentedId); newState.ready = true; } this.setState(newState); - /* - if (this.scrollStateMap[roomId]) { - var scrollState = this.scrollStateMap[roomId]; - this.refs.roomView.restoreScrollState(scrollState); - }*/ + if (this.refs.roomView && showSettings) { this.refs.roomView.showSettings(true); } @@ -515,9 +535,11 @@ module.exports = React.createClass({ if (self.starting_room_alias) { dis.dispatch({ action: 'view_room_alias', - room_alias: self.starting_room_alias + room_alias: self.starting_room_alias, + event_id: self.starting_event_id, }); delete self.starting_room_alias; + delete self.starting_event_id; } else if (!self.state.page_type) { if (!self.state.currentRoom) { var firstRoom = null; @@ -635,23 +657,35 @@ module.exports = React.createClass({ action: 'start_post_registration', }); } else if (screen.indexOf('room/') == 0) { - var roomString = screen.split('/')[1]; + var roomString = screen.substring(5); + var eventId; + + // extract event id, if one is given + var idx = roomString.indexOf('/'); + if (idx >= 0) { + eventId = roomString.substring(idx+1); + roomString = roomString.substring(0, idx); + } + if (roomString[0] == '#') { if (this.state.logged_in) { dis.dispatch({ action: 'view_room_alias', - room_alias: roomString + room_alias: roomString, + event_id: eventId, }); } else { // Okay, we'll take you here soon... this.starting_room_alias = roomString; + this.starting_event_id = eventId; // ...but you're still going to have to log in. this.notifyNewScreen('login'); } } else { dis.dispatch({ action: 'view_room', - room_id: roomString + room_id: roomString, + event_id: eventId, }); } } @@ -828,6 +862,9 @@ module.exports = React.createClass({ diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 6cc90bfc7d..be690e4cf2 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -60,7 +60,20 @@ module.exports = React.createClass({ displayName: 'RoomView', propTypes: { ConferenceHandler: React.PropTypes.any, - roomId: React.PropTypes.string, + roomId: React.PropTypes.string.isRequired, + + // id of an event to jump to. If not given, will use the read-up-to-marker. + eventId: React.PropTypes.string, + + // where to position the event given by eventId, in pixels from the + // bottom of the viewport. If not given, will try to put the event in the + // middle of the viewprt. + eventPixelOffset: React.PropTypes.number, + + // ID of an event to highlight. If undefined, no event will be highlighted. + // Typically this will either be the same as 'eventId', or undefined. + highlightedEventId: React.PropTypes.string, + autoPeek: React.PropTypes.bool, // should we try to peek the room on mount, or has whoever invoked us already initiated a peek? }, @@ -84,7 +97,7 @@ module.exports = React.createClass({ syncState: MatrixClientPeg.get().getSyncState(), hasUnsentMessages: this._hasUnsentMessages(room), callState: null, - timelineLoaded: false, // track whether our room timeline has loaded + timelineLoading: true, // track whether our room timeline is loading guestsCanJoin: false, canPeek: false, readMarkerEventId: room ? room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId) : null, @@ -153,20 +166,69 @@ module.exports = React.createClass({ // Next, load the timeline. roomProm.then((room) => { this._calculatePeekRules(room); - this._timelineWindow = new Matrix.TimelineWindow( - MatrixClientPeg.get(), room, - {windowLimit: TIMELINE_CAP}); + return this._initTimeline(this.props); + }).done(); + }, - return this._timelineWindow.load(undefined, - INITIAL_SIZE); - }).then(() => { + _initTimeline: function(props) { + var initialEvent = props.eventId; + if (!initialEvent) { + // go to the 'read-up-to' mark if no explicit event given + initialEvent = this.state.readMarkerEventId; + } + + var pixelOffset = props.eventPixelOffset; + return this._loadTimeline(initialEvent, pixelOffset); + }, + + /** + * (re)-load the event timeline, and initialise the scroll state, centered + * around the given event. + * + * @param {string?} eventId the event to focus on. If undefined, will + * scroll to the bottom of the room. + * + * @param {number?} pixelOffset offset to position the given event at + * (pixels from the bottom of the view). If undefined, will put the + * event in the middle of the view. + * + * returns a promise which will resolve when the load completes. + */ + _loadTimeline: function(eventId, pixelOffset) { + // TODO: we could optimise this, by not resetting the window if the + // event is in the current window (though it's not obvious how we can + // tell if the current window is on the live event stream) + + this.setState({ + events: [], + timelineLoading: true, + }); + + this._timelineWindow = new Matrix.TimelineWindow( + MatrixClientPeg.get(), this.state.room, + {windowLimit: TIMELINE_CAP}); + + return this._timelineWindow.load(eventId, INITIAL_SIZE).then(() => { debuglog("RoomView: timeline loaded"); this._onTimelineUpdated(true); }).finally(() => { this.setState({ - timelineLoaded: true + timelineLoading: false, + }, () => { + // initialise the scroll state of the message panel + if (!this.refs.messagePanel) { + // this shouldn't happen. + console.log("can't initialise scroll state because " + + "messagePanel didn't load"); + return; + } + if (eventId) { + this.refs.messagePanel.scrollToToken(eventId, pixelOffset); + } else { + this.refs.messagePanel.scrollToBottom(); + } }); - }).done(); + }); }, componentWillUnmount: function() { @@ -234,10 +296,6 @@ module.exports = React.createClass({ var callState; if (call) { - // Call state has changed so we may be loading video elements - // which will obscure the message log. - // scroll to bottom - this.scrollToBottom(); callState = call.call_state; } else { @@ -274,11 +332,17 @@ module.exports = React.createClass({ }); }, - // MatrixRoom still showing the messages from the old room? - // Set the key to the room_id. Sadly you can no longer get at - // the key from inside the component, or we'd check this in code. - /*componentWillReceiveProps: function(props) { - },*/ + componentWillReceiveProps: function(newProps) { + if (newProps.roomId != this.props.roomId) { + throw new Error("changing room on a RoomView is not supported"); + } + + if (newProps.eventId != this.props.eventId) { + console.log("RoomView switching to eventId " + newProps.eventId + + " (was " + this.props.eventId + ")"); + return this._initTimeline(newProps); + } + }, onRoomTimeline: function(ev, room, toStartOfTimeline, removed, data) { if (this.unmounted) return; @@ -296,7 +360,7 @@ module.exports = React.createClass({ if (ev.getSender() !== MatrixClientPeg.get().credentials.userId) { // update unread count when scrolled up - if (!this.state.searchResults && this.refs.messagePanel && this.refs.messagePanel.isAtBottom()) { + if (!this.state.searchResults && this.state.atBottom) { // no change } else { @@ -392,6 +456,20 @@ module.exports = React.createClass({ readMarkerEventId: readMarkerEventId, readMarkerGhostEventId: readMarkerGhostEventId, }); + + + // if the scrollpanel is following the timeline, attempt to scroll + // it to bring the read message up to the middle of the panel. This + // will have no immediate effect (since we are already at the + // bottom), but will ensure that if there is no further user + // activity, but room activity continues, the read message will + // scroll up to the middle of the window, but no further. + // + // we do this here as well as in sendReadReceipt to deal with + // people using two clients at once. + if (this.refs.messagePanel && this.state.atBottom) { + this.refs.messagePanel.scrollToToken(readMarkerEventId); + } } }, @@ -513,7 +591,6 @@ module.exports = React.createClass({ var messagePanel = ReactDOM.findDOMNode(this.refs.messagePanel); this.refs.messagePanel.initialised = true; - this.scrollToBottom(); this.sendReadReceipt(); this.updateTint(); @@ -618,7 +695,8 @@ module.exports = React.createClass({ }, onMessageListScroll: function(ev) { - if (this.refs.messagePanel.isAtBottom()) { + if (this.refs.messagePanel.isAtBottom() && + !this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) { if (this.state.numUnreadMessages != 0) { this.setState({ numUnreadMessages: 0 }); } @@ -912,9 +990,11 @@ module.exports = React.createClass({ } var eventId = mxEv.getId(); + var highlight = (eventId == this.props.highlightedEventId); ret.push(
  • - +
    • ); @@ -1199,9 +1279,22 @@ module.exports = React.createClass({ sendReadReceipt: function() { if (!this.state.room) return; + if (!this.refs.messagePanel) return; + var currentReadUpToEventId = this.state.room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId); var currentReadUpToEventIndex = this._indexForEventId(currentReadUpToEventId); + // We want to avoid sending out read receipts when we are looking at + // events in the past. + // + // For now, let's apply a heuristic: if (a) the server has a + // readUpToEvent for us, (b) we can't find it, and (c) we could + // forward-paginate the event timeline, then suppress read receipts. + if (currentReadUpToEventId && currentReadUpToEventIndex === null && + this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) { + return; + } + var lastReadEventIndex = this._getLastDisplayedEventIndexIgnoringOwn(); if (lastReadEventIndex === null) return; @@ -1214,6 +1307,19 @@ module.exports = React.createClass({ // it failed, so allow retries next time the user is active this.last_rr_sent_event_id = undefined; }); + + // if the scrollpanel is following the timeline, attempt to scroll + // it to bring the read message up to the middle of the panel. This + // will have no immediate effect (since we are already at the + // bottom), but will ensure that if there is no further user + // activity, but room activity continues, the read message will + // scroll up to the middle of the window, but no further. + // + // we do this here as well as in onRoomReceipt to cater for guest users + // (which do not send out read receipts). + if (this.state.atBottom) { + this.refs.messagePanel.scrollToToken(lastReadEvent.getId()); + } } }, @@ -1339,18 +1445,52 @@ module.exports = React.createClass({ }, scrollToBottom: function() { - var messagePanel = this.refs.messagePanel; - if (!messagePanel) return; - messagePanel.scrollToBottom(); + // if we can't forward-paginate the existing timeline, then there + // is no point reloading it - just jump straight to the bottom. + // + // Otherwise, reload the timeline rather than trying to paginate + // through all of space-time. + if (this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) { + this._loadTimeline(); + } else { + if (this.refs.messagePanel) { + this.refs.messagePanel.scrollToBottom(); + } + } }, // get the current scroll position of the room, so that it can be - // restored when we switch back to it + // restored when we switch back to it. + // + // This returns an object with the following properties: + // + // focussedEvent: the ID of the 'focussed' event. Typically this is the + // last event fully visible in the viewport, though if we have done + // an explicit scroll to an explicit event, it will be that event. + // + // pixelOffset: the number of pixels the window is scrolled down from + // the focussedEvent. + // + // If there are no visible events, returns null. + // getScrollState: function() { var messagePanel = this.refs.messagePanel; if (!messagePanel) return null; - return messagePanel.getScrollState(); + var scrollState = messagePanel.getScrollState(); + + if (scrollState.atBottom) { + // we don't really expect to be in this state, but it will + // occasionally happen when we are in a transition. Treat it the + // same as having no saved state (which will cause us to scroll to + // last unread on reload). + return null; + } + + return { + focussedEvent: scrollState.lastDisplayedScrollToken, + pixelOffset: scrollState.pixelOffset, + }; }, onResize: function(e) { @@ -1444,11 +1584,11 @@ module.exports = React.createClass({ var ScrollPanel = sdk.getComponent("structures.ScrollPanel"); var TintableSvg = sdk.getComponent("elements.TintableSvg"); var RoomPreviewBar = sdk.getComponent("rooms.RoomPreviewBar"); + var Loader = sdk.getComponent("elements.Spinner"); if (!this._timelineWindow) { if (this.props.roomId) { - if (!this.state.timelineLoaded) { - var Loader = sdk.getComponent("elements.Spinner"); + if (this.state.timelineLoading) { return (
    @@ -1481,7 +1621,6 @@ module.exports = React.createClass({ var myMember = this.state.room.getMember(myUserId); if (myMember && myMember.membership == 'invite') { if (this.state.joining || this.state.rejecting) { - var Loader = sdk.getComponent("elements.Spinner"); return (
    @@ -1620,7 +1759,6 @@ module.exports = React.createClass({ aux = ; } else if (this.state.uploadingRoomSettings) { - var Loader = sdk.getComponent("elements.Spinner"); aux = ; } else if (this.state.searching) { @@ -1746,15 +1884,38 @@ module.exports = React.createClass({ hideMessagePanel = true; } - var messagePanel = ( + var messagePanel; + + // just show a spinner while the timeline loads. + // + // put it in a div of the right class so that the order in the + // roomview flexbox is correct. + // + // Note that the click-on-search-result functionality relies on the + // fact that the messagePanel is hidden while the timeline reloads, + // but that the RoomHeader (complete with search term) continues to + // exist. + if (this.state.timelineLoading) { + messagePanel = ( +
    + +
    + ); + } else { + // it's important that stickyBottom = false on this, otherwise if somebody hits the + // bottom of the loaded events when viewing historical messages, we get stuck in a + // loop of paginating our way through the entire history of the room. + messagePanel = ( + style={ hideMessagePanel ? { display: 'none' } : {} } + stickyBottom={ false }>
  • {this.getEventTiles()}     - ); + ); + } return (
    diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 8d26b2e365..a577be2e52 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -37,14 +37,31 @@ if (DEBUG_SCROLL) { * It also provides a hook which allows parents to provide more list elements * when we get close to the start or end of the list. * - * We don't save the absolute scroll offset, because that would be affected by - * window width, zoom level, amount of scrollback, etc. Instead we save an - * identifier for the last fully-visible message, and the number of pixels the - * window was scrolled below it - which is hopefully be near enough. - * * Each child element should have a 'data-scroll-token'. This token is used to * serialise the scroll state, and returned as the 'lastDisplayedScrollToken' * attribute by getScrollState(). + * + * Some notes about the implementation: + * + * The saved 'scrollState' can exist in one of two states: + * + * - atBottom: (the default, and restored by resetScrollState): the viewport + * is scrolled down as far as it can be. When the children are updated, the + * scroll position will be updated to ensure it is still at the bottom. + * + * - fixed, in which the viewport is conceptually tied at a specific scroll + * offset. We don't save the absolute scroll offset, because that would be + * affected by window width, zoom level, amount of scrollback, etc. Instead + * we save an identifier for the last fully-visible message, and the number + * of pixels the window was scrolled below it - which is hopefully be near + * enough. + * + * The 'stickyBottom' property controls the behaviour when we reach the bottom + * of the window (either through a user-initiated scroll, or by calling + * scrollToBottom). If stickyBottom is enabled, the scrollState will enter + * 'atBottom' state - ensuring that new additions cause the window to scroll + * down further. If stickyBottom is disabled, we just save the scroll offset as + * normal. */ module.exports = React.createClass({ displayName: 'ScrollPanel', @@ -145,8 +162,15 @@ module.exports = React.createClass({ this.recentEventScroll = undefined; } - this.scrollState = this._calculateScrollState(); - debuglog("Saved scroll state", this.scrollState); + // If there weren't enough children to fill the viewport, the scroll we + // got might be different to the scroll we wanted; we don't want to + // forget what we wanted, so don't overwrite the saved state unless + // this appears to be a user-initiated scroll. + if (sn.scrollTop != this._lastSetScroll) { + this._saveScrollState(); + } else { + debuglog("Ignoring scroll echo"); + } this.props.onScroll(ev); @@ -154,14 +178,16 @@ module.exports = React.createClass({ }, // return true if the content is fully scrolled down right now; else false. - // - // Note that if the content hasn't yet been fully populated, this may - // spuriously return true even if the user wanted to be looking at earlier - // content. So don't call it in render() cycles. isAtBottom: function() { var sn = this._getScrollNode(); - // + 1 here to avoid fractional pixel rounding errors - return sn.scrollHeight - sn.scrollTop <= sn.clientHeight + 1; + + // there seems to be some bug with flexbox/gemini/chrome/richvdh's + // understanding of the box model, wherein the scrollNode ends up 2 + // pixels higher than the available space, even when there are less + // than a screenful of messages. + 3 is a fudge factor to pretend + // that we're at the bottom when we're still a few pixels off. + + return sn.scrollHeight - Math.ceil(sn.scrollTop) <= sn.clientHeight + 3; }, // check the scroll state and send out backfill requests if necessary. @@ -230,9 +256,9 @@ module.exports = React.createClass({ } q.finally(fillPromise, () => { - debuglog("ScrollPanel: "+dir+" fill complete"); this._pendingFillRequests[dir] = false; }).then((hasMoreResults) => { + debuglog("ScrollPanel: "+dir+" fill complete; hasMoreResults:"+hasMoreResults); if (hasMoreResults) { // further pagination requests have been disabled until now, so // it's time to check the fill state again in case the pagination @@ -249,35 +275,65 @@ module.exports = React.createClass({ }, /* reset the saved scroll state. - * - * This will cause the scroll to be reinitialised on the next update of the - * child list. * * This is useful if the list is being replaced, and you don't want to * preserve scroll even if new children happen to have the same scroll * tokens as old ones. + * + * This will cause the viewport to be scrolled down to the bottom on the + * next update of the child list. This is different to scrollToBottom(), + * which would save the current bottom-most child as the active one (so is + * no use if no children exist yet, or if you are about to replace the + * child list.) */ resetScrollState: function() { - this.scrollState = null; - }, - - scrollToTop: function() { - this._getScrollNode().scrollTop = 0; - debuglog("Scrolled to top"); + this.scrollState = {atBottom: true}; }, scrollToBottom: function() { + // the easiest way to make sure that the scroll state is correctly + // saved is to do the scroll, then save the updated state. (Calculating + // it ourselves is hard, and we can't rely on an onScroll callback + // happening, since there may be no user-visible change here). var scrollNode = this._getScrollNode(); + scrollNode.scrollTop = scrollNode.scrollHeight; debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop); + this._lastSetScroll = scrollNode.scrollTop; + + this._saveScrollState(); }, - // scroll the message list to the node with the given scrollToken. See - // notes in _calculateScrollState on how this works. - // - // pixel_offset gives the number of pixels between the bottom of the node - // and the bottom of the container. + // pixelOffset gives the number of pixels between the bottom of the node + // and the bottom of the container. If undefined, it will put the node + // in the middle of the container. scrollToToken: function(scrollToken, pixelOffset) { + var scrollNode = this._getScrollNode(); + + // default to the middle + if (pixelOffset === undefined) { + pixelOffset = scrollNode.clientHeight / 2; + } + + // save the desired scroll state. It's important we do this here rather + // than as a result of the scroll event, because (a) we might not *get* + // a scroll event, and (b) it might not currently be possible to set + // the requested scroll state (eg, because we hit the end of the + // timeline and need to do more pagination); we want to save the + // *desired* scroll state rather than what we end up achieving. + this.scrollState = { + atBottom: false, + lastDisplayedScrollToken: scrollToken, + pixelOffset: pixelOffset + }; + + // ... then make it so. + this._restoreSavedScrollState(); + }, + + // set the scrollTop attribute appropriately to position the given child at the + // given offset in the window. A helper for _restoreSavedScrollState. + _scrollToToken: function(scrollToken, pixelOffset) { /* find the dom node with the right scrolltoken */ var node; var messages = this.refs.itemlist.children; @@ -291,7 +347,7 @@ module.exports = React.createClass({ } if (!node) { - console.error("No node with scrollToken '"+scrollToken+"'"); + debuglog("ScrollPanel: No node with scrollToken '"+scrollToken+"'"); return; } @@ -312,15 +368,12 @@ module.exports = React.createClass({ debuglog("recentEventScroll now "+this.recentEventScroll); }, - _calculateScrollState: function() { - // Our scroll implementation is agnostic of the precise contents of the - // message list (since it needs to work with both search results and - // timelines). 'refs.messageList' is expected to be a DOM node with a - // number of children, each of which may have a 'data-scroll-token' - // attribute. It is this token which is stored as the - // 'lastDisplayedScrollToken'. - - var atBottom = this.isAtBottom(); + _saveScrollState: function() { + if (this.props.stickyBottom && this.isAtBottom()) { + this.scrollState = { atBottom: true }; + debuglog("Saved scroll state", this.scrollState); + return; + } var itemlist = this.refs.itemlist; var wrapperRect = ReactDOM.findDOMNode(this).getBoundingClientRect(); @@ -332,28 +385,34 @@ module.exports = React.createClass({ var boundingRect = node.getBoundingClientRect(); if (boundingRect.bottom < wrapperRect.bottom) { - return { - atBottom: atBottom, + this.scrollState = { + atBottom: false, lastDisplayedScrollToken: node.dataset.scrollToken, pixelOffset: wrapperRect.bottom - boundingRect.bottom, } + debuglog("Saved scroll state", this.scrollState); + return; } } - // apparently the entire timeline is below the viewport. Give up. - return { atBottom: true }; + debuglog("Unable to save scroll state: found no children in the viewport"); }, _restoreSavedScrollState: function() { var scrollState = this.scrollState; - if (!scrollState || (this.props.stickyBottom && scrollState.atBottom)) { - this.scrollToBottom(); + var scrollNode = this._getScrollNode(); + + if (scrollState.atBottom) { + scrollNode.scrollTop = scrollNode.scrollHeight; + debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop); } else if (scrollState.lastDisplayedScrollToken) { - this.scrollToToken(scrollState.lastDisplayedScrollToken, + this._scrollToToken(scrollState.lastDisplayedScrollToken, scrollState.pixelOffset); } + this._lastSetScroll = scrollNode.scrollTop; }, + /* get the DOM node which has the scrollTop property we care about for our * message panel. */ diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index cfac6cc9b2..6d786294ad 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -98,6 +98,9 @@ module.exports = React.createClass({ /* a function to be called when the highlight is clicked */ onHighlightClick: React.PropTypes.func, + + /* is this the focussed event */ + selectedEvent: React.PropTypes.bool, }, getInitialState: function() { @@ -273,6 +276,7 @@ module.exports = React.createClass({ ) !== -1, mx_EventTile_notSent: this.props.mxEvent.status == 'not_sent', mx_EventTile_highlight: this.shouldHighlight(), + mx_EventTile_selected: this.props.selectedEvent, mx_EventTile_continuation: this.props.continuation, mx_EventTile_last: this.props.last, mx_EventTile_contextual: this.props.contextual, From 3b1ed3a0140a115ba3549b6860701d91d4164870 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 2 Feb 2016 17:59:11 +0000 Subject: [PATCH 5/6] Fix react warnings when a RR animation is happening during room switch If the animation of an RR removal is active when we change room, we end up getting a callback after the RoomView has been unmounted. Guard against this to avoid getting React warnings. --- 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 e2ed8231ef..37866ca646 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -915,7 +915,7 @@ module.exports = React.createClass({ var hr; hr = (
    ); ret.splice(ghostIndex, 0, ( From c82b364ca84c429f0151293738e3fa01ceef7e19 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 3 Feb 2016 08:03:10 +0000 Subject: [PATCH 6/6] Address review comments Mostly renaming things and adding comments. --- src/components/structures/MatrixChat.js | 12 +--- src/components/structures/RoomView.js | 70 +++++++++++++++--------- src/components/structures/ScrollPanel.js | 52 ++++++++++++------ src/components/views/rooms/EventTile.js | 4 +- 4 files changed, 83 insertions(+), 55 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 9d9e7758b6..325b2ca459 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -657,15 +657,9 @@ module.exports = React.createClass({ action: 'start_post_registration', }); } else if (screen.indexOf('room/') == 0) { - var roomString = screen.substring(5); - var eventId; - - // extract event id, if one is given - var idx = roomString.indexOf('/'); - if (idx >= 0) { - eventId = roomString.substring(idx+1); - roomString = roomString.substring(0, idx); - } + var segments = screen.substring(5).split('/'); + var roomString = segments[0]; + var eventId = segments[1]; // undefined if no event id given if (roomString[0] == '#') { if (this.state.logged_in) { diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index be690e4cf2..f0c2e61e58 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -102,7 +102,11 @@ module.exports = React.createClass({ canPeek: false, readMarkerEventId: room ? room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId) : null, readMarkerGhostEventId: undefined, - atBottom: true, + + // 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. + atEndOfLiveTimeline: true, } }, @@ -360,7 +364,7 @@ module.exports = React.createClass({ if (ev.getSender() !== MatrixClientPeg.get().credentials.userId) { // update unread count when scrolled up - if (!this.state.searchResults && this.state.atBottom) { + if (!this.state.searchResults && this.state.atEndOfLiveTimeline) { // no change } else { @@ -467,7 +471,7 @@ module.exports = React.createClass({ // // we do this here as well as in sendReadReceipt to deal with // people using two clients at once. - if (this.refs.messagePanel && this.state.atBottom) { + if (this.refs.messagePanel && this.state.atEndOfLiveTimeline) { this.refs.messagePanel.scrollToToken(readMarkerEventId); } } @@ -700,14 +704,14 @@ module.exports = React.createClass({ if (this.state.numUnreadMessages != 0) { this.setState({ numUnreadMessages: 0 }); } - if (!this.state.atBottom) { - this.setState({ atBottom: true }); + if (!this.state.atEndOfLiveTimeline) { + this.setState({ atEndOfLiveTimeline: true }); } } else { - if (this.state.atBottom) { - this.setState({ atBottom: false }); - } + if (this.state.atEndOfLiveTimeline) { + this.setState({ atEndOfLiveTimeline: false }); + } } }, @@ -994,7 +998,7 @@ module.exports = React.createClass({ ret.push(
  • + last={last} isSelectedEvent={highlight}/>
    • ); @@ -1285,11 +1289,18 @@ module.exports = React.createClass({ var currentReadUpToEventIndex = this._indexForEventId(currentReadUpToEventId); // We want to avoid sending out read receipts when we are looking at - // events in the past. + // events in the past which are before the latest RR. + // + // For now, let's apply a heuristic: if (a) the event corresponding to + // the latest RR (either from the server, or sent by ourselves) doesn't + // appear in our timeline, and (b) we could forward-paginate the event + // timeline, then don't send any more RRs. + // + // This isn't watertight, as we could be looking at a section of + // timeline which is *after* the latest RR (so we should actually send + // RRs) - but that is a bit of a niche case. It will sort itself out when + // the user eventually hits the live timeline. // - // For now, let's apply a heuristic: if (a) the server has a - // readUpToEvent for us, (b) we can't find it, and (c) we could - // forward-paginate the event timeline, then suppress read receipts. if (currentReadUpToEventId && currentReadUpToEventIndex === null && this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) { return; @@ -1317,7 +1328,7 @@ module.exports = React.createClass({ // // we do this here as well as in onRoomReceipt to cater for guest users // (which do not send out read receipts). - if (this.state.atBottom) { + if (this.state.atEndOfLiveTimeline) { this.refs.messagePanel.scrollToToken(lastReadEvent.getId()); } } @@ -1444,7 +1455,8 @@ module.exports = React.createClass({ return this.state.numUnreadMessages + " new message" + (this.state.numUnreadMessages > 1 ? "s" : ""); }, - scrollToBottom: function() { + // jump down to the bottom of this room, where new events are arriving + jumpToLiveTimeline: function() { // if we can't forward-paginate the existing timeline, then there // is no point reloading it - just jump straight to the bottom. // @@ -1479,16 +1491,20 @@ module.exports = React.createClass({ var scrollState = messagePanel.getScrollState(); - if (scrollState.atBottom) { + if (scrollState.stuckAtBottom) { // we don't really expect to be in this state, but it will - // occasionally happen when we are in a transition. Treat it the - // same as having no saved state (which will cause us to scroll to - // last unread on reload). + // occasionally happen when no scroll state has been set on the + // messagePanel (ie, we didn't have an initial event (so it's + // probably a new room), there has been no user-initiated scroll, and + // no read-receipts have arrived to update the scroll position). + // + // Return null, which will cause us to scroll to last unread on + // reload. return null; } return { - focussedEvent: scrollState.lastDisplayedScrollToken, + focussedEvent: scrollState.trackedScrollToken, pixelOffset: scrollState.pixelOffset, }; }, @@ -1731,7 +1747,7 @@ module.exports = React.createClass({ // set when you've scrolled up else if (unreadMsgs) { statusBar = ( -
    +
    {unreadMsgs}
    @@ -1745,9 +1761,9 @@ module.exports = React.createClass({
    ); } - else if (!this.state.atBottom) { + else if (!this.state.atEndOfLiveTimeline) { statusBar = ( -
    +
    Scroll to bottom of page
    ); @@ -1888,8 +1904,10 @@ module.exports = React.createClass({ // just show a spinner while the timeline loads. // - // put it in a div of the right class so that the order in the - // roomview flexbox is correct. + // put it in a div of the right class (mx_RoomView_messagePanel) so + // that the order in the roomview flexbox is correct, and + // mx_RoomView_messageListWrapper to position the inner div in the + // right place. // // Note that the click-on-search-result functionality relies on the // fact that the messagePanel is hidden while the timeline reloads, @@ -1897,7 +1915,7 @@ module.exports = React.createClass({ // exist. if (this.state.timelineLoading) { messagePanel = ( -
    +
    ); diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index a577be2e52..514937f877 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -38,16 +38,17 @@ if (DEBUG_SCROLL) { * when we get close to the start or end of the list. * * Each child element should have a 'data-scroll-token'. This token is used to - * serialise the scroll state, and returned as the 'lastDisplayedScrollToken' + * serialise the scroll state, and returned as the 'trackedScrollToken' * attribute by getScrollState(). * * Some notes about the implementation: * * The saved 'scrollState' can exist in one of two states: * - * - atBottom: (the default, and restored by resetScrollState): the viewport - * is scrolled down as far as it can be. When the children are updated, the - * scroll position will be updated to ensure it is still at the bottom. + * - stuckAtBottom: (the default, and restored by resetScrollState): the + * viewport is scrolled down as far as it can be. When the children are + * updated, the scroll position will be updated to ensure it is still at + * the bottom. * * - fixed, in which the viewport is conceptually tied at a specific scroll * offset. We don't save the absolute scroll offset, because that would be @@ -59,9 +60,9 @@ if (DEBUG_SCROLL) { * The 'stickyBottom' property controls the behaviour when we reach the bottom * of the window (either through a user-initiated scroll, or by calling * scrollToBottom). If stickyBottom is enabled, the scrollState will enter - * 'atBottom' state - ensuring that new additions cause the window to scroll - * down further. If stickyBottom is disabled, we just save the scroll offset as - * normal. + * 'stuckAtBottom' state - ensuring that new additions cause the window to + * scroll down further. If stickyBottom is disabled, we just save the scroll + * offset as normal. */ module.exports = React.createClass({ displayName: 'ScrollPanel', @@ -178,6 +179,10 @@ module.exports = React.createClass({ }, // return true if the content is fully scrolled down right now; else false. + // + // note that this is independent of the 'stuckAtBottom' state - it is simply + // about whether the the content is scrolled down right now, irrespective of + // whether it will stay that way when the children update. isAtBottom: function() { var sn = this._getScrollNode(); @@ -268,8 +273,19 @@ module.exports = React.createClass({ }).done(); }, - // get the current scroll position of the room, so that it can be - // restored later + /* get the current scroll state. This returns an object with the following + * properties: + * + * boolean stuckAtBottom: true if we are tracking the bottom of the + * scroll. false if we are tracking a particular child. + * + * string trackedScrollToken: undefined if stuckAtBottom is true; if it is + * false, the data-scroll-token of the child which we are tracking. + * + * number pixelOffset: undefined if stuckAtBottom is true; if it is false, + * the number of pixels the bottom of the tracked child is above the + * bottom of the scroll panel. + */ getScrollState: function() { return this.scrollState; }, @@ -287,7 +303,7 @@ module.exports = React.createClass({ * child list.) */ resetScrollState: function() { - this.scrollState = {atBottom: true}; + this.scrollState = {stuckAtBottom: true}; }, scrollToBottom: function() { @@ -322,8 +338,8 @@ module.exports = React.createClass({ // timeline and need to do more pagination); we want to save the // *desired* scroll state rather than what we end up achieving. this.scrollState = { - atBottom: false, - lastDisplayedScrollToken: scrollToken, + stuckAtBottom: false, + trackedScrollToken: scrollToken, pixelOffset: pixelOffset }; @@ -370,7 +386,7 @@ module.exports = React.createClass({ _saveScrollState: function() { if (this.props.stickyBottom && this.isAtBottom()) { - this.scrollState = { atBottom: true }; + this.scrollState = { stuckAtBottom: true }; debuglog("Saved scroll state", this.scrollState); return; } @@ -386,8 +402,8 @@ module.exports = React.createClass({ var boundingRect = node.getBoundingClientRect(); if (boundingRect.bottom < wrapperRect.bottom) { this.scrollState = { - atBottom: false, - lastDisplayedScrollToken: node.dataset.scrollToken, + stuckAtBottom: false, + trackedScrollToken: node.dataset.scrollToken, pixelOffset: wrapperRect.bottom - boundingRect.bottom, } debuglog("Saved scroll state", this.scrollState); @@ -402,11 +418,11 @@ module.exports = React.createClass({ var scrollState = this.scrollState; var scrollNode = this._getScrollNode(); - if (scrollState.atBottom) { + if (scrollState.stuckAtBottom) { scrollNode.scrollTop = scrollNode.scrollHeight; debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop); - } else if (scrollState.lastDisplayedScrollToken) { - this._scrollToToken(scrollState.lastDisplayedScrollToken, + } else if (scrollState.trackedScrollToken) { + this._scrollToToken(scrollState.trackedScrollToken, scrollState.pixelOffset); } this._lastSetScroll = scrollNode.scrollTop; diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 6d786294ad..f580686128 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -100,7 +100,7 @@ module.exports = React.createClass({ onHighlightClick: React.PropTypes.func, /* is this the focussed event */ - selectedEvent: React.PropTypes.bool, + isSelectedEvent: React.PropTypes.bool, }, getInitialState: function() { @@ -276,7 +276,7 @@ module.exports = React.createClass({ ) !== -1, mx_EventTile_notSent: this.props.mxEvent.status == 'not_sent', mx_EventTile_highlight: this.shouldHighlight(), - mx_EventTile_selected: this.props.selectedEvent, + mx_EventTile_selected: this.props.isSelectedEvent, mx_EventTile_continuation: this.props.continuation, mx_EventTile_last: this.props.last, mx_EventTile_contextual: this.props.contextual,