Iterate things

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

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

@@ -48,7 +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.orderByRecents',
         'RoomList.orderByImportance',
         'breadcrumbs',
     ];

src/settings/Settings.js

@@ -26,7 +26,6 @@ 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'];
@@ -434,11 +433,10 @@ export const SETTINGS = {
             deny: [],
         },
     },
-    "RoomList.orderingAlgorithm": {
+    "RoomList.orderByRecents": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("Order rooms by message activity instead of by name"),
-        default: "recent", // controller maps boolean onto algorithm for future flexibility to >2 algorithms
+        default: true,
-        controller: new RoomListOrderingController(),
     },
     "RoomList.orderByImportance": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,

src/settings/SettingsStore.js

@@ -420,13 +420,9 @@ 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

@@ -1,35 +0,0 @@
-/*
-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";
-    }
-
-    getValueOverride(level: string, roomId: String, calculatedValue: *, calculatedAtLevel: string): * {
-        switch (calculatedValue) {
-            case "alphabetic":
-                return false;
-            case "recent":
-            default:
-                return true;
-        }
-    }
-}

src/stores/RoomListStore.js

@@ -56,6 +56,8 @@ export const ALGO_RECENT = "recent";
 
 const CATEGORY_ORDER = [CATEGORY_RED, CATEGORY_GREY, CATEGORY_BOLD, CATEGORY_IDLE];
 const getListAlgorithm = (listKey, settingAlgorithm) => {
+    // apply manual sorting only to m.favourite, otherwise respect the global setting
+    // all the known tags are listed explicitly here to simplify future changes
     switch (listKey) {
         case "m.favourite":
             return ALGO_MANUAL;
@@ -64,9 +66,8 @@ const getListAlgorithm = (listKey, settingAlgorithm) => {
         case "im.vector.fake.direct":
         case "im.vector.fake.archived":
         case "m.lowpriority":
-            return settingAlgorithm;
         default:
-            return ALGO_MANUAL; // TODO verify this is desired
+            return settingAlgorithm;
     }
 };
 
@@ -129,7 +130,7 @@ class RoomListStore extends Store {
         };
 
         SettingsStore.monitorSetting('RoomList.orderByImportance', null);
-        SettingsStore.monitorSetting('RoomList.orderingAlgorithm', null);
+        SettingsStore.monitorSetting('RoomList.orderByRecents', null);
         SettingsStore.monitorSetting('feature_custom_tags', null);
     }
 
@@ -156,8 +157,9 @@ class RoomListStore extends Store {
                 if (!logicallyReady) break;
 
                 switch (payload.settingName) {
-                    case "RoomList.orderingAlgorithm":
+                    case "RoomList.orderByRecents":
-                        this.updateSortingAlgorithm(payload.newValue, this._state.orderImportantFirst);
+                        this.updateSortingAlgorithm(payload.newValue ? ALGO_RECENT : ALGO_ALPHABETIC,
+                            this._state.orderImportantFirst);
                         break;
                     case "RoomList.orderByImportance":
                         this.updateSortingAlgorithm(this._state.algorithm, payload.newValue);
@@ -183,9 +185,9 @@ class RoomListStore extends Store {
 
                 this._matrixClient = payload.matrixClient;
 
-                const algorithm = SettingsStore.getValue("RoomList.orderingAlgorithm");
+                const orderByRecents = SettingsStore.getValue("RoomList.orderByRecents");
                 const orderByImportance = SettingsStore.getValue("RoomList.orderByImportance");
-                this.updateSortingAlgorithm(algorithm, orderByImportance);
+                this.updateSortingAlgorithm(orderByRecents ? ALGO_RECENT : ALGO_ALPHABETIC, orderByImportance);
             }
             break;
             case 'MatrixActions.Room.receipt': {
@@ -446,11 +448,9 @@ 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;
@@ -502,19 +502,16 @@ 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 (getListAlgorithm(key, this._state.algorithm) === ALGO_MANUAL) {
+            } else if (getListAlgorithm(key, this._state.algorithm) === ALGO_MANUAL || !this._state.orderImportantFirst) {
                 // 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);
 
@@ -561,23 +558,16 @@ 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 (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});
     }
 
@@ -660,27 +650,36 @@ class RoomListStore extends Store {
 
         Object.keys(lists).forEach((listKey) => {
             let comparator;
-            console.log("DEBUG sorting", listKey, "using", getListAlgorithm(listKey, this._state.algorithm));
             switch (getListAlgorithm(listKey, this._state.algorithm)) {
                 case ALGO_RECENT:
-                    comparator = (entryA, entryB) => {
+                    comparator = (entryA, entryB) => this._recentsComparator(entryA, entryB, tsOfNewestEventFn);
-                        return this._recentsComparator(entryA, entryB, tsOfNewestEventFn);
-                    };
                     break;
                 case ALGO_ALPHABETIC:
-                    comparator = (entryA, entryB) => this._alphabeticComparator(entryA, entryB);
+                    comparator = this._lexicographicalComparator;
                     break;
                 case ALGO_MANUAL:
                 default:
                     comparator = this._getManualComparator(listKey);
                     break;
             }
-            console.log("DEBUG before", listKey, lists[listKey].map(e => e.room.name));
+
-            lists[listKey].sort(comparator); // TODO inline the common CATEGORY comparator here?
+            if (this._state.orderImportantFirst) {
-            console.log("DEBUG after", listKey, lists[listKey].map(e => e.room.name));
+                lists[listKey].sort((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
+                    }
+                    return comparator(entryA, entryB);
+                });
+            } else {
+                // skip the category comparison even though it should no-op when orderImportantFirst disabled
+                lists[listKey].sort(comparator);
+            }
         });
 
-        console.log("DEBUG setting lists after comparator");
         this._setState({
             lists,
             ready: true, // Ready to receive updates to ordering
@@ -739,34 +738,13 @@ class RoomListStore extends Store {
 
     _recentsComparator(entryA, entryB, tsOfNewestEventFn) {
         console.trace("DEBUG recents");
-        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
-        }
-
         const timestampA = tsOfNewestEventFn(entryA.room);
         const timestampB = tsOfNewestEventFn(entryB.room);
         return timestampB - timestampA;
     }
 
-    _alphabeticComparator(entryA, entryB) {
+    _lexicographicalComparator(entryA, entryB) {
-        if (entryA.category !== entryB.category) {
+        return entryA.room.name.localeCompare(entryB.room.name);
-            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.localeCompare(roomB.name);
     }
 
     _getManualComparator(tagName, optimisticRequest) {
@@ -791,7 +769,7 @@ class RoomListStore extends Store {
                 return -1;
             }
 
-            return a === b ? this._lexicographicalComparator(roomA, roomB) : (a > b ? 1 : -1);
+            return a === b ? this._lexicographicalComparator(entryA, entryB) : (a > b ? 1 : -1);
         };
     }