diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 318489d9fd..fe9643a7fc 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -43,6 +43,13 @@ var INITIAL_SIZE = 20; 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({ displayName: 'RoomView', propTypes: { @@ -330,8 +337,6 @@ module.exports = React.createClass({ this.scrollToBottom(); this.sendReadReceipt(); - - this.refs.messagePanel.checkFillState(); }, componentDidUpdate: function() { @@ -346,53 +351,49 @@ module.exports = React.createClass({ }, _paginateCompleted: function() { - if (DEBUG_SCROLL) console.log("paginate complete"); + debuglog("paginate complete"); this.setState({ room: MatrixClientPeg.get().getRoom(this.props.roomId) }); 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. - - if (this.refs.messagePanel) { - this.refs.messagePanel.checkFillState(); - } }, onSearchResultsFillRequest: function(backwards) { - if (!backwards || this.state.searchInProgress) - return; + if (!backwards) + return q(false); - if (this.nextSearchBatch) { - if (DEBUG_SCROLL) console.log("requesting more search results"); - this._getSearchBatch(this.state.searchTerm, - this.state.searchScope); + if (this.state.searchResults.next_batch) { + debuglog("requesting more search results"); + var searchPromise = MatrixClientPeg.get().backPaginateRoomEventsSearch( + this.state.searchResults); + return this._handleSearchResult(searchPromise); } else { - if (DEBUG_SCROLL) console.log("no more search results"); + debuglog("no more search results"); + return q(false); } }, // set off a pagination request. onMessageListFillRequest: function(backwards) { - if (!backwards || this.state.paginating) - return; + if (!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); - if (DEBUG_SCROLL) console.log("winding back message cap to", cap); + 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; - if (DEBUG_SCROLL) console.log("starting paginate to cap", cap); + debuglog("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); } }, @@ -486,10 +487,8 @@ module.exports = React.createClass({ this.setState({ searchTerm: term, searchScope: scope, - searchResults: [], + searchResults: {}, searchHighlights: [], - searchCount: null, - searchCanPaginate: null, }); // if we already have a search panel, we need to tell it to forget @@ -498,64 +497,68 @@ module.exports = React.createClass({ this.refs.searchResultsPanel.resetScrollState(); } - this.nextSearchBatch = null; - this._getSearchBatch(term, scope); + // make sure that we don't end up showing results from + // an aborted search by keeping a unique id. + // + // todo: should cancel any previous search requests. + this.searchId = new Date().getTime(); + + var filter; + if (scope === "Room") { + filter = { + // XXX: it's unintuitive that the filter for searching doesn't have the same shape as the v2 filter API :( + rooms: [ + this.props.roomId + ] + }; + } + + debuglog("sending search request"); + + var searchPromise = MatrixClientPeg.get().searchRoomEvents({ + filter: filter, + term: term, + }); + this._handleSearchResult(searchPromise).done(); }, - // fire off a request for a batch of search results - _getSearchBatch: function(term, scope) { + _handleSearchResult: function(searchPromise) { + var self = this; + + // keep a record of the current search id, so that if the search terms + // change before we get a response, we can ignore the results. + var localSearchId = this.searchId; + this.setState({ searchInProgress: true, }); - // make sure that we don't end up merging results from - // different searches by keeping a unique id. - // - // todo: should cancel any previous search requests. - var searchId = this.searchId = new Date().getTime(); - - var self = this; - - if (DEBUG_SCROLL) console.log("sending search request"); - 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) { + return searchPromise.then(function(results) { + debuglog("search complete"); + if (!self.state.searching || self.searchId != localSearchId) { console.error("Discarding stale search results"); return; } - var results = data.search_categories.room_events; + // postgres on synapse returns us precise details of the strings + // which actually got matched for highlighting. + // + // In either case, we want to highlight the literal search term + // whether it was used by the search engine or not. - // postgres on synapse returns us precise details of the - // strings which actually got matched for highlighting. - - // combine the highlight list with our existing list; build an object - // to avoid O(N^2) fail - var highlights = {}; - results.highlights.forEach(function(hl) { highlights[hl] = 1; }); - self.state.searchHighlights.forEach(function(hl) { highlights[hl] = 1; }); - - // turn it back into an ordered list. For overlapping highlights, - // favour longer (more specific) terms first - highlights = Object.keys(highlights).sort(function(a, b) { b.length - a.length }); - - // sqlite doesn't give us any highlights, so just try to highlight the literal search term - if (highlights.length == 0) { - highlights = [ term ]; + var highlights = results.highlights; + if (highlights.indexOf(self.state.searchTerm) < 0) { + highlights = highlights.concat(self.state.searchTerm); } - // append the new results to our existing results - var events = self.state.searchResults.concat(results.results); + // For overlapping highlights, + // favour longer (more specific) terms first + highlights = highlights.sort(function(a, b) { b.length - a.length }); self.setState({ searchHighlights: highlights, - searchResults: events, - searchCount: results.count, - searchCanPaginate: !!(results.next_batch), + searchResults: results, }); - self.nextSearchBatch = results.next_batch; }, function(error) { var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); Modal.createDialog(ErrorDialog, { @@ -566,51 +569,27 @@ module.exports = React.createClass({ self.setState({ searchInProgress: false }); - }).done(); + }); }, - _getSearchCondition: function(term, scope) { - var filter; - - if (scope === "Room") { - filter = { - // XXX: it's unintuitive that the filter for searching doesn't have the same shape as the v2 filter API :( - rooms: [ - this.props.roomId - ] - }; - } - - return { - search_categories: { - room_events: { - search_term: term, - filter: filter, - order_by: "recent", - event_context: { - before_limit: 1, - after_limit: 1, - include_profile: true, - } - } - } - } - }, getSearchResultTiles: function() { var DateSeparator = sdk.getComponent('messages.DateSeparator'); - var cli = MatrixClientPeg.get(); - - var ret = []; - var EventTile = sdk.getComponent('rooms.EventTile'); + var cli = MatrixClientPeg.get(); // XXX: todo: merge overlapping results somehow? // XXX: why doesn't searching on name work? + if (this.state.searchResults.results === undefined) { + // awaiting results + return []; + } - if (this.state.searchCanPaginate === false) { - if (this.state.searchResults.length == 0) { + var ret = []; + + if (!this.state.searchResults.next_batch) { + if (this.state.searchResults.results.length == 0) { ret.push(

  • No results

    • @@ -625,9 +604,10 @@ module.exports = React.createClass({ var lastRoomId; - for (var i = this.state.searchResults.length - 1; i >= 0; i--) { - var result = this.state.searchResults[i]; - var mxEv = new Matrix.MatrixEvent(result.result); + for (var i = this.state.searchResults.results.length - 1; i >= 0; i--) { + var result = this.state.searchResults.results[i]; + + var mxEv = result.context.getEvent(); if (!EventTile.haveTileForEvent(mxEv)) { // XXX: can this ever happen? It will make the result count @@ -638,29 +618,28 @@ module.exports = React.createClass({ var eventId = mxEv.getId(); if (this.state.searchScope === 'All') { - var roomId = result.result.room_id; + var roomId = mxEv.getRoomId(); if(roomId != lastRoomId) { ret.push(

  • Room: { cli.getRoom(roomId).name }

    • ); lastRoomId = roomId; } } - var ts1 = result.result.origin_server_ts; + var ts1 = mxEv.getTs(); ret.push(
    • ); // Rank: {resultList[i].rank} - if (result.context.events_before[0]) { - var mxEv2 = new Matrix.MatrixEvent(result.context.events_before[0]); - if (EventTile.haveTileForEvent(mxEv2)) { - ret.push(
    • ); + var timeline = result.context.getTimeline(); + for (var j = 0; j < timeline.length; j++) { + var ev = timeline[j]; + var highlights; + var contextual = (j != result.context.getOurEventIndex()); + if (!contextual) { + highlights = this.state.searchHighlights; } - } - - ret.push(
    • ); - - if (result.context.events_after[0]) { - var mxEv2 = new Matrix.MatrixEvent(result.context.events_after[0]); - if (EventTile.haveTileForEvent(mxEv2)) { - ret.push(
    • ); + if (EventTile.haveTileForEvent(ev)) { + ret.push(
  • + +
    • ); } } } @@ -1296,7 +1275,7 @@ module.exports = React.createClass({ searchInfo = { searchTerm : this.state.searchTerm, searchScope : this.state.searchScope, - searchCount : this.state.searchCount, + searchCount : this.state.searchResults.count, }; } diff --git a/src/components/structures/ScrollPanel.js b/src/components/structures/ScrollPanel.js index 4a45bf5a39..052424529a 100644 --- a/src/components/structures/ScrollPanel.js +++ b/src/components/structures/ScrollPanel.js @@ -17,9 +17,17 @@ 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; +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. * * It wraps a list of
  • children; when items are added to the start or end @@ -51,7 +59,16 @@ 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 a promise; no more calls will be made until the + * promise completes. + * + * The promise should resolve to true if there is more data to be + * retrieved in this direction (in which case onFillRequest may be + * 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, @@ -71,25 +88,33 @@ module.exports = React.createClass({ getDefaultProps: function() { return { stickyBottom: true, - onFillRequest: function(backwards) {}, + onFillRequest: function(backwards) { return q(false); }, onScroll: function() {}, }; }, 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) { 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 // ignored. (Or rather, it is successfully written, but on the next @@ -113,7 +138,7 @@ module.exports = React.createClass({ } this.scrollState = this._calculateScrollState(); - if (DEBUG_SCROLL) console.log("Saved scroll state", this.scrollState); + debuglog("Saved scroll state", this.scrollState); this.props.onScroll(ev); @@ -135,11 +160,74 @@ module.exports = React.createClass({ checkFillState: function() { 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) { - // there's less than a screenful of messages left - try to get some - // more messages. - this.props.onFillRequest(true); + // need to back-fill + this._maybeFill(true); } + if (sn.scrollTop > sn.scrollHeight - sn.clientHeight * 2) { + // need to forward-fill + this._maybeFill(false); + } + }, + + // 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]) { + debuglog("ScrollPanel: Already a "+dir+" fill in progress - not starting another"); + return; + } + + debuglog("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 fillPromise; + try { + fillPromise = this.props.onFillRequest(backwards); + } catch (e) { + this._pendingFillRequests[dir] = false; + throw e; + } + + q.finally(fillPromise, () => { + debuglog("ScrollPanel: "+dir+" fill complete"); + this._pendingFillRequests[dir] = false; + }).then((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 + // was insufficient. + this.checkFillState(); + } + }).done(); }, // get the current scroll position of the room, so that it can be @@ -163,13 +251,13 @@ module.exports = React.createClass({ scrollToTop: function() { this._getScrollNode().scrollTop = 0; - if (DEBUG_SCROLL) console.log("Scrolled to top"); + debuglog("Scrolled to top"); }, scrollToBottom: function() { var scrollNode = this._getScrollNode(); 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 @@ -206,10 +294,10 @@ module.exports = React.createClass({ this.recentEventScroll = scrollNode.scrollTop; } - if (DEBUG_SCROLL) { - console.log("Scrolled to token", node.dataset.scrollToken, "+", pixelOffset+":", scrollNode.scrollTop, "(delta: "+scrollDelta+")"); - console.log("recentEventScroll now "+this.recentEventScroll); - } + debuglog("Scrolled to token", node.dataset.scrollToken, "+", + pixelOffset+":", scrollNode.scrollTop, + "(delta: "+scrollDelta+")"); + debuglog("recentEventScroll now "+this.recentEventScroll); }, _calculateScrollState: function() { diff --git a/src/components/views/rooms/MemberList.js b/src/components/views/rooms/MemberList.js index 8ca24f8992..719e4af9b3 100644 --- a/src/components/views/rooms/MemberList.js +++ b/src/components/views/rooms/MemberList.js @@ -254,7 +254,7 @@ module.exports = React.createClass({ } else { return (
    - +
    ); } diff --git a/src/components/views/rooms/RoomHeader.js b/src/components/views/rooms/RoomHeader.js index 13959a16b9..513b4bf192 100644 --- a/src/components/views/rooms/RoomHeader.js +++ b/src/components/views/rooms/RoomHeader.js @@ -109,8 +109,8 @@ module.exports = React.createClass({ var searchStatus; // don't display the search count until the search completes and - // gives us a non-null searchCount. - if (this.props.searchInfo && this.props.searchInfo.searchCount !== null) { + // gives us a valid (possibly zero) searchCount. + if (this.props.searchInfo && this.props.searchInfo.searchCount !== undefined && this.props.searchInfo.searchCount !== null) { searchStatus =
     (~{ this.props.searchInfo.searchCount } results)
    ; }