public-Repo-Mirrors/Top-Bar-Organizer
1
Fork 0
mirror of https://gitlab.gnome.org/julianschacher/top-bar-organizer.git synced 2025-10-27 15:19:09 +00:00
Code Releases Activity

Fix: Consider all boxes, when adding items to box orders

Browse Source 
Fix the following issues:

1. Let's assume we have a default (reduced) Gnome Shell top bar like
   this:
   - left box: activities, appMenu
   - center box: dateMenu
   - right box: keyboard, aggregateMenu
   And now assume, we changed the (reduced) configured box orders to
   define the top bar like this (while the extension wasn't running for
   this test case):
   - left box: activites, keyboard, appMenu
   - center box: dateMenu
   - right box: aggreateMenu
   Now for the old version of this extension, the following would
   happen:
   The extension would put the "keyboard" into the configured right box
   order on extension start, despite it being already in the configured
   left box order.
   This would happen, because the extension would look at each box
   individually at startup to determine if new items should be added to
   its box order.
   And therefore it would add "keyboard" to the configured right box
   order, leading to the (reduced) corrupted box orders looking like
   this:
   - left box: activites, keyboard, appMenu
   - center box: dateMenu
   - right box: keyboard, aggreateMenu
   And then, since the extension orders the top bar initially on start,
   the extension would put the "keyboard" item into the right box (since
   the right box would get ordered after the left box).
   (And even if the config situation were different, so that the
   originally configured box would have been ordered after the wrongly
   configured box, the configured box orders would have still been
   corrupted.)

   Fix this issue by considering all top bar boxes in
   `_createUpdatedBoxOrder/s`.

2. Let's assume we have the default (reduced) configured box orders like
   this:
   - left box:
   - center box:
   - right box: appindicator-kstatusnotifieritem-KeePassXC
   And now assume, we changed the (reduced) configured box orders to
   define the top bar like this (while the extension and the
   "KStatusNotifierItem/AppIndicator Support" extension weren't running
   for this test case):
   - left box: appindicator-kstatusnotifieritem-KeePassXC
   - center box:
   - right box:
   Now we would first start this extension and then the
   "KStatusNotifierItem/AppIndicator Support" extension.
   For the old version of this extension, the following would happen:
   The extension would have put the KeePassXC
   KStatusNotifierItem/AppIndicator into the configured right box order
   and the right box, leading to the (reduced) corrupted configured box
   orders looking like this:
   - left box: appindicator-kstatusnotifieritem-KeePassXC
   - center box:
   - right box: appindicator-kstatusnotifieritem-KeePassXC
   This would happen, because the logic for the `Panel._addToPanelBox`
   overwrite assumed that the new item should be added to the box the
   item addition initiator requested (even tho the item might be
   configured to go into another box).

   Fix this issue by considering all top bar boxes for the
   `Panel._addToPanelBox` overwrite.

One can see based on those two test cases, how those two issues create
problems.
So fix the issues as explained above and fix them together in this one
commit, since they are related.
This commit is contained in:
Julian 2021-05-21 04:15:20 +02:00
parent 4faf976155
commit a3f5ad96f4
Signed by: julian
GPG Key ID: 094C2AC34192FA11
1 changed files with 192 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

244
src/extension.js
View File

@ -49,7 +49,18 @@ class Extension {
/// For the case, where the currently saved box order is based /// For the case, where the currently saved box order is based
/// on a permutation of an outdated box order, get an updated /// on a permutation of an outdated box order, get an updated
/// box order and save it, if needed. /// box order and save it, if needed.
const updatedBoxOrder = this._createUpdatedBoxOrder(box); let updatedBoxOrder;
switch (box) {
case "left":
updatedBoxOrder = this._createUpdatedBoxOrders().left;
break;
case "center":
updatedBoxOrder = this._createUpdatedBoxOrders().center;
break;
case "right":
updatedBoxOrder = this._createUpdatedBoxOrders().right;
break;
}
// Only save the updated box order to settings, if it is // Only save the updated box order to settings, if it is
// different, to avoid looping. // different, to avoid looping.
const currentBoxOrder = this.settings.get_strv(`${box}-box-order`); const currentBoxOrder = this.settings.get_strv(`${box}-box-order`);
@ -84,9 +95,10 @@ class Extension {
* bar to the box orders. * bar to the box orders.
*/ */
_addNewItemsToBoxOrders() { _addNewItemsToBoxOrders() {
this.settings.set_strv("left-box-order", this._createUpdatedBoxOrder("left")); const boxOrders = this._createUpdatedBoxOrders();
this.settings.set_strv("center-box-order", this._createUpdatedBoxOrder("center")); this.settings.set_strv("left-box-order", boxOrders.left);
this.settings.set_strv("right-box-order", this._createUpdatedBoxOrder("right")); this.settings.set_strv("center-box-order", boxOrders.center);
this.settings.set_strv("right-box-order", boxOrders.right);
} }
/** /**
@ -99,10 +111,17 @@ class Extension {
this._orderTopBarItems("right"); this._orderTopBarItems("right");
} }
/**
* An object containing a position and box overwrite.
* @typedef PositionAndBoxOverwrite
* @property {Number} position - The position overwrite.
* @property {string} box - The position box overwrite.
*/
/** /**
* Overwrite `Panel._addToPanelBox` with a custom method, which handles top * Overwrite `Panel._addToPanelBox` with a custom method, which handles top
* bar item additions to make sure that they are added in the correct * bar item additions to make sure that they are added in the correct
* position. * position and box.
*/ */
_overwritePanelAddToPanelBox() { _overwritePanelAddToPanelBox() {
// Add the original `Panel._addToPanelBox` method as // Add the original `Panel._addToPanelBox` method as
@ -110,10 +129,15 @@ class Extension {
Panel.Panel.prototype._originalAddToPanelBox = Panel.Panel.prototype._addToPanelBox; Panel.Panel.prototype._originalAddToPanelBox = Panel.Panel.prototype._addToPanelBox;
// This function gets used by the `Panel._addToPanelBox` overwrite to // This function gets used by the `Panel._addToPanelBox` overwrite to
// determine the position for a new item. // determine the position and box for a new item.
// It also adds the new item to the relevant box order, if it isn't in // It also adds the new item to the relevant box order, if it isn't in
// it already. // it already.
const getPositionOverwrite = (role, box, indicator) => { const getPositionAndBoxOverwrite = (role, box, indicator) => {
const boxOrders = {
left: this.settings.get_strv("left-box-order"),
center: this.settings.get_strv("center-box-order"),
right: this.settings.get_strv("right-box-order"),
};
let boxOrder; let boxOrder;
// Handle the case where the new item is a // Handle the case where the new item is a
@ -122,66 +146,85 @@ class Extension {
switch (box) { switch (box) {
case "left": case "left":
boxOrder = this.settings.get_strv("left-box-order"); boxOrder = this.settings.get_strv("left-box-order");
this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder); this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder, boxOrders);
this.settings.set_strv("left-box-order", boxOrder); this.settings.set_strv("left-box-order", boxOrder);
break; break;
case "center": case "center":
boxOrder = this.settings.get_strv("center-box-order"); boxOrder = this.settings.get_strv("center-box-order");
this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder); this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder, boxOrders);
this.settings.set_strv("center-box-order", boxOrder); this.settings.set_strv("center-box-order", boxOrder);
break; break;
case "right": case "right":
boxOrder = this.settings.get_strv("right-box-order"); boxOrder = this.settings.get_strv("right-box-order");
this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder, true); this._handleAppIndicatorKStatusNotifierItemItem(indicator.container, role, boxOrder, boxOrders, true);
this.settings.set_strv("right-box-order", boxOrder); this.settings.set_strv("right-box-order", boxOrder);
break; break;
} }
} }
// Get the resolved box order for the box order. // Get the resolved box orders for all boxes.
boxOrder = this._createResolvedBoxOrder(box); const resolvedBoxOrders = {
// Also get the restricted valid box order. left: this._createResolvedBoxOrder("left"),
const restrictedValidBoxOrder = this._createRestrictedValidBoxOrder(box); center: this._createResolvedBoxOrder("center"),
right: this._createResolvedBoxOrder("right"),
};
// Also get the restricted valid box order of the target box.
const restrictedValidBoxOrderOfTargetBox = this._createRestrictedValidBoxOrder(box);
// Get the index of the role in the box order. // Get the index of the role for each box order.
const index = boxOrder.indexOf(role); const indices = {
left: resolvedBoxOrders.left.indexOf(role),
center: resolvedBoxOrders.center.indexOf(role),
right: resolvedBoxOrders.right.indexOf(role),
};
// If the role is not already configured in the box order, just add // If the role is not already configured in one of the box orders,
// it to the box order at the end/beginning, save the updated box // just add it to the target box order at the end/beginning, save
// order and return the relevant position. // the updated box order and return the relevant position and box.
if (index === -1) { if (indices.left === -1
&& indices.center === -1
&& indices.right === -1) {
switch (box) { switch (box) {
// For the left and center box, insert the role at the end, // For the left and center box, insert the role at the end,
// since they're LTR. // since they're LTR.
case "left": case "left":
boxOrder.push(role); boxOrders["left"].push(role);
this.settings.set_strv("left-box-order", boxOrder); this.settings.set_strv("left-box-order", boxOrders["left"]);
return restrictedValidBoxOrder.length - 1; return {
position: restrictedValidBoxOrderOfTargetBox.length - 1,
box: box
};
case "center": case "center":
boxOrder.push(role); boxOrders["center"].push(role);
this.settings.set_strv("center-box-order", boxOrder); this.settings.set_strv("center-box-order", boxOrders["center"]);
return restrictedValidBoxOrder.length - 1; return {
position: restrictedValidBoxOrderOfTargetBox.length - 1,
box: box
};
// For the right box, insert the role at the beginning, // For the right box, insert the role at the beginning,
// since it's RTL. // since it's RTL.
case "right": case "right":
boxOrder.unshift(role); boxOrders["right"].unshift(role);
this.settings.set_strv("right-box-order", boxOrder); this.settings.set_strv("right-box-order", boxOrders["right"]);
return 0; return {
position: 0,
box: box
};
} }
} }
// Since the role is already configured in the box order, determine /// Since the role is already configured in one of the box orders,
// the correct insertion index for the position. /// determine the correct insertion index for the position.
const determineInsertionIndex = (index, restrictedValidBoxOrder, boxOrder) => {
// Set the insertion index initially to 0, so that if no closest // Set the insertion index initially to 0, so that if no closest
// item can be found, the new item just gets inserted at the // item can be found, the new item just gets inserted at the
// beginning. // beginning.
let insertionIndex = 0; let insertionIndex = 0;
// Find the index of the closest item, which is also in the valid // Find the index of the closest item, which is also in the
// box order and before the new item. // valid box order and before the new item.
// This way, we can insert the new item just after the index of this // This way, we can insert the new item just after the index of
// closest item. // this closest item.
for (let i = index - 1; i >= 0; i--) { for (let i = index - 1; i >= 0; i--) {
let potentialClosestItemIndex = restrictedValidBoxOrder.indexOf(boxOrder[i]); let potentialClosestItemIndex = restrictedValidBoxOrder.indexOf(boxOrder[i]);
if (potentialClosestItemIndex !== -1) { if (potentialClosestItemIndex !== -1) {
@ -193,25 +236,58 @@ class Extension {
return insertionIndex; return insertionIndex;
}; };
if (indices.left !== -1) {
return {
position: determineInsertionIndex(indices.left, this._createRestrictedValidBoxOrder("left"), boxOrders.left),
box: "left"
};
}
if (indices.center !== -1) {
return {
position: determineInsertionIndex(indices.center, this._createRestrictedValidBoxOrder("center"), boxOrders.center),
box: "center"
};
}
if (indices.right !== -1) {
return {
position: determineInsertionIndex(indices.right, this._createRestrictedValidBoxOrder("right"), boxOrders.right),
box: "right"
};
}
};
// Overwrite `Panel._addToPanelBox`. // Overwrite `Panel._addToPanelBox`.
Panel.Panel.prototype._addToPanelBox = function (role, indicator, position, box) { Panel.Panel.prototype._addToPanelBox = function (role, indicator, position, box) {
// Get the position overwrite. // Get the position and box overwrite.
let positionOverwrite; let positionBoxOverwrite;
switch (box) { switch (box) {
case this._leftBox: case this._leftBox:
positionOverwrite = getPositionOverwrite(role, "left", indicator); positionBoxOverwrite = getPositionAndBoxOverwrite(role, "left", indicator);
break; break;
case this._centerBox: case this._centerBox:
positionOverwrite = getPositionOverwrite(role, "center", indicator); positionBoxOverwrite = getPositionAndBoxOverwrite(role, "center", indicator);
break; break;
case this._rightBox: case this._rightBox:
positionOverwrite = getPositionOverwrite(role, "right", indicator); positionBoxOverwrite = getPositionAndBoxOverwrite(role, "right", indicator);
break; break;
} }
// Call the original `Panel._addToPanelBox` with the position // Call the original `Panel._addToPanelBox` with the position
// overwrite as the position argument. // overwrite as the position argument and the box determined by the
this._originalAddToPanelBox(role, indicator, positionOverwrite, box); // box overwrite as the box argument.
switch (positionBoxOverwrite.box) {
case "left":
this._originalAddToPanelBox(role, indicator, positionBoxOverwrite.position, Main.panel._leftBox);
break;
case "center":
this._originalAddToPanelBox(role, indicator, positionBoxOverwrite.position, Main.panel._centerBox);
break;
case "right":
this._originalAddToPanelBox(role, indicator, positionBoxOverwrite.position, Main.panel._rightBox);
break;
}
}; };
} }
@ -220,14 +296,26 @@ class Extension {
//////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////
/** /**
* This method adds all new items currently present in the specified Gnome * An object containing a box order for the left, center and right top bar
* Shell top bar box to a box order and returns that new box order. * box.
* @param {string} box - The box to create the updated box order for. * @typedef {Object} BoxOrders
* @returns {string[]} - The updated box order. * @property {string[]} left - The box order for the left top bar box.
* @property {string[]} center - The box order for the center top bar box.
* @property {string[]} right - The box order for the right top bar box.
*/ */
_createUpdatedBoxOrder(box) {
// Load the configured box order from settings. /**
const boxOrder = this.settings.get_strv(`${box}-box-order`); * This method adds all new items currently present in the Gnome Shell top
* bar to the correct box order and returns the new box orders.
* @returns {BoxOrders} - The updated box orders.
*/
_createUpdatedBoxOrders() {
// Load the configured box orders from settings.
const boxOrders = {
left: this.settings.get_strv("left-box-order"),
center: this.settings.get_strv("center-box-order"),
right: this.settings.get_strv("right-box-order"),
};
// Get items (or rather their roles) currently present in the Gnome // Get items (or rather their roles) currently present in the Gnome
// Shell top bar and index them using their associated indicator // Shell top bar and index them using their associated indicator
@ -238,28 +326,19 @@ class Extension {
} }
// Get the indicator containers (of the items) currently present in the // Get the indicator containers (of the items) currently present in the
// Gnome Shell top bar box. // Gnome Shell top bar boxes.
let boxIndicatorContainers; const boxOrderIndicatorContainers = {
switch (box) { left: Main.panel._leftBox.get_children(),
case "left": center: Main.panel._centerBox.get_children(),
boxIndicatorContainers = Main.panel._leftBox.get_children(); // Reverse this array, since the items in the left and center box
break; // are logically LTR, while the items in the right box are RTL.
case "center": right: Main.panel._rightBox.get_children().reverse()
boxIndicatorContainers = Main.panel._centerBox.get_children(); };
break;
case "right":
// Reverse this array, since the items in the left and center
// box are logically LTR, while the items in the right box are
// RTL.
boxIndicatorContainers = Main.panel._rightBox.get_children().reverse();
break;
}
/// Go through the items (or rather their indicator containers) of the
/// box and add new items (or rather their roles) to the box order.
// Create a box order set from the box order for fast easy access.
const boxOrderSet = new Set(boxOrder);
// This function goes through the items (or rather their indicator
// containers) of the given box and adds new items (or rather their
// roles) to the box order.
const addNewItemsToBoxOrder = (boxIndicatorContainers, boxOrder, box) => {
for (const indicatorContainer of boxIndicatorContainers) { for (const indicatorContainer of boxIndicatorContainers) {
// First get the role associated with the current indicator // First get the role associated with the current indicator
// container. // container.
@ -267,12 +346,14 @@ class Extension {
// Handle an AppIndicator/KStatusNotifierItem item differently. // Handle an AppIndicator/KStatusNotifierItem item differently.
if (associatedRole.startsWith("appindicator-")) { if (associatedRole.startsWith("appindicator-")) {
this._handleAppIndicatorKStatusNotifierItemItem(indicatorContainer, associatedRole, boxOrder, box === "right"); this._handleAppIndicatorKStatusNotifierItemItem(indicatorContainer, associatedRole, boxOrder, boxOrders, box === "right");
continue; continue;
} }
// Add the role to the box order, if it isn't in it already. // Add the role to the box order, if it isn't in in one already.
if (!boxOrderSet.has(associatedRole)) { if (!boxOrders.left.includes(associatedRole)
&& !boxOrders.center.includes(associatedRole)
&& !boxOrders.right.includes(associatedRole)) {
if (box === "right") { if (box === "right") {
// Add the items to the beginning for this array, since // Add the items to the beginning for this array, since
// its RTL. // its RTL.
@ -282,8 +363,13 @@ class Extension {
} }
} }
} }
};
return boxOrder; addNewItemsToBoxOrder(boxOrderIndicatorContainers.left, boxOrders.left, "left");
addNewItemsToBoxOrder(boxOrderIndicatorContainers.center, boxOrders.center, "center");
addNewItemsToBoxOrder(boxOrderIndicatorContainers.right, boxOrders.right, "right");
return boxOrders;
} }
/** /**
@ -464,10 +550,12 @@ class Extension {
* item. * item.
* @param {string[]} - The box order the placeholder should be added to, if * @param {string[]} - The box order the placeholder should be added to, if
* needed. * needed.
* @param {BoxOrders} boxOrders - An object containing the box orders, which
* is currently getting worked on.
* @param {boolean} - Whether to add the placeholder to the beginning of the * @param {boolean} - Whether to add the placeholder to the beginning of the
* box order. * box order.
*/ */
_handleAppIndicatorKStatusNotifierItemItem(indicatorContainer, role, boxOrder, atToBeginning = false) { _handleAppIndicatorKStatusNotifierItemItem(indicatorContainer, role, boxOrder, boxOrders, atToBeginning = false) {
// Get the application the AppIndicator/KStatusNotifierItem is // Get the application the AppIndicator/KStatusNotifierItem is
// associated with. // associated with.
const application = indicatorContainer.get_child()._indicator.id; const application = indicatorContainer.get_child()._indicator.id;
@ -488,7 +576,9 @@ class Extension {
// (Then later the placeholder can be replaced with the relevant roles // (Then later the placeholder can be replaced with the relevant roles
// using `this._applicationRoleMap`.) // using `this._applicationRoleMap`.)
const placeholder = `appindicator-kstatusnotifieritem-${application}`; const placeholder = `appindicator-kstatusnotifieritem-${application}`;
if (!boxOrder.includes(placeholder)) { if (!boxOrders.left.includes(placeholder)
&& !boxOrders.center.includes(placeholder)
&& !boxOrders.right.includes(placeholder)) {
if (atToBeginning) { if (atToBeginning) {
boxOrder.unshift(placeholder); boxOrder.unshift(placeholder);
} else { } else {