diff --git a/src/stores/RoomListStore.js b/src/stores/RoomListStore.js index 0a11c2774a..50a7dd22a5 100644 --- a/src/stores/RoomListStore.js +++ b/src/stores/RoomListStore.js @@ -180,7 +180,7 @@ class RoomListStore extends Store { break; case 'MatrixActions.Room.myMembership': { if (!logicallyReady) break; - this._roomUpdateTriggered(payload.room.roomId); + this._roomUpdateTriggered(payload.room.roomId, true); } break; // This could be a new room that we've been invited to, joined or created @@ -188,7 +188,7 @@ class RoomListStore extends Store { // a member. case 'MatrixActions.Room': { if (!logicallyReady) break; - this._roomUpdateTriggered(payload.room.roomId); + this._roomUpdateTriggered(payload.room.roomId, true); } break; // TODO: Re-enable optimistic updates when we support dragging again @@ -230,12 +230,12 @@ class RoomListStore extends Store { } } - _roomUpdateTriggered(roomId) { + _roomUpdateTriggered(roomId, ignoreSticky) { // We don't calculate categories for sticky rooms because we have a moderate // interest in trying to maintain the category that they were last in before // being artificially flagged as IDLE. Also, this reduces the amount of time // we spend in _setRoomCategory ever so slightly. - if (this._state.stickyRoomId !== roomId) { + if (this._state.stickyRoomId !== roomId || ignoreSticky) { // Micro optimization: Only look up the room if we're confident we'll need it. const room = this._matrixClient.getRoom(roomId); if (!room) return; @@ -245,6 +245,31 @@ class RoomListStore extends Store { } } + _getRecommendedTagsForRoom(room) { + const tags = []; + + const myMembership = room.getMyMembership(); + if (myMembership === 'join' || myMembership === 'invite') { + // Stack the user's tags on top + // TODO: Consider whether tags are enabled at all + tags.push(...(room.tags || [])); + + const dmRoomMap = DMRoomMap.shared(); + if (dmRoomMap.getUserIdForRoomId(room.roomId)) { + tags.push("im.vector.fake.direct"); + } else if (myMembership === 'invite') { + tags.push("im.vector.fake.invite"); + } else if (tags.length === 0) { + tags.push("im.vector.fake.recent"); + } + } else { + tags.push("im.vector.fake.archived"); + } + + + return tags; + } + _setRoomCategory(room, category) { if (!room) return; // This should only happen in tests @@ -260,127 +285,116 @@ class RoomListStore extends Store { return _targetTimestamp; }; - const myMembership = room.getMyMembership(); - let doInsert = true; - const targetTags = []; - if (myMembership !== "join" && myMembership !== "invite") { - doInsert = false; - } else { - const dmRoomMap = DMRoomMap.shared(); - if (dmRoomMap.getUserIdForRoomId(room.roomId)) { - targetTags.push('im.vector.fake.direct'); - } else { - targetTags.push('im.vector.fake.recent'); - } - } + const targetTags = this._getRecommendedTagsForRoom(room); + const insertedIntoTags = []; - // We need to update all instances of a room to ensure that they are correctly organized - // in the list. We do this by shallow-cloning the entire `lists` object using a single - // iterator. Within the loop, we also rebuild the list of rooms per tag (key) so that the - // updated room gets slotted into the right spot. This sacrifices code clarity for not - // iterating on potentially large collections multiple times. + // We need to make sure all the tags (lists) are updated with the room's new position. We + // generally only get called here when there's a new room to insert or a room has potentially + // changed positions within the list. + // + // We do all our checks by iterating over the rooms in the existing lists, trying to insert + // our room where we can. As a guiding principle, we should be removing the room from all + // tags, and insert the room into targetTags. We should perform the deletion before the addition + // where possible to keep a consistent state. By the end of this, targetTags should be the + // same as insertedIntoTags. - let inserted = false; for (const key of Object.keys(this._state.lists)) { - const hasRoom = this._state.lists[key].some((e) => e.room.roomId === room.roomId); + const shouldHaveRoom = targetTags.includes(key); - // Speed optimization: Skip the loop below if we're not going to do anything productive - if (!hasRoom || LIST_ORDERS[key] !== 'recent') { - listsClone[key] = this._state.lists[key]; - if (LIST_ORDERS[key] !== 'recent' && (hasRoom || targetTags.includes(key))) { - // Ensure that we don't try and sort the room into the tag - inserted = true; - doInsert = false; - } - continue; + // Speed optimization: Don't do complicated math if we don't have to. + if (!shouldHaveRoom) { + listsClone[key] = this._state.lists[key].filter((e) => e.room.roomId !== room.roomId); } else { listsClone[key] = []; - } - // We track where the boundary within listsClone[key] is just in case our timestamp - // ordering fails. If we can't stick the room in at the correct place in the category - // grouping based on timestamp, we'll stick it at the top of the group which will be - // the index we track here. - let desiredCategoryBoundaryIndex = 0; - let foundBoundary = false; - let pushedEntry = false; + // Tags sorted by recents are more complicated than manually ordered tags, so hope + // for the best. + // TODO: Use the SettingsStore watcher to determine if tags are enabled or not + if (LIST_ORDERS[key] !== 'recent') { + // TODO: Actually insert the room + } else { + // We track where the boundary within listsClone[key] is just in case our timestamp + // ordering fails. If we can't stick the room in at the correct place in the category + // grouping based on timestamp, we'll stick it at the top of the group which will be + // the index we track here. + let desiredCategoryBoundaryIndex = 0; + let foundBoundary = false; + let pushedEntry = false; - for (const entry of this._state.lists[key]) { - // if the list is a recent list, and the room appears in this list, and we're not looking at a sticky - // room (sticky rooms have unreliable categories), try to slot the new room in - if (entry.room.roomId !== this._state.stickyRoomId) { - if (!pushedEntry && doInsert && (targetTags.length === 0 || targetTags.includes(key))) { - // Micro optimization: Support lazily loading the last timestamp in a room - let _entryTimestamp = null; - const entryTimestamp = () => { - if (_entryTimestamp === null) { - _entryTimestamp = this._tsOfNewestEvent(entry.room); - } - return _entryTimestamp; - }; - - const entryCategoryIndex = CATEGORY_ORDER.indexOf(entry.category); - - // As per above, check if we're meeting that boundary we wanted to locate. - if (entryCategoryIndex >= targetCategoryIndex && !foundBoundary) { - desiredCategoryBoundaryIndex = listsClone[key].length - 1; - foundBoundary = true; + for (const entry of this._state.lists[key]) { + // We insert our own record as needed, so don't let the old one through. + if (entry.room.roomId === room.roomId) { + continue; } - // If we've hit the top of a boundary beyond our target category, insert at the top of - // the grouping to ensure the room isn't slotted incorrectly. Otherwise, try to insert - // based on most recent timestamp. - const changedBoundary = entryCategoryIndex > targetCategoryIndex; - const currentCategory = entryCategoryIndex === targetCategoryIndex; - if (changedBoundary || (currentCategory && targetTimestamp() >= entryTimestamp())) { - if (changedBoundary) { - // If we changed a boundary, then we've gone too far - go to the top of the last - // section instead. - listsClone[key].splice(desiredCategoryBoundaryIndex, 0, {room, category}); - } else { - // If we're ordering by timestamp, just insert normally - listsClone[key].push({room, category}); + // if the list is a recent list, and the room appears in this list, and we're + // not looking at a sticky room (sticky rooms have unreliable categories), try + // to slot the new room in + if (entry.room.roomId !== this._state.stickyRoomId) { + if (!pushedEntry && shouldHaveRoom) { + // Micro optimization: Support lazily loading the last timestamp in a room + let _entryTimestamp = null; + const entryTimestamp = () => { + if (_entryTimestamp === null) { + _entryTimestamp = this._tsOfNewestEvent(entry.room); + } + return _entryTimestamp; + }; + + const entryCategoryIndex = CATEGORY_ORDER.indexOf(entry.category); + + // As per above, check if we're meeting that boundary we wanted to locate. + if (entryCategoryIndex >= targetCategoryIndex && !foundBoundary) { + desiredCategoryBoundaryIndex = listsClone[key].length - 1; + foundBoundary = true; + } + + // If we've hit the top of a boundary beyond our target category, insert at the top of + // the grouping to ensure the room isn't slotted incorrectly. Otherwise, try to insert + // based on most recent timestamp. + const changedBoundary = entryCategoryIndex > targetCategoryIndex; + const currentCategory = entryCategoryIndex === targetCategoryIndex; + if (changedBoundary || (currentCategory && targetTimestamp() >= entryTimestamp())) { + if (changedBoundary) { + // If we changed a boundary, then we've gone too far - go to the top of the last + // section instead. + listsClone[key].splice(desiredCategoryBoundaryIndex, 0, {room, category}); + } else { + // If we're ordering by timestamp, just insert normally + listsClone[key].push({room, category}); + } + pushedEntry = true; + insertedIntoTags.push(key); + } } - pushedEntry = true; - inserted = true; } + + // Fall through and clone the list. + listsClone[key].push(entry); } - // We insert our own record as needed, so don't let the old one through. - if (entry.room.roomId === room.roomId) { - continue; + if (!pushedEntry) { + if (listsClone[key].length === 0) { + listsClone[key].push({room, category}); + insertedIntoTags.push(key); + } else { + // In theory, this should never happen + console.warn(`!! Room ${room.roomId} lost: No position available`); + } } } - - // Fall through and clone the list. - listsClone[key].push(entry); } } - if (!inserted) { - // There's a good chance that we just joined the room, so we need to organize it - // We also could have left it... - let tags = []; - if (doInsert) { - tags = Object.keys(room.tags); - if (tags.length === 0) { - tags = targetTags; - } - if (tags.length === 0) { - tags = [myMembership === 'join' ? 'im.vector.fake.recent' : 'im.vector.fake.invite']; - } - } else { - tags = ['im.vector.fake.archived']; + // Double check that we inserted the room in the right places + for (const targetTag of targetTags) { + let count = 0; + for (const insertedTag of insertedIntoTags) { + if (insertedTag === targetTag) count++; } - for (const tag of tags) { - for (let i = 0; i < listsClone[tag].length; i++) { - // Just find the top of our category grouping and insert it there. - const catIdxAtPosition = CATEGORY_ORDER.indexOf(listsClone[tag][i].category); - if (catIdxAtPosition >= targetCategoryIndex) { - listsClone[tag].splice(i, 0, {room: room, category: category}); - break; - } - } + + if (count !== 1) { + console.warn(`!! Room ${room.roomId} inserted ${count} times`); } } @@ -574,7 +588,7 @@ class RoomListStore extends Store { return -1; } - return a === b ? this._lexicographicalComparator(roomA, roomB) : ( a > b ? 1 : -1); + return a === b ? this._lexicographicalComparator(roomA, roomB) : (a > b ? 1 : -1); }; }