Re: [pbs-devel] [PATCH proxmox-backup] Close #3058: ui: RemoteEdit: Add empty texts

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Thanks for trying to improve user experience, I always appreciate that.
But, we normally use emptyText for defaults, or at least make it very
clear that they are not, if used as hint. You may want to switch those
over to tooltips with more complete sentences.

some more comments inline

On 06.10.20 12:10, Dominic Jäger wrote:
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
>  www/window/RemoteEdit.js | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/www/window/RemoteEdit.js b/www/window/RemoteEdit.js
> index 56a248e0..695ad422 100644
> --- a/www/window/RemoteEdit.js
> +++ b/www/window/RemoteEdit.js
> @@ -24,7 +24,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  	me.method = name ? 'PUT' : 'POST';
>  	me.autoLoad = !!name;
>  	return {
> -	    passwordEmptyText: me.isCreate ? '' : gettext('Unchanged'),
> +	    passwordEmptyText: me.isCreate ? '•'.repeat(16) : gettext('Unchanged'),
>  	};
>      },
>  
> @@ -35,6 +35,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  		xtype: 'pmxDisplayEditField',
>  		name: 'name',
>  		fieldLabel: gettext('Remote'),
> +		emptyText: 'new_remote',

Suggests that this is the default, makes no sense IMO.

maybe add a "The unique ID for this remote" as tooltip and/or
change the fieldLabel to 'ID' instead.

>  		renderer: Ext.htmlEncode,
>  		allowBlank: false,
>  		minLength: 4,
> @@ -49,6 +50,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  		submitValue: false,
>  		vtype: 'HostPort',
>  		fieldLabel: gettext('Host'),
> +		emptyText: '192.168.10.10',

This suggests we use that IP as default, rather use a descriptive hint.
If it's long, then it could be also added as tooltip instead, which job
is to explain context better.


Could also change fieldLabel to 'Remote Address'

>  		listeners: {
>  		    change: function(field, newvalue) {
>  			let host = newvalue;
> @@ -95,6 +97,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  		allowBlank: false,
>  		name: 'userid',
>  		fieldLabel: gettext('Userid'),
> +		emptyText: 'root@pam',

Is this the default? As else it is wrong to show as such, if we do this
then we normally use something like "E.g., root@pam" or "For example, root@pam"
to indicate that this is not the default but a hint. A tooltip could work better
also here.

>  	    },
>  	    {
>  		xtype: 'textfield',
> @@ -116,6 +119,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  		    deleteEmpty: '{!isCreate}',
>  		},
>  		fieldLabel: gettext('Fingerprint'),
> +		emptyText: '0f:04:9a:2d:48:23:52:5d:d5:f4:1d:9e:5d:37:8e:60:93:d8:b0:03:31:8c:5c:d5:49:ff:96:31:9e:c2:1d:74',

no, why would yo u use a real FP here? Rather something which indicates what we expect here
but does not suggests a default..

"ab:cd:ef:..."

or like we do in the PVE "Add PBS" storage addition dialog.

>  	    },
>  	    {
>  		xtype: 'proxmoxtextfield',
> @@ -124,6 +128,7 @@ Ext.define('PBS.window.RemoteEdit', {
>  		    deleteEmpty: '{!isCreate}',
>  		},
>  		fieldLabel: gettext('Comment'),
> +		emptyText: gettext('A very long comment'),
>  	    },
>  	],
>      },
>