Re: [pve-devel] [PATCH v2 manager 3/5] ui/UploadToStorage: add checksum and algorithm

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline

On 7/22/21 15:06, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   www/manager6/window/UploadToStorage.js | 32 ++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/www/manager6/window/UploadToStorage.js b/www/manager6/window/UploadToStorage.js
> index fb9850b3..0b4d991a 100644
> --- a/www/manager6/window/UploadToStorage.js
> +++ b/www/manager6/window/UploadToStorage.js
> @@ -130,6 +130,18 @@ Ext.define('PVE.window.UploadToStorage', {
>   	    vm.set('size', (fileInput.files[0] && Proxmox.Utils.format_size(fileInput.files[0].size)) || '-');
>   	    vm.set('mimetype', (fileInput.files[0] && fileInput.files[0].type) || '-');
>   	},
> +
> +	hashChange: function(field) {
> +	    const checksum = Ext.getCmp('downloadUrlChecksum');

since we already have a controller, please use 'reference' instead
of id, and 'this.lookup' instead of Ext.getCmp

> +	    if (field.getValue() === '__default__') {

the second parameter of the change event is the value, so you
could omit the call to getValue here..

> +		checksum.setDisabled(true);
> +		checksum.setValue("");
> +		checksum.allowBlank = true;

is this correct? wouldn't it be

checksum.setAllowBlank ?

also why set that in the first place? just leave it
set to 'false'. Disabled fields will not get submitted
or calculated on form validation anyway

> +	    } else {
> +		checksum.setDisabled(false);
> +		checksum.allowBlank = false;
> +	    }
> +	},
>       },
>   
>       items: [
> @@ -184,6 +196,26 @@ Ext.define('PVE.window.UploadToStorage', {
>   			value: '{mimetype}',
>   		    },
>   		},
> +		{
> +		    xtype: 'pveHashAlgorithmSelector',
> +		    name: 'checksum-algorithm',
> +		    fieldLabel: gettext('Hash algorithm'),
> +		    allowBlank: true,
> +		    hasNoneOption: true,
> +		    value: '__default__',
> +		    listeners: {
> +			change: 'hashChange',
> +		    },
> +		},
> +		{
> +		    xtype: 'textfield',
> +		    name: 'checksum',
> +		    fieldLabel: gettext('Checksum'),
> +		    allowBlank: true,
> +		    disabled: true,
> +		    emptyText: gettext('none'),
> +		    id: 'downloadUrlChecksum',
> +		},
>   		{
>   		    xtype: 'progressbar',
>   		    text: 'Ready',
>