Merge pull request #4918 from matrix-org/travis/room-list/respect-old-settings

Respect and fix understanding of legacy options in new room list

src/settings/Settings.js

@@ -478,13 +478,13 @@ export const SETTINGS = {
             deny: [],
         },
     },
-    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14231
+    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14373
     "RoomList.orderAlphabetically": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("Order rooms by name"),
         default: false,
     },
-    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14231
+    // TODO: Remove setting: https://github.com/vector-im/riot-web/issues/14373
     "RoomList.orderByImportance": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td("Show rooms with unread notifications first"),

src/stores/room-list/RoomListStore2.ts

@@ -31,6 +31,7 @@ import RoomViewStore from "../RoomViewStore";
 import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm";
 import { EffectiveMembership, getEffectiveMembership } from "./membership";
 import { ListLayout } from "./ListLayout";
+import { isNullOrUndefined } from "matrix-js-sdk/src/utils";
 
 interface IState {
     tagsEnabled?: boolean;
@@ -318,6 +319,28 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         return <SortAlgorithm>localStorage.getItem(`mx_tagSort_${tagId}`);
     }
 
+    // logic must match calculateListOrder
+    private calculateTagSorting(tagId: TagID): SortAlgorithm {
+        const defaultSort = SortAlgorithm.Alphabetic;
+        const settingAlphabetical = SettingsStore.getValue("RoomList.orderAlphabetically", null, true);
+        const definedSort = this.getTagSorting(tagId);
+        const storedSort = this.getStoredTagSorting(tagId);
+
+        // We use the following order to determine which of the 4 flags to use:
+        // Stored > Settings > Defined > Default
+
+        let tagSort = defaultSort;
+        if (storedSort) {
+            tagSort = storedSort;
+        } else if (!isNullOrUndefined(settingAlphabetical)) {
+            tagSort = settingAlphabetical ? SortAlgorithm.Alphabetic : SortAlgorithm.Recent;
+        } else if (definedSort) {
+            tagSort = definedSort;
+        } // else default (already set)
+
+        return tagSort;
+    }
+
     public async setListOrder(tagId: TagID, order: ListAlgorithm) {
         await this.algorithm.setListOrdering(tagId, order);
         // TODO: Per-account? https://github.com/vector-im/riot-web/issues/14114
@@ -334,19 +357,35 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         return <ListAlgorithm>localStorage.getItem(`mx_listOrder_${tagId}`);
     }
 
-    private async updateAlgorithmInstances() {
-        const defaultSort = SortAlgorithm.Alphabetic;
+    // logic must match calculateTagSorting
+    private calculateListOrder(tagId: TagID): ListAlgorithm {
         const defaultOrder = ListAlgorithm.Natural;
+        const settingImportance = SettingsStore.getValue("RoomList.orderByImportance", null, true);
+        const definedOrder = this.getListOrder(tagId);
+        const storedOrder = this.getStoredListOrder(tagId);
 
+        // We use the following order to determine which of the 4 flags to use:
+        // Stored > Settings > Defined > Default
+
+        let listOrder = defaultOrder;
+        if (storedOrder) {
+            listOrder = storedOrder;
+        } else if (!isNullOrUndefined(settingImportance)) {
+            listOrder = settingImportance ? ListAlgorithm.Importance : ListAlgorithm.Natural;
+        } else if (definedOrder) {
+            listOrder = definedOrder;
+        } // else default (already set)
+
+        return listOrder;
+    }
+
+    private async updateAlgorithmInstances() {
         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);
+            const tagSort = this.calculateTagSorting(tag);
+            const listOrder = this.calculateListOrder(tag);
 
             if (tagSort !== definedSort) {
                 await this.setTagSorting(tag, tagSort);
@@ -375,8 +414,8 @@ export class RoomListStore2 extends AsyncStore<ActionPayload> {
         const sorts: ITagSortingMap = {};
         const orders: IListOrderingMap = {};
         for (const tagId of OrderedDefaultTagIDs) {
-            sorts[tagId] = this.getStoredTagSorting(tagId) || SortAlgorithm.Alphabetic;
-            orders[tagId] = this.getStoredListOrder(tagId) || ListAlgorithm.Natural;
+            sorts[tagId] = this.calculateTagSorting(tagId);
+            orders[tagId] = this.calculateListOrder(tagId);
         }
 
         if (this.state.tagsEnabled) {

src/stores/room-list/algorithms/Algorithm.ts

@@ -109,6 +109,7 @@ export class Algorithm extends EventEmitter {
     }
 
     public getTagSorting(tagId: TagID): SortAlgorithm {
+        if (!this.sortAlgorithms) return null;
         return this.sortAlgorithms[tagId];
     }
 
@@ -125,6 +126,7 @@ export class Algorithm extends EventEmitter {
     }
 
     public getListOrdering(tagId: TagID): ListAlgorithm {
+        if (!this.listAlgorithms) return null;
         return this.listAlgorithms[tagId];
     }