From 5c5307c6650b7fb51b031c7a9f05b49748a6b096 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Wed, 3 Jan 2018 17:12:31 +0000 Subject: [PATCH 1/3] Improve performance of tag panel selection (when tags are selected) Deselecting all tags is now slightly less performant than selecting a tag but mostly due to the number of RoomTiles being rendered. Swapping between different tags (a supposed common use-case) feels much more spritely! --- src/components/views/rooms/RoomList.js | 31 +++++++++++++++++--------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index f14ba5529d..9fc3a5f231 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -311,11 +311,6 @@ module.exports = React.createClass({ }); }, - isRoomInSelectedTags: function(room) { - // No selected tags = every room is visible in the list - return this.state.selectedTags.length === 0 || this._visibleRooms.includes(room.roomId); - }, - refreshRoomList: function() { // TODO: ideally we'd calculate this once at start, and then maintain // any changes to it incrementally, updating the appropriate sublists @@ -344,7 +339,26 @@ module.exports = React.createClass({ lists["im.vector.fake.archived"] = []; const dmRoomMap = DMRoomMap.shared(); - MatrixClientPeg.get().getRooms().forEach((room) => { + + // If there are any tags selected, constrain the rooms listed to the + // visible rooms as determined by this._visibleRooms. Here, we + // de-duplicate and filter out rooms that the client doesn't know + // about (hence the Set and the null-guard on `room`). + let rooms = []; + if (this.state.selectedTags.length > 0) { + const roomSet = new Set(); + this._visibleRooms.forEach((roomId) => { + const room = MatrixClientPeg.get().getRoom(roomId); + if (room) { + roomSet.add(room); + } + }); + rooms = Array.from(roomSet); + } else { + rooms = MatrixClientPeg.get().getRooms(); + } + + rooms.forEach((room, index) => { const me = room.getMember(MatrixClientPeg.get().credentials.userId); if (!me) return; @@ -362,11 +376,6 @@ module.exports = React.createClass({ // Used to split rooms via tags const tagNames = Object.keys(room.tags); - // Apply TagPanel filtering, derived from FilterStore - if (!this.isRoomInSelectedTags(room)) { - return; - } - if (tagNames.length) { for (let i = 0; i < tagNames.length; i++) { const tagName = tagNames[i]; From cdd1a57569a03f0dfbecf90466e5a7500273f307 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Thu, 4 Jan 2018 11:50:33 +0000 Subject: [PATCH 2/3] Calculate visible rooms when tags change instead of every time we getRoomLists --- src/components/views/rooms/RoomList.js | 53 ++++++++++++++------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index 9fc3a5f231..c32811e57f 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -93,7 +93,8 @@ module.exports = React.createClass({ // $groupId: [$roomId1, $roomId2, ...], }; // All rooms that should be kept in the room list when filtering - this._visibleRooms = []; + // By default, set to `null` meaning "all rooms visible" + this._visibleRooms = null; // When the selected tags are changed, initialise a group store if necessary this._filterStoreToken = FilterStore.addListener(() => { FilterStore.getSelectedTags().forEach((tag) => { @@ -297,15 +298,35 @@ module.exports = React.createClass({ // Update which rooms and users should appear according to which tags are selected updateVisibleRooms: function() { - this._visibleRooms = []; - FilterStore.getSelectedTags().forEach((tag) => { - (this._visibleRoomsForGroup[tag] || []).forEach( - (roomId) => this._visibleRooms.push(roomId), + const selectedTags = FilterStore.getSelectedTags(); + + let visibleGroupRooms = []; + selectedTags.forEach((tag) => { + visibleGroupRooms = visibleGroupRooms.concat( + this._visibleRoomsForGroup[tag] || [], ); }); + // If there are any tags selected, constrain the rooms listed to the + // visible rooms as determined by visibleGroupRooms. Here, we + // de-duplicate and filter out rooms that the client doesn't know + // about (hence the Set and the null-guard on `room`). + if (selectedTags.length > 0) { + const roomSet = new Set(); + visibleGroupRooms.forEach((roomId) => { + const room = MatrixClientPeg.get().getRoom(roomId); + if (room) { + roomSet.add(room); + } + }); + this._visibleRooms = Array.from(roomSet); + } else { + // Show all rooms + this._visibleRooms = null; + } + this.setState({ - selectedTags: FilterStore.getSelectedTags(), + selectedTags, }, () => { this.refreshRoomList(); }); @@ -340,25 +361,7 @@ module.exports = React.createClass({ const dmRoomMap = DMRoomMap.shared(); - // If there are any tags selected, constrain the rooms listed to the - // visible rooms as determined by this._visibleRooms. Here, we - // de-duplicate and filter out rooms that the client doesn't know - // about (hence the Set and the null-guard on `room`). - let rooms = []; - if (this.state.selectedTags.length > 0) { - const roomSet = new Set(); - this._visibleRooms.forEach((roomId) => { - const room = MatrixClientPeg.get().getRoom(roomId); - if (room) { - roomSet.add(room); - } - }); - rooms = Array.from(roomSet); - } else { - rooms = MatrixClientPeg.get().getRooms(); - } - - rooms.forEach((room, index) => { + (this._visibleRooms || MatrixClientPeg.get().getRooms()).forEach((room, index) => { const me = room.getMember(MatrixClientPeg.get().credentials.userId); if (!me) return; From 8ade357349443334c265e0fcc016c0d4a10a63a9 Mon Sep 17 00:00:00 2001 From: lukebarnard Date: Fri, 5 Jan 2018 12:33:26 +0000 Subject: [PATCH 3/3] Handle newly added rooms and removed rooms. --- src/components/views/rooms/RoomList.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/views/rooms/RoomList.js b/src/components/views/rooms/RoomList.js index e21f539d3f..1c548b3f72 100644 --- a/src/components/views/rooms/RoomList.js +++ b/src/components/views/rooms/RoomList.js @@ -92,9 +92,9 @@ module.exports = React.createClass({ this._visibleRoomsForGroup = { // $groupId: [$roomId1, $roomId2, ...], }; - // All rooms that should be kept in the room list when filtering - // By default, set to `null` meaning "all rooms visible" - this._visibleRooms = null; + // All rooms that should be kept in the room list when filtering. + // By default, show all rooms. + this._visibleRooms = MatrixClientPeg.get().getRooms(); // When the selected tags are changed, initialise a group store if necessary this._tagStoreToken = TagOrderStore.addListener(() => { TagOrderStore.getSelectedTags().forEach((tag) => { @@ -197,11 +197,11 @@ module.exports = React.createClass({ }, onRoom: function(room) { - this._delayedRefreshRoomList(); + this.updateVisibleRooms(); }, onDeleteRoom: function(roomId) { - this._delayedRefreshRoomList(); + this.updateVisibleRooms(); }, onArchivedHeaderClick: function(isHidden, scrollToPosition) { @@ -321,7 +321,7 @@ module.exports = React.createClass({ this._visibleRooms = Array.from(roomSet); } else { // Show all rooms - this._visibleRooms = null; + this._visibleRooms = MatrixClientPeg.get().getRooms(); } this.setState({ @@ -360,7 +360,7 @@ module.exports = React.createClass({ const dmRoomMap = DMRoomMap.shared(); - (this._visibleRooms || MatrixClientPeg.get().getRooms()).forEach((room, index) => { + this._visibleRooms.forEach((room, index) => { const me = room.getMember(MatrixClientPeg.get().credentials.userId); if (!me) return;