From d498c5a81ef9fc7069d78fc4504df2f0b7a52f30 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 10 Feb 2016 09:18:52 +0000 Subject: [PATCH] Implementation of new read-marker semantics Separate the read-up-to marker from the read-receipts, and show a status bar when the read marker is above the screen. Move the read-marker down when it is on the screen and there is user activity. (This requires corresponding changes in vector-web, to provide the CSS and img) --- src/component-index.js | 1 + src/components/structures/MessagePanel.js | 67 ++++- src/components/structures/RoomView.js | 42 +++ src/components/structures/TimelinePanel.js | 243 +++++++++++++++--- .../views/rooms/TopUnreadMessagesBar.js | 48 ++++ 5 files changed, 349 insertions(+), 52 deletions(-) create mode 100644 src/components/views/rooms/TopUnreadMessagesBar.js diff --git a/src/component-index.js b/src/component-index.js index eaf286520e..9b7a1e8751 100644 --- a/src/component-index.js +++ b/src/component-index.js @@ -88,6 +88,7 @@ module.exports.components['views.rooms.RoomTile'] = require('./components/views/ module.exports.components['views.rooms.SearchableEntityList'] = require('./components/views/rooms/SearchableEntityList'); module.exports.components['views.rooms.SearchResultTile'] = require('./components/views/rooms/SearchResultTile'); module.exports.components['views.rooms.TabCompleteBar'] = require('./components/views/rooms/TabCompleteBar'); +module.exports.components['views.rooms.TopUnreadMessagesBar'] = require('./components/views/rooms/TopUnreadMessagesBar'); module.exports.components['views.rooms.UserTile'] = require('./components/views/rooms/UserTile'); module.exports.components['views.settings.ChangeAvatar'] = require('./components/views/settings/ChangeAvatar'); module.exports.components['views.settings.ChangeDisplayName'] = require('./components/views/settings/ChangeDisplayName'); diff --git a/src/components/structures/MessagePanel.js b/src/components/structures/MessagePanel.js index 75ca902fc3..21a7ab9330 100644 --- a/src/components/structures/MessagePanel.js +++ b/src/components/structures/MessagePanel.js @@ -35,6 +35,9 @@ module.exports = React.createClass({ // event after which we should show a read marker readMarkerEventId: React.PropTypes.string, + // whether the read marker should be visible + readMarkerVisible: React.PropTypes.bool, + // the userid of our user. This is used to suppress the read marker // for pending messages. ourUserId: React.PropTypes.string, @@ -96,6 +99,34 @@ module.exports = React.createClass({ return this.refs.scrollPanel.getScrollState(); }, + // returns one of: + // + // null: there is no read marker + // -1: read marker is above the window + // 0: read marker is within the window + // +1: read marker is below the window + getReadMarkerPosition: function() { + var readMarker = this.refs.readMarkerNode; + var messageWrapper = this.refs.scrollPanel; + + if (!readMarker || !messageWrapper) { + return null; + } + + var wrapperRect = ReactDOM.findDOMNode(messageWrapper).getBoundingClientRect(); + var readMarkerRect = readMarker.getBoundingClientRect(); + + // the read-marker pretends to have zero height when it is actually + // two pixels high; +2 here to account for that. + if (readMarkerRect.bottom + 2 < wrapperRect.top) { + return -1; + } else if (readMarkerRect.top < wrapperRect.bottom) { + return 0; + } else { + return 1; + } + }, + /* jump to the bottom of the content. */ scrollToBottom: function() { @@ -125,6 +156,8 @@ module.exports = React.createClass({ }, _getEventTiles: function() { + this.eventNodes = {}; + var EventTile = sdk.getComponent('rooms.EventTile'); this.eventNodes = {}; @@ -189,17 +222,19 @@ module.exports = React.createClass({ var mxEv = eventsToShow[i]; var wantTile = true; - // insert the read marker if appropriate. Note that doing it here - // implicitly means that we never put it at the end of the timeline, - // because i will never reach eventsToShow.length. + // insert the read marker if appropriate. if (i == readMarkerIndex) { + var visible = this.props.readMarkerVisible; + + // XXX is this still needed? // suppress the read marker if the next event is sent by us; this // is a nonsensical and temporary situation caused by the delay between // us sending a message and receiving the synthesized receipt. - if (mxEv.sender && mxEv.sender.userId != this.props.ourUserId) { - ret.push(this._getReadMarkerTile()); - readMarkerVisible = true; + if (mxEv.sender && mxEv.sender.userId == this.props.ourUserId) { + visible = false; } + ret.push(this._getReadMarkerTile(visible)); + readMarkerVisible = visible; } else if (i == ghostIndex) { ret.push(this._getReadMarkerGhostTile()); } @@ -214,7 +249,15 @@ module.exports = React.createClass({ prevEvent = mxEv; } + // if the read marker comes at the end of the timeline, we don't want + // to show it, but we still want to create the
  • for it so that the + // algorithms which depend on its position on the screen aren't confused. + if (i == readMarkerIndex) { + ret.push(this._getReadMarkerTile(false)); + } + this.currentReadMarkerEventId = readMarkerVisible ? this.props.readMarkerEventId : null; + return ret; }, @@ -261,14 +304,16 @@ module.exports = React.createClass({ return ret; }, - _getReadMarkerTile: function() { + _getReadMarkerTile: function(visible) { var hr; - hr =
    ; + if (visible) { + hr =
    ; + } return ( -
  • {hr}
    • diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 8687cb444c..077ff50360 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -90,6 +90,8 @@ module.exports = React.createClass({ // 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, + + showTopUnreadMessagesBar: false, } }, @@ -553,6 +555,7 @@ module.exports = React.createClass({ atEndOfLiveTimeline: false, }); } + this._updateTopUnreadMessagesBar(); }, onDragOver: function(ev) { @@ -873,6 +876,30 @@ module.exports = React.createClass({ this.refs.messagePanel.jumpToLiveTimeline(); }, + // jump up to wherever our read marker is + jumpToReadMarker: function() { + this.refs.messagePanel.jumpToReadMarker(); + }, + + // update the read marker to match the read-receipt + forgetReadMarker: function() { + this.refs.messagePanel.forgetReadMarker(); + }, + + // decide whether or not the top 'unread messages' bar should be shown + _updateTopUnreadMessagesBar: function() { + if (!this.refs.messagePanel) + return; + + var pos = this.refs.messagePanel.getReadMarkerPosition(); + + // we want to show the bar if the read-marker is off the top of the + // screen. + var showBar = (pos < 0); + + this.setState({showTopUnreadMessagesBar: showBar}); + }, + // get the current scroll position of the room, so that it can be // restored when we switch back to it. // @@ -1272,8 +1299,22 @@ module.exports = React.createClass({ this.props.ConferenceHandler.isConferenceUser : null } onScroll={ this.onMessageListScroll } + onReadMarkerUpdated={ this._updateTopUnreadMessagesBar } />); + var topUnreadMessagesBar = null; + if (this.state.showTopUnreadMessagesBar) { + var TopUnreadMessagesBar = sdk.getComponent('rooms.TopUnreadMessagesBar'); + topUnreadMessagesBar = ( +
    + +
    + ); + } + return (
    + { topUnreadMessagesBar } { messagePanel } { searchResultsPanel }
    diff --git a/src/components/structures/TimelinePanel.js b/src/components/structures/TimelinePanel.js index 14da1e9ab8..d80f3fe3f1 100644 --- a/src/components/structures/TimelinePanel.js +++ b/src/components/structures/TimelinePanel.js @@ -29,6 +29,11 @@ var PAGINATE_SIZE = 20; var INITIAL_SIZE = 20; var TIMELINE_CAP = 1000; // the most events to show in a timeline +// consider that the user remains "active" for this many milliseconds after a +// user_activity event (and thus don't make the read-marker visible on new +// events) +var CONSIDER_USER_ACTIVE_FOR_MS = 500; + var DEBUG = false; if (DEBUG) { @@ -43,7 +48,7 @@ if (DEBUG) { * * Also responsible for handling and sending read receipts. */ -module.exports = React.createClass({ +var TimelinePanel = React.createClass({ displayName: 'TimelinePanel', propTypes: { @@ -74,14 +79,34 @@ module.exports = React.createClass({ // callback which is called when the panel is scrolled. onScroll: React.PropTypes.func, + + // callback which is called when the read-up-to mark is updated. + onReadMarkerUpdated: React.PropTypes.func, + }, + + statics: { + // a map from room id to read marker event ID + roomReadMarkerMap: {}, + + // a map from room id to read marker event timestamp + roomReadMarkerTsMap: {}, }, getInitialState: function() { + var initialReadMarker = + TimelinePanel.roomReadMarkerMap[this.props.room.roomId] + || this._getCurrentReadReceipt(); + return { events: [], timelineLoading: true, // track whether our room timeline is loading canBackPaginate: true, - readMarkerEventId: this._getCurrentReadReceipt(), + + // start with the read-marker visible, so that we see its animated + // disappearance when swtitching into the room. + readMarkerVisible: true, + + readMarkerEventId: initialReadMarker, }; }, @@ -89,9 +114,10 @@ module.exports = React.createClass({ debuglog("TimelinePanel: mounting"); this.last_rr_sent_event_id = undefined; + this._resetActivityTimer(); + this.dispatcherRef = dis.register(this.onAction); MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); - MatrixClientPeg.get().on("Room.receipt", this.onRoomReceipt); this._initTimeline(this.props); }, @@ -120,7 +146,6 @@ module.exports = React.createClass({ var client = MatrixClientPeg.get(); if (client) { client.removeListener("Room.timeline", this.onRoomTimeline); - client.removeListener("Room.receipt", this.onRoomReceipt); } }, @@ -139,9 +164,26 @@ module.exports = React.createClass({ }); }, + onMessageListScroll: function () { + if (this.props.onScroll) { + this.props.onScroll(); + } + + // we hide the read marker when it first comes onto the screen, but if + // it goes back off the top of the screen (presumably because the user + // clicks on the 'jump to bottom' button), we need to re-enable it. + if (this.getReadMarkerPosition() < 0) { + this.setState({readMarkerVisible: true}); + } + }, + onAction: function(payload) { switch (payload.action) { case 'user_activity': + this._resetActivityTimer(); + + // fall-through! + case 'user_activity_end': // we could treat user_activity_end differently and not // send receipts for messages that have arrived between @@ -149,10 +191,15 @@ module.exports = React.createClass({ // being active, but let's see if this is actually // necessary. this.sendReadReceipt(); + this.updateReadMarker(); break; } }, + _resetActivityTimer: function() { + this.user_last_active = Date.now(); + }, + onRoomTimeline: function(ev, room, toStartOfTimeline, removed, data) { // ignore events for other rooms if (room !== this.props.room) return; @@ -161,6 +208,26 @@ module.exports = React.createClass({ // updates from pagination will happen when the paginate completes. if (toStartOfTimeline || !data || !data.liveEvent) return; + if (!this.refs.messagePanel) return; + + // when a new event arrives when the user is not watching the window, but the + // window is in its auto-scroll mode, make sure the read marker is visible. + // + // We consider the user to be watching the window if they performed an action + // less than CONSIDER_USER_ACTIVE_FOR_MS ago. + // + // We ignore events we have sent ourselves; we don't want to see the + // read-marker when a remote echo of an event we have just sent takes + // more than CONSIDER_USER_ACTIVE_FOR_MS. + // + var myUserId = MatrixClientPeg.get().credentials.userId; + var sender = ev.sender ? ev.sender.userId : null; + var activity_age = Date.now() - this.user_last_active; + if (sender != myUserId && this.refs.messagePanel.getScrollState().stuckAtBottom + && activity_age > CONSIDER_USER_ACTIVE_FOR_MS) { + this.setState({readMarkerVisible: true}); + } + // 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 - @@ -171,37 +238,6 @@ module.exports = React.createClass({ } }, - onRoomReceipt: function(receiptEvent, room) { - if (room !== this.props.room) - return; - - // the received event may or may not be for our user; but it turns out - // to be easier to do the processing anyway than to figure out if it - // is. - var oldReadMarker = this.state.readMarkerEventId; - var newReadMarker = this._getCurrentReadReceipt(); - - if (newReadMarker == oldReadMarker) { - return; - } - - // suppress the animation when moving forward over an event which was sent - // by us; the original RM will have been suppressed so we don't want to show - // the animation either. - var oldReadMarkerIndex = this._indexForEventId(oldReadMarker); - if (oldReadMarkerIndex + 1 < this.state.events.length) { - var myUserId = MatrixClientPeg.get().credentials.userId; - var nextEvent = this.state.events[oldReadMarkerIndex + 1]; - if (nextEvent.sender && nextEvent.sender.userId == myUserId) { - oldReadMarker = undefined; - } - } - - this.setState({ - readMarkerEventId: newReadMarker, - }); - }, - sendReadReceipt: function() { if (!this.refs.messagePanel) return; @@ -226,7 +262,9 @@ module.exports = React.createClass({ return; } - var lastReadEventIndex = this._getLastDisplayedEventIndexIgnoringOwn(); + var lastReadEventIndex = this._getLastDisplayedEventIndex({ + ignoreOwn: true + }); if (lastReadEventIndex === null) return; var lastReadEvent = this.state.events[lastReadEventIndex]; @@ -243,6 +281,43 @@ module.exports = React.createClass({ } }, + // if the read marker is on the screen, we can now assume we've caught up to the end + // of the screen, so move the marker down to the bottom of the screen. + updateReadMarker: function() { + if (this.getReadMarkerPosition() !== 0) { + return; + } + + var currentIndex = this._indexForEventId(this.state.readMarkerEventId); + + // move the RM to *after* the message at the bottom of the screen. This + // avoids a problem whereby we never advance the RM if there is a huge + // message which doesn't fit on the screen. + // + // But ignore local echoes for this - they have a temporary event ID + // and we'll get confused when their ID changes and we can't figure out + // where the RM is pointing to. The read marker will be invisible for + // now anyway, so this doesn't really matter. + var lastDisplayedIndex = this._getLastDisplayedEventIndex({ + allowPartial: true, + ignoreEchoes: true, + }); + + if (lastDisplayedIndex === null) { + return; + } + + var lastDisplayedEvent = this.state.events[lastDisplayedIndex]; + this._setReadMarker(lastDisplayedEvent.getId(), + lastDisplayedEvent.getTs()); + + // the read-marker should become invisible, so that if the user scrolls + // down, they don't see it. + this.setState({ + readMarkerVisible: false, + }); + }, + /* jump down to the bottom of this room, where new events are arriving */ jumpToLiveTimeline: function() { @@ -260,6 +335,35 @@ module.exports = React.createClass({ } }, + /* scroll to show the read-up-to marker + */ + jumpToReadMarker: function() { + if (!this.state.readMarkerEventId) + return; + if (!this.refs.messagePanel) + return; + this.refs.messagePanel.scrollToEvent(this.state.readMarkerEventId); + }, + + + /* update the read-up-to marker to match the read receipt + */ + forgetReadMarker: function() { + var rmId = this._getCurrentReadReceipt(); + + // see if we know the timestamp for the rr event + var tl = this.props.room.getTimelineForEvent(rmId); + var rmTs; + if (tl) { + var event = tl.getEvents().find((e) => { return e.getId() == rmId }); + if (event) { + rmTs = event.getTs(); + } + } + + this._setReadMarker(rmId, rmTs); + }, + /* return true if the content is fully scrolled down and we are * at the end of the live timeline. */ @@ -281,6 +385,33 @@ module.exports = React.createClass({ return this.refs.messagePanel.getScrollState(); }, + // returns one of: + // + // null: there is no read marker + // -1: read marker is above the window + // 0: read marker is visible + // +1: read marker is below the window + getReadMarkerPosition: function() { + if (!this.refs.messagePanel) { return null; } + var ret = this.refs.messagePanel.getReadMarkerPosition(); + if (ret !== null) { + return ret; + } + + // the messagePanel doesn't know where the read marker is. + // if we know the timestamp of the read marker, make a guess based on that. + var rmTs = TimelinePanel.roomReadMarkerTsMap[this.props.room.roomId]; + if (rmTs && this.state.events) { + if (rmTs < this.state.events[0].getTs()) { + return -1; + } else { + return 1; + } + } + + return null; + }, + _initTimeline: function(props) { var initialEvent = props.eventId; var pixelOffset = props.eventPixelOffset; @@ -335,6 +466,7 @@ module.exports = React.createClass({ } this.sendReadReceipt(); + this.updateReadMarker(); }); }); }, @@ -361,16 +493,27 @@ module.exports = React.createClass({ return null; }, - _getLastDisplayedEventIndexIgnoringOwn: function() { + _getLastDisplayedEventIndex: function(opts) { + opts = opts || {}; + var ignoreOwn = opts.ignoreOwn || false; + var ignoreEchoes = opts.ignoreEchoes || false; + var allowPartial = opts.allowPartial || false; + var messagePanel = this.refs.messagePanel; if (messagePanel === undefined) return null; var wrapperRect = ReactDOM.findDOMNode(messagePanel).getBoundingClientRect(); + var myUserId = MatrixClientPeg.get().credentials.userId; 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) { + if (ignoreOwn && ev.sender && ev.sender.userId == myUserId) { + continue; + } + + // local echoes have a fake event ID + if (ignoreEchoes && ev.status) { continue; } @@ -378,8 +521,8 @@ module.exports = React.createClass({ if (!node) continue; var boundingRect = node.getBoundingClientRect(); - - if (boundingRect.bottom < wrapperRect.bottom) { + if ((allowPartial && boundingRect.top < wrapperRect.bottom) || + (!allowPartial && boundingRect.bottom < wrapperRect.bottom)) { return i; } } @@ -399,6 +542,21 @@ module.exports = React.createClass({ return this.props.room.getEventReadUpTo(myUserId); }, + _setReadMarker: function(eventId, eventTs) { + // ideally we'd sync these via the server, but for now just stash them + // in a map. + TimelinePanel.roomReadMarkerMap[this.props.room.roomId] = eventId; + + // in order to later figure out if the read marker is + // above or below the visible timeline, we stash the timestamp. + TimelinePanel.roomReadMarkerTsMap[this.props.room.roomId] = eventTs; + + // run the render cycle before calling the callback, so that + // getReadMarkerPosition() returns the right thing. + this.setState({ + readMarkerEventId: eventId, + }, this.props.onReadMarkerUpdated); + }, render: function() { var MessagePanel = sdk.getComponent("structures.MessagePanel"); @@ -439,13 +597,16 @@ module.exports = React.createClass({ events={ this.state.events } highlightedEventId={ this.props.highlightedEventId } readMarkerEventId={ this.state.readMarkerEventId } + readMarkerVisible={ this.state.readMarkerVisible } suppressFirstDateSeparator={ this.state.canBackPaginate } ourUserId={ MatrixClientPeg.get().credentials.userId } stickyBottom={ stickyBottom } isConferenceUser={ this.props.isConferenceUser } - onScroll={ this.props.onScroll } + onScroll={ this.onMessageListScroll } onFillRequest={ this.onMessageListFillRequest } /> ); }, }); + +module.exports = TimelinePanel; diff --git a/src/components/views/rooms/TopUnreadMessagesBar.js b/src/components/views/rooms/TopUnreadMessagesBar.js new file mode 100644 index 0000000000..a1e24e2eac --- /dev/null +++ b/src/components/views/rooms/TopUnreadMessagesBar.js @@ -0,0 +1,48 @@ +/* +Copyright 2016 OpenMarket Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +'use strict'; + +var React = require('react'); +var sdk = require('../../../index'); + +module.exports = React.createClass({ + displayName: 'TopUnreadMessagesBar', + + propTypes: { + onScrollUpClick: React.PropTypes.func, + onCloseClick: React.PropTypes.func, + }, + + render: function() { + return ( +
    +
    + Scroll to unread messages + Unread messages +
    + Close +
    + ); + }, +}); +