Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior

[Dominik Csapak <d.csapak@proxmox.com>]

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,
>   	    });
>