* [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering
@ 2026-08-18 8:31 Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 1/2] " Elias Huhsovitz
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Elias Huhsovitz @ 2026-08-18 8:31 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
This patch series fixes bug #7136, where resources in the Folder View
were ordered lexicographically instead of following the order
defined by the `getTypeOrder` function. It also includes a follow-up
cleanup to align variable declarations with modern javascript
standards.
Previously, grouping nodes in the Folder View were assigned the literal
type 'type' instead of their actual resource type, such as 'node' or
'network'. This caused `getTypeOrder` to return a default value for all
groups, resulting in a lexicographical sorting fallback.
Other views, such as Server View, pass the correct types to
`getTypeOrder`.
Patch 1/2 fixes the root cause by setting the grouping node's type to
the actual resource type when grouping by 'type'. This restores the
correct sort order (LXC, QEMU, Node, SDN, Network, Storage) and allows
icon and text resolution via typeDefaults.
Patch 2/2 replaces let with const for variables that are never
reassigned. This aligns the with modern JavaScript best practices.
Elias Huhsovitz (2):
fix #7136: ui: tree: harmonize folder view resource ordering
ui: tree: reduce reduce usage of `let` keyword
www/manager6/tree/ResourceTree.js | 148 ++++++++++++++++--------------
1 file changed, 77 insertions(+), 71 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH manager 1/2] fix #7136: ui: tree: harmonize folder view resource ordering
2026-08-18 8:31 [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
@ 2026-08-18 8:31 ` Elias Huhsovitz
2026-08-18 9:15 ` Dominik Csapak
2026-08-18 8:31 ` [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword Elias Huhsovitz
2026-08-18 12:09 ` superseded: [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
2 siblings, 1 reply; 7+ messages in thread
From: Elias Huhsovitz @ 2026-08-18 8:31 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Grouping nodes in the Folder view are assigned the literal type: 'type'
instead of their actual type. This causes `getTypeOrder` to return a
default value for all groups.
This causes Resources in the Folder View to be ordered
lexicographically, whereas the Server View correctly relies on the
`getTypeOrder` function.
Match the Server View ordering by setting the grouping node's `type` to
the actual resource type when grouping by `type`. This way
`getTypeOrder` receives the correct type for sorting.
Simplify the text resolution logic in `addChildSorted`.
Propagate `iconCls` from `typeDefaults` to ensure grouping nodes
display the correct icons.
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
www/manager6/tree/ResourceTree.js | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index 6ea28919..a7723095 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -232,13 +232,13 @@ Ext.define('PVE.tree.ResourceTree', {
if (info.groupbyid) {
if (me.viewFilter.groupRenderer) {
info.text = me.viewFilter.groupRenderer(info);
- } else if (info.type === 'type') {
+ } else {
let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
if (defaults && defaults.text) {
info.text = defaults.text;
+ } else {
+ info.text = info.groupbyid;
}
- } else {
- info.text = info.groupbyid;
}
}
let child = Ext.create('PVETree', info);
@@ -267,11 +267,17 @@ Ext.define('PVE.tree.ResourceTree', {
if (info.type === groupBy) {
groupinfo = info;
} else {
+ const type = groupBy === 'type' ? v : groupBy;
groupinfo = {
- type: groupBy,
+ type: type,
id: groupBy + '/' + v,
};
- if (groupBy !== 'type') {
+ if (groupBy === 'type') {
+ let defaults = PVE.tree.ResourceTree.typeDefaults[v];
+ if (defaults && defaults.iconCls) {
+ groupinfo.iconCls = defaults.iconCls;
+ }
+ } else {
groupinfo[groupBy] = v;
}
}
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword
2026-08-18 8:31 [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 1/2] " Elias Huhsovitz
@ 2026-08-18 8:31 ` Elias Huhsovitz
2026-08-18 9:15 ` Dominik Csapak
2026-08-18 12:09 ` superseded: [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
2 siblings, 1 reply; 7+ messages in thread
From: Elias Huhsovitz @ 2026-08-18 8:31 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Replace `let` with `const` the for varaibles that are not re-assigned.
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
www/manager6/tree/ResourceTree.js | 134 +++++++++++++++---------------
1 file changed, 67 insertions(+), 67 deletions(-)
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index a7723095..93d2a85b 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -52,15 +52,15 @@ Ext.define('PVE.tree.ResourceTree', {
flex: 1,
dataIndex: 'text',
renderer: function (val, meta, rec) {
- let info = rec.data;
+ const info = rec.data;
let text = info.text;
let status = '';
if (info.type === 'storage') {
- let usage = info.disk / info.maxdisk;
+ const usage = info.disk / info.maxdisk;
if (usage >= 0.0 && usage <= 1.0) {
- let barHeight = (usage * 100).toFixed(0);
- let remainingHeight = (100 - barHeight).toFixed(0);
+ const barHeight = (usage * 100).toFixed(0);
+ const remainingHeight = (100 - barHeight).toFixed(0);
status = '<div class="usage-wrapper">';
status += `<div class="usage-negative" style="height: ${remainingHeight}%"></div>`;
status += `<div class="usage" style="height: ${barHeight}%"></div>`;
@@ -112,16 +112,16 @@ Ext.define('PVE.tree.ResourceTree', {
// private
nodeSortFn: function (node1, node2) {
- let me = this;
- let n1 = node1.data,
+ const me = this;
+ const n1 = node1.data,
n2 = node2.data;
if (!n1.groupbyid === !n2.groupbyid) {
- let n1IsGuest = n1.type === 'qemu' || n1.type === 'lxc';
- let n2IsGuest = n2.type === 'qemu' || n2.type === 'lxc';
+ const n1IsGuest = n1.type === 'qemu' || n1.type === 'lxc';
+ const n2IsGuest = n2.type === 'qemu' || n2.type === 'lxc';
if (me['group-guest-types'] || !n1IsGuest || !n2IsGuest) {
// first sort (group) by type
- let res = me.getTypeOrder(n1.type) - me.getTypeOrder(n2.type);
+ const res = me.getTypeOrder(n1.type) - me.getTypeOrder(n2.type);
if (res !== 0) {
return res;
}
@@ -155,15 +155,15 @@ Ext.define('PVE.tree.ResourceTree', {
// private: fast binary search
findInsertIndex: function (node, child, start, end) {
- let me = this;
+ const me = this;
- let diff = end - start;
+ const diff = end - start;
if (diff <= 0) {
return start;
}
- let mid = start + (diff >> 1);
+ const mid = start + (diff >> 1);
- let res = me.nodeSortFn(child, node.childNodes[mid]);
+ const res = me.nodeSortFn(child, node.childNodes[mid]);
if (res <= 0) {
return me.findInsertIndex(node, child, start, mid);
} else {
@@ -172,14 +172,14 @@ Ext.define('PVE.tree.ResourceTree', {
},
setIconCls: function (info) {
- let cls = PVE.Utils.get_object_icon_class(info.type, info);
+ const cls = PVE.Utils.get_object_icon_class(info.type, info);
if (cls !== '') {
info.iconCls = cls;
}
},
getToolTip: function (info) {
- let qtips = [];
+ const qtips = [];
if (info.qmpstatus || info.status) {
qtips.push(Ext.String.format(gettext('Status: {0}'), info.qmpstatus || info.status));
}
@@ -190,7 +190,7 @@ Ext.define('PVE.tree.ResourceTree', {
qtips.push(Ext.String.format(gettext('HA State: {0}'), info.hastate));
}
if (info.type === 'storage') {
- let usage = info.disk / info.maxdisk;
+ const usage = info.disk / info.maxdisk;
if (usage >= 0.0 && usage <= 1.0) {
qtips.push(Ext.String.format(gettext('Usage: {0}%'), (usage * 100).toFixed(2)));
}
@@ -200,20 +200,20 @@ Ext.define('PVE.tree.ResourceTree', {
return undefined;
}
- let tip = qtips.join(', ');
+ const tip = qtips.join(', ');
info.tip = tip;
return tip;
},
// private
addChildSorted: function (node, info, insertPool = false) {
- let me = this;
+ const me = this;
me.setIconCls(info);
- let nestPools = PVE.UIOptions.getTreeSortingValue('nest-pools');
+ const nestPools = PVE.UIOptions.getTreeSortingValue('nest-pools');
if (info.type === 'pool' && info.pool && !insertPool && nestPools) {
- let parentPool = info.pool.split('/').slice(0, -1).join('/');
+ const parentPool = info.pool.split('/').slice(0, -1).join('/');
if (parentPool.length > 0) {
let parent = node.findChild('id', `/pool/${parentPool}`, true);
if (parent !== node) {
@@ -233,7 +233,7 @@ Ext.define('PVE.tree.ResourceTree', {
if (me.viewFilter.groupRenderer) {
info.text = me.viewFilter.groupRenderer(info);
} else {
- let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
+ const defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
if (defaults && defaults.text) {
info.text = defaults.text;
} else {
@@ -241,10 +241,10 @@ Ext.define('PVE.tree.ResourceTree', {
}
}
}
- let child = Ext.create('PVETree', info);
+ const child = Ext.create('PVETree', info);
if (node.childNodes) {
- let pos = me.findInsertIndex(node, child, 0, node.childNodes.length);
+ const pos = me.findInsertIndex(node, child, 0, node.childNodes.length);
node.insertBefore(child, node.childNodes[pos]);
} else {
node.insertBefore(child);
@@ -255,10 +255,10 @@ Ext.define('PVE.tree.ResourceTree', {
// private
groupChild: function (node, info, groups, level) {
- let me = this;
+ const me = this;
- let groupBy = groups[level];
- let v = info[groupBy];
+ const groupBy = groups[level];
+ const v = info[groupBy];
if (v) {
let group = node.findChild('groupbyid', v, true);
@@ -273,7 +273,7 @@ Ext.define('PVE.tree.ResourceTree', {
id: groupBy + '/' + v,
};
if (groupBy === 'type') {
- let defaults = PVE.tree.ResourceTree.typeDefaults[v];
+ const defaults = PVE.tree.ResourceTree.typeDefaults[v];
if (defaults && defaults.iconCls) {
groupinfo.iconCls = defaults.iconCls;
}
@@ -296,10 +296,10 @@ Ext.define('PVE.tree.ResourceTree', {
},
saveSortingOptions: function () {
- let me = this;
+ const me = this;
let changed = false;
for (const key of ['sort-field', 'group-templates', 'group-guest-types', 'nest-pools']) {
- let newValue = PVE.UIOptions.getTreeSortingValue(key);
+ const newValue = PVE.UIOptions.getTreeSortingValue(key);
if (me[key] !== newValue) {
me[key] = newValue;
changed = true;
@@ -309,22 +309,22 @@ Ext.define('PVE.tree.ResourceTree', {
},
initComponent: function () {
- let me = this;
+ const me = this;
me.saveSortingOptions();
- let rstore = PVE.data.ResourceStore;
- let sp = Ext.state.Manager.getProvider();
+ const rstore = PVE.data.ResourceStore;
+ const sp = Ext.state.Manager.getProvider();
if (!me.viewFilter) {
me.viewFilter = {};
}
- let pdata = {
+ const pdata = {
dataIndex: {},
updateCount: 0,
};
- let store = Ext.create('Ext.data.TreeStore', {
+ const store = Ext.create('Ext.data.TreeStore', {
model: 'PVETree',
root: {
expanded: true,
@@ -334,7 +334,7 @@ Ext.define('PVE.tree.ResourceTree', {
},
});
- let stateid = 'rid';
+ const stateid = 'rid';
const changedFields = [
'disk',
@@ -352,18 +352,18 @@ Ext.define('PVE.tree.ResourceTree', {
];
// special case ids from the tag view, since they change the id in the state
- let idMapFn = function (id) {
+ const idMapFn = function (id) {
if (!id) {
return undefined;
}
if (id.startsWith('qemu') || id.startsWith('lxc')) {
- let [realId, _tag] = id.split('-');
+ const [realId, _tag] = id.split('-');
return realId;
}
return id;
};
- let findNode = function (rootNode, id) {
+ const findNode = function (rootNode, id) {
if (!id) {
return undefined;
}
@@ -380,7 +380,7 @@ Ext.define('PVE.tree.ResourceTree', {
let firstUpdate = true;
- let updateTree = function () {
+ const updateTree = function () {
store.suspendEvents();
let rootnode;
@@ -395,30 +395,30 @@ Ext.define('PVE.tree.ResourceTree', {
rootnode = me.store.getRootNode();
}
// remember selected node (and all parents)
- let sm = me.getSelectionModel();
+ const sm = me.getSelectionModel();
let lastsel = sm.getSelection()[0];
- let parents = [];
- let sorting_changed = me.saveSortingOptions();
+ const parents = [];
+ const sorting_changed = me.saveSortingOptions();
for (let node = lastsel; node; node = node.parentNode) {
parents.push(node);
}
- let groups = me.viewFilter.groups || [];
+ const groups = me.viewFilter.groups || [];
// explicitly check for node/template, as those are not always grouping attributes
- let attrMoveChecks = me.viewFilter.attrMoveChecks ?? {};
+ const attrMoveChecks = me.viewFilter.attrMoveChecks ?? {};
// also check for name for when the tree is sorted by name
- let moveCheckAttrs = groups.concat(['node', 'template', 'name']);
- let filterFn = me.viewFilter.getFilterFn ? me.viewFilter.getFilterFn() : Ext.identityFn;
+ const moveCheckAttrs = groups.concat(['node', 'template', 'name']);
+ const filterFn = me.viewFilter.getFilterFn ? me.viewFilter.getFilterFn() : Ext.identityFn;
let reselect = false; // for disappeared nodes
- let index = pdata.dataIndex;
+ const index = pdata.dataIndex;
// remove vanished or moved items and update changed items in-place
for (const [key, olditem] of Object.entries(index)) {
// getById() use find(), which is slow (ExtJS4 DP5)
- let oldid = olditem.data.id;
- let id = idMapFn(olditem.data.id);
- let item = rstore.data.get(id);
+ const oldid = olditem.data.id;
+ const id = idMapFn(olditem.data.id);
+ const item = rstore.data.get(id);
let changed = sorting_changed,
moved = sorting_changed;
@@ -448,7 +448,7 @@ Ext.define('PVE.tree.ResourceTree', {
if (changed) {
olditem.beginEdit();
- let info = olditem.data;
+ const info = olditem.data;
Ext.apply(info, item.data);
if (info.id !== oldid) {
info.id = oldid;
@@ -458,7 +458,7 @@ Ext.define('PVE.tree.ResourceTree', {
}
if ((!item || moved) && olditem.isLeaf()) {
delete index[key];
- let parentNode = olditem.parentNode;
+ const parentNode = olditem.parentNode;
// a selected item moved (migration) or disappeared (destroyed), so deselect that
// node now and try to reselect the moved (or its parent) node later
if (lastsel && olditem.data.id === lastsel.data.id) {
@@ -469,25 +469,25 @@ Ext.define('PVE.tree.ResourceTree', {
store.remove(olditem);
parentNode.removeChild(olditem, true);
if (parentNode.childNodes.length < 1 && parentNode.parentNode) {
- let grandParent = parentNode.parentNode;
+ const grandParent = parentNode.parentNode;
grandParent.removeChild(parentNode, true);
}
}
}
- let items = rstore.getData().items.flatMap(me.viewFilter.itemMap ?? Ext.identityFn);
+ const items = rstore.getData().items.flatMap(me.viewFilter.itemMap ?? Ext.identityFn);
items.forEach(function (item) {
// add new items
- let olditem = index[item.data.id];
+ const olditem = index[item.data.id];
if (olditem) {
return;
}
if (filterFn && !filterFn(item)) {
return;
}
- let info = Ext.apply({ leaf: true }, item.data);
+ const info = Ext.apply({ leaf: true }, item.data);
- let child = me.groupChild(rootnode, info, groups, 0);
+ const child = me.groupChild(rootnode, info, groups, 0);
if (child) {
index[item.data.id] = child;
}
@@ -496,7 +496,7 @@ Ext.define('PVE.tree.ResourceTree', {
store.resumeEvents();
store.fireEvent('refresh', store);
- let foundChild = findNode(rootnode, lastsel?.data.id);
+ const foundChild = findNode(rootnode, lastsel?.data.id);
// select parent node if original selected node vanished
if (lastsel && !foundChild) {
@@ -544,12 +544,12 @@ Ext.define('PVE.tree.ResourceTree', {
rstore.un('load', updateTree);
},
beforecellmousedown: function (tree, td, cellIndex, record, tr, rowIndex, ev) {
- let sm = me.getSelectionModel();
+ const sm = me.getSelectionModel();
// disable selection when right clicking except if the record is already selected
me.allowSelection = ev.button !== 2 || sm.isSelected(record);
},
beforeselect: function (tree, record, index, eopts) {
- let allow = me.allowSelection;
+ const allow = me.allowSelection;
me.allowSelection = true;
return allow;
},
@@ -558,7 +558,7 @@ Ext.define('PVE.tree.ResourceTree', {
if (me.tip) {
return;
}
- let selectors = [
+ const selectors = [
'.x-tree-node-text > span:not(.proxmox-tag-dark):not(.proxmox-tag-light)',
'.x-tree-icon',
];
@@ -569,8 +569,8 @@ Ext.define('PVE.tree.ResourceTree', {
renderTo: Ext.getBody(),
listeners: {
beforeshow: function (tip) {
- let rec = me.getView().getRecord(tip.triggerElement);
- let tipText = me.getToolTip(rec.data);
+ const rec = me.getView().getRecord(tip.triggerElement);
+ const tipText = me.getToolTip(rec.data);
if (tipText) {
tip.update(tipText);
return true;
@@ -587,7 +587,7 @@ Ext.define('PVE.tree.ResourceTree', {
},
clearTree: function () {
pdata.updateCount = 0;
- let rootnode = me.store.getRootNode();
+ const rootnode = me.store.getRootNode();
rootnode.collapse();
rootnode.removeAll();
pdata.dataIndex = {};
@@ -598,7 +598,7 @@ Ext.define('PVE.tree.ResourceTree', {
updateTree();
},
selectExpand: function (node) {
- let sm = me.getSelectionModel();
+ const sm = me.getSelectionModel();
if (!sm.isSelected(node)) {
sm.select(node);
for (let iter = node; iter; iter = iter.parentNode) {
@@ -610,7 +610,7 @@ Ext.define('PVE.tree.ResourceTree', {
}
},
selectById: function (nodeid) {
- let rootnode = me.store.getRootNode();
+ const rootnode = me.store.getRootNode();
let node;
if (nodeid === 'root') {
node = rootnode;
@@ -643,7 +643,7 @@ Ext.define('PVE.tree.ResourceTree', {
before: function (node) {
if (node.data.groupbyid) {
node.beginEdit();
- let info = node.data;
+ const info = node.data;
me.setIconCls(info);
if (me.viewFilter.groupRenderer) {
info.text = me.viewFilter.groupRenderer(info);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH manager 1/2] fix #7136: ui: tree: harmonize folder view resource ordering
2026-08-18 8:31 ` [PATCH manager 1/2] " Elias Huhsovitz
@ 2026-08-18 9:15 ` Dominik Csapak
2026-08-18 9:36 ` Elias Huhsovitz
0 siblings, 1 reply; 7+ messages in thread
From: Dominik Csapak @ 2026-08-18 9:15 UTC (permalink / raw)
To: Elias Huhsovitz, pve-devel
while the code here looks ok this produces an issue with all places
where we use the type:
* selecting a category now tries to load the wrong panel type
(see Workspace.js:236)
* right clicking on vm/container throws an exception
* resource pool folder has the wrong text ('Root' instead of
'Resource-Pools')
to fix the bug, wouldn't it be easier to use the 'groupbyid' field
in case the type === 'type' ?
then we don't have to touch that semantic at all
also one line is not correctly formatted, running make tidy before
submitting would be great :)
On 8/18/26 10:31 AM, Elias Huhsovitz wrote:
> Grouping nodes in the Folder view are assigned the literal type: 'type'
> instead of their actual type. This causes `getTypeOrder` to return a
> default value for all groups.
>
> This causes Resources in the Folder View to be ordered
> lexicographically, whereas the Server View correctly relies on the
> `getTypeOrder` function.
>
> Match the Server View ordering by setting the grouping node's `type` to
> the actual resource type when grouping by `type`. This way
> `getTypeOrder` receives the correct type for sorting.
>
> Simplify the text resolution logic in `addChildSorted`.
>
> Propagate `iconCls` from `typeDefaults` to ensure grouping nodes
> display the correct icons.
>
> Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
> ---
> www/manager6/tree/ResourceTree.js | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
> index 6ea28919..a7723095 100644
> --- a/www/manager6/tree/ResourceTree.js
> +++ b/www/manager6/tree/ResourceTree.js
> @@ -232,13 +232,13 @@ Ext.define('PVE.tree.ResourceTree', {
> if (info.groupbyid) {
> if (me.viewFilter.groupRenderer) {
> info.text = me.viewFilter.groupRenderer(info);
> - } else if (info.type === 'type') {
> + } else {
> let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
> if (defaults && defaults.text) {
> info.text = defaults.text;
> + } else {
> + info.text = info.groupbyid;
> }
> - } else {
> - info.text = info.groupbyid;
> }
> }
> let child = Ext.create('PVETree', info);
> @@ -267,11 +267,17 @@ Ext.define('PVE.tree.ResourceTree', {
> if (info.type === groupBy) {
> groupinfo = info;
> } else {
> + const type = groupBy === 'type' ? v : groupBy;
> groupinfo = {
> - type: groupBy,
> + type: type,
> id: groupBy + '/' + v,
> };
> - if (groupBy !== 'type') {
> + if (groupBy === 'type') {
> + let defaults = PVE.tree.ResourceTree.typeDefaults[v];
> + if (defaults && defaults.iconCls) {
> + groupinfo.iconCls = defaults.iconCls;
> + }
> + } else {
> groupinfo[groupBy] = v;
> }
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword
2026-08-18 8:31 ` [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword Elias Huhsovitz
@ 2026-08-18 9:15 ` Dominik Csapak
0 siblings, 0 replies; 7+ messages in thread
From: Dominik Csapak @ 2026-08-18 9:15 UTC (permalink / raw)
To: Elias Huhsovitz, pve-devel
typo in the subject: 'reduce reduce'
also, not sure if this gains us anything.
yes, our style guide says if sensible, use const, but we don't
have to go out of our way to replace existing lets with consts.
Or is there some improvement to gain from this?
In general, I would prefer to leave such code as is, and only
introduce these with new code, except if there is a clear
gain (like fixing a bug or exposing some uninteded behavior).
On 8/18/26 10:31 AM, Elias Huhsovitz wrote:
> Replace `let` with `const` the for varaibles that are not re-assigned.
>
> Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
> ---
> www/manager6/tree/ResourceTree.js | 134 +++++++++++++++---------------
> 1 file changed, 67 insertions(+), 67 deletions(-)
>
> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
> index a7723095..93d2a85b 100644
> --- a/www/manager6/tree/ResourceTree.js
> +++ b/www/manager6/tree/ResourceTree.js
> @@ -52,15 +52,15 @@ Ext.define('PVE.tree.ResourceTree', {
> flex: 1,
> dataIndex: 'text',
> renderer: function (val, meta, rec) {
> - let info = rec.data;
> + const info = rec.data;
>
> let text = info.text;
> let status = '';
> if (info.type === 'storage') {
> - let usage = info.disk / info.maxdisk;
> + const usage = info.disk / info.maxdisk;
> if (usage >= 0.0 && usage <= 1.0) {
> - let barHeight = (usage * 100).toFixed(0);
> - let remainingHeight = (100 - barHeight).toFixed(0);
> + const barHeight = (usage * 100).toFixed(0);
> + const remainingHeight = (100 - barHeight).toFixed(0);
> status = '<div class="usage-wrapper">';
> status += `<div class="usage-negative" style="height: ${remainingHeight}%"></div>`;
> status += `<div class="usage" style="height: ${barHeight}%"></div>`;
> @@ -112,16 +112,16 @@ Ext.define('PVE.tree.ResourceTree', {
>
> // private
> nodeSortFn: function (node1, node2) {
> - let me = this;
> - let n1 = node1.data,
> + const me = this;
> + const n1 = node1.data,
> n2 = node2.data;
>
> if (!n1.groupbyid === !n2.groupbyid) {
> - let n1IsGuest = n1.type === 'qemu' || n1.type === 'lxc';
> - let n2IsGuest = n2.type === 'qemu' || n2.type === 'lxc';
> + const n1IsGuest = n1.type === 'qemu' || n1.type === 'lxc';
> + const n2IsGuest = n2.type === 'qemu' || n2.type === 'lxc';
> if (me['group-guest-types'] || !n1IsGuest || !n2IsGuest) {
> // first sort (group) by type
> - let res = me.getTypeOrder(n1.type) - me.getTypeOrder(n2.type);
> + const res = me.getTypeOrder(n1.type) - me.getTypeOrder(n2.type);
> if (res !== 0) {
> return res;
> }
> @@ -155,15 +155,15 @@ Ext.define('PVE.tree.ResourceTree', {
>
> // private: fast binary search
> findInsertIndex: function (node, child, start, end) {
> - let me = this;
> + const me = this;
>
> - let diff = end - start;
> + const diff = end - start;
> if (diff <= 0) {
> return start;
> }
> - let mid = start + (diff >> 1);
> + const mid = start + (diff >> 1);
>
> - let res = me.nodeSortFn(child, node.childNodes[mid]);
> + const res = me.nodeSortFn(child, node.childNodes[mid]);
> if (res <= 0) {
> return me.findInsertIndex(node, child, start, mid);
> } else {
> @@ -172,14 +172,14 @@ Ext.define('PVE.tree.ResourceTree', {
> },
>
> setIconCls: function (info) {
> - let cls = PVE.Utils.get_object_icon_class(info.type, info);
> + const cls = PVE.Utils.get_object_icon_class(info.type, info);
> if (cls !== '') {
> info.iconCls = cls;
> }
> },
>
> getToolTip: function (info) {
> - let qtips = [];
> + const qtips = [];
> if (info.qmpstatus || info.status) {
> qtips.push(Ext.String.format(gettext('Status: {0}'), info.qmpstatus || info.status));
> }
> @@ -190,7 +190,7 @@ Ext.define('PVE.tree.ResourceTree', {
> qtips.push(Ext.String.format(gettext('HA State: {0}'), info.hastate));
> }
> if (info.type === 'storage') {
> - let usage = info.disk / info.maxdisk;
> + const usage = info.disk / info.maxdisk;
> if (usage >= 0.0 && usage <= 1.0) {
> qtips.push(Ext.String.format(gettext('Usage: {0}%'), (usage * 100).toFixed(2)));
> }
> @@ -200,20 +200,20 @@ Ext.define('PVE.tree.ResourceTree', {
> return undefined;
> }
>
> - let tip = qtips.join(', ');
> + const tip = qtips.join(', ');
> info.tip = tip;
> return tip;
> },
>
> // private
> addChildSorted: function (node, info, insertPool = false) {
> - let me = this;
> + const me = this;
>
> me.setIconCls(info);
>
> - let nestPools = PVE.UIOptions.getTreeSortingValue('nest-pools');
> + const nestPools = PVE.UIOptions.getTreeSortingValue('nest-pools');
> if (info.type === 'pool' && info.pool && !insertPool && nestPools) {
> - let parentPool = info.pool.split('/').slice(0, -1).join('/');
> + const parentPool = info.pool.split('/').slice(0, -1).join('/');
> if (parentPool.length > 0) {
> let parent = node.findChild('id', `/pool/${parentPool}`, true);
> if (parent !== node) {
> @@ -233,7 +233,7 @@ Ext.define('PVE.tree.ResourceTree', {
> if (me.viewFilter.groupRenderer) {
> info.text = me.viewFilter.groupRenderer(info);
> } else {
> - let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
> + const defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
> if (defaults && defaults.text) {
> info.text = defaults.text;
> } else {
> @@ -241,10 +241,10 @@ Ext.define('PVE.tree.ResourceTree', {
> }
> }
> }
> - let child = Ext.create('PVETree', info);
> + const child = Ext.create('PVETree', info);
>
> if (node.childNodes) {
> - let pos = me.findInsertIndex(node, child, 0, node.childNodes.length);
> + const pos = me.findInsertIndex(node, child, 0, node.childNodes.length);
> node.insertBefore(child, node.childNodes[pos]);
> } else {
> node.insertBefore(child);
> @@ -255,10 +255,10 @@ Ext.define('PVE.tree.ResourceTree', {
>
> // private
> groupChild: function (node, info, groups, level) {
> - let me = this;
> + const me = this;
>
> - let groupBy = groups[level];
> - let v = info[groupBy];
> + const groupBy = groups[level];
> + const v = info[groupBy];
>
> if (v) {
> let group = node.findChild('groupbyid', v, true);
> @@ -273,7 +273,7 @@ Ext.define('PVE.tree.ResourceTree', {
> id: groupBy + '/' + v,
> };
> if (groupBy === 'type') {
> - let defaults = PVE.tree.ResourceTree.typeDefaults[v];
> + const defaults = PVE.tree.ResourceTree.typeDefaults[v];
> if (defaults && defaults.iconCls) {
> groupinfo.iconCls = defaults.iconCls;
> }
> @@ -296,10 +296,10 @@ Ext.define('PVE.tree.ResourceTree', {
> },
>
> saveSortingOptions: function () {
> - let me = this;
> + const me = this;
> let changed = false;
> for (const key of ['sort-field', 'group-templates', 'group-guest-types', 'nest-pools']) {
> - let newValue = PVE.UIOptions.getTreeSortingValue(key);
> + const newValue = PVE.UIOptions.getTreeSortingValue(key);
> if (me[key] !== newValue) {
> me[key] = newValue;
> changed = true;
> @@ -309,22 +309,22 @@ Ext.define('PVE.tree.ResourceTree', {
> },
>
> initComponent: function () {
> - let me = this;
> + const me = this;
> me.saveSortingOptions();
>
> - let rstore = PVE.data.ResourceStore;
> - let sp = Ext.state.Manager.getProvider();
> + const rstore = PVE.data.ResourceStore;
> + const sp = Ext.state.Manager.getProvider();
>
> if (!me.viewFilter) {
> me.viewFilter = {};
> }
>
> - let pdata = {
> + const pdata = {
> dataIndex: {},
> updateCount: 0,
> };
>
> - let store = Ext.create('Ext.data.TreeStore', {
> + const store = Ext.create('Ext.data.TreeStore', {
> model: 'PVETree',
> root: {
> expanded: true,
> @@ -334,7 +334,7 @@ Ext.define('PVE.tree.ResourceTree', {
> },
> });
>
> - let stateid = 'rid';
> + const stateid = 'rid';
>
> const changedFields = [
> 'disk',
> @@ -352,18 +352,18 @@ Ext.define('PVE.tree.ResourceTree', {
> ];
>
> // special case ids from the tag view, since they change the id in the state
> - let idMapFn = function (id) {
> + const idMapFn = function (id) {
> if (!id) {
> return undefined;
> }
> if (id.startsWith('qemu') || id.startsWith('lxc')) {
> - let [realId, _tag] = id.split('-');
> + const [realId, _tag] = id.split('-');
> return realId;
> }
> return id;
> };
>
> - let findNode = function (rootNode, id) {
> + const findNode = function (rootNode, id) {
> if (!id) {
> return undefined;
> }
> @@ -380,7 +380,7 @@ Ext.define('PVE.tree.ResourceTree', {
>
> let firstUpdate = true;
>
> - let updateTree = function () {
> + const updateTree = function () {
> store.suspendEvents();
>
> let rootnode;
> @@ -395,30 +395,30 @@ Ext.define('PVE.tree.ResourceTree', {
> rootnode = me.store.getRootNode();
> }
> // remember selected node (and all parents)
> - let sm = me.getSelectionModel();
> + const sm = me.getSelectionModel();
> let lastsel = sm.getSelection()[0];
> - let parents = [];
> - let sorting_changed = me.saveSortingOptions();
> + const parents = [];
> + const sorting_changed = me.saveSortingOptions();
> for (let node = lastsel; node; node = node.parentNode) {
> parents.push(node);
> }
>
> - let groups = me.viewFilter.groups || [];
> + const groups = me.viewFilter.groups || [];
> // explicitly check for node/template, as those are not always grouping attributes
> - let attrMoveChecks = me.viewFilter.attrMoveChecks ?? {};
> + const attrMoveChecks = me.viewFilter.attrMoveChecks ?? {};
>
> // also check for name for when the tree is sorted by name
> - let moveCheckAttrs = groups.concat(['node', 'template', 'name']);
> - let filterFn = me.viewFilter.getFilterFn ? me.viewFilter.getFilterFn() : Ext.identityFn;
> + const moveCheckAttrs = groups.concat(['node', 'template', 'name']);
> + const filterFn = me.viewFilter.getFilterFn ? me.viewFilter.getFilterFn() : Ext.identityFn;
>
> let reselect = false; // for disappeared nodes
> - let index = pdata.dataIndex;
> + const index = pdata.dataIndex;
> // remove vanished or moved items and update changed items in-place
> for (const [key, olditem] of Object.entries(index)) {
> // getById() use find(), which is slow (ExtJS4 DP5)
> - let oldid = olditem.data.id;
> - let id = idMapFn(olditem.data.id);
> - let item = rstore.data.get(id);
> + const oldid = olditem.data.id;
> + const id = idMapFn(olditem.data.id);
> + const item = rstore.data.get(id);
>
> let changed = sorting_changed,
> moved = sorting_changed;
> @@ -448,7 +448,7 @@ Ext.define('PVE.tree.ResourceTree', {
>
> if (changed) {
> olditem.beginEdit();
> - let info = olditem.data;
> + const info = olditem.data;
> Ext.apply(info, item.data);
> if (info.id !== oldid) {
> info.id = oldid;
> @@ -458,7 +458,7 @@ Ext.define('PVE.tree.ResourceTree', {
> }
> if ((!item || moved) && olditem.isLeaf()) {
> delete index[key];
> - let parentNode = olditem.parentNode;
> + const parentNode = olditem.parentNode;
> // a selected item moved (migration) or disappeared (destroyed), so deselect that
> // node now and try to reselect the moved (or its parent) node later
> if (lastsel && olditem.data.id === lastsel.data.id) {
> @@ -469,25 +469,25 @@ Ext.define('PVE.tree.ResourceTree', {
> store.remove(olditem);
> parentNode.removeChild(olditem, true);
> if (parentNode.childNodes.length < 1 && parentNode.parentNode) {
> - let grandParent = parentNode.parentNode;
> + const grandParent = parentNode.parentNode;
> grandParent.removeChild(parentNode, true);
> }
> }
> }
>
> - let items = rstore.getData().items.flatMap(me.viewFilter.itemMap ?? Ext.identityFn);
> + const items = rstore.getData().items.flatMap(me.viewFilter.itemMap ?? Ext.identityFn);
> items.forEach(function (item) {
> // add new items
> - let olditem = index[item.data.id];
> + const olditem = index[item.data.id];
> if (olditem) {
> return;
> }
> if (filterFn && !filterFn(item)) {
> return;
> }
> - let info = Ext.apply({ leaf: true }, item.data);
> + const info = Ext.apply({ leaf: true }, item.data);
>
> - let child = me.groupChild(rootnode, info, groups, 0);
> + const child = me.groupChild(rootnode, info, groups, 0);
> if (child) {
> index[item.data.id] = child;
> }
> @@ -496,7 +496,7 @@ Ext.define('PVE.tree.ResourceTree', {
> store.resumeEvents();
> store.fireEvent('refresh', store);
>
> - let foundChild = findNode(rootnode, lastsel?.data.id);
> + const foundChild = findNode(rootnode, lastsel?.data.id);
>
> // select parent node if original selected node vanished
> if (lastsel && !foundChild) {
> @@ -544,12 +544,12 @@ Ext.define('PVE.tree.ResourceTree', {
> rstore.un('load', updateTree);
> },
> beforecellmousedown: function (tree, td, cellIndex, record, tr, rowIndex, ev) {
> - let sm = me.getSelectionModel();
> + const sm = me.getSelectionModel();
> // disable selection when right clicking except if the record is already selected
> me.allowSelection = ev.button !== 2 || sm.isSelected(record);
> },
> beforeselect: function (tree, record, index, eopts) {
> - let allow = me.allowSelection;
> + const allow = me.allowSelection;
> me.allowSelection = true;
> return allow;
> },
> @@ -558,7 +558,7 @@ Ext.define('PVE.tree.ResourceTree', {
> if (me.tip) {
> return;
> }
> - let selectors = [
> + const selectors = [
> '.x-tree-node-text > span:not(.proxmox-tag-dark):not(.proxmox-tag-light)',
> '.x-tree-icon',
> ];
> @@ -569,8 +569,8 @@ Ext.define('PVE.tree.ResourceTree', {
> renderTo: Ext.getBody(),
> listeners: {
> beforeshow: function (tip) {
> - let rec = me.getView().getRecord(tip.triggerElement);
> - let tipText = me.getToolTip(rec.data);
> + const rec = me.getView().getRecord(tip.triggerElement);
> + const tipText = me.getToolTip(rec.data);
> if (tipText) {
> tip.update(tipText);
> return true;
> @@ -587,7 +587,7 @@ Ext.define('PVE.tree.ResourceTree', {
> },
> clearTree: function () {
> pdata.updateCount = 0;
> - let rootnode = me.store.getRootNode();
> + const rootnode = me.store.getRootNode();
> rootnode.collapse();
> rootnode.removeAll();
> pdata.dataIndex = {};
> @@ -598,7 +598,7 @@ Ext.define('PVE.tree.ResourceTree', {
> updateTree();
> },
> selectExpand: function (node) {
> - let sm = me.getSelectionModel();
> + const sm = me.getSelectionModel();
> if (!sm.isSelected(node)) {
> sm.select(node);
> for (let iter = node; iter; iter = iter.parentNode) {
> @@ -610,7 +610,7 @@ Ext.define('PVE.tree.ResourceTree', {
> }
> },
> selectById: function (nodeid) {
> - let rootnode = me.store.getRootNode();
> + const rootnode = me.store.getRootNode();
> let node;
> if (nodeid === 'root') {
> node = rootnode;
> @@ -643,7 +643,7 @@ Ext.define('PVE.tree.ResourceTree', {
> before: function (node) {
> if (node.data.groupbyid) {
> node.beginEdit();
> - let info = node.data;
> + const info = node.data;
> me.setIconCls(info);
> if (me.viewFilter.groupRenderer) {
> info.text = me.viewFilter.groupRenderer(info);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH manager 1/2] fix #7136: ui: tree: harmonize folder view resource ordering
2026-08-18 9:15 ` Dominik Csapak
@ 2026-08-18 9:36 ` Elias Huhsovitz
0 siblings, 0 replies; 7+ messages in thread
From: Elias Huhsovitz @ 2026-08-18 9:36 UTC (permalink / raw)
To: Dominik Csapak, pve-devel
Thanks for the quick reply!
Sorry, i didnt notice the bugs, i am always a bit blind when it comes to
this stuff.
I guess in that case i will opt for simply checking
type === 'type'
It felt like a band-aid fix, so i wanted to avoid it, but considering
the intricacies of the frontend code base i belive you are correct.
I will prepare the patch!
On Tue Aug 18, 2026 at 11:15 AM CEST, Dominik Csapak wrote:
> while the code here looks ok this produces an issue with all places
> where we use the type:
> * selecting a category now tries to load the wrong panel type
> (see Workspace.js:236)
> * right clicking on vm/container throws an exception
> * resource pool folder has the wrong text ('Root' instead of
> 'Resource-Pools')
>
> to fix the bug, wouldn't it be easier to use the 'groupbyid' field
> in case the type === 'type' ?
>
> then we don't have to touch that semantic at all
>
>
> also one line is not correctly formatted, running make tidy before
> submitting would be great :)
>
> On 8/18/26 10:31 AM, Elias Huhsovitz wrote:
>> Grouping nodes in the Folder view are assigned the literal type: 'type'
>> instead of their actual type. This causes `getTypeOrder` to return a
>> default value for all groups.
>>
>> This causes Resources in the Folder View to be ordered
>> lexicographically, whereas the Server View correctly relies on the
>> `getTypeOrder` function.
>>
>> Match the Server View ordering by setting the grouping node's `type` to
>> the actual resource type when grouping by `type`. This way
>> `getTypeOrder` receives the correct type for sorting.
>>
>> Simplify the text resolution logic in `addChildSorted`.
>>
>> Propagate `iconCls` from `typeDefaults` to ensure grouping nodes
>> display the correct icons.
>>
>> Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
>> ---
>> www/manager6/tree/ResourceTree.js | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
>> index 6ea28919..a7723095 100644
>> --- a/www/manager6/tree/ResourceTree.js
>> +++ b/www/manager6/tree/ResourceTree.js
>> @@ -232,13 +232,13 @@ Ext.define('PVE.tree.ResourceTree', {
>> if (info.groupbyid) {
>> if (me.viewFilter.groupRenderer) {
>> info.text = me.viewFilter.groupRenderer(info);
>> - } else if (info.type === 'type') {
>> + } else {
>> let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
>> if (defaults && defaults.text) {
>> info.text = defaults.text;
>> + } else {
>> + info.text = info.groupbyid;
>> }
>> - } else {
>> - info.text = info.groupbyid;
>> }
>> }
>> let child = Ext.create('PVETree', info);
>> @@ -267,11 +267,17 @@ Ext.define('PVE.tree.ResourceTree', {
>> if (info.type === groupBy) {
>> groupinfo = info;
>> } else {
>> + const type = groupBy === 'type' ? v : groupBy;
>> groupinfo = {
>> - type: groupBy,
>> + type: type,
>> id: groupBy + '/' + v,
>> };
>> - if (groupBy !== 'type') {
>> + if (groupBy === 'type') {
>> + let defaults = PVE.tree.ResourceTree.typeDefaults[v];
>> + if (defaults && defaults.iconCls) {
>> + groupinfo.iconCls = defaults.iconCls;
>> + }
>> + } else {
>> groupinfo[groupBy] = v;
>> }
>> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* superseded: [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering
2026-08-18 8:31 [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 1/2] " Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword Elias Huhsovitz
@ 2026-08-18 12:09 ` Elias Huhsovitz
2 siblings, 0 replies; 7+ messages in thread
From: Elias Huhsovitz @ 2026-08-18 12:09 UTC (permalink / raw)
To: Elias Huhsovitz, pve-devel
Superseded-by: https://lore.proxmox.com/pve-devel/20260818115237.91447-1-e.huhsovitz@proxmox.com/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-18 12:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 8:31 [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 1/2] " Elias Huhsovitz
2026-08-18 9:15 ` Dominik Csapak
2026-08-18 9:36 ` Elias Huhsovitz
2026-08-18 8:31 ` [PATCH manager 2/2] ui: tree: reduce reduce usage of `let` keyword Elias Huhsovitz
2026-08-18 9:15 ` Dominik Csapak
2026-08-18 12:09 ` superseded: [PATCH manager 0/2] fix #7136: ui: tree: harmonize folder view resource ordering Elias Huhsovitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.