Re: [pbs-devel] [PATCH v5 proxmox-backup 21/31] ui: add view with separate grids for pull and push sync jobs

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

one nit and one question inline

On 10/18/24 10:42, Christian Ebner wrote:
> Show sync jobs in pull and in push direction in two separate grids,
> visually separating them to limit possible misconfiguration.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 4:
> - no changes
> 
> changes since version 3:
> - no changes
> 
>   www/Makefile                   |  1 +
>   www/config/SyncPullPushView.js | 60 ++++++++++++++++++++++++++++++++++
>   www/config/SyncView.js         | 11 ++++++-
>   www/datastore/DataStoreList.js |  2 +-
>   www/datastore/Panel.js         |  2 +-
>   5 files changed, 73 insertions(+), 3 deletions(-)
>   create mode 100644 www/config/SyncPullPushView.js
> 
> diff --git a/www/Makefile b/www/Makefile
> index 609a0ba67..d35e81283 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -61,6 +61,7 @@ JSSRC=							\
>   	config/TrafficControlView.js			\
>   	config/ACLView.js				\
>   	config/SyncView.js				\
> +	config/SyncPullPushView.js			\
>   	config/VerifyView.js				\
>   	config/PruneView.js				\
>   	config/GCView.js				\
> diff --git a/www/config/SyncPullPushView.js b/www/config/SyncPullPushView.js
> new file mode 100644
> index 000000000..1588207c9
> --- /dev/null
> +++ b/www/config/SyncPullPushView.js
> @@ -0,0 +1,60 @@
> +Ext.define('PBS.config.SyncPullPush', {
> +    extend: 'Ext.panel.Panel',
> +    alias: 'widget.pbsSyncJobPullPushView',
> +    title: gettext('Sync Jobs'),
> +
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    layout: {
> +	type: 'vbox',
> +	align: 'stretch',
> +	multi: true,
> +    },
> +    defaults: {
> +	collapsible: false,
> +	margin: '7 10 3 10',

nit: not really a fan of such "specific" values (though i know we have them
all over the place...

couldn't it work to use e.g. bodyPadding: 5 on the panel and
margin: 5 to the defaults, that way the layout is such that we have

5 px bodyPadding from the top,bottom,right,left
around each child again 5px margin
which sums up to 10px on the outer borders
and 10px between childs

> +    },
> +    scrollable: true,
> +    items: [
> +	{
> +	    xtype: 'pbsSyncJobView',
> +	    itemId: 'syncJobsPull',
> +	    syncDirection: 'pull',
> +	    cbind: {
> +		datastore: '{datastore}',
> +	    },
> +	    minHeight: 125, // shows at least one line of content

just to feed my curiosity (and it's not mentioned here)
why is this height

> +	},
> +	{
> +	    xtype: 'splitter',
> +	    performCollapse: false,
> +	},
> +	{
> +	    xtype: 'pbsSyncJobView',
> +	    itemId: 'syncJobsPush',
> +	    syncDirection: 'push',
> +	    cbind: {
> +		datastore: '{datastore}',
> +	    },
> +	    flex: 1,
> +	    minHeight: 160, // shows at least one line of content

different than this height?

the columns should take the same amount of space no?
son one line should take the same height for both panels?
(or am i missing something here?)

> +	},
> +    ],
> +    initComponent: function() {
> +	let me = this;
> +
> +	let subPanelIds = me.items.map(el => el.itemId).filter(id => !!id);
> +
> +	me.callParent();
> +
> +	for (const itemId of subPanelIds) {
> +	    let component = me.getComponent(itemId);
> +	    component.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> +	}
> +    },
> +
> +    cbindData: function(initialConfig) {
> +        let me = this;
> +        me.datastore = initialConfig.datastore ? initialConfig.datastore : undefined;
> +    },
> +});
> diff --git a/www/config/SyncView.js b/www/config/SyncView.js
> index 4669a23e2..68a147615 100644
> --- a/www/config/SyncView.js
> +++ b/www/config/SyncView.js
> @@ -29,7 +29,7 @@ Ext.define('PBS.config.SyncJobView', {
>       stateful: true,
>       stateId: 'grid-sync-jobs-v1',
>   
> -    title: gettext('Sync Jobs'),
> +    title: gettext('Sync Jobs - Pull Direction'),
>   
>       controller: {
>   	xclass: 'Ext.app.ViewController',
> @@ -39,6 +39,7 @@ Ext.define('PBS.config.SyncJobView', {
>   	    let view = me.getView();
>               Ext.create('PBS.window.SyncJobEdit', {
>   		datastore: view.datastore,
> +		syncDirection: view.syncDirection,
>   		listeners: {
>   		    destroy: function() {
>   			me.reload();
> @@ -55,6 +56,7 @@ Ext.define('PBS.config.SyncJobView', {
>   
>               Ext.create('PBS.window.SyncJobEdit', {
>   		datastore: view.datastore,
> +		syncDirection: view.syncDirection,
>                   id: selection[0].data.id,
>   		listeners: {
>   		    destroy: function() {
> @@ -117,6 +119,9 @@ Ext.define('PBS.config.SyncJobView', {
>   	    if (view.datastore !== undefined) {
>   		params.store = view.datastore;
>   	    }
> +	    if (view.syncDirection !== undefined) {
> +		params["sync-direction"] = view.syncDirection;
> +	    }
>   	    view.getStore().rstore.getProxy().setExtraParams(params);
>   	    Proxmox.Utils.monStoreErrors(view, view.getStore().rstore);
>   	},
> @@ -303,6 +308,10 @@ Ext.define('PBS.config.SyncJobView', {
>   	    }
>   	}
>   
> +	if (me.syncDirection === 'push') {
> +	    me.title = gettext('Sync Jobs - Push Direction');
> +	}
> +
>   	me.callParent();
>       },
>   });
> diff --git a/www/datastore/DataStoreList.js b/www/datastore/DataStoreList.js
> index fc68cfc10..22ef18540 100644
> --- a/www/datastore/DataStoreList.js
> +++ b/www/datastore/DataStoreList.js
> @@ -239,7 +239,7 @@ Ext.define('PBS.datastore.DataStores', {
>   	{
>   	    iconCls: 'fa fa-refresh',
>   	    itemId: 'syncjobs',
> -	    xtype: 'pbsSyncJobView',
> +	    xtype: 'pbsSyncJobPullPushView',
>   	},
>   	{
>   	    iconCls: 'fa fa-check-circle',
> diff --git a/www/datastore/Panel.js b/www/datastore/Panel.js
> index ad9fc10fe..e1da7cfac 100644
> --- a/www/datastore/Panel.js
> +++ b/www/datastore/Panel.js
> @@ -68,7 +68,7 @@ Ext.define('PBS.DataStorePanel', {
>   	{
>   	    iconCls: 'fa fa-refresh',
>   	    itemId: 'syncjobs',
> -	    xtype: 'pbsSyncJobView',
> +	    xtype: 'pbsSyncJobPullPushView',
>   	    cbind: {
>   		datastore: '{datastore}',
>   	    },



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