public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling
@ 2020-11-02 14:01 Aaron Lauterer
  2020-11-02 14:01 ` [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior Aaron Lauterer
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Aaron Lauterer @ 2020-11-02 14:01 UTC (permalink / raw)
  To: pve-devel

especially on chromium based browser (e.g. chrome, edge) it can happen,
depending on the zoom level, that the last column does not fit next to
the other columns and is moved below the other columns.

This results in an ugly looking UI and in the worst case makes it
unusable.

This can also be triggered if the monitor is set to a higher scaling /
different DPI settings. I was able to have the same problem in Edge when
setting the scaling in the windows display settings to 125% (Clone VM).

Changing the layout from columns with 0.5 width to extjs HBOXes with
flex 1 works as expected.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v1 -> v2: changed approach, use HBOX layouts instead of columns with
columnwidths slightly lower than 0.5.

 www/manager6/window/Clone.js    | 7 +++----
 www/manager6/window/Migrate.js  | 8 +++-----
 www/manager6/window/Settings.js | 9 +++------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/www/manager6/window/Clone.js b/www/manager6/window/Clone.js
index 5c6c2f75..95ab1cd7 100644
--- a/www/manager6/window/Clone.js
+++ b/www/manager6/window/Clone.js
@@ -256,22 +256,21 @@ Ext.define('PVE.window.Clone', {
 	    bodyPadding: 10,
 	    reference: 'cloneform',
 	    border: false,
-	    layout: 'column',
+	    layout: 'hbox',
 	    defaultType: 'container',
-	    columns: 2,
 	    fieldDefaults: {
 		labelWidth: 100,
 		anchor: '100%'
 	    },
 	    items: [
 		{
-		    columnWidth: 0.5,
+		    flex: 1,
 		    padding: '0 10 0 0',
 		    layout: 'anchor',
 		    items: col1
 		},
 		{
-		    columnWidth: 0.5,
+		    flex: 1,
 		    padding: '0 0 0 10',
 		    layout: 'anchor',
 		    items: col2
diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 3096dd1a..91d1b30b 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -308,13 +308,11 @@ Ext.define('PVE.window.Migrate', {
 	    reference: 'formPanel',
 	    bodyPadding: 10,
 	    border: false,
-	    layout: {
-		type: 'column'
-	    },
+	    layout: 'hbox',
 	    items: [
 		{
 		    xtype: 'container',
-		    columnWidth: 0.5,
+		    flex: 1,
 		    items: [{
 			xtype: 'displayfield',
 			name: 'source',
@@ -334,7 +332,7 @@ Ext.define('PVE.window.Migrate', {
 		},
 		{
 		    xtype: 'container',
-		    columnWidth: 0.5,
+		    flex: 1,
 		    items: [{
 			xtype: 'pveNodeSelector',
 			reference: 'pveNodeSelector',
diff --git a/www/manager6/window/Settings.js b/www/manager6/window/Settings.js
index e3519b1f..3bf974f0 100644
--- a/www/manager6/window/Settings.js
+++ b/www/manager6/window/Settings.js
@@ -23,10 +23,7 @@ Ext.define('PVE.window.Settings', {
 	}
     ],
 
-    layout: {
-	type: 'column',
-	align: 'top'
-    },
+    layout: 'hbox',
 
     controller: {
 	xclass: 'Ext.app.ViewController',
@@ -185,7 +182,7 @@ Ext.define('PVE.window.Settings', {
 
     items: [{
 	xtype: 'fieldset',
-	columnWidth: 0.5,
+	flex: 1,
 	title: gettext('Webinterface Settings'),
 	margin: '5',
 	layout: {
@@ -311,7 +308,7 @@ Ext.define('PVE.window.Settings', {
     {
 	xtype: 'container',
 	layout: 'vbox',
-	columnWidth: 0.5,
+	flex: 1,
 	margin: '5',
 	defaults: {
 	    width: '100%',
-- 
2.20.1





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
  2020-11-02 14:01 [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Aaron Lauterer
@ 2020-11-02 14:01 ` Aaron Lauterer
  2020-11-17  9:29   ` Dominik Csapak
  2020-11-16 17:23 ` [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Thomas Lamprecht
  2020-11-17  9:26 ` [pve-devel] applied: " Dominik Csapak
  2 siblings, 1 reply; 8+ messages in thread
From: Aaron Lauterer @ 2020-11-02 14:01 UTC (permalink / raw)
  To: pve-devel

When scaling the browsers content either via the browser itself or
because the OS has a different scaling / DPI setting, it can happen that
not all columns have enough space next to each other and thus the last
column is moved further below.

This happens especially on chromium bases browsers (e.g. chrome, edge).

Changing the layout to use extjs HBOXes with flex instead of columns
solves works well.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v1 -> v2: changed approach, use HBOX layouts instead of columns with
columnwidths slightly lower than 0.5.

 src/panel/InputPanel.js | 127 +++++++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 54 deletions(-)

diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js
index 0ac5e48..d5d6186 100644
--- a/src/panel/InputPanel.js
+++ b/src/panel/InputPanel.js
@@ -82,70 +82,80 @@ Ext.define('Proxmox.panel.InputPanel', {
 	let items;
 
 	if (me.items) {
-	    me.columns = 1;
 	    items = [
 		{
-		    columnWidth: 1,
 		    layout: 'anchor',
 		    items: me.items,
 		},
 	    ];
 	    me.items = undefined;
 	} else if (me.column4) {
-	    me.columns = 4;
 	    items = [
 		{
-		    columnWidth: 0.25,
-		    padding: '0 10 0 0',
-		    layout: 'anchor',
-		    items: me.column1,
-		},
-		{
-		    columnWidth: 0.25,
-		    padding: '0 10 0 0',
-		    layout: 'anchor',
-		    items: me.column2,
-		},
-		{
-		    columnWidth: 0.25,
-		    padding: '0 10 0 0',
-		    layout: 'anchor',
-		    items: me.column3,
-		},
-		{
-		    columnWidth: 0.25,
-		    padding: '0 0 0 10',
-		    layout: 'anchor',
-		    items: me.column4,
+		    layout: 'hbox',
+		    defaults: {
+			border: false,
+		    },
+		    items: [
+			{
+			    flex: 1,
+			    padding: '0 10 0 0',
+			    layout: 'anchor',
+			    items: me.column1,
+			},
+			{
+			    flex: 1,
+			    padding: '0 10 0 0',
+			    layout: 'anchor',
+			    items: me.column2,
+			},
+			{
+			    flex: 1,
+			    padding: '0 10 0 0',
+			    layout: 'anchor',
+			    items: me.column3,
+			},
+			{
+			    flex: 1,
+			    padding: '0 0 0 10',
+			    layout: 'anchor',
+			    items: me.column4,
+			},
+		    ],
 		},
 	    ];
 	    if (me.columnB) {
 		items.push({
-		    columnWidth: 1,
 		    padding: '10 0 0 0',
 		    layout: 'anchor',
 		    items: me.columnB,
 		});
 	    }
 	} else if (me.column1) {
-	    me.columns = 2;
 	    items = [
 		{
-		    columnWidth: 0.5,
-		    padding: '0 10 0 0',
-		    layout: 'anchor',
-		    items: me.column1,
-		},
-		{
-		    columnWidth: 0.5,
-		    padding: '0 0 0 10',
-		    layout: 'anchor',
-		    items: me.column2 || [], // allow empty column
+		    layout: 'hbox',
+		    defaults: {
+			border: false,
+		    },
+		    items: [
+			{
+			    flex: 1,
+			    padding: '0 10 0 0',
+			    layout: 'anchor',
+			    items: me.column1,
+			},
+			{
+			    flex: 1,
+			    padding: '0 0 0 10',
+			    layout: 'anchor',
+			    items: me.column2 || [], // allow empty column
+			},
+		    ],
 		},
 	    ];
 	    if (me.columnB) {
 		items.push({
-		    columnWidth: 1,
 		    padding: '10 0 0 0',
 		    layout: 'anchor',
 		    items: me.columnB,
@@ -159,7 +169,6 @@ Ext.define('Proxmox.panel.InputPanel', {
 	if (me.advancedItems) {
 	    advItems = [
 		{
-		    columnWidth: 1,
 		    layout: 'anchor',
 		    items: me.advancedItems,
 		},
@@ -168,16 +177,27 @@ Ext.define('Proxmox.panel.InputPanel', {
 	} else if (me.advancedColumn1) {
 	    advItems = [
 		{
-		    columnWidth: 0.5,
-		    padding: '0 10 0 0',
-		    layout: 'anchor',
-		    items: me.advancedColumn1,
-		},
-		{
-		    columnWidth: 0.5,
-		    padding: '0 0 0 10',
-		    layout: 'anchor',
-		    items: me.advancedColumn2 || [], // allow empty column
+		    layout: {
+			type: 'hbox',
+			align: 'begin',
+		    },
+		    defaults: {
+			border: false,
+		    },
+		    items: [
+			{
+			    flex: 1,
+			    padding: '0 10 0 0',
+			    layout: 'anchor',
+			    items: me.advancedColumn1,
+			},
+			{
+			    flex: 1,
+			    padding: '0 0 0 10',
+			    layout: 'anchor',
+			    items: me.advancedColumn2 || [], // allow empty column
+			},
+		    ],
 		},
 	    ];
 
@@ -186,7 +206,6 @@ Ext.define('Proxmox.panel.InputPanel', {
 
 	    if (me.advancedColumnB) {
 		advItems.push({
-		    columnWidth: 1,
 		    padding: '10 0 0 0',
 		    layout: 'anchor',
 		    items: me.advancedColumnB,
@@ -198,7 +217,6 @@ Ext.define('Proxmox.panel.InputPanel', {
 	if (advItems) {
 	    me.hasAdvanced = true;
 	    advItems.unshift({
-		columnWidth: 1,
 		xtype: 'box',
 		hidden: false,
 		border: true,
@@ -207,11 +225,9 @@ Ext.define('Proxmox.panel.InputPanel', {
 		},
 	    });
 	    items.push({
-		columnWidth: 1,
 		xtype: 'container',
 		itemId: 'advancedContainer',
 		hidden: !me.showAdvanced,
-		layout: 'column',
 		defaults: {
 		    border: false,
 		},
@@ -230,7 +246,10 @@ Ext.define('Proxmox.panel.InputPanel', {
 	    });
 	} else {
 	    Ext.apply(me, {
-		layout: 'column',
+		layout: {
+		    type: 'vbox',
+		    align: 'stretch',
+		},
 		defaultType: 'container',
 		items: items,
 	    });
-- 
2.20.1





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling
  2020-11-02 14:01 [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Aaron Lauterer
  2020-11-02 14:01 ` [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior Aaron Lauterer
@ 2020-11-16 17:23 ` Thomas Lamprecht
  2020-11-17  9:26 ` [pve-devel] applied: " Dominik Csapak
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2020-11-16 17:23 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak

On 02.11.20 15:01, Aaron Lauterer wrote:
> especially on chromium based browser (e.g. chrome, edge) it can happen,
> depending on the zoom level, that the last column does not fit next to
> the other columns and is moved below the other columns.
> 
> This results in an ugly looking UI and in the worst case makes it
> unusable.
> 
> This can also be triggered if the monitor is set to a higher scaling /
> different DPI settings. I was able to have the same problem in Edge when
> setting the scaling in the windows display settings to 125% (Clone VM).
> 
> Changing the layout from columns with 0.5 width to extjs HBOXes with
> flex 1 works as expected.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> v1 -> v2: changed approach, use HBOX layouts instead of columns with
> columnwidths slightly lower than 0.5.

@Dominik, can you please take a look at this and apply (or a Reviewed-by tag).

> 
>  www/manager6/window/Clone.js    | 7 +++----
>  www/manager6/window/Migrate.js  | 8 +++-----
>  www/manager6/window/Settings.js | 9 +++------
>  3 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/www/manager6/window/Clone.js b/www/manager6/window/Clone.js
> index 5c6c2f75..95ab1cd7 100644
> --- a/www/manager6/window/Clone.js
> +++ b/www/manager6/window/Clone.js
> @@ -256,22 +256,21 @@ Ext.define('PVE.window.Clone', {
>  	    bodyPadding: 10,
>  	    reference: 'cloneform',
>  	    border: false,
> -	    layout: 'column',
> +	    layout: 'hbox',
>  	    defaultType: 'container',
> -	    columns: 2,
>  	    fieldDefaults: {
>  		labelWidth: 100,
>  		anchor: '100%'
>  	    },
>  	    items: [
>  		{
> -		    columnWidth: 0.5,
> +		    flex: 1,
>  		    padding: '0 10 0 0',
>  		    layout: 'anchor',
>  		    items: col1
>  		},
>  		{
> -		    columnWidth: 0.5,
> +		    flex: 1,
>  		    padding: '0 0 0 10',
>  		    layout: 'anchor',
>  		    items: col2
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
> index 3096dd1a..91d1b30b 100644
> --- a/www/manager6/window/Migrate.js
> +++ b/www/manager6/window/Migrate.js
> @@ -308,13 +308,11 @@ Ext.define('PVE.window.Migrate', {
>  	    reference: 'formPanel',
>  	    bodyPadding: 10,
>  	    border: false,
> -	    layout: {
> -		type: 'column'
> -	    },
> +	    layout: 'hbox',
>  	    items: [
>  		{
>  		    xtype: 'container',
> -		    columnWidth: 0.5,
> +		    flex: 1,
>  		    items: [{
>  			xtype: 'displayfield',
>  			name: 'source',
> @@ -334,7 +332,7 @@ Ext.define('PVE.window.Migrate', {
>  		},
>  		{
>  		    xtype: 'container',
> -		    columnWidth: 0.5,
> +		    flex: 1,
>  		    items: [{
>  			xtype: 'pveNodeSelector',
>  			reference: 'pveNodeSelector',
> diff --git a/www/manager6/window/Settings.js b/www/manager6/window/Settings.js
> index e3519b1f..3bf974f0 100644
> --- a/www/manager6/window/Settings.js
> +++ b/www/manager6/window/Settings.js
> @@ -23,10 +23,7 @@ Ext.define('PVE.window.Settings', {
>  	}
>      ],
>  
> -    layout: {
> -	type: 'column',
> -	align: 'top'
> -    },
> +    layout: 'hbox',
>  
>      controller: {
>  	xclass: 'Ext.app.ViewController',
> @@ -185,7 +182,7 @@ Ext.define('PVE.window.Settings', {
>  
>      items: [{
>  	xtype: 'fieldset',
> -	columnWidth: 0.5,
> +	flex: 1,
>  	title: gettext('Webinterface Settings'),
>  	margin: '5',
>  	layout: {
> @@ -311,7 +308,7 @@ Ext.define('PVE.window.Settings', {
>      {
>  	xtype: 'container',
>  	layout: 'vbox',
> -	columnWidth: 0.5,
> +	flex: 1,
>  	margin: '5',
>  	defaults: {
>  	    width: '100%',
> 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] applied: [PATCH manager v2] ui: fix column behavior with browser scaling
  2020-11-02 14:01 [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Aaron Lauterer
  2020-11-02 14:01 ` [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior Aaron Lauterer
  2020-11-16 17:23 ` [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Thomas Lamprecht
@ 2020-11-17  9:26 ` Dominik Csapak
  2 siblings, 0 replies; 8+ messages in thread
From: Dominik Csapak @ 2020-11-17  9:26 UTC (permalink / raw)
  To: pve-devel

applied, thanks :)




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
  2020-11-02 14:01 ` [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior Aaron Lauterer
@ 2020-11-17  9:29   ` Dominik Csapak
  2020-11-17 11:41     ` Aaron Lauterer
  0 siblings, 1 reply; 8+ messages in thread
From: Dominik Csapak @ 2020-11-17  9:29 UTC (permalink / raw)
  To: pve-devel

some comments inline, looks good otherwise

On 11/2/20 3:01 PM, Aaron Lauterer wrote:
> When scaling the browsers content either via the browser itself or
> because the OS has a different scaling / DPI setting, it can happen that
> not all columns have enough space next to each other and thus the last
> column is moved further below.
> 
> This happens especially on chromium bases browsers (e.g. chrome, edge).
> 
> Changing the layout to use extjs HBOXes with flex instead of columns
> solves works well.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> v1 -> v2: changed approach, use HBOX layouts instead of columns with
> columnwidths slightly lower than 0.5.
> 
>   src/panel/InputPanel.js | 127 +++++++++++++++++++++++-----------------
>   1 file changed, 73 insertions(+), 54 deletions(-)
> 
> diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js
> index 0ac5e48..d5d6186 100644
> --- a/src/panel/InputPanel.js
> +++ b/src/panel/InputPanel.js
> @@ -82,70 +82,80 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	let items;
>   
>   	if (me.items) {
> -	    me.columns = 1;
>   	    items = [
>   		{
> -		    columnWidth: 1,
>   		    layout: 'anchor',
>   		    items: me.items,
>   		},
>   	    ];
>   	    me.items = undefined;
>   	} else if (me.column4) {
> -	    me.columns = 4;
>   	    items = [
>   		{
> -		    columnWidth: 0.25,
> -		    padding: '0 10 0 0',
> -		    layout: 'anchor',
> -		    items: me.column1,
> -		},
> -		{
> -		    columnWidth: 0.25,
> -		    padding: '0 10 0 0',
> -		    layout: 'anchor',
> -		    items: me.column2,
> -		},
> -		{
> -		    columnWidth: 0.25,
> -		    padding: '0 10 0 0',
> -		    layout: 'anchor',
> -		    items: me.column3,
> -		},
> -		{
> -		    columnWidth: 0.25,
> -		    padding: '0 0 0 10',
> -		    layout: 'anchor',
> -		    items: me.column4,
> +		    layout: 'hbox',
> +		    defaults: {
> +			border: false,
> +		    },

i guess we could add

layout: 'anchor'

here too and save 3 lines?

> +		    items: [
> +			{
> +			    flex: 1,
> +			    padding: '0 10 0 0',
> +			    layout: 'anchor',
> +			    items: me.column1,
> +			},
> +			{
> +			    flex: 1,
> +			    padding: '0 10 0 0',
> +			    layout: 'anchor',
> +			    items: me.column2,
> +			},
> +			{
> +			    flex: 1,
> +			    padding: '0 10 0 0',
> +			    layout: 'anchor',
> +			    items: me.column3,
> +			},
> +			{
> +			    flex: 1,
> +			    padding: '0 0 0 10',
> +			    layout: 'anchor',
> +			    items: me.column4,
> +			},
> +		    ],
>   		},
>   	    ];
>   	    if (me.columnB) {
>   		items.push({
> -		    columnWidth: 1,
>   		    padding: '10 0 0 0',
>   		    layout: 'anchor',
>   		    items: me.columnB,
>   		});
>   	    }
>   	} else if (me.column1) {
> -	    me.columns = 2;
>   	    items = [
>   		{
> -		    columnWidth: 0.5,
> -		    padding: '0 10 0 0',
> -		    layout: 'anchor',
> -		    items: me.column1,
> -		},
> -		{
> -		    columnWidth: 0.5,
> -		    padding: '0 0 0 10',
> -		    layout: 'anchor',
> -		    items: me.column2 || [], // allow empty column
> +		    layout: 'hbox',
> +		    defaults: {
> +			border: false,
> +		    },
> +		    items: [
> +			{
> +			    flex: 1,
> +			    padding: '0 10 0 0',
> +			    layout: 'anchor',
> +			    items: me.column1,
> +			},
> +			{
> +			    flex: 1,
> +			    padding: '0 0 0 10',
> +			    layout: 'anchor',
> +			    items: me.column2 || [], // allow empty column
> +			},
> +		    ],
>   		},
>   	    ];
>   	    if (me.columnB) {
>   		items.push({
> -		    columnWidth: 1,
>   		    padding: '10 0 0 0',
>   		    layout: 'anchor',
>   		    items: me.columnB,
> @@ -159,7 +169,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	if (me.advancedItems) {
>   	    advItems = [
>   		{
> -		    columnWidth: 1,
>   		    layout: 'anchor',
>   		    items: me.advancedItems,
>   		},
> @@ -168,16 +177,27 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	} else if (me.advancedColumn1) {
>   	    advItems = [
>   		{
> -		    columnWidth: 0.5,
> -		    padding: '0 10 0 0',
> -		    layout: 'anchor',
> -		    items: me.advancedColumn1,
> -		},
> -		{
> -		    columnWidth: 0.5,
> -		    padding: '0 0 0 10',
> -		    layout: 'anchor',
> -		    items: me.advancedColumn2 || [], // allow empty column
> +		    layout: {
> +			type: 'hbox',
> +			align: 'begin',
> +		    },
> +		    defaults: {
> +			border: false,
> +		    },
> +		    items: [
> +			{
> +			    flex: 1,
> +			    padding: '0 10 0 0',
> +			    layout: 'anchor',
> +			    items: me.advancedColumn1,
> +			},
> +			{
> +			    flex: 1,
> +			    padding: '0 0 0 10',
> +			    layout: 'anchor',
> +			    items: me.advancedColumn2 || [], // allow empty column
> +			},
> +		    ],
>   		},
>   	    ];
>   
> @@ -186,7 +206,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>   
>   	    if (me.advancedColumnB) {
>   		advItems.push({
> -		    columnWidth: 1,
>   		    padding: '10 0 0 0',
>   		    layout: 'anchor',
>   		    items: me.advancedColumnB,
> @@ -198,7 +217,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	if (advItems) {
>   	    me.hasAdvanced = true;
>   	    advItems.unshift({
> -		columnWidth: 1,
>   		xtype: 'box',
>   		hidden: false,
>   		border: true,
> @@ -207,11 +225,9 @@ Ext.define('Proxmox.panel.InputPanel', {
>   		},
>   	    });
>   	    items.push({
> -		columnWidth: 1,
>   		xtype: 'container',
>   		itemId: 'advancedContainer',
>   		hidden: !me.showAdvanced,
> -		layout: 'column',
>   		defaults: {
>   		    border: false,
>   		},

i guess we would have to change this here to vbox as well?
do we even use 'fieldContainer' anywhere anymore?

> @@ -230,7 +246,10 @@ Ext.define('Proxmox.panel.InputPanel', {
>   	    });
>   	} else {
>   	    Ext.apply(me, {
> -		layout: 'column',
> +		layout: {
> +		    type: 'vbox',
> +		    align: 'stretch',
> +		},
>   		defaultType: 'container',
>   		items: items,
>   	    });
> 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
  2020-11-17  9:29   ` Dominik Csapak
@ 2020-11-17 11:41     ` Aaron Lauterer
  2020-11-17 12:05       ` Dominik Csapak
  0 siblings, 1 reply; 8+ messages in thread
From: Aaron Lauterer @ 2020-11-17 11:41 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominik Csapak



On 11/17/20 10:29 AM, Dominik Csapak wrote:
> some comments inline, looks good otherwise
> 
> On 11/2/20 3:01 PM, Aaron Lauterer wrote:
>> When scaling the browsers content either via the browser itself or
>> because the OS has a different scaling / DPI setting, it can happen that
>> not all columns have enough space next to each other and thus the last
>> column is moved further below.
>>
>> This happens especially on chromium bases browsers (e.g. chrome, edge).
>>
>> Changing the layout to use extjs HBOXes with flex instead of columns
>> solves works well.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> v1 -> v2: changed approach, use HBOX layouts instead of columns with
>> columnwidths slightly lower than 0.5.
>>
>>   src/panel/InputPanel.js | 127 +++++++++++++++++++++++-----------------
>>   1 file changed, 73 insertions(+), 54 deletions(-)
>>
>> diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js
>> index 0ac5e48..d5d6186 100644
>> --- a/src/panel/InputPanel.js
>> +++ b/src/panel/InputPanel.js
>> @@ -82,70 +82,80 @@ Ext.define('Proxmox.panel.InputPanel', {
>>       let items;
>>       if (me.items) {
>> -        me.columns = 1;
>>           items = [
>>           {
>> -            columnWidth: 1,
>>               layout: 'anchor',
>>               items: me.items,
>>           },
>>           ];
>>           me.items = undefined;
>>       } else if (me.column4) {
>> -        me.columns = 4;
>>           items = [
>>           {
>> -            columnWidth: 0.25,
>> -            padding: '0 10 0 0',
>> -            layout: 'anchor',
>> -            items: me.column1,
>> -        },
>> -        {
>> -            columnWidth: 0.25,
>> -            padding: '0 10 0 0',
>> -            layout: 'anchor',
>> -            items: me.column2,
>> -        },
>> -        {
>> -            columnWidth: 0.25,
>> -            padding: '0 10 0 0',
>> -            layout: 'anchor',
>> -            items: me.column3,
>> -        },
>> -        {
>> -            columnWidth: 0.25,
>> -            padding: '0 0 0 10',
>> -            layout: 'anchor',
>> -            items: me.column4,
>> +            layout: 'hbox',
>> +            defaults: {
>> +            border: false,
>> +            },
> 
> i guess we could add
> 
> layout: 'anchor'
> 
> here too and save 3 lines?

instead of the hbox and disabling borders for the 4column layout?

Doesn't work for me on Firefox when I do that and check against the PMG -> Configuration -> Spam Detector -> Options -> Languages panel

> 
>> +            items: [
>> +            {
>> +                flex: 1,
>> +                padding: '0 10 0 0',
>> +                layout: 'anchor',
>> +                items: me.column1,
>> +            },
>> +            {
>> +                flex: 1,
>> +                padding: '0 10 0 0',
>> +                layout: 'anchor',
>> +                items: me.column2,
>> +            },
>> +            {
>> +                flex: 1,
>> +                padding: '0 10 0 0',
>> +                layout: 'anchor',
>> +                items: me.column3,
>> +            },
>> +            {
>> +                flex: 1,
>> +                padding: '0 0 0 10',
>> +                layout: 'anchor',
>> +                items: me.column4,
>> +            },
>> +            ],
>>           },
>>           ];
>>           if (me.columnB) {
>>           items.push({
>> -            columnWidth: 1,
>>               padding: '10 0 0 0',
>>               layout: 'anchor',
>>               items: me.columnB,
>>           });
>>           }
>>       } else if (me.column1) {
>> -        me.columns = 2;
>>           items = [
>>           {
>> -            columnWidth: 0.5,
>> -            padding: '0 10 0 0',
>> -            layout: 'anchor',
>> -            items: me.column1,
>> -        },
>> -        {
>> -            columnWidth: 0.5,
>> -            padding: '0 0 0 10',
>> -            layout: 'anchor',
>> -            items: me.column2 || [], // allow empty column
>> +            layout: 'hbox',
>> +            defaults: {
>> +            border: false,
>> +            },
>> +            items: [
>> +            {
>> +                flex: 1,
>> +                padding: '0 10 0 0',
>> +                layout: 'anchor',
>> +                items: me.column1,
>> +            },
>> +            {
>> +                flex: 1,
>> +                padding: '0 0 0 10',
>> +                layout: 'anchor',
>> +                items: me.column2 || [], // allow empty column
>> +            },
>> +            ],
>>           },
>>           ];
>>           if (me.columnB) {
>>           items.push({
>> -            columnWidth: 1,
>>               padding: '10 0 0 0',
>>               layout: 'anchor',
>>               items: me.columnB,
>> @@ -159,7 +169,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>       if (me.advancedItems) {
>>           advItems = [
>>           {
>> -            columnWidth: 1,
>>               layout: 'anchor',
>>               items: me.advancedItems,
>>           },
>> @@ -168,16 +177,27 @@ Ext.define('Proxmox.panel.InputPanel', {
>>       } else if (me.advancedColumn1) {
>>           advItems = [
>>           {
>> -            columnWidth: 0.5,
>> -            padding: '0 10 0 0',
>> -            layout: 'anchor',
>> -            items: me.advancedColumn1,
>> -        },
>> -        {
>> -            columnWidth: 0.5,
>> -            padding: '0 0 0 10',
>> -            layout: 'anchor',
>> -            items: me.advancedColumn2 || [], // allow empty column
>> +            layout: {
>> +            type: 'hbox',
>> +            align: 'begin',
>> +            },
>> +            defaults: {
>> +            border: false,
>> +            },
>> +            items: [
>> +            {
>> +                flex: 1,
>> +                padding: '0 10 0 0',
>> +                layout: 'anchor',
>> +                items: me.advancedColumn1,
>> +            },
>> +            {
>> +                flex: 1,
>> +                padding: '0 0 0 10',
>> +                layout: 'anchor',
>> +                items: me.advancedColumn2 || [], // allow empty column
>> +            },
>> +            ],
>>           },
>>           ];
>> @@ -186,7 +206,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>           if (me.advancedColumnB) {
>>           advItems.push({
>> -            columnWidth: 1,
>>               padding: '10 0 0 0',
>>               layout: 'anchor',
>>               items: me.advancedColumnB,
>> @@ -198,7 +217,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>       if (advItems) {
>>           me.hasAdvanced = true;
>>           advItems.unshift({
>> -        columnWidth: 1,
>>           xtype: 'box',
>>           hidden: false,
>>           border: true,
>> @@ -207,11 +225,9 @@ Ext.define('Proxmox.panel.InputPanel', {
>>           },
>>           });
>>           items.push({
>> -        columnWidth: 1,
>>           xtype: 'container',
>>           itemId: 'advancedContainer',
>>           hidden: !me.showAdvanced,
>> -        layout: 'column',
>>           defaults: {
>>               border: false,
>>           },
> 
> i guess we would have to change this here to vbox as well?
> do we even use 'fieldContainer' anywhere anymore?

There are a few places where we have

xtype: 'fieldcontainer',
extend: 'Ext.form.FieldContainer',

if you grep the widget toolkit, pmg-gui and pve-manager repos, but nothing where we set

me.useFieldContainer

manually. Unless that is some property that extjs is setting automagically.

> 
>> @@ -230,7 +246,10 @@ Ext.define('Proxmox.panel.InputPanel', {
>>           });
>>       } else {
>>           Ext.apply(me, {
>> -        layout: 'column',
>> +        layout: {
>> +            type: 'vbox',
>> +            align: 'stretch',
>> +        },
>>           defaultType: 'container',
>>           items: items,
>>           });
>>
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
  2020-11-17 11:41     ` Aaron Lauterer
@ 2020-11-17 12:05       ` Dominik Csapak
  2020-11-17 12:43         ` Aaron Lauterer
  0 siblings, 1 reply; 8+ messages in thread
From: Dominik Csapak @ 2020-11-17 12:05 UTC (permalink / raw)
  To: Aaron Lauterer, Proxmox VE development discussion

On 11/17/20 12:41 PM, Aaron Lauterer wrote:
> 
> 
> On 11/17/20 10:29 AM, Dominik Csapak wrote:
>> some comments inline, looks good otherwise
>>
>> On 11/2/20 3:01 PM, Aaron Lauterer wrote:
>>> When scaling the browsers content either via the browser itself or
>>> because the OS has a different scaling / DPI setting, it can happen that
>>> not all columns have enough space next to each other and thus the last
>>> column is moved further below.
>>>
>>> This happens especially on chromium bases browsers (e.g. chrome, edge).
>>>
>>> Changing the layout to use extjs HBOXes with flex instead of columns
>>> solves works well.
>>>
>>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>>> ---
>>> v1 -> v2: changed approach, use HBOX layouts instead of columns with
>>> columnwidths slightly lower than 0.5.
>>>
>>>   src/panel/InputPanel.js | 127 +++++++++++++++++++++++-----------------
>>>   1 file changed, 73 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js
>>> index 0ac5e48..d5d6186 100644
>>> --- a/src/panel/InputPanel.js
>>> +++ b/src/panel/InputPanel.js
>>> @@ -82,70 +82,80 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>       let items;
>>>       if (me.items) {
>>> -        me.columns = 1;
>>>           items = [
>>>           {
>>> -            columnWidth: 1,
>>>               layout: 'anchor',
>>>               items: me.items,
>>>           },
>>>           ];
>>>           me.items = undefined;
>>>       } else if (me.column4) {
>>> -        me.columns = 4;
>>>           items = [
>>>           {
>>> -            columnWidth: 0.25,
>>> -            padding: '0 10 0 0',
>>> -            layout: 'anchor',
>>> -            items: me.column1,
>>> -        },
>>> -        {
>>> -            columnWidth: 0.25,
>>> -            padding: '0 10 0 0',
>>> -            layout: 'anchor',
>>> -            items: me.column2,
>>> -        },
>>> -        {
>>> -            columnWidth: 0.25,
>>> -            padding: '0 10 0 0',
>>> -            layout: 'anchor',
>>> -            items: me.column3,
>>> -        },
>>> -        {
>>> -            columnWidth: 0.25,
>>> -            padding: '0 0 0 10',
>>> -            layout: 'anchor',
>>> -            items: me.column4,
>>> +            layout: 'hbox',
>>> +            defaults: {
>>> +            border: false,
>>> +            },
>>
>> i guess we could add
>>
>> layout: 'anchor'
>>
>> here too and save 3 lines?
> 
> instead of the hbox and disabling borders for the 4column layout?
> 
> Doesn't work for me on Firefox when I do that and check against the PMG 
> -> Configuration -> Spam Detector -> Options -> Languages panel
> 

hmm? i meant adding using

layout: 'hbox',
defaults: {
     border: false,
     layout: 'anchor',
     flex: 1,
},

(i just noticed we could also set 'flex: 1' there)
instead of adding layout: 'anchor' on every child
'defaults' corresponds to the default settings of all child items

>>
>>> +            items: [
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 10 0 0',
>>> +                layout: 'anchor',
>>> +                items: me.column1,
>>> +            },
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 10 0 0',
>>> +                layout: 'anchor',
>>> +                items: me.column2,
>>> +            },
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 10 0 0',
>>> +                layout: 'anchor',
>>> +                items: me.column3,
>>> +            },
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 0 0 10',
>>> +                layout: 'anchor',
>>> +                items: me.column4,
>>> +            },
>>> +            ],
>>>           },
>>>           ];
>>>           if (me.columnB) {
>>>           items.push({
>>> -            columnWidth: 1,
>>>               padding: '10 0 0 0',
>>>               layout: 'anchor',
>>>               items: me.columnB,
>>>           });
>>>           }
>>>       } else if (me.column1) {
>>> -        me.columns = 2;
>>>           items = [
>>>           {
>>> -            columnWidth: 0.5,
>>> -            padding: '0 10 0 0',
>>> -            layout: 'anchor',
>>> -            items: me.column1,
>>> -        },
>>> -        {
>>> -            columnWidth: 0.5,
>>> -            padding: '0 0 0 10',
>>> -            layout: 'anchor',
>>> -            items: me.column2 || [], // allow empty column
>>> +            layout: 'hbox',
>>> +            defaults: {
>>> +            border: false,
>>> +            },
>>> +            items: [
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 10 0 0',
>>> +                layout: 'anchor',
>>> +                items: me.column1,
>>> +            },
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 0 0 10',
>>> +                layout: 'anchor',
>>> +                items: me.column2 || [], // allow empty column
>>> +            },
>>> +            ],
>>>           },
>>>           ];
>>>           if (me.columnB) {
>>>           items.push({
>>> -            columnWidth: 1,
>>>               padding: '10 0 0 0',
>>>               layout: 'anchor',
>>>               items: me.columnB,
>>> @@ -159,7 +169,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>       if (me.advancedItems) {
>>>           advItems = [
>>>           {
>>> -            columnWidth: 1,
>>>               layout: 'anchor',
>>>               items: me.advancedItems,
>>>           },
>>> @@ -168,16 +177,27 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>       } else if (me.advancedColumn1) {
>>>           advItems = [
>>>           {
>>> -            columnWidth: 0.5,
>>> -            padding: '0 10 0 0',
>>> -            layout: 'anchor',
>>> -            items: me.advancedColumn1,
>>> -        },
>>> -        {
>>> -            columnWidth: 0.5,
>>> -            padding: '0 0 0 10',
>>> -            layout: 'anchor',
>>> -            items: me.advancedColumn2 || [], // allow empty column
>>> +            layout: {
>>> +            type: 'hbox',
>>> +            align: 'begin',
>>> +            },
>>> +            defaults: {
>>> +            border: false,
>>> +            },
>>> +            items: [
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 10 0 0',
>>> +                layout: 'anchor',
>>> +                items: me.advancedColumn1,
>>> +            },
>>> +            {
>>> +                flex: 1,
>>> +                padding: '0 0 0 10',
>>> +                layout: 'anchor',
>>> +                items: me.advancedColumn2 || [], // allow empty column
>>> +            },
>>> +            ],
>>>           },
>>>           ];
>>> @@ -186,7 +206,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>           if (me.advancedColumnB) {
>>>           advItems.push({
>>> -            columnWidth: 1,
>>>               padding: '10 0 0 0',
>>>               layout: 'anchor',
>>>               items: me.advancedColumnB,
>>> @@ -198,7 +217,6 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>       if (advItems) {
>>>           me.hasAdvanced = true;
>>>           advItems.unshift({
>>> -        columnWidth: 1,
>>>           xtype: 'box',
>>>           hidden: false,
>>>           border: true,
>>> @@ -207,11 +225,9 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>           },
>>>           });
>>>           items.push({
>>> -        columnWidth: 1,
>>>           xtype: 'container',
>>>           itemId: 'advancedContainer',
>>>           hidden: !me.showAdvanced,
>>> -        layout: 'column',
>>>           defaults: {
>>>               border: false,
>>>           },
>>
>> i guess we would have to change this here to vbox as well?
>> do we even use 'fieldContainer' anywhere anymore?
> 
> There are a few places where we have
> 
> xtype: 'fieldcontainer',
> extend: 'Ext.form.FieldContainer',

thats something different

> 
> if you grep the widget toolkit, pmg-gui and pve-manager repos, but 
> nothing where we set
> 
> me.useFieldContainer
> 
> manually. Unless that is some property that extjs is setting automagically.

i cannot remember, but a short search in extjs source does
not turn anything up for 'useFieldContainer' so i'd
say this is dead code and we should/could remove it altogether...
(i also quickly grepped proxmox-backup, but no use of it there either)

> 
>>
>>> @@ -230,7 +246,10 @@ Ext.define('Proxmox.panel.InputPanel', {
>>>           });
>>>       } else {
>>>           Ext.apply(me, {
>>> -        layout: 'column',
>>> +        layout: {
>>> +            type: 'vbox',
>>> +            align: 'stretch',
>>> +        },
>>>           defaultType: 'container',
>>>           items: items,
>>>           });
>>>
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
  2020-11-17 12:05       ` Dominik Csapak
@ 2020-11-17 12:43         ` Aaron Lauterer
  0 siblings, 0 replies; 8+ messages in thread
From: Aaron Lauterer @ 2020-11-17 12:43 UTC (permalink / raw)
  To: Dominik Csapak, Proxmox VE development discussion



On 11/17/20 1:05 PM, Dominik Csapak wrote:
> On 11/17/20 12:41 PM, Aaron Lauterer wrote:
>>
>>
>> On 11/17/20 10:29 AM, Dominik Csapak wrote:
>>> some comments inline, looks good otherwise
>>>
>>> On 11/2/20 3:01 PM, Aaron Lauterer wrote:
>>>> When scaling the browsers content either via the browser itself or


[...]

>>>> +            layout: 'hbox',
>>>> +            defaults: {
>>>> +            border: false,
>>>> +            },
>>>
>>> i guess we could add
>>>
>>> layout: 'anchor'
>>>
>>> here too and save 3 lines?
>>
>> instead of the hbox and disabling borders for the 4column layout?
>>
>> Doesn't work for me on Firefox when I do that and check against the PMG -> Configuration -> Spam Detector -> Options -> Languages panel
>>
> 
> hmm? i meant adding using
> 
> layout: 'hbox',
> defaults: {
>      border: false,
>      layout: 'anchor',
>      flex: 1,
> },
> 
> (i just noticed we could also set 'flex: 1' there)
> instead of adding layout: 'anchor' on every child
> 'defaults' corresponds to the default settings of all child items
>

ah now I get it. Yep seems to be working. I will send in v3 with those changes

[...]

>>>>           defaults: {
>>>>               border: false,
>>>>           },
>>>
>>> i guess we would have to change this here to vbox as well?
>>> do we even use 'fieldContainer' anywhere anymore?
>>
>> There are a few places where we have
>>
>> xtype: 'fieldcontainer',
>> extend: 'Ext.form.FieldContainer',
> 
> thats something different

thanks for confirming that

> 
>>
>> if you grep the widget toolkit, pmg-gui and pve-manager repos, but nothing where we set
>>
>> me.useFieldContainer
>>
>> manually. Unless that is some property that extjs is setting automagically.
> 
> i cannot remember, but a short search in extjs source does
> not turn anything up for 'useFieldContainer' so i'd
> say this is dead code and we should/could remove it altogether...
> (i also quickly grepped proxmox-backup, but no use of it there either)
> 

I'll add a separate commit on the v3 to remove that code




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-11-17 12:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 14:01 [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Aaron Lauterer
2020-11-02 14:01 ` [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior Aaron Lauterer
2020-11-17  9:29   ` Dominik Csapak
2020-11-17 11:41     ` Aaron Lauterer
2020-11-17 12:05       ` Dominik Csapak
2020-11-17 12:43         ` Aaron Lauterer
2020-11-16 17:23 ` [pve-devel] [PATCH manager v2] ui: fix column behavior with browser scaling Thomas Lamprecht
2020-11-17  9:26 ` [pve-devel] applied: " Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal