[pve-devel] [PATCH v3 widget-toolkit] add network selector widget

[Tim Marx]

> Dominik Csapak <d.csapak at proxmox.com> hat am 11. Juni 2019 um 16:58 geschrieben:
> 
> 
> high level looks ok, some comments inline
> 
> On 6/11/19 2:04 PM, Tim Marx wrote:
> > Signed-off-by: Tim Marx <t.marx at proxmox.com>
> > ---
> >   Makefile                |   1 +
> >   form/NetworkSelector.js | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 122 insertions(+)
> >   create mode 100644 form/NetworkSelector.js
> > 
> > diff --git a/Makefile b/Makefile
> > index b9dc8b9..d12a4da 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -34,6 +34,7 @@ JSSRC=					\
> >   	form/ComboGrid.js		\
> >   	form/RRDTypeSelector.js		\
> >   	form/BondModeSelector.js	\
> > +	form/NetworkSelector.js		\
> >   	button/Button.js		\
> >   	button/HelpButton.js		\
> >   	grid/ObjectGrid.js		\
> > diff --git a/form/NetworkSelector.js b/form/NetworkSelector.js
> > new file mode 100644
> > index 0000000..fa512e7
> > --- /dev/null
> > +++ b/form/NetworkSelector.js
> > @@ -0,0 +1,121 @@
> > +Ext.define('Proxmox.form.NetworkSelectorController', {
> > +    extend: 'Ext.app.ViewController',
> > +    alias: 'controller.proxmoxNetworkSelectorController',
> > +
> > +    init: function(view) {
> > +	var me = this;
> > +
> > +	if (!view.nodename) {
> > +	    throw "missing custom view config: nodename";
> > +	}
> > +	view.getStore().getProxy().setUrl('/api2/json/nodes/'+ view.nodename + '/network');
> > +    }
> > +});
> 
> any reason why you define it outside of the NetworkSelector (like we do 
> everywhere else with xclass: 'Ext.app.ViewController' ?)
> 

Not a specific one, it's just a little bit more readable for me.

> > +
> > +Ext.define('Proxmox.data.NetworkSelector', {
> > +    extend: 'Ext.data.Model',
> > +    fields: [
> > +	{name: 'active'},
> > +	{name: 'cidr'},
> > +	{name: 'cidr6'},
> > +	{name: 'comments'},
> > +	{name: 'iface'},
> > +	{name: 'slaves'},
> > +	{name: 'type'}
> > +    ]
> > +});
> > +
> > +Ext.define('Proxmox.form.NetworkSelector', {
> > +    extend: 'Proxmox.form.ComboGrid',
> > +    alias: 'widget.proxmoxNetworkSelector',
> > +
> > +    nodename: 'localhost',
> > +    controller: 'proxmoxNetworkSelectorController',
> > +    setNodename: function(nodename) {
> > +	this.nodename = nodename;
> > +	var networkSelectorStore = this.getStore();
> > +	networkSelectorStore.removeAll();
> > +	// because of manual local copy of data for ip4/6
> > +	this.getPicker().refresh();
> > +	if (networkSelectorStore && typeof networkSelectorStore.getProxy === 'function') {
> > +	    networkSelectorStore.getProxy().setUrl('/api2/json/nodes/'+ nodename + '/network');
> > +	    networkSelectorStore.load();
> > +	}
> > +    },
> > +    // set default value to empty array, else it inits it with
> > +    // null and after the store load it is an empty array,
> > +    // triggering dirtychange
> > +    value: [],
> > +    valueField: 'cidr',
> > +    displayField: 'cidr',
> > +    store: {
> > +	autoLoad: true,
> > +	model: 'Proxmox.data.NetworkSelector',
> > +	proxy: {
> > +	    type: 'proxmox'
> > +	},
> > +	sorters: [
> > +	    {
> > +		property : 'iface',
> > +		direction: 'ASC'
> > +	    }
> > +	],
> > +	filters: [
> > +	      function(item) {
> > +		return item.data.cidr;
> we should not ignore interfaces with ipv6 only
> 

Had a convert logic in place already, but than switched to copy each record if they have both defined. The convert got lost on the way. Will send a v4 with the adaptions.

> > +	    }
> > +	],
> > +	listeners: {
> > +	    load: function(records, successfull) {
> 
> this signature is wrong, the load event has function(store, records, 
> success,...)
> 
> it works by accident since the this is the store, the store has an 
> 'each' method  if there are no records it does not call your function

Yeah you right, I somehow mixed the load function and load event from the ext docs. Thanks for that^^

> 
> > +		var store = this;
> > +		if(successfull) {
> > +		    records.each(function(record) {
> > +			if(record.data.cidr && record.data.cidr6) {
> > +			    var tempcopy = record.copy(null);
> > +			    tempcopy.data.cidr = tempcopy.data.cidr6;
> > +			    tempcopy.data.comment = tempcopy.data.comments6;
> > +			    store.add(tempcopy);
> > +			}
> > +		    });
> > +		}
> > +	    }
> > +	}
> > +    },
> > +    listConfig: {
> > +	width: 600,
> > +	columns: [
> > +	    {
> > +		header: gettext('Interface'),
> > +		sortable: true,
> > +		flex:1,
> > +		dataIndex: 'iface'
> > +	    },
> > +	    {
> > +		header: gettext('Active'),
> > +		sortable: true,
> > +		flex:1,
> > +		dataIndex: 'active'
> > +	    },
> > +	    {
> > +
> > +		header: gettext('CIDR'),
> > +		dataIndex: 'cidr',
> > +		sortable: true,
> > +		hideable: false,
> > +		flex:1
> > +	    },
> 
> same here regarding ignoring ipv6 only, if we have an interface with 
> ipv6 only, it does not show up better would be probably a 'calculated 
> field' that returns
> cidr || cidr6
> and show that

same as above
> 
> > +	    {
> > +		header: gettext('Type'),
> > +		sortable: true,
> > +		flex:1,
> > +		dataIndex: 'type'
> > +	    },
> > +	    {
> > +		header: gettext('Comment'),
> > +		sortable: true,
> > +		flex:1,
> > +		dataIndex: 'comments'
> > +	    }
> > +	]
> > +    }
> > +});
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel