Re: [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support

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

comments inline:

On 11/22/24 15:47, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   www/window/DataStoreEdit.js | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
> index b8e866df2..7b6aff1e7 100644
> --- a/www/window/DataStoreEdit.js
> +++ b/www/window/DataStoreEdit.js
> @@ -63,6 +63,20 @@ Ext.define('PBS.DataStoreEdit', {
>   			emptyText: gettext('An absolute path'),
>   			validator: val => val?.trim() !== '/',
>   		    },
> +		    {
> +			xtype: 'pmxDisplayEditField',
> +			fieldLabel: gettext('Device'),
> +			name: 'backing-device',
> +			disabled: true,
> +			cbind: {
> +			    editable: '{isCreate}',
> +			},
> +			editConfig: {
> +			    xtype: 'pbsPartitionSelector',
> +			    allowBlank: true,
> +			},
> +			emptyText: gettext('Device path'),
> +		    },

it's a bit tricky to see from the code, but this editwindow is never
actually called in an editable context, so the displayedit field here
would actually not be necessary, because if you omit this

>   		],
>   		column2: [
>   		    {
> @@ -88,6 +102,29 @@ Ext.define('PBS.DataStoreEdit', {
>   		    },
>   		],
>   		columnB: [
> +		    {
> +			xtype: 'checkbox',
> +			boxLabel: gettext('Removable datastore'),
> +			submitValue: false,
> +			listeners: {
> +			    change: function(checkbox, isRemovable) {
> +				let inputPanel = checkbox.up('inputpanel');
> +				let pathField = inputPanel.down('[name=path]');
> +				let uuidField = inputPanel.down('pbsPartitionSelector[name=backing-device]');
> +				let uuidEditField = inputPanel.down('[name=backing-device]');

this 'double' field modifying would also not be necessary.

even if we leave the displayedit field, this code here does not make much sense,
since the checkbox is always visible/editable, but the device itself is not?

either this is editable on an existing datastore (then the device should also
be editable afterwards), or this is not changeable, then the checkbox
must also vanish on edit

in any case, the checkbox change can/should only happen when the device is editable
so we can omit trying to modifying the display field here?

> +
> +				uuidField.allowBlank = !isRemovable;
> +				uuidEditField.setDisabled(!isRemovable);
> +				uuidField.setDisabled(!isRemovable);
> +				uuidField.setValue('');
> +				if (isRemovable) {
> +				    pathField.setFieldLabel(gettext('On device path'));
> +				} else {
> +				    pathField.setFieldLabel(gettext('Backing Path'));
> +				}
> +			    },
> +			},
> +		    },
>   		    {
>   			xtype: 'textfield',
>   			name: 'comment',



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel