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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 758A99287 for ; Wed, 8 Mar 2023 13:15:33 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3E3832022C for ; Wed, 8 Mar 2023 13:15:03 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 ESMTPS for ; Wed, 8 Mar 2023 13:15:02 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2F2C745E50 for ; Wed, 8 Mar 2023 13:15:02 +0100 (CET) Message-ID: <972c7e4f-d881-32cb-eaec-89d66408a74f@proxmox.com> Date: Wed, 8 Mar 2023 13:15:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Content-Language: en-US To: Proxmox VE development discussion , Aaron Lauterer References: <20230113150930.857270-1-a.lauterer@proxmox.com> <20230113150930.857270-4-a.lauterer@proxmox.com> From: Dominik Csapak In-Reply-To: <20230113150930.857270-4-a.lauterer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.061 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH manager 3/3] ui: ceph pool edit: rework with controller and formulas 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: Wed, 08 Mar 2023 12:15:33 -0000 some (minor) comments inline On 1/13/23 16:09, Aaron Lauterer wrote: > instead of relying purely on listeners that then manually change other > components, we can use binds, formulas and a basic controller. > > This makes it quite a bit easier to let multiple components react to > changes. > > A cbind is used for the size component to set the initial start value. > Other options, like using setValue in the controller init, will trigger > the change listener and therefore can affect the min size without any > user interaction. > > Signed-off-by: Aaron Lauterer > --- > www/manager6/ceph/Pool.js | 87 ++++++++++++++++++++++++++++----------- > 1 file changed, 63 insertions(+), 24 deletions(-) > > diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js > index 86f83ffb..a38c5b3f 100644 > --- a/www/manager6/ceph/Pool.js > +++ b/www/manager6/ceph/Pool.js > @@ -7,6 +7,49 @@ Ext.define('PVE.CephPoolInputPanel', { > onlineHelp: 'pve_ceph_pools', > > subject: 'Ceph Pool', > + > + viewModel: { typically we have the viewmodel between the controller and the layout, but no big issue (though we're probably not 100% consistent on that) > + data: { > + minSize: null, > + size: null, > + }, > + formulas: { > + minSizeLabel: (get) => { > + if (get('showMinSizeOneWarning') || get('showMinSizeHalfWarning')) { > + return `${gettext('Min. Size')} `; > + } > + return gettext('Min. Size'); > + }, > + showMinSizeOneWarning: (get) => get('minSize') === 1, > + showMinSizeHalfWarning: (get) => { > + let minSize = get('minSize'); > + let size = get('size'); > + if (minSize === 1) { > + return false; > + } > + return minSize < (size / 2) && minSize !== size; > + }, mhmm... i generally don't want to have negated variable names, but since we bind that to the 'hidden' property (negated) would it maybe make sense to reverse the logic in here and rename them to 'hide*Warning' ? not sure about that though > + }, > + }, > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + init: function(view) { > + let vm = this.getViewModel(); > + vm.set('size', view.defaultSize ? Number(view.defaultSize) : 3); > + vm.set('minSize', view.defaultMinSize ? Number(view.defaultMinSize) : 2); > + }, > + sizeChange: function(field, val) { > + let vm = this.getViewModel(); > + let minSize = Math.round(val / 2); > + if (minSize > 1) { > + vm.set('minSize', minSize); > + } > + vm.set('size', val); // bind does not work in a pmxDisplayEditField, update manually > + }, > + }, > + > column1: [ > { > xtype: 'pmxDisplayEditField', > @@ -34,12 +77,7 @@ Ext.define('PVE.CephPoolInputPanel', { > maxValue: 7, > allowBlank: false, > listeners: { > - change: function(field, val) { > - let size = Math.round(val / 2); > - if (size > 1) { > - field.up('inputpanel').down('field[name=min_size]').setValue(size); > - } > - }, > + change: 'sizeChange', > }, > }, > }, > @@ -77,10 +115,13 @@ Ext.define('PVE.CephPoolInputPanel', { > advancedColumn1: [ > { > xtype: 'proxmoxintegerfield', > - fieldLabel: gettext('Min. Size'), > + bind: { > + fieldLabel: '{minSizeLabel}', > + value: '{minSize}', > + }, > name: 'min_size', > + reference: 'min_size', it seems you don't actually use the reference, maybe a leftover? > cbind: { > - value: (get) => get('defaultMinSize') ?? 2, > minValue: (get) => { > if (Number(get('defaultMinSize')) === 1) { > return 1; > @@ -91,28 +132,26 @@ Ext.define('PVE.CephPoolInputPanel', { > }, > maxValue: 7, > allowBlank: false, > - listeners: { > - change: function(field, minSize) { > - let panel = field.up('inputpanel'); > - let size = panel.down('field[name=size]').getValue(); > - > - let showWarning = minSize < (size / 2) && minSize !== size; > - > - let fieldLabel = gettext('Min. Size'); > - if (showWarning) { > - fieldLabel = gettext('Min. Size') + ' '; > - } > - panel.down('field[name=min_size-warning]').setHidden(!showWarning); > - field.setFieldLabel(fieldLabel); > - }, > - }, > }, > { > xtype: 'displayfield', > - name: 'min_size-warning', > + name: 'min_size_half-warning', a field that's not set/read does not really need a name, so we can simply omit that > + bind: { > + hidden: '{!showMinSizeHalfWarning}', > + }, > + hidden: true, > userCls: 'pmx-hint', > value: gettext('min_size < size/2 can lead to data loss, incomplete PGs or unfound objects.'), > + }, > + { > + xtype: 'displayfield', > + name: 'min_size_one-warning', same here > + bind: { > + hidden: '{!showMinSizeOneWarning}', > + }, > hidden: true, > + userCls: 'pmx-hint', > + value: gettext('a min_size of 1 is not recommended and can lead to data loss'), > }, > { > xtype: 'pmxDisplayEditField',