> Dominik Csapak <d.csa...@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.m...@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@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
