From 49abfc1fb20235e7c3ea3ac55cd0321f9635c0eb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 30 Jul 2020 15:06:04 -0600 Subject: [PATCH] Ensure sublists are updated when rooms are removed from them Fixes https://github.com/vector-im/riot-web/issues/14798 (part 2) This is in two parts itself: The `RoomSublist` needs to break its references to the `RoomListStore`, so it now clones the room arrays. The `Algorithm` is the other part, which is slightly more complicated. It turns out that we weren't handling splicing as a change in the `ImportanceAlgorithm`, therefore the `Algorithm` wasn't really feeling like it needed to change anything. Further, the `Algorithm` was using the wrong reference to where it should be dumping rooms (`this.cachedRooms` is a getter which returns a different object depending on conditions), so having fixed that we need to ensure that the filtered and sticky maps are also updated when we remove a room. Because we send the new tag through a Timeline update, we'll end up updating the tag later on and don't need to update the filter and sticky collections. --- src/components/views/rooms/RoomSublist.tsx | 6 +++--- src/stores/room-list/algorithms/Algorithm.ts | 8 +++++--- .../algorithms/list-ordering/ImportanceAlgorithm.ts | 3 +++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index c6730e5a65..f6d0d1c22e 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -46,7 +46,7 @@ import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; -import { arrayHasOrderChange } from "../../../utils/arrays"; +import { arrayFastClone, arrayHasOrderChange } from "../../../utils/arrays"; import { objectExcluding, objectHasDiff } from "../../../utils/objects"; import TemporaryTile from "./TemporaryTile"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; @@ -115,7 +115,7 @@ export default class RoomSublist extends React.Component { isResizing: false, isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed, height: 0, // to be fixed in a moment, we need `rooms` to calculate this. - rooms: RoomListStore.instance.orderedLists[this.props.tagId] || [], + rooms: arrayFastClone(RoomListStore.instance.orderedLists[this.props.tagId] || []), }; // Why Object.assign() and not this.state.height? Because TypeScript says no. this.state = Object.assign(this.state, {height: this.calculateInitialHeight()}); @@ -255,7 +255,7 @@ export default class RoomSublist extends React.Component { } const currentRooms = this.state.rooms; - const newRooms = RoomListStore.instance.orderedLists[this.props.tagId] || []; + const newRooms = arrayFastClone(RoomListStore.instance.orderedLists[this.props.tagId] || []); if (arrayHasOrderChange(currentRooms, newRooms)) { stateUpdates.rooms = newRooms; } diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 9b2779d900..2654a8b460 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -715,7 +715,9 @@ export class Algorithm extends EventEmitter { const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); - this.cachedRooms[rmTag] = algorithm.orderedRooms; + this._cachedRooms[rmTag] = algorithm.orderedRooms; + this.recalculateFilteredRoomsForTag(rmTag); // update filter to re-sort the list + this.recalculateStickyRoom(rmTag); // update sticky room to make sure it moves if needed } for (const addTag of diff.added) { if (SettingsStore.getValue("advancedRoomListLogging")) { @@ -725,7 +727,7 @@ export class Algorithm extends EventEmitter { const algorithm: OrderingAlgorithm = this.algorithms[addTag]; if (!algorithm) throw new Error(`No algorithm for ${addTag}`); await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); - this.cachedRooms[addTag] = algorithm.orderedRooms; + this._cachedRooms[addTag] = algorithm.orderedRooms; } // Update the tag map so we don't regen it in a moment @@ -821,7 +823,7 @@ export class Algorithm extends EventEmitter { if (!algorithm) throw new Error(`No algorithm for ${tag}`); await algorithm.handleRoomUpdate(room, cause); - this.cachedRooms[tag] = algorithm.orderedRooms; + this._cachedRooms[tag] = algorithm.orderedRooms; // Flag that we've done something this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 9cb7679e89..e4aa5ff06f 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -136,6 +136,9 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { } else { throw new Error(`Unhandled splice: ${cause}`); } + + // changes have been made if we made it here, so say so + return true; } public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise {