diff --git a/docs/img/RoomListStore2.png b/docs/img/RoomListStore2.png new file mode 100644 index 0000000000..9952d1c910 Binary files /dev/null and b/docs/img/RoomListStore2.png differ diff --git a/src/stores/room-list/README.md b/docs/room-list-store.md similarity index 82% rename from src/stores/room-list/README.md rename to docs/room-list-store.md index ba34691d34..53f0527209 100644 --- a/src/stores/room-list/README.md +++ b/docs/room-list-store.md @@ -2,20 +2,31 @@ It's so complicated it needs its own README. +![](img/RoomListStore2.png) + +Legend: +* Orange = External event. +* Purple = Deterministic flow. +* Green = Algorithm definition. +* Red = Exit condition/point. +* Blue = Process definition. + ## Algorithms involved There's two main kinds of algorithms involved in the room list store: list ordering and tag sorting. Throughout the code an intentional decision has been made to call them the List Algorithm and Sorting -Algorithm respectively. The list algorithm determines the behaviour of the room list whereas the sorting -algorithm determines how rooms get ordered within tags affected by the list algorithm. +Algorithm respectively. The list algorithm determines the primary ordering of a given tag whereas the +tag sorting defines how rooms within that tag get sorted, at the discretion of the list ordering. + +Behaviour of the overall room list (sticky rooms, etc) are determined by the generically-named Algorithm +class. Here is where much of the coordination from the room list store is done to figure out which list +algorithm to call, instead of having all the logic in the room list store itself. -Behaviour of the room list takes the shape of determining what features the room list supports, as well -as determining where and when to apply the sorting algorithm in a tag. The importance algorithm, which -is described later in this doc, is an example of an algorithm which makes heavy behavioural changes -to the room list. Tag sorting is effectively the comparator supplied to the list algorithm. This gives the list algorithm -the power to decide when and how to apply the tag sorting, if at all. +the power to decide when and how to apply the tag sorting, if at all. For example, the importance algorithm, +later described in this document, heavily uses the list ordering behaviour to break the tag into categories. +Each category then gets sorted by the appropriate tag sorting algorithm. ### Tag sorting algorithm: Alphabetical @@ -70,11 +81,11 @@ Conveniently, each tag gets ordered by those categories as presented: red rooms above bold, etc. Once the algorithm has determined which rooms belong in which categories, the tag sorting algorithm -gets applied to each category in a sub-sub-list fashion. This should result in the red rooms (for example) +gets applied to each category in a sub-list fashion. This should result in the red rooms (for example) being sorted alphabetically amongst each other as well as the grey rooms sorted amongst each other, but collectively the tag will be sorted into categories with red being at the top. -### Sticky rooms +## Sticky rooms When the user visits a room, that room becomes 'sticky' in the list, regardless of ordering algorithm. From a code perspective, the underlying algorithm is not aware of a sticky room and instead the base class @@ -128,13 +139,13 @@ maintain the caching behaviour described above. ## Class breakdowns -The `RoomListStore` is the major coordinator of various `Algorithm` implementations, which take care -of the various `ListAlgorithm` and `SortingAlgorithm` options. The `Algorithm` superclass is also -responsible for figuring out which tags get which rooms, as Matrix specifies them as a reverse map: -tags get defined on rooms and are not defined as a collection of rooms (unlike how they are presented -to the user). Various list-specific utilities are also included, though they are expected to move -somewhere more general when needed. For example, the `membership` utilities could easily be moved -elsewhere as needed. +The `RoomListStore` is the major coordinator of various algorithm implementations, which take care +of the various `ListAlgorithm` and `SortingAlgorithm` options. The `Algorithm` class is responsible +for figuring out which tags get which rooms, as Matrix specifies them as a reverse map: tags get +defined on rooms and are not defined as a collection of rooms (unlike how they are presented to the +user). Various list-specific utilities are also included, though they are expected to move somewhere +more general when needed. For example, the `membership` utilities could easily be moved elsewhere +as needed. The various bits throughout the room list store should also have jsdoc of some kind to help describe what they do and how they work. diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 95a0ba4cc5..e57f03cefc 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -314,6 +314,14 @@ limitations under the License. font-size: $font-15px; line-height: $font-20px; font-weight: 600; - margin-bottom: 12px; + margin-bottom: 4px; + } + + .mx_RadioButton, .mx_Checkbox { + margin-top: 8px; + } + + .mx_Checkbox { + margin-left: -8px; // to counteract the indent from the component } } diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index aea5b4ae23..9f8b8579c3 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -27,8 +27,11 @@ import RoomTile2 from "./RoomTile2"; import { ResizableBox, ResizeCallbackData } from "react-resizable"; import { ListLayout } from "../../../stores/room-list/ListLayout"; import NotificationBadge, { ListNotificationState } from "./NotificationBadge"; -import {ContextMenu, ContextMenuButton} from "../../structures/ContextMenu"; +import { ContextMenu, ContextMenuButton } from "../../structures/ContextMenu"; import StyledCheckbox from "../elements/StyledCheckbox"; +import StyledRadioButton from "../elements/StyledRadioButton"; +import RoomListStore from "../../../stores/room-list/RoomListStore2"; +import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models"; /******************************************************************* * CAUTION * @@ -116,9 +119,14 @@ export default class RoomSublist2 extends React.Component { this.setState({menuDisplayed: false}); }; - private onUnreadFirstChanged = () => { - // TODO: Support per-list algorithm changes - console.log("Unread first changed"); + private onUnreadFirstChanged = async () => { + const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.layout.tagId) === ListAlgorithm.Importance; + const newAlgorithm = isUnreadFirst ? ListAlgorithm.Natural : ListAlgorithm.Importance; + await RoomListStore.instance.setListOrder(this.props.layout.tagId, newAlgorithm); + }; + + private onTagSortChanged = async (sort: SortAlgorithm) => { + await RoomListStore.instance.setTagSorting(this.props.layout.tagId, sort); }; private onMessagePreviewChanged = () => { @@ -149,6 +157,8 @@ export default class RoomSublist2 extends React.Component { let contextMenu = null; if (this.state.menuDisplayed) { const elementRect = this.menuButtonRef.current.getBoundingClientRect(); + const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.layout.tagId) === SortAlgorithm.Alphabetic; + const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.layout.tagId) === ListAlgorithm.Importance; contextMenu = ( {
{_t("Sort by")}
- TODO: Radios are blocked by https://github.com/matrix-org/matrix-react-sdk/pull/4731 + this.onTagSortChanged(SortAlgorithm.Recent)} + checked={!isAlphabetical} + name={`mx_${this.props.layout.tagId}_sortBy`} + > + {_t("Activity")} + + this.onTagSortChanged(SortAlgorithm.Alphabetic)} + checked={isAlphabetical} + name={`mx_${this.props.layout.tagId}_sortBy`} + > + {_t("A-Z")} +
{_t("Unread rooms")}
{_t("Always show first")} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 81577d740e..31da52a494 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1138,6 +1138,8 @@ "Not now": "Not now", "Don't ask me again": "Don't ask me again", "Sort by": "Sort by", + "Activity": "Activity", + "A-Z": "A-Z", "Unread rooms": "Unread rooms", "Always show first": "Always show first", "Show": "Show", diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 63a06dcf81..1c4e66c4b0 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -17,25 +17,21 @@ limitations under the License. import { MatrixClient } from "matrix-js-sdk/src/client"; import SettingsStore from "../../settings/SettingsStore"; -import { DefaultTagID, OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models"; -import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/list-ordering/Algorithm"; +import { OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models"; import TagOrderStore from "../TagOrderStore"; import { AsyncStore } from "../AsyncStore"; import { Room } from "matrix-js-sdk/src/models/room"; -import { ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models"; -import { getListAlgorithmInstance } from "./algorithms/list-ordering"; +import { IListOrderingMap, ITagMap, ITagSortingMap, ListAlgorithm, SortAlgorithm } from "./algorithms/models"; import { ActionPayload } from "../../dispatcher/payloads"; import defaultDispatcher from "../../dispatcher/dispatcher"; import { readReceiptChangeIsFor } from "../../utils/read-receipts"; import { IFilterCondition } from "./filters/IFilterCondition"; import { TagWatcher } from "./TagWatcher"; import RoomViewStore from "../RoomViewStore"; +import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; interface IState { tagsEnabled?: boolean; - - preferredSort?: SortAlgorithm; - preferredAlgorithm?: ListAlgorithm; } /** @@ -48,7 +44,7 @@ export class RoomListStore2 extends AsyncStore { private _matrixClient: MatrixClient; private initialListsGenerated = false; private enabled = false; - private algorithm: Algorithm; + private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); @@ -64,6 +60,7 @@ export class RoomListStore2 extends AsyncStore { this.checkEnabled(); for (const settingName of this.watchedSettings) SettingsStore.monitorSetting(settingName, null); RoomViewStore.addListener(this.onRVSUpdate); + this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); } public get orderedLists(): ITagMap { @@ -85,14 +82,10 @@ export class RoomListStore2 extends AsyncStore { private async readAndCacheSettingsFromStore() { const tagsEnabled = SettingsStore.isFeatureEnabled("feature_custom_tags"); - const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance"); - const orderAlphabetically = SettingsStore.getValue("RoomList.orderAlphabetically"); await this.updateState({ tagsEnabled, - preferredSort: orderAlphabetically ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent, - preferredAlgorithm: orderByImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural, }); - this.setAlgorithmClass(); + await this.updateAlgorithmInstances(); } private onRVSUpdate = () => { @@ -259,17 +252,57 @@ export class RoomListStore2 extends AsyncStore { } } - private getSortAlgorithmFor(tagId: TagID): SortAlgorithm { - switch (tagId) { - case DefaultTagID.Invite: - case DefaultTagID.Untagged: - case DefaultTagID.Archived: - case DefaultTagID.LowPriority: - case DefaultTagID.DM: - return this.state.preferredSort; - case DefaultTagID.Favourite: - default: - return SortAlgorithm.Manual; + public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + await this.algorithm.setTagSorting(tagId, sort); + localStorage.setItem(`mx_tagSort_${tagId}`, sort); + } + + public getTagSorting(tagId: TagID): SortAlgorithm { + return this.algorithm.getTagSorting(tagId); + } + + // noinspection JSMethodCanBeStatic + private getStoredTagSorting(tagId: TagID): SortAlgorithm { + return localStorage.getItem(`mx_tagSort_${tagId}`); + } + + public async setListOrder(tagId: TagID, order: ListAlgorithm) { + await this.algorithm.setListOrdering(tagId, order); + localStorage.setItem(`mx_listOrder_${tagId}`, order); + } + + public getListOrder(tagId: TagID): ListAlgorithm { + return this.algorithm.getListOrdering(tagId); + } + + // noinspection JSMethodCanBeStatic + private getStoredListOrder(tagId: TagID): ListAlgorithm { + return localStorage.getItem(`mx_listOrder_${tagId}`); + } + + private async updateAlgorithmInstances() { + const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance"); + const orderAlphabetically = SettingsStore.getValue("RoomList.orderAlphabetically"); + + const defaultSort = orderAlphabetically ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent; + const defaultOrder = orderByImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural; + + for (const tag of Object.keys(this.orderedLists)) { + const definedSort = this.getTagSorting(tag); + const definedOrder = this.getListOrder(tag); + + const storedSort = this.getStoredTagSorting(tag); + const storedOrder = this.getStoredListOrder(tag); + + const tagSort = storedSort ? storedSort : (definedSort ? definedSort : defaultSort); + const listOrder = storedOrder ? storedOrder : (definedOrder ? definedOrder : defaultOrder); + + if (tagSort !== definedSort) { + await this.setTagSorting(tag, tagSort); + } + if (listOrder !== definedOrder) { + await this.setListOrder(tag, listOrder); + } } } @@ -279,15 +312,6 @@ export class RoomListStore2 extends AsyncStore { await super.updateState(newState); } - private setAlgorithmClass() { - if (this.algorithm) { - this.algorithm.off(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); - } - this.algorithm = getListAlgorithmInstance(this.state.preferredAlgorithm); - this.algorithm.setFilterConditions(this.filterConditions); - this.algorithm.on(LIST_UPDATED_EVENT, this.onAlgorithmListUpdated); - } - private onAlgorithmListUpdated = () => { console.log("Underlying algorithm has triggered a list update - refiring"); this.emit(LISTS_UPDATE_EVENT, this); @@ -296,9 +320,11 @@ export class RoomListStore2 extends AsyncStore { private async regenerateAllLists() { console.warn("Regenerating all room lists"); - const tags: ITagSortingMap = {}; + const sorts: ITagSortingMap = {}; + const orders: IListOrderingMap = {}; for (const tagId of OrderedDefaultTagIDs) { - tags[tagId] = this.getSortAlgorithmFor(tagId); + sorts[tagId] = this.getStoredTagSorting(tagId) || SortAlgorithm.Alphabetic; + orders[tagId] = this.getStoredListOrder(tagId) || ListAlgorithm.Natural; } if (this.state.tagsEnabled) { @@ -307,7 +333,7 @@ export class RoomListStore2 extends AsyncStore { console.log("rtags", roomTags); } - await this.algorithm.populateTags(tags); + await this.algorithm.populateTags(sorts, orders); await this.algorithm.setKnownRooms(this.matrixClient.getRooms()); this.initialListsGenerated = true; diff --git a/src/stores/room-list/algorithms/list-ordering/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts similarity index 76% rename from src/stores/room-list/algorithms/list-ordering/Algorithm.ts rename to src/stores/room-list/algorithms/Algorithm.ts index a8512c0bd6..e8b167c1ba 100644 --- a/src/stores/room-list/algorithms/list-ordering/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -14,17 +14,25 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { DefaultTagID, RoomUpdateCause, TagID } from "../../models"; import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; -import { EffectiveMembership, splitRoomsByMembership } from "../../membership"; -import { ITagMap, ITagSortingMap } from "../models"; -import DMRoomMap from "../../../../utils/DMRoomMap"; -import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../../filters/IFilterCondition"; +import DMRoomMap from "../../../utils/DMRoomMap"; import { EventEmitter } from "events"; -import { UPDATE_EVENT } from "../../../AsyncStore"; -import { ArrayUtil } from "../../../../utils/arrays"; -import { getEnumValues } from "../../../../utils/enums"; +import { arrayHasDiff, ArrayUtil } from "../../../utils/arrays"; +import { getEnumValues } from "../../../utils/enums"; +import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; +import { + IListOrderingMap, + IOrderingAlgorithmMap, + ITagMap, + ITagSortingMap, + ListAlgorithm, + SortAlgorithm +} from "./models"; +import { FILTER_CHANGED, FilterPriority, IFilterCondition } from "../filters/IFilterCondition"; +import { EffectiveMembership, splitRoomsByMembership } from "../membership"; +import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; +import { getListAlgorithmInstance } from "./list-ordering"; // TODO: Add locking support to avoid concurrent writes? @@ -44,21 +52,22 @@ interface IStickyRoom { * management (which rooms go in which tags) and ask the implementation to * deal with ordering mechanics. */ -export abstract class Algorithm extends EventEmitter { +export class Algorithm extends EventEmitter { private _cachedRooms: ITagMap = {}; private _cachedStickyRooms: ITagMap = {}; // a clone of the _cachedRooms, with the sticky room private filteredRooms: ITagMap = {}; private _stickyRoom: IStickyRoom = null; - - protected sortAlgorithms: ITagSortingMap; - protected rooms: Room[] = []; - protected roomIdsToTags: { + private sortAlgorithms: ITagSortingMap; + private listAlgorithms: IListOrderingMap; + private algorithms: IOrderingAlgorithmMap; + private rooms: Room[] = []; + private roomIdsToTags: { [roomId: string]: TagID[]; } = {}; - protected allowedByFilter: Map = new Map(); - protected allowedRoomsByFilters: Set = new Set(); + private allowedByFilter: Map = new Map(); + private allowedRoomsByFilters: Set = new Set(); - protected constructor() { + public constructor() { super(); } @@ -68,6 +77,7 @@ export abstract class Algorithm extends EventEmitter { public set stickyRoom(val: Room) { // setters can't be async, so we call a private function to do the work + // noinspection JSIgnoredPromiseFromCall this.updateStickyRoom(val); } @@ -89,14 +99,38 @@ export abstract class Algorithm extends EventEmitter { return this._cachedRooms; } - /** - * Sets the filter conditions the Algorithm should use. - * @param filterConditions The filter conditions to use. - */ - public setFilterConditions(filterConditions: IFilterCondition[]): void { - for (const filter of filterConditions) { - this.addFilterCondition(filter); - } + public getTagSorting(tagId: TagID): SortAlgorithm { + return this.sortAlgorithms[tagId]; + } + + public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + if (!tagId) throw new Error("Tag ID must be defined"); + if (!sort) throw new Error("Algorithm must be defined"); + this.sortAlgorithms[tagId] = sort; + + const algorithm: OrderingAlgorithm = this.algorithms[tagId]; + await algorithm.setSortAlgorithm(sort); + this._cachedRooms[tagId] = algorithm.orderedRooms; + this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list + this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed + } + + public getListOrdering(tagId: TagID): ListAlgorithm { + return this.listAlgorithms[tagId]; + } + + public async setListOrdering(tagId: TagID, order: ListAlgorithm) { + if (!tagId) throw new Error("Tag ID must be defined"); + if (!order) throw new Error("Algorithm must be defined"); + this.listAlgorithms[tagId] = order; + + const algorithm = getListAlgorithmInstance(order, tagId, this.sortAlgorithms[tagId]); + this.algorithms[tagId] = algorithm; + + await algorithm.setRooms(this._cachedRooms[tagId]) + this._cachedRooms[tagId] = algorithm.orderedRooms; + this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list + this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed } public addFilterCondition(filterCondition: IFilterCondition): void { @@ -310,11 +344,21 @@ export abstract class Algorithm extends EventEmitter { * as reference for which lists to generate and which way to generate * them. * @param {ITagSortingMap} tagSortingMap The tags to generate. + * @param {IListOrderingMap} listOrderingMap The ordering of those tags. * @returns {Promise<*>} A promise which resolves when complete. */ - public async populateTags(tagSortingMap: ITagSortingMap): Promise { - if (!tagSortingMap) throw new Error(`Map cannot be null or empty`); + public async populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): Promise { + if (!tagSortingMap) throw new Error(`Sorting map cannot be null or empty`); + if (!listOrderingMap) throw new Error(`Ordering ma cannot be null or empty`); + if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) { + throw new Error(`Both maps must contain the exact same tags`); + } this.sortAlgorithms = tagSortingMap; + this.listAlgorithms = listOrderingMap; + this.algorithms = {}; + for (const tag of Object.keys(tagSortingMap)) { + this.algorithms[tag] = getListAlgorithmInstance(this.listAlgorithms[tag], tag, this.sortAlgorithms[tag]); + } return this.setKnownRooms(this.rooms); } @@ -428,7 +472,17 @@ export abstract class Algorithm extends EventEmitter { * be mutated in place. * @returns {Promise<*>} A promise which resolves when complete. */ - protected abstract generateFreshTags(updatedTagMap: ITagMap): Promise; + private async generateFreshTags(updatedTagMap: ITagMap): Promise { + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + + for (const tag of Object.keys(updatedTagMap)) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.setRooms(updatedTagMap[tag]); + updatedTagMap[tag] = algorithm.orderedRooms; + } + } /** * Asks the Algorithm to update its knowledge of a room. For example, when @@ -441,5 +495,48 @@ export abstract class Algorithm extends EventEmitter { * depending on whether or not getOrderedRooms() should be called after * processing. */ - public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; + public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { + if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); + + if (cause === RoomUpdateCause.PossibleTagChange) { + // TODO: Be smarter and splice rather than regen the planet. + // TODO: No-op if no change. + await this.setKnownRooms(this.rooms); + return true; + } + + if (cause === RoomUpdateCause.NewRoom) { + // TODO: Be smarter and insert rather than regen the planet. + await this.setKnownRooms([room, ...this.rooms]); + return true; + } + + if (cause === RoomUpdateCause.RoomRemoved) { + // TODO: Be smarter and splice rather than regen the planet. + await this.setKnownRooms(this.rooms.filter(r => r !== room)); + return true; + } + + let tags = this.roomIdsToTags[room.roomId]; + if (!tags) { + console.warn(`No tags known for "${room.name}" (${room.roomId})`); + return false; + } + + let changed = false; + for (const tag of tags) { + const algorithm: OrderingAlgorithm = this.algorithms[tag]; + if (!algorithm) throw new Error(`No algorithm for ${tag}`); + + await algorithm.handleRoomUpdate(room, cause); + this.cachedRooms[tag] = algorithm.orderedRooms; + + // Flag that we've done something + this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list + this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed + changed = true; + } + + return true; + }; } diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 7cbcc504c2..325aaf19e6 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -15,12 +15,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomUpdateCause, TagID } from "../../models"; -import { ITagMap, SortAlgorithm } from "../models"; +import { SortAlgorithm } from "../models"; import { sortRoomsWithAlgorithm } from "../tag-sorting"; import * as Unread from '../../../../Unread'; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; /** * The determined category of a room. @@ -77,32 +77,16 @@ const CATEGORY_ORDER = [Category.Red, Category.Grey, Category.Bold, Category.Idl * within the same category. For more information, see the comments contained * within the class. */ -export class ImportanceAlgorithm extends Algorithm { +export class ImportanceAlgorithm extends OrderingAlgorithm { + // This tracks the category for the tag it represents by tracking the index of + // each category within the list, where zero is the top of the list. This then + // tracks when rooms change categories and splices the orderedRooms array as + // needed, preventing many ordering operations. - // HOW THIS WORKS - // -------------- - // - // This block of comments assumes you've read the README two levels higher. - // You should do that if you haven't already. - // - // Tags are fed into the algorithmic functions from the Algorithm superclass, - // which cause subsequent updates to the room list itself. Categories within - // those tags are tracked as index numbers within the array (zero = top), with - // each sticky room being tracked separately. Internally, the category index - // can be found from `this.indices[tag][category]`. - // - // The room list store is always provided with the `this.cachedRooms` results, which are - // updated as needed and not recalculated often. For example, when a room needs to - // move within a tag, the array in `this.cachedRooms` will be spliced instead of iterated. - // The `indices` help track the positions of each category to make splicing easier. + private indices: ICategoryIndex = {}; - private indices: { - // @ts-ignore - TS wants this to be a string but we know better than it - [tag: TagID]: ICategoryIndex; - } = {}; - - constructor() { - super(); + public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + super(tagId, initialSortingAlgorithm); console.log("Constructed an ImportanceAlgorithm"); } @@ -143,122 +127,86 @@ export class ImportanceAlgorithm extends Algorithm { return Category.Idle; } - protected async generateFreshTags(updatedTagMap: ITagMap): Promise { - for (const tagId of Object.keys(updatedTagMap)) { - const unorderedRooms = updatedTagMap[tagId]; - - const sortBy = this.sortAlgorithms[tagId]; - if (!sortBy) throw new Error(`${tagId} does not have a sorting algorithm`); - - if (sortBy === SortAlgorithm.Manual) { - // Manual tags essentially ignore the importance algorithm, so don't do anything - // special about them. - updatedTagMap[tagId] = await sortRoomsWithAlgorithm(unorderedRooms, tagId, sortBy); - } else { - // Every other sorting type affects the categories, not the whole tag. - const categorized = this.categorizeRooms(unorderedRooms); - for (const category of Object.keys(categorized)) { - const roomsToOrder = categorized[category]; - categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, tagId, sortBy); - } - - const newlyOrganized: Room[] = []; - const newIndices: ICategoryIndex = {}; - - for (const category of CATEGORY_ORDER) { - newIndices[category] = newlyOrganized.length; - newlyOrganized.push(...categorized[category]); - } - - this.indices[tagId] = newIndices; - updatedTagMap[tagId] = newlyOrganized; + public async setRooms(rooms: Room[]): Promise { + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); + } else { + // Every other sorting type affects the categories, not the whole tag. + const categorized = this.categorizeRooms(rooms); + for (const category of Object.keys(categorized)) { + const roomsToOrder = categorized[category]; + categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm); } + + const newlyOrganized: Room[] = []; + const newIndices: ICategoryIndex = {}; + + for (const category of CATEGORY_ORDER) { + newIndices[category] = newlyOrganized.length; + newlyOrganized.push(...categorized[category]); + } + + this.indices = newIndices; + this.cachedOrderedRooms = newlyOrganized; } } public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { - if (cause === RoomUpdateCause.PossibleTagChange) { - // TODO: Be smarter and splice rather than regen the planet. - // TODO: No-op if no change. - await this.setKnownRooms(this.rooms); - return; + // TODO: Handle NewRoom and RoomRemoved + if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { + throw new Error(`Unsupported update cause: ${cause}`); } - if (cause === RoomUpdateCause.NewRoom) { - // TODO: Be smarter and insert rather than regen the planet. - await this.setKnownRooms([room, ...this.rooms]); - return; - } - - if (cause === RoomUpdateCause.RoomRemoved) { - // TODO: Be smarter and splice rather than regen the planet. - await this.setKnownRooms(this.rooms.filter(r => r !== room)); - return; - } - - let tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); - return false; - } const category = this.getRoomCategory(room); - let changed = false; - for (const tag of tags) { - if (this.sortAlgorithms[tag] === SortAlgorithm.Manual) { - continue; // Nothing to do here. - } - - const taggedRooms = this.cachedRooms[tag]; - const indices = this.indices[tag]; - let roomIdx = taggedRooms.indexOf(room); - if (roomIdx === -1) { - console.warn(`Degrading performance to find missing room in "${tag}": ${room.roomId}`); - roomIdx = taggedRooms.findIndex(r => r.roomId === room.roomId); - } - if (roomIdx === -1) { - throw new Error(`Room ${room.roomId} has no index in ${tag}`); - } - - // Try to avoid doing array operations if we don't have to: only move rooms within - // the categories if we're jumping categories - const oldCategory = this.getCategoryFromIndices(roomIdx, indices); - if (oldCategory !== category) { - // Move the room and update the indices - this.moveRoomIndexes(1, oldCategory, category, indices); - taggedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) - taggedRooms.splice(indices[category], 0, room); // splice in the new room (pre-adjusted) - // Note: if moveRoomIndexes() is called after the splice then the insert operation - // will happen in the wrong place. Because we would have already adjusted the index - // for the category, we don't need to determine how the room is moving in the list. - // If we instead tried to insert before updating the indices, we'd have to determine - // whether the room was moving later (towards IDLE) or earlier (towards RED) from its - // current position, as it'll affect the category's start index after we remove the - // room from the array. - } - - // The room received an update, so take out the slice and sort it. This should be relatively - // quick because the room is inserted at the top of the category, and most popular sorting - // algorithms will deal with trying to keep the active room at the top/start of the category. - // For the few algorithms that will have to move the thing quite far (alphabetic with a Z room - // for example), the list should already be sorted well enough that it can rip through the - // array and slot the changed room in quickly. - const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1] - ? Number.MAX_SAFE_INTEGER - : indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]]; - const startIdx = indices[category]; - const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine - const unsortedSlice = taggedRooms.splice(startIdx, numSort); - const sorted = await sortRoomsWithAlgorithm(unsortedSlice, tag, this.sortAlgorithms[tag]); - taggedRooms.splice(startIdx, 0, ...sorted); - - // Finally, flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + return; // Nothing to do here. } - return changed; + + let roomIdx = this.cachedOrderedRooms.indexOf(room); + if (roomIdx === -1) { // can only happen if the js-sdk's store goes sideways. + console.warn(`Degrading performance to find missing room in "${this.tagId}": ${room.roomId}`); + roomIdx = this.cachedOrderedRooms.findIndex(r => r.roomId === room.roomId); + } + if (roomIdx === -1) { + throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); + } + + // Try to avoid doing array operations if we don't have to: only move rooms within + // the categories if we're jumping categories + const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); + if (oldCategory !== category) { + // Move the room and update the indices + this.moveRoomIndexes(1, oldCategory, category, this.indices); + this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) + this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + // Note: if moveRoomIndexes() is called after the splice then the insert operation + // will happen in the wrong place. Because we would have already adjusted the index + // for the category, we don't need to determine how the room is moving in the list. + // If we instead tried to insert before updating the indices, we'd have to determine + // whether the room was moving later (towards IDLE) or earlier (towards RED) from its + // current position, as it'll affect the category's start index after we remove the + // room from the array. + } + + // The room received an update, so take out the slice and sort it. This should be relatively + // quick because the room is inserted at the top of the category, and most popular sorting + // algorithms will deal with trying to keep the active room at the top/start of the category. + // For the few algorithms that will have to move the thing quite far (alphabetic with a Z room + // for example), the list should already be sorted well enough that it can rip through the + // array and slot the changed room in quickly. + const nextCategoryStartIdx = category === CATEGORY_ORDER[CATEGORY_ORDER.length - 1] + ? Number.MAX_SAFE_INTEGER + : this.indices[CATEGORY_ORDER[CATEGORY_ORDER.indexOf(category) + 1]]; + const startIdx = this.indices[category]; + const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine + const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort); + const sorted = await sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); + this.cachedOrderedRooms.splice(startIdx, 0, ...sorted); + + return true; // change made } + // noinspection JSMethodCanBeStatic private getCategoryFromIndices(index: number, indices: ICategoryIndex): Category { for (let i = 0; i < CATEGORY_ORDER.length; i++) { const category = CATEGORY_ORDER[i]; @@ -274,6 +222,7 @@ export class ImportanceAlgorithm extends Algorithm { throw new Error("Programming error: somehow you've ended up with an index that isn't in a category"); } + // noinspection JSMethodCanBeStatic private moveRoomIndexes(nRooms: number, fromCategory: Category, toCategory: Category, indices: ICategoryIndex) { // We have to update the index of the category *after* the from/toCategory variables // in order to update the indices correctly. Because the room is moving from/to those diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index d544b1196f..cce7372986 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -14,49 +14,37 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; -import { ITagMap } from "../models"; +import { SortAlgorithm } from "../models"; import { sortRoomsWithAlgorithm } from "../tag-sorting"; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; +import { RoomUpdateCause, TagID } from "../../models"; +import { Room } from "matrix-js-sdk/src/models/room"; /** * Uses the natural tag sorting algorithm order to determine tag ordering. No * additional behavioural changes are present. */ -export class NaturalAlgorithm extends Algorithm { +export class NaturalAlgorithm extends OrderingAlgorithm { - constructor() { - super(); + public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + super(tagId, initialSortingAlgorithm); console.log("Constructed a NaturalAlgorithm"); } - protected async generateFreshTags(updatedTagMap: ITagMap): Promise { - for (const tagId of Object.keys(updatedTagMap)) { - const unorderedRooms = updatedTagMap[tagId]; - - const sortBy = this.sortAlgorithms[tagId]; - if (!sortBy) throw new Error(`${tagId} does not have a sorting algorithm`); - - updatedTagMap[tagId] = await sortRoomsWithAlgorithm(unorderedRooms, tagId, sortBy); - } + public async setRooms(rooms: Room[]): Promise { + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); } public async handleRoomUpdate(room, cause): Promise { - const tags = this.roomIdsToTags[room.roomId]; - if (!tags) { - console.warn(`No tags known for "${room.name}" (${room.roomId})`); - return false; + // TODO: Handle NewRoom and RoomRemoved + if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { + throw new Error(`Unsupported update cause: ${cause}`); } - let changed = false; - for (const tag of tags) { - // TODO: Optimize this loop to avoid useless operations - // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags - this.cachedRooms[tag] = await sortRoomsWithAlgorithm(this.cachedRooms[tag], tag, this.sortAlgorithms[tag]); - // Flag that we've done something - this.recalculateFilteredRoomsForTag(tag); // update filter to re-sort the list - this.recalculateStickyRoom(tag); // update sticky room to make sure it appears if needed - changed = true; - } - return changed; + // TODO: Optimize this to avoid useless operations + // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags + this.cachedOrderedRooms = await sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm); + + return true; } } diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts new file mode 100644 index 0000000000..263e8a4cd4 --- /dev/null +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -0,0 +1,72 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { Room } from "matrix-js-sdk/src/models/room"; +import { RoomUpdateCause, TagID } from "../../models"; +import { SortAlgorithm } from "../models"; + +/** + * Represents a list ordering algorithm. Subclasses should populate the + * `cachedOrderedRooms` field. + */ +export abstract class OrderingAlgorithm { + protected cachedOrderedRooms: Room[]; + protected sortingAlgorithm: SortAlgorithm; + + protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { + // noinspection JSIgnoredPromiseFromCall + this.setSortAlgorithm(initialSortingAlgorithm); // we use the setter for validation + } + + /** + * The rooms as ordered by the algorithm. + */ + public get orderedRooms(): Room[] { + return this.cachedOrderedRooms || []; + } + + /** + * Sets the sorting algorithm to use within the list. + * @param newAlgorithm The new algorithm. Must be defined. + * @returns Resolves when complete. + */ + public async setSortAlgorithm(newAlgorithm: SortAlgorithm) { + if (!newAlgorithm) throw new Error("A sorting algorithm must be defined"); + this.sortingAlgorithm = newAlgorithm; + + // Force regeneration of the rooms + await this.setRooms(this.orderedRooms); + } + + /** + * Sets the rooms the algorithm should be handling, implying a reconstruction + * of the ordering. + * @param rooms The rooms to use going forward. + * @returns Resolves when complete. + */ + public abstract setRooms(rooms: Room[]): Promise; + + /** + * Handle a room update. The Algorithm will only call this for causes which + * the list ordering algorithm can handle within the same tag. For example, + * tag changes will not be sent here. + * @param room The room where the update happened. + * @param cause The cause of the update. + * @returns True if the update requires the Algorithm to update the presentation layers. + */ + // XXX: TODO: We assume this will only ever be a position update and NOT a NewRoom or RemoveRoom change!! + public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; +} diff --git a/src/stores/room-list/algorithms/list-ordering/index.ts b/src/stores/room-list/algorithms/list-ordering/index.ts index bcccd150cd..8156c3a250 100644 --- a/src/stores/room-list/algorithms/list-ordering/index.ts +++ b/src/stores/room-list/algorithms/list-ordering/index.ts @@ -14,25 +14,32 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Algorithm } from "./Algorithm"; import { ImportanceAlgorithm } from "./ImportanceAlgorithm"; -import { ListAlgorithm } from "../models"; +import { ListAlgorithm, SortAlgorithm } from "../models"; import { NaturalAlgorithm } from "./NaturalAlgorithm"; +import { TagID } from "../../models"; +import { OrderingAlgorithm } from "./OrderingAlgorithm"; -const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: () => Algorithm } = { - [ListAlgorithm.Natural]: () => new NaturalAlgorithm(), - [ListAlgorithm.Importance]: () => new ImportanceAlgorithm(), +interface AlgorithmFactory { + (tagId: TagID, initialSortingAlgorithm: SortAlgorithm): OrderingAlgorithm; +} + +const ALGORITHM_FACTORIES: { [algorithm in ListAlgorithm]: AlgorithmFactory } = { + [ListAlgorithm.Natural]: (tagId, initSort) => new NaturalAlgorithm(tagId, initSort), + [ListAlgorithm.Importance]: (tagId, initSort) => new ImportanceAlgorithm(tagId, initSort), }; /** * Gets an instance of the defined algorithm * @param {ListAlgorithm} algorithm The algorithm to get an instance of. + * @param {TagID} tagId The tag the algorithm is for. + * @param {SortAlgorithm} initSort The initial sorting algorithm for the ordering algorithm. * @returns {Algorithm} The algorithm instance. */ -export function getListAlgorithmInstance(algorithm: ListAlgorithm): Algorithm { +export function getListAlgorithmInstance(algorithm: ListAlgorithm, tagId: TagID, initSort: SortAlgorithm): OrderingAlgorithm { if (!ALGORITHM_FACTORIES[algorithm]) { throw new Error(`${algorithm} is not a known algorithm`); } - return ALGORITHM_FACTORIES[algorithm](); + return ALGORITHM_FACTORIES[algorithm](tagId, initSort); } diff --git a/src/stores/room-list/algorithms/models.ts b/src/stores/room-list/algorithms/models.ts index 284600a776..943d833b67 100644 --- a/src/stores/room-list/algorithms/models.ts +++ b/src/stores/room-list/algorithms/models.ts @@ -16,6 +16,7 @@ limitations under the License. import { TagID } from "../models"; import { Room } from "matrix-js-sdk/src/models/room"; +import { OrderingAlgorithm } from "./list-ordering/OrderingAlgorithm"; export enum SortAlgorithm { Manual = "MANUAL", @@ -36,6 +37,16 @@ export interface ITagSortingMap { [tagId: TagID]: SortAlgorithm; } +export interface IListOrderingMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: ListAlgorithm; +} + +export interface IOrderingAlgorithmMap { + // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. + [tagId: TagID]: OrderingAlgorithm; +} + export interface ITagMap { // @ts-ignore - TypeScript really wants this to be [tagId: string] but we know better. [tagId: TagID]: Room[];