From f2a24521dc4b7f9e1568755c8ea5ed529a41e12a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 24 Dec 2015 00:10:21 +0000 Subject: [PATCH] Make ScrollPanel keep track of when fill requests are happening The dance to avoid doing repeated fill requests on every update is common, so add it to ScrollPanel. Let onFillRequest return a promise, which prevents any updates until it completes. --- src/components/structures/RoomView.js | 27 +++++----- src/components/structures/ScrollPanel.js | 63 +++++++++++++++++++++++- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 127fe2fdb8..fc909f69ab 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -330,8 +330,6 @@ module.exports = React.createClass({ this.scrollToBottom(); this.sendReadReceipt(); - - this.refs.messagePanel.checkFillState(); }, componentDidUpdate: function() { @@ -353,30 +351,25 @@ module.exports = React.createClass({ }); this.setState({paginating: false}); - - // we might not have got enough (or, indeed, any) results from the - // pagination request, so give the messagePanel a chance to set off - // another. - - this.refs.messagePanel.checkFillState(); }, onSearchResultsFillRequest: function(backwards) { - if (!backwards || this.state.searchInProgress) - return; + if (!backwards) + return false; if (this.nextSearchBatch) { if (DEBUG_SCROLL) console.log("requesting more search results"); - this._getSearchBatch(this.state.searchTerm, - this.state.searchScope); + return this._getSearchBatch(this.state.searchTerm, + this.state.searchScope).then(true); } else { if (DEBUG_SCROLL) console.log("no more search results"); + return false; } }, // set off a pagination request. onMessageListFillRequest: function(backwards) { - if (!backwards || this.state.paginating) + if (!backwards) return; // Either wind back the message cap (if there are enough events in the @@ -386,11 +379,13 @@ module.exports = React.createClass({ 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); this.setState({messageCap: cap}); + return true; } else if(this.state.room.oldState.paginationToken) { var cap = this.state.messageCap + PAGINATE_SIZE; if (DEBUG_SCROLL) console.log("starting paginate to cap", cap); this.setState({messageCap: cap, paginating: true}); - MatrixClientPeg.get().scrollback(this.state.room, PAGINATE_SIZE).finally(this._paginateCompleted).done(); + return MatrixClientPeg.get().scrollback(this.state.room, PAGINATE_SIZE). + finally(this._paginateCompleted).then(true); } }, @@ -515,8 +510,8 @@ module.exports = React.createClass({ var self = this; if (DEBUG_SCROLL) console.log("sending search request"); - MatrixClientPeg.get().search({ body: this._getSearchCondition(term, scope), - next_batch: this.nextSearchBatch }) + return MatrixClientPeg.get().search({ body: this._getSearchCondition(term, scope), + next_batch: this.nextSearchBatch }) .then(function(data) { if (DEBUG_SCROLL) console.log("search complete"); if (!self.state.searching || self.searchId != searchId) { diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 2c68562ada..fc0b630f26 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -17,6 +17,7 @@ limitations under the License. var React = require("react"); var ReactDOM = require("react-dom"); var GeminiScrollbar = require('react-gemini-scrollbar'); +var q = require("q"); var DEBUG_SCROLL = false; @@ -51,7 +52,15 @@ module.exports = React.createClass({ /* onFillRequest(backwards): a callback which is called on scroll when * 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 + * there is no more data in this directon (at this time) - which will + * stop the pagination cycle until the user scrolls again. + * + * This can return a promise; if it does, no more calls will be made + * until the promise completes. The promise should resolve to true or + * false as above. */ onFillRequest: React.PropTypes.func, @@ -77,14 +86,22 @@ module.exports = React.createClass({ }, componentWillMount: function() { + this._pendingFillRequests = {b: null, f: null}; this.resetScrollState(); }, + componentDidMount: function() { + this.checkFillState(); + }, + componentDidUpdate: function() { // after adding event tiles, we may need to tweak the scroll (either to // keep at the bottom of the timeline, or to maintain the view after // adding events to the top). this._restoreSavedScrollState(); + + // we also re-check the fill state, in case the paginate was inadequate + this.checkFillState(); }, onScroll: function(ev) { @@ -131,10 +148,52 @@ module.exports = React.createClass({ if (sn.scrollTop < sn.clientHeight) { // there's less than a screenful of messages left - try to get some // more messages. - this.props.onFillRequest(true); + this._maybeFill(true); } }, + // check if there is already a pending fill request. If not, set one off. + _maybeFill: function(backwards) { + var dir = backwards ? 'b' : 'f'; + if (this._pendingFillRequests[dir]) { + if (DEBUG_SCROLL) { + console.log("ScrollPanel: Already a "+dir+" fill in progress - not starting another"); + } + return; + } + + if (DEBUG_SCROLL) { + console.log("ScrollPanel: starting "+dir+" fill"); + } + + // onFillRequest can end up calling us recursively (via onScroll + // 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 + // fill request, so wrap it in a try/catch. + this._pendingFillRequests[dir] = true; + var r; + try { + r = this.props.onFillRequest(backwards); + } catch (e) { + this._pendingFillRequests[dir] = false; + throw e; + } + + q.finally(r, () => { + if (DEBUG_SCROLL) { + console.log("ScrollPanel: "+dir+" fill complete"); + } + this._pendingFillRequests[dir] = false; + }).then((res) => { + if (res) { + // further pagination requests have been disabled until now, so + // it's time to check the fill state again in case the pagination + // was insufficient. + this.checkFillState(); + } + }).done(); + }, + // get the current scroll position of the room, so that it can be // restored later getScrollState: function() {