mirror of https://github.com/vector-im/riot-web
				
				
				
			Fix sticky room disappearing/jumping in search results
Fixes https://github.com/vector-im/riot-web/issues/14124 Fixes https://github.com/vector-im/riot-web/issues/14154 (which was technically supposed to say that the sticky room when filtering was always last) This is all a bit complicated, but the theory is that we end up with a stable list even through filtering. There's some notes within, though I suspect it'll be difficult to understand :(pull/21833/head
							parent
							
								
									74e4ea7d48
								
							
						
					
					
						commit
						f93d67fc65
					
				| 
						 | 
				
			
			@ -170,12 +170,16 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
        // When we do have a room though, we expect to be able to find it
 | 
			
		||||
        const tag = this.roomIdsToTags[val.roomId][0];
 | 
			
		||||
        if (!tag) throw new Error(`${val.roomId} does not belong to a tag and cannot be sticky`);
 | 
			
		||||
        let position = this.cachedRooms[tag].indexOf(val);
 | 
			
		||||
 | 
			
		||||
        // We specifically do NOT use the ordered rooms set as it contains the sticky room, which
 | 
			
		||||
        // means we'll be off by 1 when the user is switching rooms. This leads to visual jumping
 | 
			
		||||
        // when the user is moving south in the list (not north, because of math).
 | 
			
		||||
        let position = this.getOrderedRoomsWithoutSticky()[tag].indexOf(val);
 | 
			
		||||
        if (position < 0) throw new Error(`${val.roomId} does not appear to be known and cannot be sticky`);
 | 
			
		||||
 | 
			
		||||
        // 🐉 Here be dragons.
 | 
			
		||||
        // Before we can go through with lying to the underlying algorithm about a room
 | 
			
		||||
        // we need to ensure that when we do we're ready for the innevitable sticky room
 | 
			
		||||
        // we need to ensure that when we do we're ready for the inevitable sticky room
 | 
			
		||||
        // update we'll receive. To prepare for that, we first remove the sticky room and
 | 
			
		||||
        // recalculate the state ourselves so that when the underlying algorithm calls for
 | 
			
		||||
        // the same thing it no-ops. After we're done calling the algorithm, we'll issue
 | 
			
		||||
| 
						 | 
				
			
			@ -208,6 +212,12 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
            position: position,
 | 
			
		||||
            tag: tag,
 | 
			
		||||
        };
 | 
			
		||||
 | 
			
		||||
        // We update the filtered rooms just in case, as otherwise users will end up visiting
 | 
			
		||||
        // a room while filtering and it'll disappear. We don't update the filter earlier in
 | 
			
		||||
        // this function simply because we don't have to.
 | 
			
		||||
        this.recalculateFilteredRoomsForTag(tag);
 | 
			
		||||
        if (lastStickyRoom && lastStickyRoom.tag !== tag) this.recalculateFilteredRoomsForTag(tag);
 | 
			
		||||
        this.recalculateStickyRoom();
 | 
			
		||||
 | 
			
		||||
        // Finally, trigger an update
 | 
			
		||||
| 
						 | 
				
			
			@ -231,9 +241,7 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
            // We optimize our lookups by trying to reduce sample size as much as possible
 | 
			
		||||
            // to the rooms we know will be deduped by the Set.
 | 
			
		||||
            const rooms = this.cachedRooms[tagId].map(r => r); // cheap clone
 | 
			
		||||
            if (this._stickyRoom && this._stickyRoom.tag === tagId && this._stickyRoom.room) {
 | 
			
		||||
                rooms.push(this._stickyRoom.room);
 | 
			
		||||
            }
 | 
			
		||||
            this.tryInsertStickyRoomToFilterSet(rooms, tagId);
 | 
			
		||||
            let remainingRooms = rooms.map(r => r);
 | 
			
		||||
            let allowedRoomsInThisTag = [];
 | 
			
		||||
            let lastFilterPriority = orderedFilters[0].relativePriority;
 | 
			
		||||
| 
						 | 
				
			
			@ -263,6 +271,7 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
        this.emit(LIST_UPDATED_EVENT);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    // TODO: Remove or use.
 | 
			
		||||
    protected addPossiblyFilteredRoomsToTag(tagId: TagID, added: Room[]): void {
 | 
			
		||||
        const filters = this.allowedByFilter.keys();
 | 
			
		||||
        for (const room of added) {
 | 
			
		||||
| 
						 | 
				
			
			@ -281,7 +290,8 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
    protected recalculateFilteredRoomsForTag(tagId: TagID): void {
 | 
			
		||||
        console.log(`Recalculating filtered rooms for ${tagId}`);
 | 
			
		||||
        delete this.filteredRooms[tagId];
 | 
			
		||||
        const rooms = this.cachedRooms[tagId];
 | 
			
		||||
        const rooms = this.cachedRooms[tagId].map(r => r); // cheap clone
 | 
			
		||||
        this.tryInsertStickyRoomToFilterSet(rooms, tagId);
 | 
			
		||||
        const filteredRooms = rooms.filter(r => this.allowedRoomsByFilters.has(r));
 | 
			
		||||
        if (filteredRooms.length > 0) {
 | 
			
		||||
            this.filteredRooms[tagId] = filteredRooms;
 | 
			
		||||
| 
						 | 
				
			
			@ -289,6 +299,17 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
        console.log(`[DEBUG] ${filteredRooms.length}/${rooms.length} rooms filtered into ${tagId}`);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    protected tryInsertStickyRoomToFilterSet(rooms: Room[], tagId: TagID) {
 | 
			
		||||
        if (!this._stickyRoom || !this._stickyRoom.room || this._stickyRoom.tag !== tagId) return;
 | 
			
		||||
 | 
			
		||||
        const position = this._stickyRoom.position;
 | 
			
		||||
        if (position >= rooms.length) {
 | 
			
		||||
            rooms.push(this._stickyRoom.room);
 | 
			
		||||
        } else {
 | 
			
		||||
            rooms.splice(position, 0, this._stickyRoom.room);
 | 
			
		||||
        }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Recalculate the sticky room position. If this is being called in relation to
 | 
			
		||||
     * a specific tag being updated, it should be given to this function to optimize
 | 
			
		||||
| 
						 | 
				
			
			@ -377,6 +398,20 @@ export class Algorithm extends EventEmitter {
 | 
			
		|||
        return this.filteredRooms;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * This returns the same as getOrderedRooms(), but without the sticky room
 | 
			
		||||
     * map as it causes issues for sticky room handling (see sticky room handling
 | 
			
		||||
     * for more information).
 | 
			
		||||
     * @returns {ITagMap} The cached list of rooms, ordered,
 | 
			
		||||
     * for each tag. May be empty, but never null/undefined.
 | 
			
		||||
     */
 | 
			
		||||
    private getOrderedRoomsWithoutSticky(): ITagMap {
 | 
			
		||||
        if (!this.hasFilters) {
 | 
			
		||||
            return this.cachedRooms;
 | 
			
		||||
        }
 | 
			
		||||
        return this.filteredRooms;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Seeds the Algorithm with a set of rooms. The algorithm will discard all
 | 
			
		||||
     * previously known information and instead use these rooms instead.
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue