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

[Dominic Jäger]

Thank you for looking at it so quickly!

On Tue, Oct 06, 2020 at 12:22:03PM +0200, Thomas Lamprecht wrote:
> 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.

Seems I checked too many online guides that suggest writing examples into
placeholders/emptyText and too little source code.

So far the only tooltips I have found on textfields are the "This field is
required" from allowBlank: false. The existing tooltips are all on some kind of
button and simply adding "tooltip: 'Test'," to a proxmoxtextfield doesn't do
anything?

=> I stuck to your other suggestion and made more descriptive labels with
either
 - "Example: <example>" or
 - a description


> On 06.10.20 12:10, Dominic Jäger wrote:
> > Signed-off-by: Dominic Jäger <d.jaeger at 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.
Changed the label and added "Example:". I think having some example here makes
sense to show that a single word/string is required and signs like underscores
are allowed.

> 
> >  		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'
Changed the label and added "Example:". Not sure about adding the port number as
it's optional.
> 
> >  		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 at pam',
> 
> Is this the default? 
Explicitly specifying the user ID is required by GUI and CLI

 ~ proxmox-backup-manager remote create test --host 1.1.1.1 --password test
 Error: parameter verification errors

 parameter 'userid': parameter is missing and it is not optional.


> As else it is wrong to show as such, if we do this
> then we normally use something like "E.g., root at pam" or "For example, root at pam"
> to indicate that this is not the default but a hint. A tooltip could work better
> also here.
Chose "Example:" because
 - we can use that in every textfield of this window without exceeding its
   length
 - it is recommended in our technical writing guide
 - It is like that in the "Add PBS" panel

Additionally, changed the example to admin at pbs because that's what we
have in "Add PBS" storage addition dialog.

> 
> >  	    },
> >  	    {
> >  		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.
Changed to the exact same descriptions as in that dialog.
> 
> >  	    },
> >  	    {
> >  		xtype: 'proxmoxtextfield',
> > @@ -124,6 +128,7 @@ Ext.define('PBS.window.RemoteEdit', {
> >  		    deleteEmpty: '{!isCreate}',
> >  		},
> >  		fieldLabel: gettext('Comment'),
> > +		emptyText: gettext('A very long comment'),
> >  	    },
> >  	],
> >      },
> > 
> 
>