[pve-devel] [PATCH widget-toolkit 0/5] some APTRepositories improvements

[pve-devel] [PATCH widget-toolkit 0/5] some APTRepositories improvements

[Dominik Csapak]

some small and basic improvements

NOTE: patch 4/5 changes how we display warnings, which currently (AFAICS)
only can happen on the 'Suites' property if we add more warnings for
different properties, we have to add them in the relevant columns as well

Dominik Csapak (5):
  APTRepositories: change separation style between panels
  APTRepositories: make panel scrollable
  APTRepositories: change updating button text/state to viewcontroller
  APTRepositories: make the Suites warnings inline errors
  APTRepositories: remove unecessary selection model

 src/node/APTRepositories.js | 167 +++++++++++++++++++-----------------
 1 file changed, 88 insertions(+), 79 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 1/5] APTRepositories: change separation style between panels

[Dominik Csapak]

having double borders does not look so nice, separate them with a bit
of padding

also change the warning panel to a simple header

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/APTRepositories.js | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 6812d46..7544a23 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -24,6 +24,8 @@ Ext.define('Proxmox.node.APTRepositoriesErrors', {
 
     store: {},
 
+    border: false,
+
     viewConfig: {
 	stripeRows: false,
 	getRowClass: () => 'proxmox-invalid-row',
@@ -52,6 +54,8 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 
     cls: 'proxmox-apt-repos', // to allow applying styling to general components with local effect
 
+    border: false,
+
     tbar: [
 	{
 	    text: gettext('Reload'),
@@ -350,18 +354,25 @@ Ext.define('Proxmox.node.APTRepositories', {
 
     items: [
 	{
-	    title: gettext('Warning'),
-	    name: 'repositoriesMainWarning',
-	    xtype: 'panel',
+	    xtype: 'header',
+	    baseCls: 'x-panel-header',
 	    bind: {
+		hidden: '{!mainWarning}',
 		title: '{mainWarning}',
+	    },
+	},
+	{
+	    xtype: 'box',
+	    bind: {
 		hidden: '{!mainWarning}',
 	    },
+	    height: 5,
 	},
 	{
 	    xtype: 'proxmoxNodeAPTRepositoriesErrors',
 	    name: 'repositoriesErrors',
 	    hidden: true,
+	    padding: '0 0 5 0',
 	    bind: {
 		hidden: '{noErrors}',
 	    },
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 2/5] APTRepositories: make panel scrollable

[Dominik Csapak]

so that it is still usable with many errors and/or repositories

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/APTRepositories.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 7544a23..1985e50 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -352,6 +352,12 @@ Ext.define('Proxmox.node.APTRepositories', {
 	},
     },
 
+    scrollable: true,
+    layout: {
+	type: 'vbox',
+	align: 'stretch',
+    },
+
     items: [
 	{
 	    xtype: 'header',
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 3/5] APTRepositories: change updating button text/state to viewcontroller

[Dominik Csapak]

we already have a viewmodel we can use, and so the handler does
not have to navigate the component tree up/down

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/APTRepositories.js | 41 ++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 1985e50..23cd984 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -81,6 +81,9 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	    altText: gettext('Disable'),
 	    id: 'repoEnableButton',
 	    disabled: true,
+	    bind: {
+		text: '{enableButtonText}',
+	    },
 	    handler: function(button, event, record) {
 		let me = this;
 		let panel = me.up('proxmoxNodeAPTRepositories');
@@ -290,19 +293,6 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 
 	me.callParent();
     },
-
-    listeners: {
-	selectionchange: function() {
-	    let me = this;
-
-	    if (me.onSelectionChange) {
-		let sm = me.getSelectionModel();
-		let rec = sm.getSelection()[0];
-
-		me.onSelectionChange(rec, sm);
-	    }
-	},
-    },
 });
 
 Ext.define('Proxmox.node.APTRepositories', {
@@ -314,6 +304,20 @@ Ext.define('Proxmox.node.APTRepositories', {
 
     product: 'Proxmox VE', // default
 
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	selectionChange: function(grid, selection) {
+	    let me = this;
+	    if (!selection || selection.length < 1) {
+		return;
+	    }
+	    let rec = selection[0];
+	    let vm = me.getViewModel();
+	    vm.set('selectionenabled', rec.get('Enabled'));
+	},
+    },
+
     viewModel: {
 	data: {
 	    product: 'Proxmox VE', // default
@@ -321,9 +325,12 @@ Ext.define('Proxmox.node.APTRepositories', {
 	    subscriptionActive: '',
 	    noSubscriptionRepo: '',
 	    enterpriseRepo: '',
+	    selectionenabled: false,
 	},
 	formulas: {
 	    noErrors: (get) => get('errorCount') === 0,
+	    enableButtonText: (get) => get('selectionenabled')
+		? gettext('Disable') : gettext('Enable'),
 	    mainWarning: function(get) {
 		// Not yet initialized
 		if (get('subscriptionActive') === '' ||
@@ -390,12 +397,8 @@ Ext.define('Proxmox.node.APTRepositories', {
 		nodename: '{nodename}',
 	    },
 	    majorUpgradeAllowed: false, // TODO get release information from an API call?
-	    onSelectionChange: function(rec, sm) {
-		let me = this;
-		if (rec) {
-		    let btn = me.up('proxmoxNodeAPTRepositories').down('#repoEnableButton');
-		    btn.setText(rec.get('Enabled') ? gettext('Disable') : gettext('Enable'));
-		}
+	    listeners: {
+		selectionchange: 'selectionChange',
 	    },
 	},
     ],
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 4/5] APTRepositories: make the Suites warnings inline errors

[Dominik Csapak]

by iterating only once over the info array and adding them directly
to the records this way we can avoid iterating over the records
again

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/APTRepositories.js | 100 +++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 54 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 23cd984..de0197b 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -157,8 +157,20 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	{
 	    header: gettext('Suites'),
 	    dataIndex: 'Suites',
-	    renderer: function(suites, cell, record) {
-		return suites.join(' ');
+	    renderer: function(suites, metaData, record) {
+		let err = '';
+		if (record.data.warnings && record.data.warnings.length > 0) {
+		    let txt = [gettext('Warning')];
+		    record.data.warnings.forEach((warning) => {
+			if (warning.property === 'Suites') {
+			    txt.push(warning.message);
+			}
+		    });
+		    metaData.tdAttr = `data-qtip="${Ext.htmlEncode(txt.join('<br>'))}"`;
+		    metaData.tdCls = 'proxmox-invalid-row';
+		    err = '<i class="fa fa-fw critical fa-exclamation-circle"></i> ';
+		}
+		return suites.join(' ') + err;
 	    },
 	    width: 130,
 	},
@@ -209,53 +221,6 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	},
     ],
 
-    addAdditionalInfos: function(gridData, infos) {
-	let me = this;
-
-	let warnings = {};
-	let origins = {};
-
-	let addLine = function(obj, key, line) {
-	    if (obj[key]) {
-		obj[key] += "\n";
-		obj[key] += line;
-	    } else {
-		obj[key] = line;
-	    }
-	};
-
-	for (const info of infos) {
-	    const key = `${info.path}:${info.index}`;
-	    if (info.kind === 'warning' ||
-		(info.kind === 'ignore-pre-upgrade-warning' && !me.majorUpgradeAllowed)
-	    ) {
-		addLine(warnings, key, gettext('Warning') + ": " + info.message);
-	    } else if (info.kind === 'origin') {
-		origins[key] = info.message;
-	    }
-	}
-
-	gridData.forEach(function(record) {
-	    const key = `${record.Path}:${record.Index}`;
-	    record.Origin = origins[key];
-	});
-
-	me.rowBodyFeature.getAdditionalData = function(innerData, rowIndex, record, orig) {
-	    let headerCt = this.view.headerCt;
-	    let colspan = headerCt.getColumnCount();
-
-	    const key = `${innerData.Path}:${innerData.Index}`;
-	    const warning_text = warnings[key];
-
-	    return {
-		rowBody: '<div style="color: red; white-space: pre-line">' +
-		    Ext.String.htmlEncode(warning_text) + '</div>',
-		rowBodyCls: warning_text ? '' : Ext.baseCSSPrefix + 'grid-row-body-hidden',
-		rowBodyColspan: colspan,
-	    };
-	};
-    },
-
     initComponent: function() {
 	let me = this;
 
@@ -274,8 +239,6 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	    ],
 	});
 
-	let rowBodyFeature = Ext.create('Ext.grid.feature.RowBody', {});
-
 	let groupingFeature = Ext.create('Ext.grid.feature.Grouping', {
 	    groupHeaderTpl: '{[ "File: " + values.name ]} ({rows.length} ' +
 		'repositor{[values.rows.length > 1 ? "ies" : "y"]})',
@@ -287,8 +250,7 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	Ext.apply(me, {
 	    store: store,
 	    selModel: sm,
-	    rowBodyFeature: rowBodyFeature,
-	    features: [groupingFeature, rowBodyFeature],
+	    features: [groupingFeature],
 	});
 
 	me.callParent();
@@ -494,16 +456,46 @@ Ext.define('Proxmox.node.APTRepositories', {
 		errors = data.errors;
 		digest = data.digest;
 
+		let infos = {};
+		for (const info of data.infos) {
+		    let path = info.path;
+		    let idx = info.index;
+
+		    if (!infos[path]) {
+			infos[path] = {};
+		    }
+		    if (!infos[path][idx]) {
+			infos[path][idx] = {
+			    origin: '',
+			    warnings: [],
+			};
+		    }
+
+		    if (info.kind === 'origin') {
+			infos[path][idx].origin = info.message;
+		    } else if (info.kind === 'warning' ||
+			(info.kind === 'ignore-pre-upgrade-warning' && !repoGrid.majorUpgradeAllowed)
+		    ) {
+			infos[path][idx].warnings.push(info);
+		    } else {
+			throw 'unknown info';
+		    }
+		}
+
+
 		files.forEach(function(file) {
 		    for (let n = 0; n < file.repositories.length; n++) {
 			let repo = file.repositories[n];
 			repo.Path = file.path;
 			repo.Index = n;
+			if (infos[file.path] && infos[file.path][n]) {
+			    repo.Origin = infos[file.path][n].origin || Proxmox.Utils.UnknownText;
+			    repo.warnings = infos[file.path][n].warnings || [];
+			}
 			gridData.push(repo);
 		    }
 		});
 
-		repoGrid.addAdditionalInfos(gridData, data.infos);
 		repoGrid.store.loadData(gridData);
 
 		me.updateStandardRepos(data['standard-repos']);
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 5/5] APTRepositories: remove unecessary selection model

[Dominik Csapak]

this is the default anyway, no reason to explicitely create it

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/node/APTRepositories.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index de0197b..04e6f97 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -245,11 +245,8 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
 	    enableGroupingMenu: false,
 	});
 
-	let sm = Ext.create('Ext.selection.RowModel', {});
-
 	Ext.apply(me, {
 	    store: store,
-	    selModel: sm,
 	    features: [groupingFeature],
 	});
 
-- 
2.30.2

[pve-devel] applied-series: [PATCH widget-toolkit 0/5] some APTRepositories improvements

[Thomas Lamprecht]

On 30.06.21 17:14, Dominik Csapak wrote:
> some small and basic improvements
> 
> NOTE: patch 4/5 changes how we display warnings, which currently (AFAICS)
> only can happen on the 'Suites' property if we add more warnings for
> different properties, we have to add them in the relevant columns as well
> 
> Dominik Csapak (5):
>   APTRepositories: change separation style between panels
>   APTRepositories: make panel scrollable
>   APTRepositories: change updating button text/state to viewcontroller
>   APTRepositories: make the Suites warnings inline errors
>   APTRepositories: remove unecessary selection model
> 
>  src/node/APTRepositories.js | 167 +++++++++++++++++++-----------------
>  1 file changed, 88 insertions(+), 79 deletions(-)
> 

applied series,  thanks!

FYI: was fine on top of Fabi's change sent at roughly the same time without
any merge conflict.