Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Ensure incremental updates to the ImportanceAlgorithm trigger A-Z order #5031

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

turt2live
Copy link
Member

Fixes element-hq/element-web#14475

Background: Sticky rooms are actually a pair of lies to the underlying algorithm as a combination of REMOVE_ROOM/NEW_ROOM calls so they don't get considered as needing to be sorted. When a room is added under the importance algorithm, it is expected that the category it is being added to will be re-sorted to account for the change, however we weren't doing that since we optimized the NewRoom path to be a splice operation.

Fixes element-hq/element-web#14475

Background: Sticky rooms are actually a pair of lies to the underlying algorithm as a combination of REMOVE_ROOM/NEW_ROOM calls so they don't get considered as needing to be sorted. When a room is added under the importance algorithm, it is expected that the category it is being added to will be re-sorted to account for the change, however we weren't doing that since we optimized the NewRoom path to be a splice operation.
@turt2live turt2live requested a review from a team July 21, 2020 20:13
@@ -123,6 +123,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
const category = this.getRoomCategory(room);
this.alterCategoryPositionBy(category, 1, this.indices);
this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted)
await this.sortCategory(category);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's annoying that this took an hour to find.

@turt2live turt2live merged commit ba73ce1 into develop Jul 21, 2020
@turt2live turt2live deleted the travis/room-list/a-z-order branch July 21, 2020 20:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New room list: A-Z ordering with "Always show unread first" doesn't put unread room back after reading it
2 participants