[pve-devel] [PATCH manager 2/4] fix lxc hostname editing

Emmanuel Kasper e.kasper at proxmox.com
Wed Jul 6 12:05:44 CEST 2016


Hi
I put the comments inline

> also instead of simply setting the emptyText to the name on
> tab creation, we load the window, which then gets the actual value
acked


> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  www/manager6/lxc/DNS.js | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/www/manager6/lxc/DNS.js b/www/manager6/lxc/DNS.js
> index eaf1719..6fa8ef2 100644
> --- a/www/manager6/lxc/DNS.js
> +++ b/www/manager6/lxc/DNS.js
> @@ -156,7 +156,7 @@ Ext.define('PVE.lxc.DNS', {
>  	var rows = {
>  	    hostname: {
>  		required: true,
> -		defaultValue: me.pveSelNode.data.name,
> +		defaultValue: 'localhost',
>  		header: gettext('Hostname'),
>  		editor: caps.vms['VM.Config.Network'] ? {
>  		    xtype: 'pveWindowEdit',
> @@ -167,8 +167,8 @@ Ext.define('PVE.lxc.DNS', {
>  			vtype: 'DnsName',
>  			value: '',
>  			fieldLabel: gettext('Hostname'),
> -			allowBlank: true,
> -			emptyText: me.pveSelNode.data.name
> +			allowBlank: false,
> +			emptyText: 'localhost'

not sure if localhost itself is not a particulary good placeholder, as
it is not unique, and will create a duplicate entry in the /etc/hosts
container

if we're not sure what to put here, I would rather get rid of the
defaultValue: localhost
emptyText: localhost
since usability for placeholders is anyway debattable
https://www.nngroup.com/articles/form-design-placeholders/

>  		    }
>  		} : undefined
>  	    },
> @@ -229,8 +229,9 @@ Ext.define('PVE.lxc.DNS', {
>  		    url: '/api2/extjs/' + baseurl
>  		}, rowdef.editor);
>  		win = Ext.createWidget(rowdef.editor.xtype, config);
> +		win.load();
>  	    }
> -	    //win.load();
> +
>  	    win.show();
>  	    win.on('destroy', reload);
>  	};
> 
good catch, having this field alwasw greyed out was confusing




More information about the pve-devel mailing list