From 52cdf609540f1883abfec9bb3998f330fa2e5bf0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 26 May 2018 20:27:48 -0600 Subject: [PATCH 01/11] Add options to pin unread/mentioned rooms to the top of the room list Fixes https://github.com/vector-im/riot-web/issues/6604 Fixes https://github.com/vector-im/riot-web/issues/732 Fixes https://github.com/vector-im/riot-web/issues/1104 Signed-off-by: Travis Ralston --- src/components/structures/RoomSubList.js | 21 ++++++++++++++++++++- src/components/structures/UserSettings.js | 2 ++ src/i18n/strings/en_EN.json | 2 ++ src/settings/Settings.js | 10 ++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index fb82ee067b..b3c9d5f1b6 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -17,6 +17,8 @@ limitations under the License. 'use strict'; +import SettingsStore from "../../settings/SettingsStore"; + var React = require('react'); var ReactDOM = require('react-dom'); var classNames = require('classnames'); @@ -98,8 +100,10 @@ var RoomSubList = React.createClass({ componentWillReceiveProps: function(newProps) { // order the room list appropriately before we re-render //if (debug) console.log("received new props, list = " + newProps.list); + const filteredRooms = this.applySearchFilter(newProps.list, newProps.searchFilter); + const sortedRooms = newProps.order === "recent" ? this.applyPinnedTileRules(filteredRooms) : filteredRooms; this.setState({ - sortedList: this.applySearchFilter(newProps.list, newProps.searchFilter), + sortedList: sortedRooms, }); }, @@ -110,6 +114,21 @@ var RoomSubList = React.createClass({ }); }, + applyPinnedTileRules: function(list) { + const pinUnread = SettingsStore.getValue("pinUnreadRooms"); + const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); + if (!pinUnread && !pinMentioned) { + return list; // Nothing to sort + } + + const mentioned = !pinMentioned ? [] : list.filter(room => room.getUnreadNotificationCount("highlight") > 0); + const unread = !pinUnread ? [] : list.filter(room => Unread.doesRoomHaveUnreadMessages(room)); + + return mentioned + .concat(unread.filter(room => !mentioned.find(other => other === room))) + .concat(list.filter(room => !unread.find(other => other === room))); + }, + // The header is collapsable if it is hidden or not stuck // The dataset elements are added in the RoomList _initAndPositionStickyHeaders method isCollapsableOnClick: function() { diff --git a/src/components/structures/UserSettings.js b/src/components/structures/UserSettings.js index c8ce79905d..3695e0cf9f 100644 --- a/src/components/structures/UserSettings.js +++ b/src/components/structures/UserSettings.js @@ -81,6 +81,8 @@ const SIMPLE_SETTINGS = [ { id: "VideoView.flipVideoHorizontally" }, { id: "TagPanel.disableTagPanel" }, { id: "enableWidgetScreenshots" }, + { id: "pinMentionedRooms" }, + { id: "pinUnreadRooms" }, ]; // These settings must be defined in SettingsStore diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index bf9e395bee..1b378c34d3 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -218,6 +218,8 @@ "Enable URL previews for this room (only affects you)": "Enable URL previews for this room (only affects you)", "Enable URL previews by default for participants in this room": "Enable URL previews by default for participants in this room", "Room Colour": "Room Colour", + "Pin unread rooms to the top of the room list": "Pin unread rooms to the top of the room list", + "Pin rooms I'm mentioned in to the top of the room list": "Pin rooms I'm mentioned in to the top of the room list", "Enable widget screenshots on supported widgets": "Enable widget screenshots on supported widgets", "Collecting app version information": "Collecting app version information", "Collecting logs": "Collecting logs", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index b1bc4161fd..1665a59dfd 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -269,6 +269,16 @@ export const SETTINGS = { default: true, controller: new AudioNotificationsEnabledController(), }, + "pinMentionedRooms": { + supportedLevels: LEVELS_ACCOUNT_SETTINGS, + displayName: _td("Pin rooms I'm mentioned in to the top of the room list"), + default: false, + }, + "pinUnreadRooms": { + supportedLevels: LEVELS_ACCOUNT_SETTINGS, + displayName: _td("Pin unread rooms to the top of the room list"), + default: false, + }, "enableWidgetScreenshots": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Enable widget screenshots on supported widgets'), From 3d8f0adf56d0bd0607d6ad553c47cf915a87cf95 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 12 Oct 2018 14:35:54 -0600 Subject: [PATCH 02/11] Move pinned rooms check to the RoomListStore --- src/components/structures/RoomSubList.js | 20 +------------------ src/stores/RoomListStore.js | 25 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/components/structures/RoomSubList.js b/src/components/structures/RoomSubList.js index 6aaa875e48..d798070659 100644 --- a/src/components/structures/RoomSubList.js +++ b/src/components/structures/RoomSubList.js @@ -19,7 +19,6 @@ limitations under the License. import React from 'react'; import classNames from 'classnames'; import sdk from '../../index'; -import SettingsStore from "../../settings/SettingsStore"; import { Droppable } from 'react-beautiful-dnd'; import { _t } from '../../languageHandler'; import dis from '../../dispatcher'; @@ -100,10 +99,8 @@ const RoomSubList = React.createClass({ componentWillReceiveProps: function(newProps) { // order the room list appropriately before we re-render //if (debug) console.log("received new props, list = " + newProps.list); - const filteredRooms = this.applySearchFilter(newProps.list, newProps.searchFilter); - const sortedRooms = newProps.order === "recent" ? this.applyPinnedTileRules(filteredRooms) : filteredRooms; this.setState({ - sortedList: sortedRooms, + sortedList: this.applySearchFilter(newProps.list, newProps.searchFilter), }); }, @@ -116,21 +113,6 @@ const RoomSubList = React.createClass({ (filter[0] === '#' && room.getAliases().some((alias) => alias.toLowerCase().startsWith(lcFilter)))); }, - applyPinnedTileRules: function(list) { - const pinUnread = SettingsStore.getValue("pinUnreadRooms"); - const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); - if (!pinUnread && !pinMentioned) { - return list; // Nothing to sort - } - - const mentioned = !pinMentioned ? [] : list.filter(room => room.getUnreadNotificationCount("highlight") > 0); - const unread = !pinUnread ? [] : list.filter(room => Unread.doesRoomHaveUnreadMessages(room)); - - return mentioned - .concat(unread.filter(room => !mentioned.find(other => other === room))) - .concat(list.filter(room => !unread.find(other => other === room))); - }, - // The header is collapsable if it is hidden or not stuck // The dataset elements are added in the RoomList _initAndPositionStickyHeaders method isCollapsableOnClick: function() { diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 67c0c13be7..4c3e10e77f 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -17,6 +17,7 @@ import {Store} from 'flux/utils'; import dis from '../dispatcher'; import DMRoomMap from '../utils/DMRoomMap'; import Unread from '../Unread'; +import SettingsStore from "../settings/SettingsStore"; /** * A class for storing application state for categorising rooms in @@ -263,6 +264,30 @@ class RoomListStore extends Store { } _recentsComparator(roomA, roomB) { + const pinUnread = SettingsStore.getValue("pinUnreadRooms"); + const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); + + // We try and set the ordering to be Mentioned > Unread > Recent + // assuming the user has the right settings, of course + + if (pinMentioned) { + const mentionsA = roomA.getUnreadNotificationCount("highlight") > 0; + const mentionsB = roomB.getUnreadNotificationCount("highlight") > 0; + if (mentionsA && !mentionsB) return -1; + if (!mentionsA && mentionsB) return 1; + if (mentionsA && mentionsB) return 0; + // If neither have mentions, fall through to remaining checks + } + + if (pinUnread) { + const unreadA = Unread.doesRoomHaveUnreadMessages(roomA); + const unreadB = Unread.doesRoomHaveUnreadMessages(roomB); + if (unreadA && !unreadB) return -1; + if (!unreadA && unreadB) return 1; + if (unreadA && unreadB) return 0; + // If neither have unread messages, fall through to remaining checks + } + // XXX: We could use a cache here and update it when we see new // events that trigger a reorder return this._tsOfNewestEvent(roomB) - this._tsOfNewestEvent(roomA); From 873133458a16a8b51e950cfd0857879e19eba591 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 31 Oct 2018 13:06:57 -0600 Subject: [PATCH 03/11] Remove the request-only stuff we don't need anymore This was introduced in https://github.com/matrix-org/matrix-react-sdk/pull/2250 but can be pulled out due to https://github.com/matrix-org/matrix-js-sdk/pull/770. See https://github.com/vector-im/riot-web/issues/7634 for more information about the future. --- karma.conf.js | 13 ------------- package.json | 1 - 2 files changed, 14 deletions(-) diff --git a/karma.conf.js b/karma.conf.js index 41ddbdf249..4d699599cb 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -199,25 +199,12 @@ module.exports = function (config) { 'matrix-react-sdk': path.resolve('test/skinned-sdk.js'), 'sinon': 'sinon/pkg/sinon.js', - - // To make webpack happy - // Related: https://github.com/request/request/issues/1529 - // (there's no mock available for fs, so we fake a mock by using - // an in-memory version of fs) - "fs": "memfs", }, modules: [ path.resolve('./test'), "node_modules" ], }, - node: { - // Because webpack is made of fail - // https://github.com/request/request/issues/1529 - // Note: 'mock' is the new 'empty' - net: 'mock', - tls: 'mock' - }, devtool: 'inline-source-map', externals: { // Don't try to bundle electron: leave it as a commonjs dependency diff --git a/package.json b/package.json index b72080cd36..8a51c0877d 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "lodash": "^4.13.1", "lolex": "2.3.2", "matrix-js-sdk": "matrix-org/matrix-js-sdk#develop", - "memfs": "^2.10.1", "optimist": "^0.6.1", "pako": "^1.0.5", "prop-types": "^15.5.8", From 5558b7a3b2b8d8beec94cd901712e3f778cfa6e6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 14:43:15 -0600 Subject: [PATCH 04/11] Avoid hitting the SettingsStore thousands of times when generating room lists Should fix https://github.com/vector-im/riot-web/issues/7646 to some degree --- src/stores/RoomListStore.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 8909dbb489..527fc0fe2e 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -217,11 +217,16 @@ class RoomListStore extends Store { } }); + // Note: we check the settings up here instead of in the forEach or + // in the _recentsComparator to avoid hitting the SettingsStore a few + // thousand times. + const pinUnread = SettingsStore.getValue("pinUnreadRooms"); + const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); Object.keys(lists).forEach((listKey) => { let comparator; switch (RoomListStore._listOrders[listKey]) { case "recent": - comparator = this._recentsComparator; + comparator = (roomA, roomB) => this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); break; case "manual": default: @@ -262,10 +267,7 @@ class RoomListStore extends Store { } } - _recentsComparator(roomA, roomB) { - const pinUnread = SettingsStore.getValue("pinUnreadRooms"); - const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); - + _recentsComparator(roomA, roomB, pinUnread, pinMentioned) { // We try and set the ordering to be Mentioned > Unread > Recent // assuming the user has the right settings, of course From 272acfa2f55b89f87658da91a503a2023ef83958 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 14:46:39 -0600 Subject: [PATCH 05/11] Appease the linter --- src/stores/RoomListStore.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 527fc0fe2e..65148ec8c4 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -226,7 +226,9 @@ class RoomListStore extends Store { let comparator; switch (RoomListStore._listOrders[listKey]) { case "recent": - comparator = (roomA, roomB) => this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); + comparator = (roomA, roomB) => { + return this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); + }; break; case "manual": default: From 0c7aadb92b0d3b7042ff8658c33df5b0b697cefe Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 16:28:13 -0600 Subject: [PATCH 06/11] Improve room list sort performance by caching common variables This won't help much if the user is in a ton of highly active rooms, but for the most part this will help those in thousands of rooms, many of which are likely to be quiet. Fixes https://github.com/vector-im/riot-web/issues/7646 Fixes https://github.com/vector-im/riot-web/issues/7645 (due to timestamp ordering) --- src/stores/RoomListStore.js | 135 ++++++++++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 15 deletions(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 65148ec8c4..e77debd2f2 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -54,6 +54,7 @@ class RoomListStore extends Store { "im.vector.fake.archived": [], }, ready: false, + roomCache: {}, // roomId => { cacheType => value } }; } @@ -85,6 +86,8 @@ class RoomListStore extends Store { !payload.isLiveUnfilteredRoomTimelineEvent || !this._eventTriggersRecentReorder(payload.event) ) break; + + this._clearCachedRoomState(payload.event.getRoomId()); this._generateRoomLists(); } break; @@ -112,6 +115,8 @@ class RoomListStore extends Store { if (liveTimeline !== eventTimeline || !this._eventTriggersRecentReorder(payload.event) ) break; + + this._clearCachedRoomState(payload.event.getRoomId()); this._generateRoomLists(); } break; @@ -222,12 +227,20 @@ class RoomListStore extends Store { // thousand times. const pinUnread = SettingsStore.getValue("pinUnreadRooms"); const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); + this._timings = {}; Object.keys(lists).forEach((listKey) => { let comparator; switch (RoomListStore._listOrders[listKey]) { case "recent": comparator = (roomA, roomB) => { - return this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); + this._timings["overall_" + roomA.roomId + "_" + roomB.roomId] = { + type: "overall", + start: performance.now(), + end: 0, + }; + const ret = this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); + this._timings["overall_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); + return ret; }; break; case "manual": @@ -238,12 +251,76 @@ class RoomListStore extends Store { lists[listKey].sort(comparator); }); + // Combine the samples for performance metrics + const samplesByType = {}; + for (const sampleName of Object.keys(this._timings)) { + const sample = this._timings[sampleName]; + if (!samplesByType[sample.type]) samplesByType[sample.type] = { + min: 999999999, + max: 0, + count: 0, + total: 0, + }; + + const record = samplesByType[sample.type]; + const duration = sample.end - sample.start; + if (duration < record.min) record.min = duration; + if (duration > record.max) record.max = duration; + record.count++; + record.total += duration; + } + + for (const category of Object.keys(samplesByType)) { + const {min, max, count, total} = samplesByType[category]; + const average = total / count; + + console.log(`RoomListSortPerf : type=${category} min=${min} max=${max} total=${total} samples=${count} average=${average}`); + } + this._setState({ lists, ready: true, // Ready to receive updates via Room.tags events }); } + _updateCachedRoomState(roomId, type, value) { + const roomCache = this._state.roomCache; + if (!roomCache[roomId]) roomCache[roomId] = {}; + + if (value) roomCache[roomId][type] = value; + else delete roomCache[roomId][type]; + + this._setState({roomCache}); + } + + _clearCachedRoomState(roomId) { + const roomCache = this._state.roomCache; + delete roomCache[roomId]; + this._setState({roomCache}); + } + + _getRoomState(room, type) { + const roomId = room.roomId; + const roomCache = this._state.roomCache; + if (roomCache[roomId] && typeof roomCache[roomId][type] !== 'undefined') { + return roomCache[roomId][type]; + } + + if (type === "timestamp") { + const ts = this._tsOfNewestEvent(room); + this._updateCachedRoomState(roomId, "timestamp", ts); + return ts; + } else if (type === "unread") { + const unread = room.getUnreadNotificationCount() > 0; + this._updateCachedRoomState(roomId, "unread", unread); + return unread; + } else if (type === "notifications") { + const notifs = room.getUnreadNotificationCount("highlight") > 0; + this._updateCachedRoomState(roomId, "notifications", notifs); + return notifs; + } else throw new Error("Unrecognized room cache type: " + type); + } + _eventTriggersRecentReorder(ev) { return ev.getTs() && ( Unread.eventTriggersUnreadCount(ev) || @@ -270,30 +347,58 @@ class RoomListStore extends Store { } _recentsComparator(roomA, roomB, pinUnread, pinMentioned) { + //console.log("Comparing " + roomA.roomId + " with " + roomB.roomId +" || pinUnread=" + pinUnread +" pinMentioned="+pinMentioned); // We try and set the ordering to be Mentioned > Unread > Recent - // assuming the user has the right settings, of course + // assuming the user has the right settings, of course. + + this._timings["timestamp_" + roomA.roomId + "_" + roomB.roomId] = { + type: "timestamp", + start: performance.now(), + end: 0, + }; + const timestampA = this._getRoomState(roomA, "timestamp"); + const timestampB = this._getRoomState(roomB, "timestamp"); + const timestampDiff = timestampB - timestampA; + this._timings["timestamp_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); if (pinMentioned) { - const mentionsA = roomA.getUnreadNotificationCount("highlight") > 0; - const mentionsB = roomB.getUnreadNotificationCount("highlight") > 0; - if (mentionsA && !mentionsB) return -1; - if (!mentionsA && mentionsB) return 1; - if (mentionsA && mentionsB) return 0; - // If neither have mentions, fall through to remaining checks + this._timings["mentioned_" + roomA.roomId + "_" + roomB.roomId] = { + type: "mentioned", + start: performance.now(), + end: 0, + }; + const mentionsA = this._getRoomState(roomA, "notifications"); + const mentionsB = this._getRoomState(roomB, "notifications"); + this._timings["mentioned_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); + if (mentionsA && !mentionsB) return -1; + if (!mentionsA && mentionsB) return 1; + + // If they both have notifications, sort by timestamp. + // If neither have notifications (the fourth check not shown + // here), then try and sort by unread messages and finally by + // timestamp. + if (mentionsA && mentionsB) return timestampDiff; } if (pinUnread) { - const unreadA = Unread.doesRoomHaveUnreadMessages(roomA); - const unreadB = Unread.doesRoomHaveUnreadMessages(roomB); + this._timings["unread_" + roomA.roomId + "_" + roomB.roomId] = { + type: "unread", + start: performance.now(), + end: 0, + }; + const unreadA = this._getRoomState(roomA, "unread"); + const unreadB = this._getRoomState(roomB, "notifications"); + this._timings["unread_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); if (unreadA && !unreadB) return -1; if (!unreadA && unreadB) return 1; - if (unreadA && unreadB) return 0; - // If neither have unread messages, fall through to remaining checks + + // If they both have unread messages, sort by timestamp + // If nether have unread message (the fourth check not shown + // here), then just sort by timestamp anyways. + if (unreadA && unreadB) return timestampDiff; } - // XXX: We could use a cache here and update it when we see new - // events that trigger a reorder - return this._tsOfNewestEvent(roomB) - this._tsOfNewestEvent(roomA); + return timestampDiff; } _lexicographicalComparator(roomA, roomB) { From 122868e32f3c27b1565c1044b3c325b96c5a0b54 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 16:30:48 -0600 Subject: [PATCH 07/11] Removing timing/performance tracking on room list store This was used to verify the fix was actually making improvements and can be safely taken out. --- src/stores/RoomListStore.js | 55 +------------------------------------ 1 file changed, 1 insertion(+), 54 deletions(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index e77debd2f2..8dc557ace0 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -227,20 +227,12 @@ class RoomListStore extends Store { // thousand times. const pinUnread = SettingsStore.getValue("pinUnreadRooms"); const pinMentioned = SettingsStore.getValue("pinMentionedRooms"); - this._timings = {}; Object.keys(lists).forEach((listKey) => { let comparator; switch (RoomListStore._listOrders[listKey]) { case "recent": comparator = (roomA, roomB) => { - this._timings["overall_" + roomA.roomId + "_" + roomB.roomId] = { - type: "overall", - start: performance.now(), - end: 0, - }; - const ret = this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); - this._timings["overall_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); - return ret; + return this._recentsComparator(roomA, roomB, pinUnread, pinMentioned); }; break; case "manual": @@ -251,32 +243,6 @@ class RoomListStore extends Store { lists[listKey].sort(comparator); }); - // Combine the samples for performance metrics - const samplesByType = {}; - for (const sampleName of Object.keys(this._timings)) { - const sample = this._timings[sampleName]; - if (!samplesByType[sample.type]) samplesByType[sample.type] = { - min: 999999999, - max: 0, - count: 0, - total: 0, - }; - - const record = samplesByType[sample.type]; - const duration = sample.end - sample.start; - if (duration < record.min) record.min = duration; - if (duration > record.max) record.max = duration; - record.count++; - record.total += duration; - } - - for (const category of Object.keys(samplesByType)) { - const {min, max, count, total} = samplesByType[category]; - const average = total / count; - - console.log(`RoomListSortPerf : type=${category} min=${min} max=${max} total=${total} samples=${count} average=${average}`); - } - this._setState({ lists, ready: true, // Ready to receive updates via Room.tags events @@ -347,29 +313,16 @@ class RoomListStore extends Store { } _recentsComparator(roomA, roomB, pinUnread, pinMentioned) { - //console.log("Comparing " + roomA.roomId + " with " + roomB.roomId +" || pinUnread=" + pinUnread +" pinMentioned="+pinMentioned); // We try and set the ordering to be Mentioned > Unread > Recent // assuming the user has the right settings, of course. - this._timings["timestamp_" + roomA.roomId + "_" + roomB.roomId] = { - type: "timestamp", - start: performance.now(), - end: 0, - }; const timestampA = this._getRoomState(roomA, "timestamp"); const timestampB = this._getRoomState(roomB, "timestamp"); const timestampDiff = timestampB - timestampA; - this._timings["timestamp_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); if (pinMentioned) { - this._timings["mentioned_" + roomA.roomId + "_" + roomB.roomId] = { - type: "mentioned", - start: performance.now(), - end: 0, - }; const mentionsA = this._getRoomState(roomA, "notifications"); const mentionsB = this._getRoomState(roomB, "notifications"); - this._timings["mentioned_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); if (mentionsA && !mentionsB) return -1; if (!mentionsA && mentionsB) return 1; @@ -381,14 +334,8 @@ class RoomListStore extends Store { } if (pinUnread) { - this._timings["unread_" + roomA.roomId + "_" + roomB.roomId] = { - type: "unread", - start: performance.now(), - end: 0, - }; const unreadA = this._getRoomState(roomA, "unread"); const unreadB = this._getRoomState(roomB, "notifications"); - this._timings["unread_" + roomA.roomId + "_" + roomB.roomId].end = performance.now(); if (unreadA && !unreadB) return -1; if (!unreadA && unreadB) return 1; From a713cc5c52309f74d1dfccef28556de037eb3db1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 17:07:05 -0600 Subject: [PATCH 08/11] Compare the right types of events --- src/stores/RoomListStore.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 8dc557ace0..f7596774b6 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -335,7 +335,7 @@ class RoomListStore extends Store { if (pinUnread) { const unreadA = this._getRoomState(roomA, "unread"); - const unreadB = this._getRoomState(roomB, "notifications"); + const unreadB = this._getRoomState(roomB, "unread"); if (unreadA && !unreadB) return -1; if (!unreadA && unreadB) return 1; From 3960ae2fcd956702c5adc4f0929438acdea53388 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 1 Nov 2018 17:17:01 -0600 Subject: [PATCH 09/11] Add more commentary around how the roomCache works --- src/stores/RoomListStore.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index f7596774b6..2b70d53b59 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -54,7 +54,24 @@ class RoomListStore extends Store { "im.vector.fake.archived": [], }, ready: false, - roomCache: {}, // roomId => { cacheType => value } + + // The room cache stores a mapping of roomId to cache record. + // Each cache record is a key/value pair for various bits of + // data used to sort the room list. Currently this stores the + // following bits of informations: + // "timestamp": number, The timestamp of the last relevant + // event in the room. + // "notifications": boolean, Whether or not the user has been + // highlighted on any unread events. + // "unread": boolean, Whether or not the user has any + // unread events. + // + // All of the cached values are lazily loaded on read in the + // recents comparator. When an event is received for a particular + // room, all the cached values are invalidated - forcing the + // next read to set new values. The entries do not expire on + // their own. + roomCache: {}, }; } From f9d5c11d8d373ce6d521ce436b5e6ccdaa4604aa Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sun, 4 Nov 2018 19:47:24 -0700 Subject: [PATCH 10/11] Regenerate the room list when m.fully_read is issued Not doing so results in the RoomListStore tracking stale data when the user reads messages on another device. The visual effect of this is rooms being incorrectly pinned in places they shouldn't be, such as the top of the list. This also fixes another visual bug where rooms don't move down once their timelines are read. This second issue is mot prominent when multiple rooms have been pinned to the top, and the middle one is read ahead of the others - it'll stick around until some other condition decides to wipe the room's cached state. Fixes https://github.com/vector-im/riot-web/issues/7653 --- src/actions/MatrixActionCreators.js | 30 +++++++++++++++++++++++++++++ src/stores/RoomListStore.js | 7 +++++++ 2 files changed, 37 insertions(+) diff --git a/src/actions/MatrixActionCreators.js b/src/actions/MatrixActionCreators.js index 31bcac3e52..2f2c23d06f 100644 --- a/src/actions/MatrixActionCreators.js +++ b/src/actions/MatrixActionCreators.js @@ -62,6 +62,35 @@ function createAccountDataAction(matrixClient, accountDataEvent) { }; } +/** + * @typedef RoomAccountDataAction + * @type {Object} + * @property {string} action 'MatrixActions.Room.accountData'. + * @property {MatrixEvent} event the MatrixEvent that triggered the dispatch. + * @property {string} event_type the type of the MatrixEvent, e.g. "m.direct". + * @property {Object} event_content the content of the MatrixEvent. + * @property {Room} room the room where the account data was changed. + */ + +/** + * Create a MatrixActions.Room.accountData action that represents a MatrixClient `Room.accountData` + * matrix event. + * + * @param {MatrixClient} matrixClient the matrix client. + * @param {MatrixEvent} accountDataEvent the account data event. + * @param {Room} room the room where account data was changed + * @returns {RoomAccountDataAction} an action of type MatrixActions.accountData. + */ +function createRoomAccountDataAction(matrixClient, accountDataEvent, room) { + return { + action: 'MatrixActions.Room.accountData', + event: accountDataEvent, + event_type: accountDataEvent.getType(), + event_content: accountDataEvent.getContent(), + room: room, + }; +} + /** * @typedef RoomAction * @type {Object} @@ -201,6 +230,7 @@ export default { start(matrixClient) { this._addMatrixClientListener(matrixClient, 'sync', createSyncAction); this._addMatrixClientListener(matrixClient, 'accountData', createAccountDataAction); + this._addMatrixClientListener(matrixClient, 'Room.accountData', createRoomAccountDataAction); this._addMatrixClientListener(matrixClient, 'Room', createRoomAction); this._addMatrixClientListener(matrixClient, 'Room.tags', createRoomTagsAction); this._addMatrixClientListener(matrixClient, 'Room.timeline', createRoomTimelineAction); diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 2b70d53b59..0f8e5d7b4d 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -142,6 +142,13 @@ class RoomListStore extends Store { this._generateRoomLists(); } break; + case 'MatrixActions.Room.accountData': { + if (payload.event_type === 'm.fully_read') { + this._clearCachedRoomState(payload.room.roomId); + this._generateRoomLists(); + } + } + break; case 'MatrixActions.Room.myMembership': { this._generateRoomLists(); } From ec2528e8b5f1c015e1c35e2f87c9c9eb9f748344 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sun, 4 Nov 2018 23:00:47 -0700 Subject: [PATCH 11/11] Update MatrixActionCreators.js --- src/actions/MatrixActionCreators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/MatrixActionCreators.js b/src/actions/MatrixActionCreators.js index 2f2c23d06f..c1d42ffd0d 100644 --- a/src/actions/MatrixActionCreators.js +++ b/src/actions/MatrixActionCreators.js @@ -79,7 +79,7 @@ function createAccountDataAction(matrixClient, accountDataEvent) { * @param {MatrixClient} matrixClient the matrix client. * @param {MatrixEvent} accountDataEvent the account data event. * @param {Room} room the room where account data was changed - * @returns {RoomAccountDataAction} an action of type MatrixActions.accountData. + * @returns {RoomAccountDataAction} an action of type MatrixActions.Room.accountData. */ function createRoomAccountDataAction(matrixClient, accountDataEvent, room) { return {