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

[Christian Ebner]

On 10/25/24 12:39, Dominik Csapak wrote:
> 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 at 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

Okay, will see to improve this, I basically took the same values as we 
already have for the prune and garbage collection view.

>> +    },
>> +    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?)

Well, you should know: see commit 7a23a7ca ("ui: prune/gc view: improve 
sizing & scrolling behaviour") ;)

Jokes aside, I can include that reasoning into the commit message as 
well. I did once again follow very closely the pre-existing prune and 
garbage collection view.