PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Address review comments

Browse source 
Make onFillRequest always return a promise
This commit is contained in:
Richard van der Hoff 2016-01-04 16:28:32 +00:00
parent 89fcf019e1
commit b5eae891b4
2 changed files with 77 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
src/components/structures/RoomView.js
View file

@ -43,6 +43,13 @@ var INITIAL_SIZE = 20;
var DEBUG_SCROLL = false; var DEBUG_SCROLL = false;
if (DEBUG_SCROLL) {
// using bind means that we get to keep useful line numbers in the console
var debuglog = console.log.bind(console);
} else {
var debuglog = function () {};
}
module.exports = React.createClass({ module.exports = React.createClass({
displayName: 'RoomView', displayName: 'RoomView',
propTypes: { propTypes: {
@ -344,7 +351,7 @@ module.exports = React.createClass({
}, },
_paginateCompleted: function() { _paginateCompleted: function() {
if (DEBUG_SCROLL) console.log("paginate complete"); debuglog("paginate complete");
this.setState({ this.setState({
room: MatrixClientPeg.get().getRoom(this.props.roomId) room: MatrixClientPeg.get().getRoom(this.props.roomId)
@ -355,34 +362,34 @@ module.exports = React.createClass({
onSearchResultsFillRequest: function(backwards) { onSearchResultsFillRequest: function(backwards) {
if (!backwards) if (!backwards)
return false; return q(false);
if (this.nextSearchBatch) { if (this.nextSearchBatch) {
if (DEBUG_SCROLL) console.log("requesting more search results"); debuglog("requesting more search results");
return this._getSearchBatch(this.state.searchTerm, return this._getSearchBatch(this.state.searchTerm,
this.state.searchScope).then(true); this.state.searchScope).then(true);
} else { } else {
if (DEBUG_SCROLL) console.log("no more search results"); debuglog("no more search results");
return false; return q(false);
} }
}, },
// set off a pagination request. // set off a pagination request.
onMessageListFillRequest: function(backwards) { onMessageListFillRequest: function(backwards) {
if (!backwards) if (!backwards)
return; return q(false);
// Either wind back the message cap (if there are enough events in the // Either wind back the message cap (if there are enough events in the
// timeline to do so), or fire off a pagination request. // timeline to do so), or fire off a pagination request.
if (this.state.messageCap < this.state.room.timeline.length) { if (this.state.messageCap < this.state.room.timeline.length) {
var cap = Math.min(this.state.messageCap + PAGINATE_SIZE, this.state.room.timeline.length); var cap = Math.min(this.state.messageCap + PAGINATE_SIZE, this.state.room.timeline.length);
if (DEBUG_SCROLL) console.log("winding back message cap to", cap); debuglog("winding back message cap to", cap);
this.setState({messageCap: cap}); this.setState({messageCap: cap});
return true; return q(true);
} else if(this.state.room.oldState.paginationToken) { } else if(this.state.room.oldState.paginationToken) {
var cap = this.state.messageCap + PAGINATE_SIZE; var cap = this.state.messageCap + PAGINATE_SIZE;
if (DEBUG_SCROLL) console.log("starting paginate to cap", cap); debuglog("starting paginate to cap", cap);
this.setState({messageCap: cap, paginating: true}); this.setState({messageCap: cap, paginating: true});
return MatrixClientPeg.get().scrollback(this.state.room, PAGINATE_SIZE). return MatrixClientPeg.get().scrollback(this.state.room, PAGINATE_SIZE).
finally(this._paginateCompleted).then(true); finally(this._paginateCompleted).then(true);
@ -492,7 +499,7 @@ module.exports = React.createClass({
} }
this.nextSearchBatch = null; this.nextSearchBatch = null;
this._getSearchBatch(term, scope); this._getSearchBatch(term, scope).done();
}, },
// fire off a request for a batch of search results // fire off a request for a batch of search results
@ -509,11 +516,11 @@ module.exports = React.createClass({
var self = this; var self = this;
if (DEBUG_SCROLL) console.log("sending search request"); debuglog("sending search request");
return MatrixClientPeg.get().search({ body: this._getSearchCondition(term, scope), return MatrixClientPeg.get().search({ body: this._getSearchCondition(term, scope),
next_batch: this.nextSearchBatch }) next_batch: this.nextSearchBatch })
.then(function(data) { .then(function(data) {
if (DEBUG_SCROLL) console.log("search complete"); debuglog("search complete");
if (!self.state.searching || self.searchId != searchId) { if (!self.state.searching || self.searchId != searchId) {
console.error("Discarding stale search results"); console.error("Discarding stale search results");
return; return;
@ -559,7 +566,7 @@ module.exports = React.createClass({
self.setState({ self.setState({
searchInProgress: false searchInProgress: false
}); });
}).done(); });
}, },
_getSearchCondition: function(term, scope) { _getSearchCondition: function(term, scope) {

88
src/components/structures/ScrollPanel.js
View file

@ -21,6 +21,13 @@ var q = require("q");
var DEBUG_SCROLL = false; var DEBUG_SCROLL = false;
if (DEBUG_SCROLL) {
// using bind means that we get to keep useful line numbers in the console
var debuglog = console.log.bind(console);
} else {
var debuglog = function () {};
}
/* This component implements an intelligent scrolling list. /* This component implements an intelligent scrolling list.
* *
* It wraps a list of <li> children; when items are added to the start or end * It wraps a list of <li> children; when items are added to the start or end
@ -54,13 +61,14 @@ module.exports = React.createClass({
* the user nears the start (backwards = true) or end (backwards = * the user nears the start (backwards = true) or end (backwards =
* false) of the list. * false) of the list.
* *
* This should return true if the pagination was successful, or false if * This should return a promise; no more calls will be made until the
* there is no more data in this directon (at this time) - which will * promise completes.
* stop the pagination cycle until the user scrolls again.
* *
* This can return a promise; if it does, no more calls will be made * The promise should resolve to true if there is more data to be
* until the promise completes. The promise should resolve to true or * retrieved in this direction (in which case onFillRequest may be
* false as above. * called again immediately), or false if there is no more data in this
* directon (at this time) - which will stop the pagination cycle until
* the user scrolls again.
*/ */
onFillRequest: React.PropTypes.func, onFillRequest: React.PropTypes.func,
@ -80,7 +88,7 @@ module.exports = React.createClass({
getDefaultProps: function() { getDefaultProps: function() {
return { return {
stickyBottom: true, stickyBottom: true,
onFillRequest: function(backwards) {}, onFillRequest: function(backwards) { return q(false); },
onScroll: function() {}, onScroll: function() {},
}; };
}, },
@ -106,7 +114,7 @@ module.exports = React.createClass({
onScroll: function(ev) { onScroll: function(ev) {
var sn = this._getScrollNode(); var sn = this._getScrollNode();
if (DEBUG_SCROLL) console.log("Scroll event: offset now:", sn.scrollTop, "recentEventScroll:", this.recentEventScroll); debuglog("Scroll event: offset now:", sn.scrollTop, "recentEventScroll:", this.recentEventScroll);
// Sometimes we see attempts to write to scrollTop essentially being // Sometimes we see attempts to write to scrollTop essentially being
// ignored. (Or rather, it is successfully written, but on the next // ignored. (Or rather, it is successfully written, but on the next
@ -130,7 +138,7 @@ module.exports = React.createClass({
} }
this.scrollState = this._calculateScrollState(); this.scrollState = this._calculateScrollState();
if (DEBUG_SCROLL) console.log("Saved scroll state", this.scrollState); debuglog("Saved scroll state", this.scrollState);
this.props.onScroll(ev); this.props.onScroll(ev);
@ -145,12 +153,36 @@ module.exports = React.createClass({
checkFillState: function() { checkFillState: function() {
var sn = this._getScrollNode(); var sn = this._getScrollNode();
// if there is less than a screenful of messages above or below the
// viewport, try to get some more messages.
//
// scrollTop is the number of pixels between the top of the content and
// the top of the viewport.
//
// scrollHeight is the total height of the content.
//
// clientHeight is the height of the viewport (excluding borders,
// margins, and scrollbars).
//
//
// .---------. - -
// | | | scrollTop |
// .-+---------+-. - - |
// | | | | | |
// | | | | | clientHeight | scrollHeight
// | | | | | |
// `-+---------+-' - |
// | | |
// | | |
// `---------' -
//
if (sn.scrollTop < sn.clientHeight) { if (sn.scrollTop < sn.clientHeight) {
// there's less than a screenful of messages left - try to get some // need to back-fill
// more messages.
this._maybeFill(true); this._maybeFill(true);
} }
if (sn.scrollTop > sn.scrollHeight - sn.clientHeight * 2) { if (sn.scrollTop > sn.scrollHeight - sn.clientHeight * 2) {
// need to forward-fill
this._maybeFill(false); this._maybeFill(false);
} }
}, },
@ -159,36 +191,30 @@ module.exports = React.createClass({
_maybeFill: function(backwards) { _maybeFill: function(backwards) {
var dir = backwards ? 'b' : 'f'; var dir = backwards ? 'b' : 'f';
if (this._pendingFillRequests[dir]) { if (this._pendingFillRequests[dir]) {
if (DEBUG_SCROLL) { debuglog("ScrollPanel: Already a "+dir+" fill in progress - not starting another");
console.log("ScrollPanel: Already a "+dir+" fill in progress - not starting another");
}
return; return;
} }
if (DEBUG_SCROLL) { debuglog("ScrollPanel: starting "+dir+" fill");
console.log("ScrollPanel: starting "+dir+" fill");
}
// onFillRequest can end up calling us recursively (via onScroll // onFillRequest can end up calling us recursively (via onScroll
// events) so make sure we set this before firing off the call. That // events) so make sure we set this before firing off the call. That
// does present the risk that we might not ever actually fire off the // does present the risk that we might not ever actually fire off the
// fill request, so wrap it in a try/catch. // fill request, so wrap it in a try/catch.
this._pendingFillRequests[dir] = true; this._pendingFillRequests[dir] = true;
var r; var fillPromise;
try { try {
r = this.props.onFillRequest(backwards); fillPromise = this.props.onFillRequest(backwards);
} catch (e) { } catch (e) {
this._pendingFillRequests[dir] = false; this._pendingFillRequests[dir] = false;
throw e; throw e;
} }
q.finally(r, () => { q.finally(fillPromise, () => {
if (DEBUG_SCROLL) { debuglog("ScrollPanel: "+dir+" fill complete");
console.log("ScrollPanel: "+dir+" fill complete");
}
this._pendingFillRequests[dir] = false; this._pendingFillRequests[dir] = false;
}).then((res) => { }).then((hasMoreResults) => {
if (res) { if (hasMoreResults) {
// further pagination requests have been disabled until now, so // further pagination requests have been disabled until now, so
// it's time to check the fill state again in case the pagination // it's time to check the fill state again in case the pagination
// was insufficient. // was insufficient.
@ -218,13 +244,13 @@ module.exports = React.createClass({
scrollToTop: function() { scrollToTop: function() {
this._getScrollNode().scrollTop = 0; this._getScrollNode().scrollTop = 0;
if (DEBUG_SCROLL) console.log("Scrolled to top"); debuglog("Scrolled to top");
}, },
scrollToBottom: function() { scrollToBottom: function() {
var scrollNode = this._getScrollNode(); var scrollNode = this._getScrollNode();
scrollNode.scrollTop = scrollNode.scrollHeight; scrollNode.scrollTop = scrollNode.scrollHeight;
if (DEBUG_SCROLL) console.log("Scrolled to bottom; offset now", scrollNode.scrollTop); debuglog("Scrolled to bottom; offset now", scrollNode.scrollTop);
}, },
// scroll the message list to the node with the given scrollToken. See // scroll the message list to the node with the given scrollToken. See
@ -261,10 +287,10 @@ module.exports = React.createClass({
this.recentEventScroll = scrollNode.scrollTop; this.recentEventScroll = scrollNode.scrollTop;
} }
if (DEBUG_SCROLL) { debuglog("Scrolled to token", node.dataset.scrollToken, "+",
console.log("Scrolled to token", node.dataset.scrollToken, "+", pixelOffset+":", scrollNode.scrollTop, "(delta: "+scrollDelta+")"); pixelOffset+":", scrollNode.scrollTop,
console.log("recentEventScroll now "+this.recentEventScroll); "(delta: "+scrollDelta+")");
} debuglog("recentEventScroll now "+this.recentEventScroll);
}, },
_calculateScrollState: function() { _calculateScrollState: function() {