Re: [pbs-devel] [PATCH proxmox-backup v5 6/6] ui: add support for optional Remote in SyncJob

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

works quite nice now, a few smaller comments inline.

Am 06/10/2023 um 16:05 schrieb Hannes Laimer:
> @@ -134,16 +143,78 @@ Ext.define('PBS.window.SyncJobEdit', {
>  		],
>  
>  		column2: [
> +		    {
> +			xtype: 'radiogroup',
> +			fieldLabel: gettext('Location'),
> +			defaultType: 'radiofield',
> +			items: [
> +			    {
> +				boxLabel: 'Local',
> +				name: 'location',
> +				inputValue: 'local',
> +				submitValue: false,
> +			    },
> +			    {
> +				boxLabel: 'Remote',
> +				name: 'location',
> +				inputValue: 'remote',
> +				submitValue: false

Seems to miss a

    checked: true,

here to correctly indicate the default from the start on.

> +			    },
> +			],
> +			listeners: {
> +			    change: function(_group, radio) {
> +				let me = this;
> +				let form = me.up('pbsSyncJobEdit');
> +				let remoteField = form.down('field[name=remote]');
> +				let storeField = form.down('field[name=remote-store]');
> +				let nsField = form.down('field[name=remote-ns]');
> +
> +				if (!storeField.value) {
> +				    nsField.clearValue();
> +				    nsField.setDisabled(true);
> +				}
> +

no hard feelings, but for below you could go for a pattern like:

let isLocalSync = radio.location === 'local';

remoteField.setDisabled(isLocalSync);
remoteField.allowBlank = isLocalSync;

and so on, as that might save us some duplication here, but as said, just a nit.

And I observed a small glitch, when switching the radio-group selection from local
to remote, the "Source Remote" field is not marked as required, i.e., a validate
call might be missing.