From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id ADDCB605FE for ; Tue, 17 Nov 2020 13:05:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 9C480F85B for ; Tue, 17 Nov 2020 13:05:04 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 69D36F84E for ; Tue, 17 Nov 2020 13:05:02 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 2F8AC43756; Tue, 17 Nov 2020 13:05:02 +0100 (CET) To: Aaron Lauterer , Proxmox VE development discussion References: <20201102140102.26141-1-a.lauterer@proxmox.com> <20201102140102.26141-2-a.lauterer@proxmox.com> <93892863-6465-7443-4881-5daa3cccc43a@proxmox.com> From: Dominik Csapak Message-ID: <39d726dc-ca26-69e6-75d9-21c6363c4b21@proxmox.com> Date: Tue, 17 Nov 2020 13:05:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Thunderbird/83.0 MIME-Version: 1.0 In-Reply-To: <93892863-6465-7443-4881-5daa3cccc43a@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.351 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pve-devel] [PATCH widget-toolkit v2] InputPanel: fix column scaling behavior X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Nov 2020 12:05:34 -0000 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 >>> --- >>> 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 >> >>