[pve-devel] [PATCH manager] gui: reset cdimage selector on change

[Thomas Lamprecht]

On 10/23/19 10:27 AM, Tim Marx wrote:
> to improve UX, disabled fields shouldn't show validation errors.

can you please hint somewhere that not the ISO selector value itself,
but the radio-button boolean value for "use CD/DVD (ISO)"?

It's quite clear if one looks at the code but not so from the patch,
and I was a bit confused how it worked ^^

Works good, still two style nits and an off-topic comment below.

> 
> Signed-off-by: Tim Marx <t.marx at proxmox.com>
> ---
>  www/manager6/qemu/CDEdit.js | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js
> index 78d758c7..a1636aa9 100644
> --- a/www/manager6/qemu/CDEdit.js
> +++ b/www/manager6/qemu/CDEdit.js
> @@ -89,7 +89,8 @@ Ext.define('PVE.qemu.CDInputPanel', {
>  		    }
>  		    me.down('field[name=cdstorage]').setDisabled(!value);
>  		    me.down('field[name=cdimage]').setDisabled(!value);
> -		    me.down('field[name=cdimage]').validate();
> +		    var cdImageField = me.down('field[name=cdimage]');

please also use above then for the previous setDisabled call

> +		    value ? cdImageField.validate() : cdImageField.reset();

can we do a real nice written out if-else? I'd prefer that over
ternary operations.

On another note: When testing this I had a strange behavior, also present
without this patch. When I select an valid ISO, then edit a few chars in
to make it invalid and then click on some arbitrary free space in the
panel (or wizard) the following happens: The value jumps back to the
previously selected valid ISO *but* the field stays invalid. I'd expect
that one of both happens, but not both. So either keep garbage value and
invalid state, or fixup value and re-validate.

Maybe you could take a look at this too :) 


>  		}
>  	    }
>  	});
> --
> 2.20.1