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

[Wolfgang Bumiller]

On Wed, Jul 06, 2016 at 12:05:44PM +0200, Emmanuel Kasper wrote:
> 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

The issue was that on the backend side if no hostname is set we do use
'localhost' rather than 'ct$vmid' as the gui suggested.

> 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
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>