public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior
Date: Tue, 17 Nov 2020 12:41:18 +0100	[thread overview]
Message-ID: <93892863-6465-7443-4881-5daa3cccc43a@proxmox.com> (raw)
In-Reply-To: <c8560f5a-c043-ef33-239b-7f281792d6e6@proxmox.com>



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




  reply	other threads:[~2020-11-17 11:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=93892863-6465-7443-4881-5daa3cccc43a@proxmox.com \
    --to=a.lauterer@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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