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

[Thomas Lamprecht]

On 07.10.20 11:53, Dominic Jäger wrote:
> 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.

There was no need to look at the source, just use PVE's webinterface and you
see it everywhere..

Please, always try to check out the webinterface from a *user* POV and try to
use the rules there, before following some random UX guides online (there are
far to many, often conflicting).

I do not think throwing in just a plain "192.168.10.11" IP helps anybody more
than reading the label does, could rather make one think that this only accepts
IPs and not DNS host names.

> 
> 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?

There are the ExtJS generate tooltips, like if a fields format limit (regex,
vtype, maximum value, ...) does not applies, but there are also quite some
manually added ones.


Some ExtJS components (e.g., buttons) provide a config "tooltip" property, for
those which don't you can use something like:

autoEl: {
    tag: 'div',
    'data-qtip': gettext('Include volume in backup job'),
},


PS: we have over 30 defined tooltips in pve-manager alone, so they are far from
being a rare.

> 
> => 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.

we never do that for user chosen IDs. Think of it that way:
If you'd have a form for registering a newborn baby, wouldn't it be weird
if there's a "Name: For example, Bobby" field?

> 
>>
>>>  		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.

That's why a tooltip can be much more helpful, as there you can say that one
can add an optional port.

>>
>>>  		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.
> 
> 

FYI: That was a rhetoric question, as you using the emptyLabel like that
suggests it is.