refactor: move logic for loading and saving box orders to priv. methods

Move logic for loading and saving box orders from/to settings to private
methods to have more readable code.

src/extension.js

@@ -16,9 +16,10 @@ export default class TopBarOrganizerExtension extends Extension {
         // Initially handle new top bar items and order top bar boxes.
         this.#handleNewItemsAndOrderTopBar();
 
-        // Overwrite `Panel._addToPanelBox` method with one handling new items
+        // Overwrite the `Panel._addToPanelBox` method with one handling new
-        // and also handle AppIndicators getting ready, to handle new items.
+        // items.
         this.#overwritePanelAddToPanelBox();
+        // Handle AppIndicators getting ready, to handle new AppIndicator items.
         this._boxOrderManager.connect("appIndicatorReady", () => {
             this.#handleNewItemsAndOrderTopBar();
         });
@@ -95,7 +96,7 @@ export default class TopBarOrganizerExtension extends Extension {
         }
 
         // Get the valid box order.
-        const validBoxOrder = this._boxOrderManager.createValidBoxOrder(box);
+        const validBoxOrder = this._boxOrderManager.getValidBoxOrder(box);
 
         // Get the relevant box of `Main.panel`.
         let panelBox;
@@ -111,10 +112,10 @@ export default class TopBarOrganizerExtension extends Extension {
                 break;
         }
 
-        /// Go through the items (or rather their roles) of the validBoxOrder
+        /// Go through the items of the validBoxOrder and order the GNOME Shell
-        /// and order the panelBox accordingly.
+        /// top bar box accordingly.
         for (let i = 0; i < validBoxOrder.length; i++) {
-            const role = validBoxOrder[i];
+            const role = validBoxOrder[i].role;
             // Get the indicator container associated with the current role.
             const associatedIndicatorContainer = Main.panel.statusArea[role].container;
 

src/extensionModules/BoxOrderManager.js

@@ -5,10 +5,18 @@ import GObject from "gi://GObject";
 import * as Main from "resource:///org/gnome/shell/ui/main.js";
 
 /**
- * This class provides methods get, set and interact with box orders, while
+ * A resolved box order item containing the items role and settings identifier.
- * taking over the work of translating between what is stored in settings and
+ * @typedef {Object} ResolvedBoxOrderItem
- * what is really useable by the other extension code.
+ * @property {string} settingsId - The settings identifier of the item.
- * It's basically a heavy wrapper around the box orders stored in the settings.
+ * @property {string} role - The role of the item.
+ */
+
+/**
+ * This class provides an interfaces to the box orders stored in settings.
+ * It takes care of handling AppIndicator items and resolving from the internal
+ * item settings identifiers to roles.
+ * In the end this results in convenient functions, which are directly useful in
+ * other extension code.
  */
 export default class BoxOrderManager extends GObject.Object {
     static {
@@ -20,31 +28,68 @@ export default class BoxOrderManager extends GObject.Object {
     }
 
     #appIndicatorReadyHandlerIdMap;
-    #appIndicatorItemApplicationRoleMap;
+    #appIndicatorItemSettingsIdToRolesMap;
     #settings;
 
     constructor(params = {}, settings) {
         super(params);
 
         this.#appIndicatorReadyHandlerIdMap = new Map();
-        this.#appIndicatorItemApplicationRoleMap = new Map();
+        this.#appIndicatorItemSettingsIdToRolesMap = new Map();
 
         this.#settings = settings;
     }
 
     /**
-     * Handles an AppIndicator/KStatusNotifierItem item by associating the role
+     * Gets a box order for the given top bar box from settings.
-     * of the given item with the application of the
+     * @param {string} box - The top bar box for which to get the box order.
-     * AppIndicator/KStatusNotifier item and returning a placeholder role.
+     * Must be one of the following values:
-     * In the case, where the application can't be determined, this method
+     * - "left"
-     * throws an error. However it also makes sure that once the app indicators
+     * - "center"
-     * "ready" signal emits, this classes "appIndicatorReady" signal emits as
+     * - "right"
-     * well.
+     * @returns {string[]} - The box order consisting of an array of item
+     * settings identifiers.
+     */
+    #getBoxOrder(box) {
+        return this.#settings.get_strv(`${box}-box-order`);
+    }
+
+    /**
+     * Save the given box order to settings, making sure to only save a changed
+     * box order, to avoid loops when listening on settings changes.
+     * @param {string} box - The top bar box for which to save the box order.
+     * Must be one of the following values:
+     * - "left"
+     * - "center"
+     * - "right"
+     * @param {string[]} boxOrder - The box order to save. Must be an array of
+     * item settings identifiers.
+     */
+    #saveBoxOrder(box, boxOrder) {
+        const currentBoxOrder = this.#getBoxOrder(box);
+
+        // Only save the given box order to settings, if it is different, to
+        // avoid loops when listening on settings changes.
+        if (JSON.stringify(boxOrder) !== JSON.stringify(currentBoxOrder)) {
+            this.#settings.set_strv(`${box}-box-order`, boxOrder);
+        }
+    }
+
+    /**
+     * Handles an AppIndicator/KStatusNotifierItem item by deriving a settings
+     * identifier and then associating the role of the given item to the items
+     * settings identifier.
+     * It then returns the derived settings identifier.
+     * In the case, where the settings identifier can't be derived, because the
+     * application can't be determined, this method throws an error. However it
+     * then also makes sure that once the app indicators "ready" signal emits,
+     * this classes "appIndicatorReady" signal emits as well, such that it and
+     * other methods can be called again to properly handle the item.
      * @param {string} indicatorContainer - The container of the indicator of the
      * AppIndicator/KStatusNotifierItem item.
      * @param {string} role - The role of the AppIndicator/KStatusNotifierItem
      * item.
-     * @returns {string} The placeholder role.
+     * @returns {string} The derived items settings identifier.
      */
     #handleAppIndicatorItem(indicatorContainer, role) {
         const appIndicator = indicatorContainer.get_child()._indicator;
@@ -66,56 +111,74 @@ export default class BoxOrderManager extends GObject.Object {
             application = "dropbox-client";
         }
 
-        // Associate the role with the application.
+        // Derive the items settings identifier from the application name.
-        let roles = this.#appIndicatorItemApplicationRoleMap.get(application);
+        const itemSettingsId = `appindicator-kstatusnotifieritem-${application}`;
+
+        // Associate the role with the items settings identifier.
+        let roles = this.#appIndicatorItemSettingsIdToRolesMap.get(itemSettingsId);
         if (roles) {
-            // If the application already has an array of associated roles, just
+            // If the settings identifier already has an array of associated
-            // add the role to it, if needed.
+            // roles, just add the role to it, if needed.
             if (!roles.includes(role)) {
                 roles.push(role);
             }
         } else {
             // Otherwise create a new array.
-            this.#appIndicatorItemApplicationRoleMap.set(application, [role]);
+            this.#appIndicatorItemSettingsIdToRolesMap.set(itemSettingsId, [role]);
         }
 
-        // Return the placeholder.
+        // Return the item settings identifier.
-        // A box order containing this placeholder can later be resolved to
+        return itemSettingsId;
-        // relevant roles using `#resolveAppIndicatorPlaceholders`.
-        return `appindicator-kstatusnotifieritem-${application}`;
     }
 
     /**
-     * Takes a box order and replaces AppIndicator placeholder roles with
+     * Gets a resolved box order for the given top bar box, where all
-     * actual roles.
+     * AppIndicator items got resolved using their roles, meaning they might be
-     * @param {string[]} - The box order of which to replace placeholder roles.
+     * present multiple times or not at all depending on the roles stored.
-     * @returns {string[]} - A box order with all placeholder roles
+     * @param {string} box - The top bar box for which to get the resolved box order.
-     * resolved/replaced to/with actual roles.
+     * Must be one of the following values:
+     * - "left"
+     * - "center"
+     * - "right"
+     * @returns {ResolvedBoxOrderItem[]} - The resolved box order.
      */
-    #resolveAppIndicatorPlaceholders(boxOrder) {
+    #getResolvedBoxOrder(box) {
+        let boxOrder = this.#getBoxOrder(box);
+
         let resolvedBoxOrder = [];
-        for (const role of boxOrder) {
+        for (const itemSettingsId of boxOrder) {
-            // If the role isn't a placeholder, just add it to the resolved box
+            const resolvedBoxOrderItem = {
-            // order.
+                settingsId: itemSettingsId,
-            if (!role.startsWith("appindicator-kstatusnotifieritem-")) {
+                role: "",
-                resolvedBoxOrder.push(role);
+            };
+
+            // If the items settings identifier doesn't indicate that the item
+            // is an AppIndicator/KStatusNotifierItem item, then its identifier
+            // is the role and it can just be added to the resolved box order.
+            if (!itemSettingsId.startsWith("appindicator-kstatusnotifieritem-")) {
+                resolvedBoxOrderItem.role = resolvedBoxOrderItem.settingsId;
+                resolvedBoxOrder.push(resolvedBoxOrderItem);
                 continue;
             }
 
-            /// If the role is a placeholder, replace it.
+            // If the items settings identifier indicates otherwise, then handle
-            // First get the application this placeholder is associated with.
+            // the item specially.
-            const application = role.replace("appindicator-kstatusnotifieritem-", "");
 
-            // Then get the actual roles associated with this application.
+            // Get the roles roles associated with the items settings id.
-            let actualRoles = this.#appIndicatorItemApplicationRoleMap.get(application);
+            let roles = this.#appIndicatorItemSettingsIdToRolesMap.get(resolvedBoxOrderItem.settingsId);
 
-            // If there are no actual roles, continue.
+            // If there are no roles associated, continue.
-            if (!actualRoles) {
+            if (!roles) {
                 continue;
             }
 
-            // Otherwise add the actual roles to the resolved box order.
+            // Otherwise create a new resolved box order item for each role and
-            resolvedBoxOrder.push(...actualRoles);
+            // add it to the resolved box order.
+            for (const role of roles) {
+                const newResolvedBoxOrderItem = JSON.parse(JSON.stringify(resolvedBoxOrderItem));
+                newResolvedBoxOrderItem.role = role;
+                resolvedBoxOrder.push(newResolvedBoxOrderItem);
+            }
         }
 
         return resolvedBoxOrder;
@@ -136,24 +199,23 @@ export default class BoxOrderManager extends GObject.Object {
     }
 
     /**
-     * This method returns a valid box order for the given top bar box.
+     * Gets a valid box order for the given top bar box, where all AppIndicator
-     * This means it returns a box order, where only roles are included, which
+     * items got resolved and where only items are included, which are in some
-     * have their associated indicator container already in some box of the
+     * GNOME Shell top bar box.
-     * Gnome Shell top bar.
      * @param {string} box - The top bar box to return the valid box order for.
      * Must be one of the following values:
      * - "left"
      * - "center"
      * - "right"
-     * @returns {string[]} - The valid box order.
+     * @returns {ResolvedBoxOrderItem[]} - The valid box order.
      */
-    createValidBoxOrder(box) {
+    getValidBoxOrder(box) {
         // Get a resolved box order.
-        let boxOrder = this.#resolveAppIndicatorPlaceholders(this.#settings.get_strv(`${box}-box-order`));
+        let resolvedBoxOrder = this.#getResolvedBoxOrder(box);
 
         // ToDo: simplify.
         // Get the indicator containers (of the items) currently present in the
-        // Gnome Shell top bar.
+        // GNOME Shell top bar.
         const indicatorContainers = [
             Main.panel._leftBox.get_children(),
             Main.panel._centerBox.get_children(),
@@ -164,16 +226,16 @@ export default class BoxOrderManager extends GObject.Object {
         // fast easy access.
         const indicatorContainerSet = new Set(indicatorContainers);
 
-        // Go through the box order and only add items to the valid box order,
+        // Go through the resolved box order and only add items to the valid box
-        // where their indicator is present in the Gnome Shell top bar
+        // order, where their indicator is currently present in the GNOME Shell
-        // currently.
+        // top bar.
         let validBoxOrder = [];
-        for (const role of boxOrder) {
+        for (const item of resolvedBoxOrder) {
-            // Get the indicator container associated with the current role.
+            // Get the indicator container associated with the items role.
-            const associatedIndicatorContainer = Main.panel.statusArea[role]?.container;
+            const associatedIndicatorContainer = Main.panel.statusArea[item.role]?.container;
 
             if (indicatorContainerSet.has(associatedIndicatorContainer)) {
-                validBoxOrder.push(role);
+                validBoxOrder.push(item);
             }
         }
 
@@ -181,24 +243,24 @@ export default class BoxOrderManager extends GObject.Object {
     }
 
     /**
-     * This method saves all new items currently present in the Gnome Shell top
+     * This method saves all new items currently present in the GNOME Shell top
-     * bar to the correct box orders.
+     * bar to the settings.
      */
     saveNewTopBarItems() {
         // Only run, when the session mode is "user" or the parent session mode
         // is "user".
-        if(Main.sessionMode.currentMode !== "user" && Main.sessionMode.parentMode !== "user") {
+        if (Main.sessionMode.currentMode !== "user" && Main.sessionMode.parentMode !== "user") {
             return;
         }
 
-        // Load the configured box orders from settings.
+        // Get the box orders.
         const boxOrders = {
-            left: this.#settings.get_strv("left-box-order"),
+            left: this.#getBoxOrder("left"),
-            center: this.#settings.get_strv("center-box-order"),
+            center: this.#getBoxOrder("center"),
-            right: this.#settings.get_strv("right-box-order"),
+            right: this.#getBoxOrder("right"),
         };
 
-        // Get roles (of items) currently present in the Gnome Shell top bar and
+        // Get roles (of items) currently present in the GNOME Shell top bar and
         // index them using their associated indicator container.
         let indicatorContainerRoleMap = new Map();
         for (const role in Main.panel.statusArea) {
@@ -206,7 +268,7 @@ export default class BoxOrderManager extends GObject.Object {
         }
 
         // Get the indicator containers (of the items) currently present in the
-        // Gnome Shell top bar boxes.
+        // GNOME Shell top bar boxes.
         const boxIndicatorContainers = {
             left: Main.panel._leftBox.get_children(),
             center: Main.panel._centerBox.get_children(),
@@ -216,8 +278,8 @@ export default class BoxOrderManager extends GObject.Object {
         };
 
         // This function goes through the indicator containers of the given box
-        // and adds roles of new items to the box order.
+        // and adds new item settings identifiers to the given box order.
-        const addNewItemsToBoxOrder = (indicatorContainers, boxOrder, box) => {
+        const addNewItemSettingsIdsToBoxOrder = (indicatorContainers, boxOrder, box) => {
             for (const indicatorContainer of indicatorContainers) {
                 // First get the role associated with the current indicator
                 // container.
@@ -226,49 +288,46 @@ export default class BoxOrderManager extends GObject.Object {
                     continue;
                 }
 
-                // Handle an AppIndicator/KStatusNotifierItem item differently.
+                // Then get a settings identifier for the item.
+                let itemSettingsId;
+                // If the role indicates that the item is an
+                // AppIndicator/KStatusNotifierItem item, then handle it
+                // differently
                 if (role.startsWith("appindicator-")) {
                     try {
-                        role = this.#handleAppIndicatorItem(indicatorContainer, role);
+                        itemSettingsId = this.#handleAppIndicatorItem(indicatorContainer, role);
                     } catch (e) {
                         if (e.message !== "Application can't be determined.") {
                             throw(e);
                         }
                         continue;
                     }
+                } else { // Otherwise just use the role as the settings identifier.
+                    itemSettingsId = role;
                 }
 
-                // Add the role to the box order, if it isn't in in one already.
+                // Add the items settings identifier to the box order, if it
-                if (!boxOrders.left.includes(role)
+                // isn't in in one already.
-                    && !boxOrders.center.includes(role)
+                if (!boxOrders.left.includes(itemSettingsId)
-                    && !boxOrders.right.includes(role)) {
+                    && !boxOrders.center.includes(itemSettingsId)
+                    && !boxOrders.right.includes(itemSettingsId)) {
                     if (box === "right") {
                         // Add the items to the beginning for this array, since
                         // its RTL.
-                        boxOrder.unshift(role);
+                        boxOrder.unshift(itemSettingsId);
                     } else {
-                        boxOrder.push(role);
+                        boxOrder.push(itemSettingsId);
                     }
                 }
             }
         };
 
-        addNewItemsToBoxOrder(boxIndicatorContainers.left, boxOrders.left, "left");
+        addNewItemSettingsIdsToBoxOrder(boxIndicatorContainers.left, boxOrders.left, "left");
-        addNewItemsToBoxOrder(boxIndicatorContainers.center, boxOrders.center, "center");
+        addNewItemSettingsIdsToBoxOrder(boxIndicatorContainers.center, boxOrders.center, "center");
-        addNewItemsToBoxOrder(boxIndicatorContainers.right, boxOrders.right, "right");
+        addNewItemSettingsIdsToBoxOrder(boxIndicatorContainers.right, boxOrders.right, "right");
 
-        // This function saves the given box order to settings.
+        this.#saveBoxOrder("left", boxOrders.left);
-        const saveBoxOrderToSettings = (boxOrder, box) => {
+        this.#saveBoxOrder("center", boxOrders.center);
-            const currentBoxOrder = this.#settings.get_strv(`${box}-box-order`);
+        this.#saveBoxOrder("right", boxOrders.right);
-            // Only save the updated box order to settings, if it is different,
-            // to avoid loops, when listening on settings changes.
-            if (JSON.stringify(currentBoxOrder) !== JSON.stringify(boxOrder)) {
-                this.#settings.set_strv(`${box}-box-order`, boxOrder);
-            }
-        };
-
-        saveBoxOrderToSettings(boxOrders.left, "left");
-        saveBoxOrderToSettings(boxOrders.center, "center");
-        saveBoxOrderToSettings(boxOrders.right, "right");
     }
 }