Re: [pve-devel] [PATCH pve-manager 10/11] sdn: add fabric edit/delete forms

[Gabriel Goller <g.goller@proxmox.com>]

>> diff --git a/www/manager6/sdn/fabrics/Common.js b/www/manager6/sdn/fabrics/Common.js
>> new file mode 100644
>> index 000000000000..72ec093fc928
>> --- /dev/null
>> +++ b/www/manager6/sdn/fabrics/Common.js
>> @@ -0,0 +1,222 @@
>> +Ext.define('PVE.sdn.Fabric.InterfacePanel', {
>> +    extend: 'Ext.grid.Panel',
>> +    mixins: ['Ext.form.field.Field'],
>> +
>> +    network_interfaces: undefined,
>> +
>> +    selectionChange: function(_grid, _selection) {
>> +	let me = this;
>> +	me.value = me.getSelection().map((rec) => {
>> +	    delete rec.data.cidr;
>> +	    delete rec.data.cidr6;
>> +	    delete rec.data.selected;
>> +	    return PVE.Parser.printPropertyString(rec.data);
>
>maybe we could explicitly select the fields we want to include here, so
>this doesn't break when we add new fields?

Depends on which fields :)
If we add fields to the interface, it won't break, if we add more
"display-only" fields it will break.

Anyway this is a common component, so we would need to pass/add a check for
the protocol and then select the protocol specific attributes.

>> +    updateSelectedInterfaces: function(values) {
>> +	let me = this;
>> +	if (values) {
>> +	    let recs = [];
>> +	    let store = me.getStore();
>> +
>> +	    for (const i of values) {
>> +		let rec = store.getById(i.name);
>> +		if (rec) {
>> +		    recs.push(rec);
>> +		}
>> +	    }
>> +	    me.suspendEvent('change');
>> +	    me.setSelection();
>> +	    me.setSelection(recs);
>> +	    me.resumeEvent('change');
>> +	} else {
>> +	    me.suspendEvent('change');
>> +	    me.setSelection();
>> +	    me.resumeEvent('change');
>> +	}
>
>could avoid some duplication by moving the methods calls above / below
>the if/else

I can extract the resumeEvent call, but keeping suspendEvent within each
branch is safer. If store operations fail between suspend and resume,
we'd risk permanently disabling the 'change' event listener.

Thanks!


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