First attempt alpha room list sorting

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js

@@ -48,6 +48,7 @@ export default class PreferencesUserSettingsTab extends React.Component {
     ];
 
     static ROOM_LIST_SETTINGS = [
+        'RoomList.orderingAlgorithm', // this has a controller which maps the boolean inputs to algorithms
         'RoomList.orderByImportance',
         'breadcrumbs',
     ];

src/i18n/strings/en_EN.json

@@ -413,8 +413,9 @@
     "Enable widget screenshots on supported widgets": "Enable widget screenshots on supported widgets",
     "Prompt before sending invites to potentially invalid matrix IDs": "Prompt before sending invites to potentially invalid matrix IDs",
     "Show developer tools": "Show developer tools",
-    "Order rooms in the room list by most important first instead of most recent": "Order rooms in the room list by most important first instead of most recent",
+    "Order rooms by message activity instead of by name": "Order rooms by message activity instead of by name",
-    "Show recently visited rooms above the room list": "Show recently visited rooms above the room list",
+    "Show rooms with unread notifications first": "Show rooms with unread notifications first",
+    "Show shortcuts to recently viewed rooms above the room list": "Show shortcuts to recently viewed rooms above the room list",
     "Show hidden events in timeline": "Show hidden events in timeline",
     "Low bandwidth mode": "Low bandwidth mode",
     "Allow fallback call assist server turn.matrix.org when your homeserver does not offer one (your IP address would be shared during a call)": "Allow fallback call assist server turn.matrix.org when your homeserver does not offer one (your IP address would be shared during a call)",

src/settings/Settings.js

@@ -26,6 +26,7 @@ import CustomStatusController from "./controllers/CustomStatusController";
 import ThemeController from './controllers/ThemeController';
 import ReloadOnChangeController from "./controllers/ReloadOnChangeController";
 import {RIGHT_PANEL_PHASES} from "../stores/RightPanelStorePhases";
+import RoomListOrderingController from "./controllers/RoomListOrderingController";
 
 // These are just a bunch of helper arrays to avoid copy/pasting a bunch of times
 const LEVELS_ROOM_SETTINGS = ['device', 'room-device', 'room-account', 'account', 'config'];
@@ -433,14 +434,20 @@ export const SETTINGS = {
             deny: [],
         },
     },
+    "RoomList.orderingAlgorithm": {
+        supportedLevels: LEVELS_ACCOUNT_SETTINGS,
+        displayName: _td("Order rooms by message activity instead of by name"),
+        default: "recent", // XXX controller maps boolean onto algorithm for future flexibility
+        controller: new RoomListOrderingController(),
+    },
     "RoomList.orderByImportance": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
-        displayName: _td('Order rooms in the room list by most important first instead of most recent'),
+        displayName: _td("Show rooms with unread notifications first"),
         default: true,
     },
     "breadcrumbs": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
-        displayName: _td("Show recently visited rooms above the room list"),
+        displayName: _td("Show shortcuts to recently viewed rooms above the room list"),
         default: true,
     },
     "showHiddenEventsInTimeline": {

src/settings/SettingsStore.js

@@ -420,9 +420,13 @@ export default class SettingsStore {
             throw new Error("User cannot set " + settingName + " at " + level + " in " + roomId);
         }
 
+        const controller = setting.controller;
+        if (controller) {
+            value = controller.augmentValue(level, roomId, value);
+        }
+
         await handler.setValue(settingName, roomId, value);
 
-        const controller = setting.controller;
         if (controller) {
             controller.onChange(level, roomId, value);
         }

src/settings/controllers/RoomListOrderingController.js

@@ -0,0 +1,25 @@
+/*
+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 SettingController from "./SettingController";
+
+export default class RoomListOrderingController extends SettingController {
+    augmentValue(level, roomId, newValue): * {
+        // currently we expose algorithm as a boolean but store it as a string for future flexibility
+        // where we may want >2 algorithms available for the user to choose between.
+        return newValue ? "recent" : "alphabetic";
+    }
+}

src/settings/controllers/SettingController.js

@@ -47,4 +47,15 @@ export default class SettingController {
     onChange(level, roomId, newValue) {
         // do nothing by default
     }
+
+    /**
+     * Gets the value which should actually get written into the store based on the input value from setValue.
+     * @param {string} level The level at which the setting has been modified.
+     * @param {String} roomId The room ID, may be null.
+     * @param {*} newValue The new value for the setting, may be null.
+     * @return {*} The value that should be used, may be null.
+     */
+    augmentValue(level, roomId, newValue) {
+        return newValue;
+    }
 }

src/stores/RoomListStore.js

@@ -36,29 +36,48 @@ const CATEGORY_GREY = "grey";   // Unread notified messages (not mentions)
 const CATEGORY_BOLD = "bold";   // Unread messages (not notified, 'Mentions Only' rooms)
 const CATEGORY_IDLE = "idle";   // Nothing of interest
 
-const CATEGORY_ORDER = [CATEGORY_RED, CATEGORY_GREY, CATEGORY_BOLD, CATEGORY_IDLE];
-const LIST_ORDERS = {
-    "m.favourite": "manual",
-    "im.vector.fake.invite": "recent",
-    "im.vector.fake.recent": "recent",
-    "im.vector.fake.direct": "recent",
-    "m.lowpriority": "recent",
-    "im.vector.fake.archived": "recent",
-};
-
 /**
- * Identifier for the "breadcrumb" (or "sort by most important room first") algorithm.
+ * Identifier for manual sorting behaviour: sort by the user defined order.
- * Includes a provision for keeping the currently open room from flying down the room
- * list.
  * @type {string}
  */
-const ALGO_IMPORTANCE = "importance";
+export const ALGO_MANUAL = "manual";
+
+/**
+ * Identifier for alphabetic sorting behaviour: sort by the room name alphabetically first.
+ * @type {string}
+ */
+export const ALGO_ALPHABETIC = "alphabetic";
 
 /**
  * Identifier for classic sorting behaviour: sort by the most recent message first.
  * @type {string}
  */
-const ALGO_RECENT = "recent";
+export const ALGO_RECENT = "recent";
+
+const CATEGORY_ORDER = [CATEGORY_RED, CATEGORY_GREY, CATEGORY_BOLD, CATEGORY_IDLE];
+const getListAlgorithm = (listKey, settingAlgorithm) => {
+    switch (listKey) {
+        case "m.favourite":
+            return ALGO_MANUAL;
+        case "im.vector.fake.invite":
+        case "im.vector.fake.recent":
+        case "im.vector.fake.direct":
+        case "im.vector.fake.archived":
+        case "m.lowpriority":
+            return settingAlgorithm;
+        default:
+            return ALGO_MANUAL; // TODO verify this is desired
+    }
+};
+
+const knownLists = new Set([
+    "m.favourite",
+    "im.vector.fake.invite",
+    "im.vector.fake.recent",
+    "im.vector.fake.direct",
+    "im.vector.fake.archived",
+    "m.lowpriority",
+]);
 
 /**
  * A class for storing application state for categorising rooms in
@@ -76,13 +95,12 @@ class RoomListStore extends Store {
     /**
      * Changes the sorting algorithm used by the RoomListStore.
      * @param {string} algorithm The new algorithm to use. Should be one of the ALGO_* constants.
+     * @param {boolean} orderImportantFirst Whether to sort by categories of importance
      */
-    updateSortingAlgorithm(algorithm) {
+    updateSortingAlgorithm(algorithm, orderImportantFirst) {
         // Dev note: We only have two algorithms at the moment, but it isn't impossible that we want
         // multiple in the future. Also constants make things slightly clearer.
-        const byImportance = algorithm === ALGO_IMPORTANCE;
+        this._setState({algorithm, orderImportantFirst});
-        console.log("Updating room sorting algorithm: sortByImportance=" + byImportance);
-        this._setState({orderRoomsByImportance: byImportance});
 
         // Trigger a resort of the entire list to reflect the change in algorithm
         this._generateInitialRoomLists();
@@ -106,10 +124,12 @@ class RoomListStore extends Store {
             presentationLists: defaultLists, // like `lists`, but with arrays of rooms instead
             ready: false,
             stickyRoomId: null,
-            orderRoomsByImportance: true,
+            algorithm: ALGO_RECENT,
+            orderImportantFirst: false,
         };
 
         SettingsStore.monitorSetting('RoomList.orderByImportance', null);
+        SettingsStore.monitorSetting('RoomList.orderingAlgorithm', null);
         SettingsStore.monitorSetting('feature_custom_tags', null);
     }
 
@@ -135,11 +155,17 @@ class RoomListStore extends Store {
             case 'setting_updated': {
                 if (!logicallyReady) break;
 
-                if (payload.settingName === 'RoomList.orderByImportance') {
+                switch (payload.settingName) {
-                    this.updateSortingAlgorithm(payload.newValue === true ? ALGO_IMPORTANCE : ALGO_RECENT);
+                    case "RoomList.orderingAlgorithm":
-                } else if (payload.settingName === 'feature_custom_tags') {
+                        this.updateSortingAlgorithm(payload.newValue, this._state.orderImportantFirst);
-                    this._setState({tagsEnabled: payload.newValue});
+                        break;
-                    this._generateInitialRoomLists(); // Tags means we have to start from scratch
+                    case "RoomList.orderByImportance":
+                        this.updateSortingAlgorithm(this._state.algorithm, payload.newValue);
+                        break;
+                    case "feature_custom_tags":
+                        this._setState({tagsEnabled: payload.newValue});
+                        this._generateInitialRoomLists(); // Tags means we have to start from scratch
+                        break;
                 }
             }
             break;
@@ -157,9 +183,9 @@ class RoomListStore extends Store {
 
                 this._matrixClient = payload.matrixClient;
 
-                const algorithm = SettingsStore.getValue("RoomList.orderByImportance")
+                const algorithm = SettingsStore.getValue("RoomList.orderingAlgorithm");
-                    ? ALGO_IMPORTANCE : ALGO_RECENT;
+                const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance");
-                this.updateSortingAlgorithm(algorithm);
+                this.updateSortingAlgorithm(algorithm, orderByImportance);
             }
             break;
             case 'MatrixActions.Room.receipt': {
@@ -188,7 +214,8 @@ class RoomListStore extends Store {
                 if (!logicallyReady ||
                     !payload.isLiveEvent ||
                     !payload.isLiveUnfilteredRoomTimelineEvent ||
-                    !this._eventTriggersRecentReorder(payload.event)
+                    !this._eventTriggersRecentReorder(payload.event) ||
+                    this._state.algorithm !== ALGO_RECENT
                 ) {
                     break;
                 }
@@ -302,7 +329,7 @@ class RoomListStore extends Store {
     _filterTags(tags) {
         tags = tags ? Object.keys(tags) : [];
         if (this._state.tagsEnabled) return tags;
-        return tags.filter((t) => !!LIST_ORDERS[t]);
+        return tags.filter((t) => knownLists.has(t));
     }
 
     _getRecommendedTagsForRoom(room) {
@@ -419,9 +446,11 @@ class RoomListStore extends Store {
                     if (changedBoundary) {
                         // If we changed a boundary, then we've gone too far - go to the top of the last
                         // section instead.
+                        console.log("DEBUG changedBoundary", room.name, room, category);
                         newList.splice(desiredCategoryBoundaryIndex, 0, {room, category});
                     } else {
                         // If we're ordering by timestamp, just insert normally
+                        console.log("DEBUG 11push", room.name, room, category);
                         newList.push({room, category});
                     }
                     pushedEntry = true;
@@ -473,16 +502,19 @@ class RoomListStore extends Store {
 
             // Speed optimization: Don't do complicated math if we don't have to.
             if (!shouldHaveRoom) {
+                console.log("DEBUG A");
                 listsClone[key] = this._state.lists[key].filter((e) => e.room.roomId !== room.roomId);
-            } else if (LIST_ORDERS[key] !== 'recent') {
+            } else if (getListAlgorithm(key, this._state.algorithm) === ALGO_MANUAL) {
                 // Manually ordered tags are sorted later, so for now we'll just clone the tag
                 // and add our room if needed
                 listsClone[key] = this._state.lists[key].filter((e) => e.room.roomId !== room.roomId);
+                console.log("DEBUG push", room.name, room, category);
                 listsClone[key].push({room, category});
                 insertedIntoTags.push(key);
             } else {
                 listsClone[key] = [];
 
+                console.log("DEBUG slot");
                 const pushedEntry = this._slotRoomIntoList(
                     room, category, key, this._state.lists[key], listsClone[key], lastTimestamp);
 
@@ -529,16 +561,23 @@ class RoomListStore extends Store {
                     console.warn(`!! List for tag ${targetTag} does not exist - creating`);
                     listsClone[targetTag] = [];
                 }
+                console.log("DEBUG123", room.name, room, category);
                 listsClone[targetTag].splice(0, 0, {room, category});
             }
         }
 
+        console.log("DEBUG targetTags", targetTags);
+
         // Sort the favourites before we set the clone
         for (const tag of Object.keys(listsClone)) {
-            if (LIST_ORDERS[tag] === 'recent') continue; // skip recents (pre-sorted)
+            if (getListAlgorithm(tag, this._state.algorithm) !== ALGO_MANUAL) continue; // skip recents (pre-sorted)
+            console.log("DEBUG applying manual sort to", tag);
             listsClone[tag].sort(this._getManualComparator(tag));
         }
 
+        console.log("DEBUG setting lists=listsClone",
+            this._state.lists["im.vector.fake.recent"].map(e => e.room.name),
+            listsClone["im.vector.fake.recent"].map(e => e.room.name));
         this._setState({lists: listsClone});
     }
 
@@ -585,8 +624,10 @@ class RoomListStore extends Store {
 
                         // Default to an arbitrary category for tags which aren't ordered by recents
                         let category = CATEGORY_IDLE;
-                        if (LIST_ORDERS[tagName] === 'recent') category = this._calculateCategory(room);
+                        if (getListAlgorithm(tagName, this._state.algorithm) !== ALGO_MANUAL) {
-                        lists[tagName].push({room, category: category});
+                            category = this._calculateCategory(room);
+                        }
+                        lists[tagName].push({room, category});
                     }
                 } else if (dmRoomMap.getUserIdForRoomId(room.roomId)) {
                     // "Direct Message" rooms (that we're still in and that aren't otherwise tagged)
@@ -605,33 +646,41 @@ class RoomListStore extends Store {
         // cache only needs to survive the sort operation below and should not be implemented outside
         // of this function, otherwise the room lists will almost certainly be out of date and wrong.
         const latestEventTsCache = {}; // roomId => timestamp
+        const tsOfNewestEventFn = (room) => {
+            if (!room) return Number.MAX_SAFE_INTEGER; // Should only happen in tests
+
+            if (latestEventTsCache[room.roomId]) {
+                return latestEventTsCache[room.roomId];
+            }
+
+            const ts = this._tsOfNewestEvent(room);
+            latestEventTsCache[room.roomId] = ts;
+            return ts;
+        };
 
         Object.keys(lists).forEach((listKey) => {
             let comparator;
-            switch (LIST_ORDERS[listKey]) {
+            console.log("DEBUG sorting", listKey, "using", getListAlgorithm(listKey, this._state.algorithm));
-                case "recent":
+            switch (getListAlgorithm(listKey, this._state.algorithm)) {
+                case ALGO_RECENT:
                     comparator = (entryA, entryB) => {
-                        return this._recentsComparator(entryA, entryB, (room) => {
+                        return this._recentsComparator(entryA, entryB, tsOfNewestEventFn);
-                            if (!room) return Number.MAX_SAFE_INTEGER; // Should only happen in tests
-
-                            if (latestEventTsCache[room.roomId]) {
-                                return latestEventTsCache[room.roomId];
-                            }
-
-                            const ts = this._tsOfNewestEvent(room);
-                            latestEventTsCache[room.roomId] = ts;
-                            return ts;
-                        });
                     };
                     break;
-                case "manual":
+                case ALGO_ALPHABETIC:
+                    comparator = (entryA, entryB) => this._alphabeticComparator(entryA, entryB);
+                    break;
+                case ALGO_MANUAL:
                 default:
                     comparator = this._getManualComparator(listKey);
                     break;
             }
-            lists[listKey].sort(comparator);
+            console.log("DEBUG before", listKey, lists[listKey].map(e => e.room.name));
+            lists[listKey].sort(comparator); // TODO inline the common CATEGORY comparator here?
+            console.log("DEBUG after", listKey, lists[listKey].map(e => e.room.name));
         });
 
+        console.log("DEBUG setting lists after comparator");
         this._setState({
             lists,
             ready: true, // Ready to receive updates to ordering
@@ -668,7 +717,7 @@ class RoomListStore extends Store {
     }
 
     _calculateCategory(room) {
-        if (!this._state.orderRoomsByImportance) {
+        if (!this._state.orderImportantFirst) {
             // Effectively disable the categorization of rooms if we're supposed to
             // be sorting by more recent messages first. This triggers the timestamp
             // comparison bit of _setRoomCategory and _recentsComparator instead of
@@ -689,26 +738,35 @@ class RoomListStore extends Store {
     }
 
     _recentsComparator(entryA, entryB, tsOfNewestEventFn) {
-        const roomA = entryA.room;
+        console.trace("DEBUG recents");
-        const roomB = entryB.room;
+        if (entryA.category !== entryB.category) {
-        const categoryA = entryA.category;
+            const idxA = CATEGORY_ORDER.indexOf(entryA.category);
-        const categoryB = entryB.category;
+            const idxB = CATEGORY_ORDER.indexOf(entryB.category);
-
-        if (categoryA !== categoryB) {
-            const idxA = CATEGORY_ORDER.indexOf(categoryA);
-            const idxB = CATEGORY_ORDER.indexOf(categoryB);
             if (idxA > idxB) return 1;
             if (idxA < idxB) return -1;
             return 0; // Technically not possible
         }
 
-        const timestampA = tsOfNewestEventFn(roomA);
+        const timestampA = tsOfNewestEventFn(entryA.room);
-        const timestampB = tsOfNewestEventFn(roomB);
+        const timestampB = tsOfNewestEventFn(entryB.room);
         return timestampB - timestampA;
     }
 
+    _alphabeticComparator(entryA, entryB) {
+        if (entryA.category !== entryB.category) {
+            const idxA = CATEGORY_ORDER.indexOf(entryA.category);
+            const idxB = CATEGORY_ORDER.indexOf(entryB.category);
+            if (idxA > idxB) return 1;
+            if (idxA < idxB) return -1;
+            return 0; // Technically not possible
+        }
+
+        // console.log("DEBUG alphabetic, same category", JSON.stringify(entryA.room.name), JSON.stringify(entryB.room.name), this._lexicographicalComparator(entryA.room, entryB.room));
+        return this._lexicographicalComparator(entryA.room, entryB.room);
+    }
+
     _lexicographicalComparator(roomA, roomB) {
-        return roomA.name > roomB.name ? 1 : -1;
+        return roomA.name.localeCompare(roomB.name);
     }
 
     _getManualComparator(tagName, optimisticRequest) {