public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH manager 1/2] fix #7136: ui: tree: harmonize folder view resource ordering
Date: Tue, 18 Aug 2026 11:36:49 +0200	[thread overview]
Message-ID: <DKRYVBDZN6OP.25ZUA0OJZR8RB@proxmox.com> (raw)
In-Reply-To: <d121fbef-aa49-4fd0-b2f9-1b6341d80e12@proxmox.com>

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;
>>                       }
>>                   }





  reply	other threads:[~2026-08-18  9:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKRYVBDZN6OP.25ZUA0OJZR8RB@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal