From a11516a984c222f7e307b1695cbf82cb92a248d3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 17:33:28 +0100 Subject: [PATCH 1/8] Make publicrooms use the new paginating API Also do filtering on the server WIP: This breaks the network dropdown --- src/components/structures/RoomDirectory.js | 110 +++++++++++------- .../vector-web/structures/RoomDirectory.css | 5 + 2 files changed, 72 insertions(+), 43 deletions(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 3d82499687..7a54f6b54a 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -29,6 +29,7 @@ var linkify = require('linkifyjs'); var linkifyString = require('linkifyjs/string'); var linkifyMatrix = require('matrix-react-sdk/lib/linkify-matrix'); var sanitizeHtml = require('sanitize-html'); +var q = require('q'); linkifyMatrix(linkify); @@ -50,7 +51,6 @@ module.exports = React.createClass({ getInitialState: function() { return { publicRooms: [], - roomAlias: '', loading: true, filterByNetwork: null, } @@ -64,6 +64,8 @@ module.exports = React.createClass({ this.networkPatterns[network] = new RegExp(this.props.config.networkPatterns[network]); } } + this.nextBatch = null; + this.filterString = null; // dis.dispatch({ // action: 'ui_opacity', @@ -73,7 +75,7 @@ module.exports = React.createClass({ }, componentDidMount: function() { - this.getPublicRooms(); + this.refreshRoomList(); }, componentWillUnmount: function() { @@ -84,24 +86,34 @@ module.exports = React.createClass({ // }); }, - getPublicRooms: function() { - var self = this; - MatrixClientPeg.get().publicRooms(function (err, data) { - if (err) { - self.setState({ loading: false }); - console.error("Failed to get publicRooms: %s", JSON.stringify(err)); - var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - Modal.createDialog(ErrorDialog, { - title: "Failed to get public room list", - description: err.message - }); - } - else { - self.setState({ - publicRooms: data.chunk, - loading: false, - }); - } + refreshRoomList: function() { + this.nextBatch = null; + this.setState({ + publicRooms: [], + }); + this.getMoreRooms().done(); + }, + + getMoreRooms: function() { + const opts = {limit: 20}; + if (this.nextBatch) opts.since = this.nextBatch; + if (this.filterString) opts.filter = { generic_search_term: this.filterString } ; + return MatrixClientPeg.get().publicRooms(opts).then((data) => { + this.nextBatch = data.next_batch; + this.setState((s) => { + s.publicRooms.push(...data.chunk); + s.loading = false; + return s; + }); + return Boolean(data.next_batch); + }, (err) => { + this.setState({ loading: false }); + console.error("Failed to get publicRooms: %s", JSON.stringify(err)); + var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); + Modal.createDialog(ErrorDialog, { + title: "Failed to get public room list", + description: err.message + }); }); }, @@ -142,10 +154,10 @@ module.exports = React.createClass({ return MatrixClientPeg.get().deleteAlias(alias); }).done(() => { modal.close(); - this.getPublicRooms(); + this.refreshRoomList(); }, function(err) { modal.close(); - this.getPublicRooms(); + this.refreshRoomList(); Modal.createDialog(ErrorDialog, { title: "Failed to "+step, description: err.toString() @@ -170,6 +182,23 @@ module.exports = React.createClass({ }); }, + onFillRequest: function(backwards) { + if (backwards || !this.nextBatch) return q(false); + + return this.getMoreRooms(); + }, + + onFilterChange: function(ev) { + const alias = ev.target.value; + + if (ev.key == "Enter") { + this.showRoomAlias(alias); + } else { + this.filterString = alias || null; + this.refreshRoomList(); + } + }, + showRoomAlias: function(alias) { this.showRoom(null, alias); }, @@ -214,23 +243,17 @@ module.exports = React.createClass({ dis.dispatch(payload); }, - getRows: function(filter) { + getRows: function() { var BaseAvatar = sdk.getComponent('avatars.BaseAvatar'); if (!this.state.publicRooms) return []; var rooms = this.state.publicRooms.filter((a) => { - // FIXME: if incrementally typing, keep narrowing down the search set - // incrementally rather than starting over each time. if (this.state.filterByNetwork) { if (!this._isRoomInNetwork(a, this.state.filterByNetwork)) return false; } - return (((a.name && a.name.toLowerCase().search(filter.toLowerCase()) >= 0) || - (a.aliases && a.aliases[0].toLowerCase().search(filter.toLowerCase()) >= 0)) && - a.num_joined_members > 0); - }).sort(function(a,b) { - return a.num_joined_members - b.num_joined_members; + return true; }); var rows = []; var self = this; @@ -259,7 +282,7 @@ module.exports = React.createClass({ var topic = rooms[i].topic || ''; topic = linkifyString(sanitizeHtml(topic)); - rows.unshift( + rows.push(
- +
- + - { this.getRows(this.state.roomAlias) } + { this.getRows() }
- +
); diff --git a/src/skins/vector/css/vector-web/structures/RoomDirectory.css b/src/skins/vector/css/vector-web/structures/RoomDirectory.css index b4a2394593..17954e1c98 100644 --- a/src/skins/vector/css/vector-web/structures/RoomDirectory.css +++ b/src/skins/vector/css/vector-web/structures/RoomDirectory.css @@ -46,6 +46,11 @@ limitations under the License. -webkit-flex-direction: column; } +.mx_RoomDirectory_list .mx_RoomView_messageListWrapper { + justify-content: flex-start; + -webkit-justify-content: flex-start; +} + .mx_RoomDirectory_listheader { display: table; width: 100%; From 2b6fbb038adaa573de9be8193ae4e4914f6ad299 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 18:41:29 +0100 Subject: [PATCH 2/8] Show headers while loading & show spinner whilst waiting for filter requests --- src/components/structures/RoomDirectory.js | 42 +++++++++++----------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 7a54f6b54a..50a247ed24 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -98,6 +98,7 @@ module.exports = React.createClass({ const opts = {limit: 20}; if (this.nextBatch) opts.since = this.nextBatch; if (this.filterString) opts.filter = { generic_search_term: this.filterString } ; + this.setState({loading: true}); return MatrixClientPeg.get().publicRooms(opts).then((data) => { this.nextBatch = data.next_batch; this.setState((s) => { @@ -327,18 +328,31 @@ module.exports = React.createClass({ }, render: function() { + let content; if (this.state.loading) { - var Loader = sdk.getComponent("elements.Spinner"); - return ( -
- -
- ); + const Loader = sdk.getComponent("elements.Spinner"); + content =
+ +
; + } else { + const ScrollPanel = sdk.getComponent("structures.ScrollPanel"); + content = + + + { this.getRows() } + +
+ ; } const SimpleRoomHeader = sdk.getComponent('rooms.SimpleRoomHeader'); const NetworkDropdown = sdk.getComponent('directory.NetworkDropdown'); - const ScrollPanel = sdk.getComponent("structures.ScrollPanel"); return (
@@ -349,19 +363,7 @@ module.exports = React.createClass({ />
- - - - { this.getRows() } - -
- + {content} ); From 6d332256b53582d288df269c7b5c77d192225fae Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 18:48:47 +0100 Subject: [PATCH 3/8] Ignore results of old requests --- src/components/structures/RoomDirectory.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 50a247ed24..9c65c6f16d 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -95,11 +95,19 @@ module.exports = React.createClass({ }, getMoreRooms: function() { + const my_filter_string = this.filterString; const opts = {limit: 20}; if (this.nextBatch) opts.since = this.nextBatch; - if (this.filterString) opts.filter = { generic_search_term: this.filterString } ; + if (this.filterString) opts.filter = { generic_search_term: my_filter_string } ; this.setState({loading: true}); return MatrixClientPeg.get().publicRooms(opts).then((data) => { + if (my_filter_string != this.filterString) { + // if the filter has changed since this request was sent, + // throw away the result (don't even clear the busy flag + // since we must still have a request in flight) + return; + } + this.nextBatch = data.next_batch; this.setState((s) => { s.publicRooms.push(...data.chunk); @@ -108,6 +116,11 @@ module.exports = React.createClass({ }); return Boolean(data.next_batch); }, (err) => { + if (my_filter_string != this.filterString) { + // as above: we don't care about errors for old + // requests either + return; + } this.setState({ loading: false }); console.error("Failed to get publicRooms: %s", JSON.stringify(err)); var ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); From 2fdec51a5bcd291589ad03193cb23d8551bf7f96 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 18:53:18 +0100 Subject: [PATCH 4/8] Wait a bit before sending filter requests avoid hammering the server with requests for each keystroke --- src/components/structures/RoomDirectory.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 9c65c6f16d..2675f22ed6 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -66,6 +66,7 @@ module.exports = React.createClass({ } this.nextBatch = null; this.filterString = null; + this.filterTimeout = null; // dis.dispatch({ // action: 'ui_opacity', @@ -209,7 +210,18 @@ module.exports = React.createClass({ this.showRoomAlias(alias); } else { this.filterString = alias || null; - this.refreshRoomList(); + + // don't send the request for a little bit, + // no point hammering the server with a + // request for every keystroke, let the + // user finish typing. + if (this.filterTimeout) { + clearTimeout(this.filterTimeout); + } + this.filterTimeout = setTimeout(() => { + this.filterTimeout = null; + this.refreshRoomList(); + }, 300); } }, From 50f05db29e10d335f0d40bb9402bf4d60cb20430 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 19:29:56 +0100 Subject: [PATCH 5/8] Don't show loading spinner if just paginating --- src/components/structures/RoomDirectory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 2675f22ed6..9f3ee04bbe 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -91,6 +91,7 @@ module.exports = React.createClass({ this.nextBatch = null; this.setState({ publicRooms: [], + loading: true, }); this.getMoreRooms().done(); }, @@ -100,7 +101,6 @@ module.exports = React.createClass({ const opts = {limit: 20}; if (this.nextBatch) opts.since = this.nextBatch; if (this.filterString) opts.filter = { generic_search_term: my_filter_string } ; - this.setState({loading: true}); return MatrixClientPeg.get().publicRooms(opts).then((data) => { if (my_filter_string != this.filterString) { // if the filter has changed since this request was sent, From 3d97061d784765b26a10ae2cae9407be32257d57 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 19:59:06 +0100 Subject: [PATCH 6/8] Check if we need to fetch more rooms after filter --- src/components/structures/RoomDirectory.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 9f3ee04bbe..716d258405 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -67,6 +67,7 @@ module.exports = React.createClass({ this.nextBatch = null; this.filterString = null; this.filterTimeout = null; + this.scrollPanel = null; // dis.dispatch({ // action: 'ui_opacity', @@ -194,6 +195,14 @@ module.exports = React.createClass({ onNetworkChange: function(network) { this.setState({ filterByNetwork: network, + }, () => { + // we just filtered out a bunch of rooms, so check to see if + // we need to fill up the scrollpanel again + // NB. Because we filter the results, the HS can keep giving + // us more rooms and we'll keep requesting more if none match + // the filter, which is pretty terrible. We need a way + // to filter by network on the server. + if (this.scrollPanel) this.scrollPanel.checkFillState(); }); }, @@ -338,6 +347,10 @@ module.exports = React.createClass({ return rows; }, + collectScrollPanel: function(element) { + this.scrollPanel = element; + }, + /** * Terrible temporary function that guess what network a public room * entry is in, until synapse is able to tell us @@ -361,7 +374,7 @@ module.exports = React.createClass({ ; } else { const ScrollPanel = sdk.getComponent("structures.ScrollPanel"); - content = Date: Fri, 16 Sep 2016 20:49:28 +0100 Subject: [PATCH 7/8] Update test now that /publicRooms is a post --- test/app-tests/joining.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/app-tests/joining.js b/test/app-tests/joining.js index 22b018d1d7..a668c2b184 100644 --- a/test/app-tests/joining.js +++ b/test/app-tests/joining.js @@ -76,7 +76,7 @@ describe('joining a room', function () { httpBackend.when('GET', '/pushrules').respond(200, {}); httpBackend.when('POST', '/filter').respond(200, { filter_id: 'fid' }); httpBackend.when('GET', '/sync').respond(200, {}); - httpBackend.when('GET', '/publicRooms').respond(200, {chunk: []}); + httpBackend.when('POST', '/publicRooms').respond(200, {chunk: []}); httpBackend.when('GET', '/directory/room/'+encodeURIComponent(ROOM_ALIAS)).respond(200, { room_id: ROOM_ID }); // start with a logged-in client From 53fd3f52faa6dc7aa9b43ed5254714160aeed3ec Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Sep 2016 20:56:14 +0100 Subject: [PATCH 8/8] Oops, onChange doesn't catch the enter key --- src/components/structures/RoomDirectory.js | 34 ++++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/components/structures/RoomDirectory.js b/src/components/structures/RoomDirectory.js index 716d258405..95e2f71c5b 100644 --- a/src/components/structures/RoomDirectory.js +++ b/src/components/structures/RoomDirectory.js @@ -215,22 +215,24 @@ module.exports = React.createClass({ onFilterChange: function(ev) { const alias = ev.target.value; - if (ev.key == "Enter") { - this.showRoomAlias(alias); - } else { - this.filterString = alias || null; + this.filterString = alias || null; - // don't send the request for a little bit, - // no point hammering the server with a - // request for every keystroke, let the - // user finish typing. - if (this.filterTimeout) { - clearTimeout(this.filterTimeout); - } - this.filterTimeout = setTimeout(() => { - this.filterTimeout = null; - this.refreshRoomList(); - }, 300); + // don't send the request for a little bit, + // no point hammering the server with a + // request for every keystroke, let the + // user finish typing. + if (this.filterTimeout) { + clearTimeout(this.filterTimeout); + } + this.filterTimeout = setTimeout(() => { + this.filterTimeout = null; + this.refreshRoomList(); + }, 300); + }, + + onFilterKeyUp: function(ev) { + if (ev.key == "Enter") { + this.showRoomAlias(ev.target.value); } }, @@ -397,7 +399,7 @@ module.exports = React.createClass({