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

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

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