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.
pull/21833/head
Travis Ralston 2020-07-30 15:06:04 -06:00
parent da05cac1b6
commit 49abfc1fb2
3 changed files with 11 additions and 6 deletions

View File

@ -46,7 +46,7 @@ import { Direction } from "re-resizable/lib/resizer";
import { polyfillTouchEvent } from "../../../@types/polyfill"; import { polyfillTouchEvent } from "../../../@types/polyfill";
import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore";
import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; 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 { objectExcluding, objectHasDiff } from "../../../utils/objects";
import TemporaryTile from "./TemporaryTile"; import TemporaryTile from "./TemporaryTile";
import { ListNotificationState } from "../../../stores/notifications/ListNotificationState"; import { ListNotificationState } from "../../../stores/notifications/ListNotificationState";
@ -115,7 +115,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
isResizing: false, isResizing: false,
isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed, isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed,
height: 0, // to be fixed in a moment, we need `rooms` to calculate this. 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. // Why Object.assign() and not this.state.height? Because TypeScript says no.
this.state = Object.assign(this.state, {height: this.calculateInitialHeight()}); this.state = Object.assign(this.state, {height: this.calculateInitialHeight()});
@ -255,7 +255,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
} }
const currentRooms = this.state.rooms; 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)) { if (arrayHasOrderChange(currentRooms, newRooms)) {
stateUpdates.rooms = newRooms; stateUpdates.rooms = newRooms;
} }

View File

@ -715,7 +715,9 @@ export class Algorithm extends EventEmitter {
const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; const algorithm: OrderingAlgorithm = this.algorithms[rmTag];
if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); if (!algorithm) throw new Error(`No algorithm for ${rmTag}`);
await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); 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) { for (const addTag of diff.added) {
if (SettingsStore.getValue("advancedRoomListLogging")) { if (SettingsStore.getValue("advancedRoomListLogging")) {
@ -725,7 +727,7 @@ export class Algorithm extends EventEmitter {
const algorithm: OrderingAlgorithm = this.algorithms[addTag]; const algorithm: OrderingAlgorithm = this.algorithms[addTag];
if (!algorithm) throw new Error(`No algorithm for ${addTag}`); if (!algorithm) throw new Error(`No algorithm for ${addTag}`);
await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); 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 // 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}`); if (!algorithm) throw new Error(`No algorithm for ${tag}`);
await algorithm.handleRoomUpdate(room, cause); await algorithm.handleRoomUpdate(room, cause);
this.cachedRooms[tag] = algorithm.orderedRooms; this._cachedRooms[tag] = algorithm.orderedRooms;
// Flag that we've done something // Flag that we've done something
this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list

View File

@ -136,6 +136,9 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
} else { } else {
throw new Error(`Unhandled splice: ${cause}`); 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<boolean> { public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise<boolean> {