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 <[email protected]> > --- > 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 _______________________________________________ pve-devel mailing list [email protected] https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
