[pve-devel] [PATCH manager] Add mac prefix to the datacenter options

[Wolfgang Bumiller]

On Fri, Jul 15, 2016 at 10:11:47AM +0200, Dominik Csapak wrote:
> comments inline
> 
> On 07/15/2016 10:02 AM, Wolfgang Bumiller wrote:
> >---
> > www/manager6/dc/OptionView.js | 36 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 36 insertions(+)
> >
> >diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
> >index 2149cb0..11f563c 100644
> >--- a/www/manager6/dc/OptionView.js
> >+++ b/www/manager6/dc/OptionView.js
> >@@ -99,6 +99,31 @@ Ext.define('PVE.dc.EmailFromEdit', {
> >     }
> > });
> >
> >+Ext.define('PVE.dc.MacPrefixEdit', {
> >+    extend: 'PVE.window.Edit',
> >+
> >+    initComponent : function() {
> >+	var me = this;
> >+
> >+	Ext.applyIf(me, {
> >+	    subject: gettext('MAC address prefix'),
> >+	    items: {
> >+		xtype: 'pvetextfield',
> >+		name: 'mac_prefix',
> >+		regex: /[a-f0-9]{2}(?::[a-f0-9]{2}){0,2}:?/i,
> 
> you should add a 'regexText' to display a better error if this regex fails
> (e.g an example)
> 
> also you are missing the ^ at the beginning and the $ at the end
> else a string like <anything>af matches also

Ah right, in the perl schema we anchor them in the backend.

> 
> >+		emptyText: 'none',
> >+		deleteEmpty: true,
> >+		value: '',
> >+		fieldLabel: gettext('MAC address prefix')
> >+	    }
> >+	});
> >+
> >+	me.callParent();
> >+
> >+	me.load();
> >+    }
> >+});
> >+
> > Ext.define('PVE.dc.OptionView', {
> >     extend: 'PVE.grid.ObjectGrid',
> >     alias: ['widget.pveDcOptionView'],
> >@@ -146,6 +171,17 @@ Ext.define('PVE.dc.OptionView', {
> > 		    }
> > 		    return value;
> > 		}
> >+	    },
> >+	    mac_prefix: {
> >+		header: gettext('MAC address prefix'),
> >+		editor: 'PVE.dc.MacPrefixEdit',
> >+		required: true,
> >+		renderer: function(value) {
> >+		    if (!value) {
> >+			return 'Do not use a prefix for MAC addresses';
> 
> this should be a shorter and more informative message like
> 'No MAC Prefix' as it is, it sounds like 'you should not use a prefix'
> instead of 'there is no prefix'
> 
> also it should be inside a gettext

I just stuck to the style from the file, eg. we show 'Do not use any
proxy' in the 'HTTP proxy' field.

> 
> >+		    }
> >+		    return value;
> >+		}
> > 	    }
> > 	};
> >
> >