[pve-devel] [PATCH manager 1/2] ui: FirewallRules: make columns flexible

[pve-devel] [PATCH manager 1/2] ui: FirewallRules: make columns flexible

[Aaron Lauterer]

Changing the width definitions to use flex will make better use on
larger monitors. Changing the `width` parameter to `minWidth` ensures
that on smaller screens it is still usable, though some horizontal
scrolling might be necessary.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/grid/FirewallRules.js | 38 ++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index c3b8fa53..5a2241a0 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -627,7 +627,8 @@ Ext.define('PVE.FirewallRules', {
 		// similar to xtype: 'rownumberer',
 		dataIndex: 'pos',
 		resizable: false,
-		width: 42,
+		minWidth: 42,
+		flex: 1,
 		sortable: false,
 		align: 'right',
 		hideable: false,
@@ -658,7 +659,8 @@ Ext.define('PVE.FirewallRules', {
 			me.updateRule(data);
 		    },
 		},
-		width: 50,
+		minWidth: 50,
+		flex: 2,
 	    },
 	    {
 		header: gettext('Type'),
@@ -666,7 +668,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('type', value, metaData, record);
 		},
-		width: 50,
+		minWidth: 50,
+		flex: 2,
 	    },
 	    {
 		header: gettext('Action'),
@@ -674,7 +677,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('action', value, metaData, record);
 		},
-		width: 80,
+		minWidth: 80,
+		flex: 3,
 	    },
 	    {
 		header: gettext('Macro'),
@@ -682,7 +686,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('macro', value, metaData, record);
 		},
-		width: 80,
+		minWidth: 80,
+		flex: 3,
 	    },
 	];
 
@@ -693,7 +698,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('iface', value, metaData, record);
 		},
-		width: 80,
+		minWidth: 80,
+		flex: 3,
 	    });
 	}
 
@@ -704,7 +710,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('source', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 4,
 	    },
 	    {
 		header: gettext('Destination'),
@@ -712,7 +719,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('dest', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 4,
 	    },
 	    {
 		header: gettext('Protocol'),
@@ -720,7 +728,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('proto', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 3,
 	    },
 	    {
 		header: gettext('Dest. port'),
@@ -728,7 +737,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('dport', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 4,
 	    },
 	    {
 		header: gettext('Source port'),
@@ -736,7 +746,8 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('sport', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 3,
 	    },
 	    {
 		header: gettext('Log level'),
@@ -744,12 +755,13 @@ Ext.define('PVE.FirewallRules', {
 		renderer: function(value, metaData, record) {
 		    return render_errors('log', value, metaData, record);
 		},
-		width: 100,
+		minWidth: 100,
+		flex: 3,
 	    },
 	    {
 		header: gettext('Comment'),
 		dataIndex: 'comment',
-		flex: 1,
+		flex: 6,
 		renderer: function(value, metaData, record) {
 		    return render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
 		},
-- 
2.20.1

[pve-devel] [PATCH manager 2/2] ui: FirewallRules: Add tooltip to comments

[Aaron Lauterer]

The comment columns might not be wide enough for longer comments. Since
it is the most right columns, it can be tricky to grab the right pixel
to drag it wider.

A tooltip that shows up on hover should be okay.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

As an additional patch as I realized that the comment column can be a
bit hard to use on smaller screens.

 www/manager6/grid/FirewallRules.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index 5a2241a0..f7f304b8 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -763,8 +763,10 @@ Ext.define('PVE.FirewallRules', {
 		dataIndex: 'comment',
 		flex: 6,
 		renderer: function(value, metaData, record) {
-		    return render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
-		},
+		    let comment = render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
+		    return `<span data-qtip="${comment}">${comment}</span>`;
+},
+
 	    },
 	);
 
-- 
2.20.1

[pve-devel] applied: [PATCH manager 2/2] ui: FirewallRules: Add tooltip to comments

[Thomas Lamprecht]

On 22.02.21 11:16, Aaron Lauterer wrote:
> The comment columns might not be wide enough for longer comments. Since
> it is the most right columns, it can be tricky to grab the right pixel
> to drag it wider.
> 
> A tooltip that shows up on hover should be okay.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> As an additional patch as I realized that the comment column can be a
> bit hard to use on smaller screens.

I fixed a style nit and bug and added a heuristic to only add it when
required (i.e., text length is ~ longer than cell width) in a followup.

> 
>  www/manager6/grid/FirewallRules.js | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
> index 5a2241a0..f7f304b8 100644
> --- a/www/manager6/grid/FirewallRules.js
> +++ b/www/manager6/grid/FirewallRules.js
> @@ -763,8 +763,10 @@ Ext.define('PVE.FirewallRules', {
>  		dataIndex: 'comment',
>  		flex: 6,
>  		renderer: function(value, metaData, record) {
> -		    return render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
> -		},
> +		    let comment = render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
> +		    return `<span data-qtip="${comment}">${comment}</span>`;

returned literally "undefined" if no comment was set

> +},


bad indentation

> +
>  	    },
>  	);
>  
> 

[pve-devel] applied: [PATCH manager 1/2] ui: FirewallRules: make columns flexible

[Thomas Lamprecht]

On 22.02.21 11:16, Aaron Lauterer wrote:
> Changing the width definitions to use flex will make better use on
> larger monitors. Changing the `width` parameter to `minWidth` ensures
> that on smaller screens it is still usable, though some horizontal
> scrolling might be necessary.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  www/manager6/grid/FirewallRules.js | 38 ++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
>

applied, thanks! Followed up with some further changes as I remembered a discussion
I had with Dietmar about to many flex in columns (see commit message for details)