* [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
@ 2020-10-01 12:18 Aaron Lauterer
2020-10-01 14:44 ` [pve-devel] applied: " Thomas Lamprecht
2020-10-01 15:10 ` [pve-devel] " Thomas Lamprecht
0 siblings, 2 replies; 6+ messages in thread
From: Aaron Lauterer @ 2020-10-01 12:18 UTC (permalink / raw)
To: pve-devel
Allows 2 digits rule numbers to be visible in the column. Made it a bit
wider than needed to account for potentially wider font rendering on
different platforms.
Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
www/manager6/grid/FirewallRules.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index ec2d1c84..42035129 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -629,7 +629,7 @@ Ext.define('PVE.FirewallRules', {
// similar to xtype: 'rownumberer',
dataIndex: 'pos',
resizable: false,
- width: 23,
+ width: 42,
sortable: false,
align: 'right',
hideable: false,
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] applied: [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
2020-10-01 12:18 [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column Aaron Lauterer
@ 2020-10-01 14:44 ` Thomas Lamprecht
2020-10-01 15:10 ` [pve-devel] " Thomas Lamprecht
1 sibling, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2020-10-01 14:44 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
On 01.10.20 14:18, Aaron Lauterer wrote:
> Allows 2 digits rule numbers to be visible in the column. Made it a bit
> wider than needed to account for potentially wider font rendering on
> different platforms.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> www/manager6/grid/FirewallRules.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
for the future please:
* do *not* use file endings in the commit message, if not really relevant.
* put the "fixes #12345:" at the beginning.
No use but makes it harder to read (and get sane, user readable d/changelogs entries from)
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
this time still: applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
2020-10-01 12:18 [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column Aaron Lauterer
2020-10-01 14:44 ` [pve-devel] applied: " Thomas Lamprecht
@ 2020-10-01 15:10 ` Thomas Lamprecht
2020-10-01 15:22 ` Aaron Lauterer
2020-10-02 12:52 ` Aaron Lauterer
1 sibling, 2 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2020-10-01 15:10 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
On 01.10.20 14:18, Aaron Lauterer wrote:
> Allows 2 digits rule numbers to be visible in the column. Made it a bit
> wider than needed to account for potentially wider font rendering on
> different platforms.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> www/manager6/grid/FirewallRules.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
> index ec2d1c84..42035129 100644
> --- a/www/manager6/grid/FirewallRules.js
> +++ b/www/manager6/grid/FirewallRules.js
> @@ -629,7 +629,7 @@ Ext.define('PVE.FirewallRules', {
> // similar to xtype: 'rownumberer',
> dataIndex: 'pos',
> resizable: false,
> - width: 23,
> + width: 42,
> sortable: false,
> align: 'right',
> hideable: false,
>
oh, and maybe you can try to switch that whole column definition away from fixed
widths to relative "flex" only. Just needs to look OK on 720p and quite good on
1080p. It's not always possible, but most of the time it works out well and not
using fixed widths is always nicer, IMO.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
2020-10-01 15:10 ` [pve-devel] " Thomas Lamprecht
@ 2020-10-01 15:22 ` Aaron Lauterer
2020-10-02 12:52 ` Aaron Lauterer
1 sibling, 0 replies; 6+ messages in thread
From: Aaron Lauterer @ 2020-10-01 15:22 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 10/1/20 5:10 PM, Thomas Lamprecht wrote:
> On 01.10.20 14:18, Aaron Lauterer wrote:
>> Allows 2 digits rule numbers to be visible in the column. Made it a bit
>> wider than needed to account for potentially wider font rendering on
>> different platforms.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> www/manager6/grid/FirewallRules.js | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
>> index ec2d1c84..42035129 100644
>> --- a/www/manager6/grid/FirewallRules.js
>> +++ b/www/manager6/grid/FirewallRules.js
>> @@ -629,7 +629,7 @@ Ext.define('PVE.FirewallRules', {
>> // similar to xtype: 'rownumberer',
>> dataIndex: 'pos',
>> resizable: false,
>> - width: 23,
>> + width: 42,
>> sortable: false,
>> align: 'right',
>> hideable: false,
>>
>
> oh, and maybe you can try to switch that whole column definition away from fixed
> widths to relative "flex" only. Just needs to look OK on 720p and quite good on
> 1080p. It's not always possible, but most of the time it works out well and not
> using fixed widths is always nicer, IMO.
>
good point. I'll take a look at it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
2020-10-01 15:10 ` [pve-devel] " Thomas Lamprecht
2020-10-01 15:22 ` Aaron Lauterer
@ 2020-10-02 12:52 ` Aaron Lauterer
2021-02-19 16:45 ` Thomas Lamprecht
1 sibling, 1 reply; 6+ messages in thread
From: Aaron Lauterer @ 2020-10-02 12:52 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 10/1/20 5:10 PM, Thomas Lamprecht wrote:
> oh, and maybe you can try to switch that whole column definition away from fixed
> widths to relative "flex" only. Just needs to look OK on 720p and quite good on
> 1080p. It's not always possible, but most of the time it works out well and not
> using fixed widths is always nicer, IMO.
>
I gave it a quick try and I think adding flex properties does help on large screens but for smaller screens I would keep the former width values as minWidth to keep the table readable and to force extjs to switch it to horizontal scrolling as it is currently doing with the fixed widths. Even 720p is already where the horizontal scroll bar shows up.
This is the git diff I used:
diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index 42035129..fee9c177 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -629,7 +629,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,
@@ -660,7 +661,8 @@ Ext.define('PVE.FirewallRules', {
me.updateRule(data);
}
},
- width: 50
+ minWidth: 50,
+ flex: 2,
},
{
header: gettext('Type'),
@@ -668,7 +670,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'),
@@ -676,7 +679,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'),
@@ -684,7 +688,8 @@ Ext.define('PVE.FirewallRules', {
renderer: function(value, metaData, record) {
return render_errors('macro', value, metaData, record);
},
- width: 80
+ minWidth: 80,
+ flex: 3,
}
];
@@ -695,7 +700,8 @@ Ext.define('PVE.FirewallRules', {
renderer: function(value, metaData, record) {
return render_errors('iface', value, metaData, record);
},
- width: 80
+ minWidth: 80,
+ flex: 3,
});
}
@@ -706,7 +712,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'),
@@ -714,7 +721,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'),
@@ -722,7 +730,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'),
@@ -730,7 +739,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'),
@@ -738,7 +748,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'),
@@ -746,12 +757,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);
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column
2020-10-02 12:52 ` Aaron Lauterer
@ 2021-02-19 16:45 ` Thomas Lamprecht
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2021-02-19 16:45 UTC (permalink / raw)
To: Proxmox VE development discussion, Aaron Lauterer
On 02.10.20 14:52, Aaron Lauterer wrote:
>
> On 10/1/20 5:10 PM, Thomas Lamprecht wrote:
>> oh, and maybe you can try to switch that whole column definition away from fixed
>> widths to relative "flex" only. Just needs to look OK on 720p and quite good on
>> 1080p. It's not always possible, but most of the time it works out well and not
>> using fixed widths is always nicer, IMO.
>>
>
> I gave it a quick try and I think adding flex properties does help on large screens but for smaller screens I would keep the former width values as minWidth to keep the table readable and to force extjs to switch it to horizontal scrolling as it is currently doing with the fixed widths. Even 720p is already where the horizontal scroll bar shows up.
>
> This is the git diff I used:
Could you send this as patch?
>
> diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
> index 42035129..fee9c177 100644
> --- a/www/manager6/grid/FirewallRules.js
> +++ b/www/manager6/grid/FirewallRules.js
> @@ -629,7 +629,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,
> @@ -660,7 +661,8 @@ Ext.define('PVE.FirewallRules', {
> me.updateRule(data);
> }
> },
> - width: 50
> + minWidth: 50,
> + flex: 2,
> },
> {
> header: gettext('Type'),
> @@ -668,7 +670,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'),
> @@ -676,7 +679,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'),
> @@ -684,7 +688,8 @@ Ext.define('PVE.FirewallRules', {
> renderer: function(value, metaData, record) {
> return render_errors('macro', value, metaData, record);
> },
> - width: 80
> + minWidth: 80,
> + flex: 3,
> }
> ];
>
> @@ -695,7 +700,8 @@ Ext.define('PVE.FirewallRules', {
> renderer: function(value, metaData, record) {
> return render_errors('iface', value, metaData, record);
> },
> - width: 80
> + minWidth: 80,
> + flex: 3,
> });
> }
>
> @@ -706,7 +712,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'),
> @@ -714,7 +721,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'),
> @@ -722,7 +730,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'),
> @@ -730,7 +739,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'),
> @@ -738,7 +748,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'),
> @@ -746,12 +757,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);
> }
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-19 16:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 12:18 [pve-devel] [PATCH manager] ui: FirewallRules.js: fix #3049 wider rule number column Aaron Lauterer
2020-10-01 14:44 ` [pve-devel] applied: " Thomas Lamprecht
2020-10-01 15:10 ` [pve-devel] " Thomas Lamprecht
2020-10-01 15:22 ` Aaron Lauterer
2020-10-02 12:52 ` Aaron Lauterer
2021-02-19 16:45 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox