[pve-devel] [PATCH v3 manager] Make CPU Model Selector a grouped grid view

[Thomas Lamprecht]

On 10.09.19 12:20, Stefan Reiter wrote:
> Uses a ComboGrid with grouping feature to segment different CPU vendors.
> Allows a user to show/hide groups.
> 
> Doesn't work with value set in widget definition for some reason, but
> we always use setValue() anyway (and if we don't, value will be '', aka
> the default, which is correct too).
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> I'm personally not 100% convinced this grid approach makes it easier to parse
> for a user, but I'd be fine with this being applied, if consensus is otherwise.
> 
> A search function like with our ISO selector for example would be nice too I
> think, but I could not get that to work with the grouping feature at all.

Hmm, yeah, I'm on your side. Base issue for me is that I do not really like
how it is now, but neither your proposed version nor mine/Dominik's one..

Multilevel "drop down" could be still better? 

Maybe also just the grid as here, but without the grouping?

On-top of you work we could have a setting of which models to show at all in
the selector? May not make sense for some admins to have Intel CPUs listed if
they run an all-AMD CPU backed cluster..

> 
> * v3: Use ComboGrid (grouped) instead of list with headings
> 
> 
>  www/manager6/form/CPUModelSelector.js | 249 ++++++++++++++++++++++----
>  www/manager6/qemu/ProcessorEdit.js    |   1 -
>  2 files changed, 214 insertions(+), 36 deletions(-)
> 
> diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
> index 9eb5b0e9..ce6c8f33 100644
> --- a/www/manager6/form/CPUModelSelector.js
> +++ b/www/manager6/form/CPUModelSelector.js
> @@ -1,38 +1,217 @@
>  Ext.define('PVE.form.CPUModelSelector', {
> -    extend: 'Proxmox.form.KVComboBox',
> +    extend: 'Proxmox.form.ComboGrid',
>      alias: ['widget.CPUModelSelector'],
> -    comboItems: [
> -	['__default__', Proxmox.Utils.defaultText + ' (kvm64)'],
> -	['486', '486'],
> -	['athlon', 'athlon'],
> -	['core2duo', 'core2duo'],
> -	['coreduo', 'coreduo'],
> -	['kvm32', 'kvm32'],
> -	['kvm64', 'kvm64'],
> -	['pentium', 'pentium'],
> -	['pentium2', 'pentium2'],
> -	['pentium3', 'pentium3'],
> -	['phenom', 'phenom'],
> -	['qemu32', 'qemu32'],
> -	['qemu64', 'qemu64'],
> -	['Conroe', 'Conroe'],
> -	['Penryn', 'Penryn'],
> -	['Nehalem', 'Nehalem'],
> -	['Westmere', 'Westmere'],
> -	['SandyBridge', 'SandyBridge'],
> -	['IvyBridge', 'IvyBridge'],
> -	['Haswell', 'Haswell'],
> -	['Haswell-noTSX','Haswell-noTSX'],
> -	['Broadwell', 'Broadwell'],
> -	['Broadwell-noTSX','Broadwell-noTSX'],
> -	['Skylake-Client','Skylake-Client'],
> -	['Opteron_G1', 'Opteron_G1'],
> -	['Opteron_G2', 'Opteron_G2'],
> -	['Opteron_G3', 'Opteron_G3'],
> -	['Opteron_G4', 'Opteron_G4'],
> -	['Opteron_G5', 'Opteron_G5'],
> -	['EPYC', 'EPYC'],
> -	['host', 'host']
> -
> -    ]
> +
> +    valueField: 'value',
> +    displayField: 'name',

Why don't you just use the same column for both, which would allow to reduce the
double definitions in the model below?

Or else format the "name" field content a bit nicer, e.g.:

s/athlon/Athlon/
/Operon_G1/Opteron G1/

> +
> +    emptyText: Proxmox.Utils.defaultText + ' (kvm64)',
> +    allowBlank: true,
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	var groupingFeature = Ext.create('Ext.grid.feature.Grouping', {
> +            groupHeaderTpl: '{[ values.name ]}',

I tried with:
startCollapsed: true,

not to sure..

> +	    enableGroupingMenu: false
> +	});
> +	

trailing white space errror in empty line above

> +	Ext.apply(me.listConfig, {
> +	    features: [groupingFeature]
> +	});
> +
> +	me.callParent();
> +    },
> +
> +    getSubmitData: function () {
> +	var me = this,
> +	    val = me.getSubmitValue(),
> +	    data;
> +
> +	if (val === '') {
> +	    data = { delete: me.getName() };
> +	} else {
> +	    data = {};
> +	    data[me.getName()] = val;
> +	}
> +
> +	return data;
> +    },
> +
> +    listConfig: {
> +	columns: [
> +	    {
> +		header: gettext('Model'),
> +		dataIndex: 'name',
> +		hideable: false,
> +		sortable: false,
> +		flex: 1
> +	    },
> +	    {
> +		header: gettext('Vendor'),
> +		dataIndex: 'vendor',
> +		hideable: false,
> +		sortable: false,
> +		width: 65
> +	    }
> +	],
> +	width: 280

imo a width of >= 300 and flex for both (3:2 or the like) looks a bit better.

> +    },
> +
> +    store: {
> +	fields: [ 'value', 'name', 'vendor'],
> +	groupField: 'vendor',
> +	data: [
> +	    {
> +		value: '486',
> +		name: '486',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'athlon',
> +		name: 'athlon',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'core2duo',
> +		name: 'core2duo',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'coreduo',
> +		name: 'coreduo',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'kvm32',
> +		name: 'kvm32',
> +		vendor: 'Other'
> +	    },
> +	    {
> +		value: 'kvm64',
> +		name: 'kvm64',
> +		vendor: 'Other'
> +	    },
> +	    {
> +		value: 'pentium',
> +		name: 'pentium',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'pentium2',
> +		name: 'pentium2',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'pentium3',
> +		name: 'pentium3',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'phenom',
> +		name: 'phenom',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'qemu32',
> +		name: 'qemu32',
> +		vendor: 'Other'
> +	    },
> +	    {
> +		value: 'qemu64',
> +		name: 'qemu64',
> +		vendor: 'Other'
> +	    },
> +	    {
> +		value: 'Conroe',
> +		name: 'Conroe',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Penryn',
> +		name: 'Penryn',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Nehalem',
> +		name: 'Nehalem',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Westmere',
> +		name: 'Westmere',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'SandyBridge',
> +		name: 'SandyBridge',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'IvyBridge',
> +		name: 'IvyBridge',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Haswell',
> +		name: 'Haswell',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Haswell-noTSX',
> +		name:'Haswell-noTSX',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Broadwell',
> +		name: 'Broadwell',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Broadwell-noTSX',
> +		name:'Broadwell-noTSX',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Skylake-Client',
> +		name:'Skylake-Client',
> +		vendor: 'Intel'
> +	    },
> +	    {
> +		value: 'Opteron_G1',
> +		name: 'Opteron_G1',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'Opteron_G2',
> +		name: 'Opteron_G2',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'Opteron_G3',
> +		name: 'Opteron_G3',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'Opteron_G4',
> +		name: 'Opteron_G4',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'Opteron_G5',
> +		name: 'Opteron_G5',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'EPYC',
> +		name: 'EPYC',
> +		vendor: 'AMD'
> +	    },
> +	    {
> +		value: 'host',
> +		name: 'host',
> +		vendor: 'Other'
> +	    }
> +	]
> +    }
>  });
> diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js
> index c62dc734..bc17e152 100644
> --- a/www/manager6/qemu/ProcessorEdit.js
> +++ b/www/manager6/qemu/ProcessorEdit.js
> @@ -103,7 +103,6 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
>  	{
>  	    xtype: 'CPUModelSelector',
>  	    name: 'cputype',
> -	    value: '__default__',
>  	    fieldLabel: gettext('Type')
>  	},
>  	{
>