* [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