Address review comments

Mostly renaming things and adding comments.
This commit is contained in:
Richard van der Hoff 2016-02-03 08:03:10 +00:00
parent d9e13780b8
commit c82b364ca8
4 changed files with 83 additions and 55 deletions

View file

@ -657,15 +657,9 @@ module.exports = React.createClass({
action: 'start_post_registration', action: 'start_post_registration',
}); });
} else if (screen.indexOf('room/') == 0) { } else if (screen.indexOf('room/') == 0) {
var roomString = screen.substring(5); var segments = screen.substring(5).split('/');
var eventId; var roomString = segments[0];
var eventId = segments[1]; // undefined if no event id given
// 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 (roomString[0] == '#') {
if (this.state.logged_in) { if (this.state.logged_in) {

View file

@ -102,7 +102,11 @@ module.exports = React.createClass({
canPeek: false, canPeek: false,
readMarkerEventId: room ? room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId) : null, readMarkerEventId: room ? room.getEventReadUpTo(MatrixClientPeg.get().credentials.userId) : null,
readMarkerGhostEventId: undefined, 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) { if (ev.getSender() !== MatrixClientPeg.get().credentials.userId) {
// update unread count when scrolled up // update unread count when scrolled up
if (!this.state.searchResults && this.state.atBottom) { if (!this.state.searchResults && this.state.atEndOfLiveTimeline) {
// no change // no change
} }
else { else {
@ -467,7 +471,7 @@ module.exports = React.createClass({
// //
// we do this here as well as in sendReadReceipt to deal with // we do this here as well as in sendReadReceipt to deal with
// people using two clients at once. // 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); this.refs.messagePanel.scrollToToken(readMarkerEventId);
} }
} }
@ -700,14 +704,14 @@ module.exports = React.createClass({
if (this.state.numUnreadMessages != 0) { if (this.state.numUnreadMessages != 0) {
this.setState({ numUnreadMessages: 0 }); this.setState({ numUnreadMessages: 0 });
} }
if (!this.state.atBottom) { if (!this.state.atEndOfLiveTimeline) {
this.setState({ atBottom: true }); this.setState({ atEndOfLiveTimeline: true });
} }
} }
else { else {
if (this.state.atBottom) { if (this.state.atEndOfLiveTimeline) {
this.setState({ atBottom: false }); this.setState({ atEndOfLiveTimeline: false });
} }
} }
}, },
@ -994,7 +998,7 @@ module.exports = React.createClass({
ret.push( ret.push(
<li key={eventId} ref={this._collectEventNode.bind(this, eventId)} data-scroll-token={eventId}> <li key={eventId} ref={this._collectEventNode.bind(this, eventId)} data-scroll-token={eventId}>
<EventTile mxEvent={mxEv} continuation={continuation} <EventTile mxEvent={mxEv} continuation={continuation}
last={last} selectedEvent={highlight}/> last={last} isSelectedEvent={highlight}/>
</li> </li>
); );
@ -1285,11 +1289,18 @@ module.exports = React.createClass({
var currentReadUpToEventIndex = this._indexForEventId(currentReadUpToEventId); var currentReadUpToEventIndex = this._indexForEventId(currentReadUpToEventId);
// We want to avoid sending out read receipts when we are looking at // 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 && if (currentReadUpToEventId && currentReadUpToEventIndex === null &&
this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) { this._timelineWindow.canPaginate(EventTimeline.FORWARDS)) {
return; return;
@ -1317,7 +1328,7 @@ module.exports = React.createClass({
// //
// we do this here as well as in onRoomReceipt to cater for guest users // we do this here as well as in onRoomReceipt to cater for guest users
// (which do not send out read receipts). // (which do not send out read receipts).
if (this.state.atBottom) { if (this.state.atEndOfLiveTimeline) {
this.refs.messagePanel.scrollToToken(lastReadEvent.getId()); 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" : ""); 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 // if we can't forward-paginate the existing timeline, then there
// is no point reloading it - just jump straight to the bottom. // is no point reloading it - just jump straight to the bottom.
// //
@ -1479,16 +1491,20 @@ module.exports = React.createClass({
var scrollState = messagePanel.getScrollState(); var scrollState = messagePanel.getScrollState();
if (scrollState.atBottom) { if (scrollState.stuckAtBottom) {
// we don't really expect to be in this state, but it will // we don't really expect to be in this state, but it will
// occasionally happen when we are in a transition. Treat it the // occasionally happen when no scroll state has been set on the
// same as having no saved state (which will cause us to scroll to // messagePanel (ie, we didn't have an initial event (so it's
// last unread on reload). // 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 null;
} }
return { return {
focussedEvent: scrollState.lastDisplayedScrollToken, focussedEvent: scrollState.trackedScrollToken,
pixelOffset: scrollState.pixelOffset, pixelOffset: scrollState.pixelOffset,
}; };
}, },
@ -1731,7 +1747,7 @@ module.exports = React.createClass({
// set when you've scrolled up // set when you've scrolled up
else if (unreadMsgs) { else if (unreadMsgs) {
statusBar = ( statusBar = (
<div className="mx_RoomView_unreadMessagesBar" onClick={ this.scrollToBottom }> <div className="mx_RoomView_unreadMessagesBar" onClick={ this.jumpToLiveTimeline }>
<img src="img/newmessages.svg" width="24" height="24" alt=""/> <img src="img/newmessages.svg" width="24" height="24" alt=""/>
{unreadMsgs} {unreadMsgs}
</div> </div>
@ -1745,9 +1761,9 @@ module.exports = React.createClass({
</div> </div>
); );
} }
else if (!this.state.atBottom) { else if (!this.state.atEndOfLiveTimeline) {
statusBar = ( statusBar = (
<div className="mx_RoomView_scrollToBottomBar" onClick={ this.scrollToBottom }> <div className="mx_RoomView_scrollToBottomBar" onClick={ this.jumpToLiveTimeline }>
<img src="img/scrolldown.svg" width="24" height="24" alt="Scroll to bottom of page" title="Scroll to bottom of page"/> <img src="img/scrolldown.svg" width="24" height="24" alt="Scroll to bottom of page" title="Scroll to bottom of page"/>
</div> </div>
); );
@ -1888,8 +1904,10 @@ module.exports = React.createClass({
// just show a spinner while the timeline loads. // just show a spinner while the timeline loads.
// //
// put it in a div of the right class so that the order in the // put it in a div of the right class (mx_RoomView_messagePanel) so
// roomview flexbox is correct. // 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 // Note that the click-on-search-result functionality relies on the
// fact that the messagePanel is hidden while the timeline reloads, // fact that the messagePanel is hidden while the timeline reloads,
@ -1897,7 +1915,7 @@ module.exports = React.createClass({
// exist. // exist.
if (this.state.timelineLoading) { if (this.state.timelineLoading) {
messagePanel = ( messagePanel = (
<div className="mx_RoomView_messagePanel" style={{display: "flex"}}> <div className="mx_RoomView_messagePanel mx_RoomView_messageListWrapper">
<Loader /> <Loader />
</div> </div>
); );

View file

@ -38,16 +38,17 @@ if (DEBUG_SCROLL) {
* when we get close to the start or end of the list. * 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 * 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(). * attribute by getScrollState().
* *
* Some notes about the implementation: * Some notes about the implementation:
* *
* The saved 'scrollState' can exist in one of two states: * The saved 'scrollState' can exist in one of two states:
* *
* - atBottom: (the default, and restored by resetScrollState): the viewport * - stuckAtBottom: (the default, and restored by resetScrollState): the
* is scrolled down as far as it can be. When the children are updated, the * viewport is scrolled down as far as it can be. When the children are
* scroll position will be updated to ensure it is still at the bottom. * 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 * - 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 * 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 * The 'stickyBottom' property controls the behaviour when we reach the bottom
* of the window (either through a user-initiated scroll, or by calling * of the window (either through a user-initiated scroll, or by calling
* scrollToBottom). If stickyBottom is enabled, the scrollState will enter * scrollToBottom). If stickyBottom is enabled, the scrollState will enter
* 'atBottom' state - ensuring that new additions cause the window to scroll * 'stuckAtBottom' state - ensuring that new additions cause the window to
* down further. If stickyBottom is disabled, we just save the scroll offset as * scroll down further. If stickyBottom is disabled, we just save the scroll
* normal. * offset as normal.
*/ */
module.exports = React.createClass({ module.exports = React.createClass({
displayName: 'ScrollPanel', displayName: 'ScrollPanel',
@ -178,6 +179,10 @@ module.exports = React.createClass({
}, },
// return true if the content is fully scrolled down right now; else false. // 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() { isAtBottom: function() {
var sn = this._getScrollNode(); var sn = this._getScrollNode();
@ -268,8 +273,19 @@ module.exports = React.createClass({
}).done(); }).done();
}, },
// get the current scroll position of the room, so that it can be /* get the current scroll state. This returns an object with the following
// restored later * 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() { getScrollState: function() {
return this.scrollState; return this.scrollState;
}, },
@ -287,7 +303,7 @@ module.exports = React.createClass({
* child list.) * child list.)
*/ */
resetScrollState: function() { resetScrollState: function() {
this.scrollState = {atBottom: true}; this.scrollState = {stuckAtBottom: true};
}, },
scrollToBottom: function() { scrollToBottom: function() {
@ -322,8 +338,8 @@ module.exports = React.createClass({
// timeline and need to do more pagination); we want to save the // timeline and need to do more pagination); we want to save the
// *desired* scroll state rather than what we end up achieving. // *desired* scroll state rather than what we end up achieving.
this.scrollState = { this.scrollState = {
atBottom: false, stuckAtBottom: false,
lastDisplayedScrollToken: scrollToken, trackedScrollToken: scrollToken,
pixelOffset: pixelOffset pixelOffset: pixelOffset
}; };
@ -370,7 +386,7 @@ module.exports = React.createClass({
_saveScrollState: function() { _saveScrollState: function() {
if (this.props.stickyBottom && this.isAtBottom()) { if (this.props.stickyBottom && this.isAtBottom()) {
this.scrollState = { atBottom: true }; this.scrollState = { stuckAtBottom: true };
debuglog("Saved scroll state", this.scrollState); debuglog("Saved scroll state", this.scrollState);
return; return;
} }
@ -386,8 +402,8 @@ module.exports = React.createClass({
var boundingRect = node.getBoundingClientRect(); var boundingRect = node.getBoundingClientRect();
if (boundingRect.bottom < wrapperRect.bottom) { if (boundingRect.bottom < wrapperRect.bottom) {
this.scrollState = { this.scrollState = {
atBottom: false, stuckAtBottom: false,
lastDisplayedScrollToken: node.dataset.scrollToken, trackedScrollToken: node.dataset.scrollToken,
pixelOffset: wrapperRect.bottom - boundingRect.bottom, pixelOffset: wrapperRect.bottom - boundingRect.bottom,
} }
debuglog("Saved scroll state", this.scrollState); debuglog("Saved scroll state", this.scrollState);
@ -402,11 +418,11 @@ module.exports = React.createClass({
var scrollState = this.scrollState; var scrollState = this.scrollState;
var scrollNode = this._getScrollNode(); var scrollNode = this._getScrollNode();
if (scrollState.atBottom) { if (scrollState.stuckAtBottom) {
scrollNode.scrollTop = scrollNode.scrollHeight; scrollNode.scrollTop = scrollNode.scrollHeight;
debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop); debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop);
} else if (scrollState.lastDisplayedScrollToken) { } else if (scrollState.trackedScrollToken) {
this._scrollToToken(scrollState.lastDisplayedScrollToken, this._scrollToToken(scrollState.trackedScrollToken,
scrollState.pixelOffset); scrollState.pixelOffset);
} }
this._lastSetScroll = scrollNode.scrollTop; this._lastSetScroll = scrollNode.scrollTop;

View file

@ -100,7 +100,7 @@ module.exports = React.createClass({
onHighlightClick: React.PropTypes.func, onHighlightClick: React.PropTypes.func,
/* is this the focussed event */ /* is this the focussed event */
selectedEvent: React.PropTypes.bool, isSelectedEvent: React.PropTypes.bool,
}, },
getInitialState: function() { getInitialState: function() {
@ -276,7 +276,7 @@ module.exports = React.createClass({
) !== -1, ) !== -1,
mx_EventTile_notSent: this.props.mxEvent.status == 'not_sent', mx_EventTile_notSent: this.props.mxEvent.status == 'not_sent',
mx_EventTile_highlight: this.shouldHighlight(), 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_continuation: this.props.continuation,
mx_EventTile_last: this.props.last, mx_EventTile_last: this.props.last,
mx_EventTile_contextual: this.props.contextual, mx_EventTile_contextual: this.props.contextual,