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 9C29DB8B22 for ; Wed, 6 Dec 2023 10:59:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 76AA037176 for ; Wed, 6 Dec 2023 10:59:04 +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, 6 Dec 2023 10:59:03 +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 A333A42509 for ; Wed, 6 Dec 2023 10:59:03 +0100 (CET) Message-ID: <9dc84de5-23c5-4ecb-a25b-41488e5709b3@proxmox.com> Date: Wed, 6 Dec 2023 10:59:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Dominik Csapak , Proxmox VE development discussion References: <20231205154458.268660-1-m.carrara@proxmox.com> <20231205154458.268660-2-m.carrara@proxmox.com> Content-Language: en-US From: Max Carrara In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [RFC proxmox-widget-toolkit 1/2] input panel: add `raw` parameter to function `getValues` 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, 06 Dec 2023 09:59:34 -0000 On 12/6/23 10:13, Dominik Csapak wrote: > hi, some comment inline > > On 12/5/23 16:44, Max Carrara wrote: >> This parameter may be used to circumvent calls to `onGetValues`. >> >> Also adds a docstring for the function. >> >> Signed-off-by: Max Carrara >> --- >>   src/panel/InputPanel.js | 15 ++++++++++++++- >>   1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js >> index 34150ef..723be42 100644 >> --- a/src/panel/InputPanel.js >> +++ b/src/panel/InputPanel.js >> @@ -31,7 +31,16 @@ Ext.define('Proxmox.panel.InputPanel', { >>       return values; >>       }, >>   -    getValues: function(dirtyOnly) { >> +    /** >> +     * Returns the submit data from the panel's form fields. >> +     * >> +     * @param {boolean} dirtyOnly `true` to return only dirty fields >> +     * (fields that have been changed from their original value). >> +     * @param {boolean} raw `true` to prevent calling >> +     * {@link Proxmox.panel.InputPanel#onGetValues onGetValues} and >> +     * instead return the original submit data. >> +     */ > > nice to see these things documented, not sure about the format, but i guess we could adapt such a standard format (maybe at one point we > could even generate nicer docs for this :) ) That's just JSDoc - I haven't really seen it used anywhere here (and to be honest, also didn't bother to look) so I figured I'd just use it here and see how people respond. ;) Ext JS uses it too, so. > > another thing is the interface, adding a parameter works and can be ok > but wouldn't it be nicer to invent a new 'getRawValues'? > > this would then not change the signature of the original function and > is more in line with what extjs does internally > (getValue/getRawValue/etc.) > > not a hard requirement for me though Absolutely not opposed to this - I think then the code is more exlicit anyway. E.g. `foo.getValues(false, true)` doesn't really say what it does at first glance. I'll definitely incorporate this! > > a slight tangent thought: > > after looking, it seems we never use(d?) the dirtyOnly > parameter... so it might be nice to remove that > > we couldn't even have used it anywhere because we always > set it to false when 'onGetValues' is  a function > so we'd  need to set it to something different, but > later we call it unconditionally as a function(?!?) > > but that is rather unrelated, just confused me > (it seems it was "always" this way, so maybe > there is some hidden svn history that i'm not seeing > where the code would make more sense) I was wondering about that too, but I didn't really want to turn over too many stones for what's supposed to be just a simple RFC. I wouldn't mind retiring that parameter for the actual patch series then, if you'd like. I'll also double-check all call sites to make sure it's not used anywhere - not that it would make a difference, but just in case the parameter is provided somewhere. > >> +    getValues: function(dirtyOnly, raw) { >>       let me = this; >>         if (Ext.isFunction(me.onGetValues)) { >> @@ -46,6 +55,10 @@ Ext.define('Proxmox.panel.InputPanel', { >>           } >>       }); >>   +    if (raw) { >> +        return values; >> +    } >> + >>       return me.onGetValues(values); >>       }, >>