Re: [pve-devel] [PATCH manager 3/3] ui: resource tree: show nodes/storages in pool/tag view by default

[Dominik Csapak <d.csapak@proxmox.com>]

On 11/12/24 14:38, Thomas Lamprecht wrote:
> Am 12.11.24 um 12:45 schrieb Dominik Csapak:
>> and make it configurable in the tree browser settings.
>> this makes now use of the new sorting order and the more efficient
>> 'getFilterFn' method of the viewSelector
> 
> works OK besides some eslint warnings (see below), but what I noticed is
> that we never listed storages under their assigned pools, which is IMO
> a bit odd and might be something to fix separately.

yeah looking at this, i noticed we don't return the pools for storages
at all currently in the resources api call.

Also we can have a storage in multiple pools (in contrast to guests)
so that is a bit of a problem here. I could return the pools as comma separated
list (that would satisfy the return schema, but isn't pretty) and
use the same mechanism for duplicating storages as for the tag view.

should i go this route or should i omit this for the pool view for now
and simply add the toggle for the tag view?

> 
> Btw. having this default one for pool view might not be welcomed by all,
> as some used this view to hide the nodes and that stuff for those people
> that are "afraid" of them (managers and such), but no hard feelings here.
> 
> Grouping the resource without tag or pool under a collapsible node, say
> "Resources without Tag/Pool" for avoiding spending time with naming, might
> be a trade-off here.
> 
> Btw. having an expand-all and collapse-all inline button between view
> selection and settings might be nice too for bigger setups, and probably
> not that much work. I.e., somewhat similar to how the API viewer endpoint
> tree has those [0], just not as header tools.
> 
> [0]: https://pve.proxmox.com/pve-docs/api-viewer/index.html
> 
> 
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> not sure if we should make each option seperate, so either
>> separate options for tag/pool view
>>
>> or separate options for nodes/storages (sdn maybe too?)
>> or both (that would be 4 new options)
> 
> 
> If I'd do a multi-select combobox for each Pool and Tag view, but we might wait
> that out to see if there is actual user demand.
> 
>>
>>   www/manager6/UIOptions.js               |  1 +
>>   www/manager6/form/ViewSelector.js       | 16 ++++++++++++++--
>>   www/manager6/tree/ResourceTree.js       |  2 +-
>>   www/manager6/window/TreeSettingsEdit.js | 13 +++++++++++++
>>   4 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/www/manager6/UIOptions.js b/www/manager6/UIOptions.js
>> index 057c8f03..97b689b0 100644
>> --- a/www/manager6/UIOptions.js
>> +++ b/www/manager6/UIOptions.js
>> @@ -93,6 +93,7 @@ Ext.define('PVE.UIOptions', {
>>   	    'sort-field': 'vmid',
>>   	    'group-templates': true,
>>   	    'group-guest-types': true,
>> +	    'more-types': true,
>>   	};
>>   
>>   	return browserValues?.[key] ?? defaults[key];
>> diff --git a/www/manager6/form/ViewSelector.js b/www/manager6/form/ViewSelector.js
>> index f5de5c8e..8c656fc8 100644
>> --- a/www/manager6/form/ViewSelector.js
>> +++ b/www/manager6/form/ViewSelector.js
>> @@ -30,12 +30,24 @@ Ext.define('PVE.form.ViewSelector', {
>>   		text: gettext('Pool View'),
>>   		groups: ['pool'],
>>   		// Pool View only lists VMs and Containers
>> -		getFilterFn: () => ({ data }) => data.type === 'qemu' || data.type === 'lxc' || data.type === 'pool',
>> +		getFilterFn: function() {
>> +		    let types = ['qemu', 'lxc', 'pool'];
>> +		    if (PVE.UIOptions.getTreeSortingValue('more-types')) {
>> +			types.push('node', 'storage');
>> +		    }
>> +		    return ({data}) => types.indexOf(data.type) !== -1;
> 
> above causes eslint to output warnings due to missing spaces for `{ data }`
> 
>> +		},
>>   	    },
>>   	    tags: {
>>   		text: gettext('Tag View'),
>>   		groups: ['tag'],
>> -		getFilterFn: () => ({ data }) => data.type === 'qemu' || data.type === 'lxc',
>> +		getFilterFn: function() {
>> +		    let types = ['qemu', 'lxc'];
>> +		    if (PVE.UIOptions.getTreeSortingValue('more-types')) {
>> +			types.push('node', 'storage');
>> +		    }
>> +		    return ({data}) => types.indexOf(data.type) !== -1;
> 
> same eslint warning as above
> 

sorry, i have to stop testing gui patches with just 'make install'...


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel