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 214BBD960 for ; Mon, 17 Jul 2023 11:03:10 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EC922B2CF for ; Mon, 17 Jul 2023 11:03:09 +0200 (CEST) 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 ; Mon, 17 Jul 2023 11:03:09 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id DB6AA426EC; Mon, 17 Jul 2023 11:03:08 +0200 (CEST) Message-ID: Date: Mon, 17 Jul 2023 11:03:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Christoph Heiss , Proxmox VE development discussion References: <20230703145116.257861-1-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 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] [PATCH manager v2] fix #4758: ui: lxc wizard: allow multiple ssh keys 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: Mon, 17 Jul 2023 09:03:10 -0000 On 7/13/23 12:09, Christoph Heiss wrote: > > On Mon, Jul 03, 2023 at 04:51:16PM +0200, Dominik Csapak wrote: >> by converting the textfield into a textarea and validate the value >> line wise (if there is more than one line) >> >> also create a 'MultiFileButton' (mostly copied from extjs) that allows >> to select multiple files at once >> > > Some small nits inline, but otherwise: > > Tested-by: Christoph Heiss >> Signed-off-by: Dominik Csapak >> --- >> changes from v2: >> * reduces lines of code (Thanks @Thomas) >> * implemented multi file select >> >> www/manager6/Makefile | 1 + >> www/manager6/form/MultiFileButton.js | 58 ++++++++++++++++++++++++++++ >> www/manager6/lxc/CreateWizard.js | 19 +++++---- >> 3 files changed, 71 insertions(+), 7 deletions(-) >> create mode 100644 www/manager6/form/MultiFileButton.js >> >> [..] >> diff --git a/www/manager6/form/MultiFileButton.js b/www/manager6/form/MultiFileButton.js >> new file mode 100644 >> index 00000000..a73662f2 >> --- /dev/null >> +++ b/www/manager6/form/MultiFileButton.js >> @@ -0,0 +1,58 @@ >> +// mostly copied from ExtJS FileButton, but added 'multiple' at the relevant >> +// places so we have a file picker where one can select multiple files >> +Ext.define('PVE.form.MultiFileButton', { >> + extend: 'Ext.form.field.FileButton', >> + alias: 'widget.pveMultiFileButton', >> + >> + afterTpl: [ >> + '> + 'type="file" size="1" name="{inputName}" unselectable="on" multiple ', >> + 'accept="{accept}"', >> + 'tabindex="{tabIndex}"', >> + '>', >> + ], >> + >> + createFileInput: function(isTemporary) { >> + var me = this, > nit: s/var/let/ > this... >> + fileInputEl, listeners; >> + >> + fileInputEl = me.fileInputEl = me.el.createChild({ > nit: Is `me.fileInputEl` used somewhere I'm not seeing? Otherwise, could > be eliminated and just be `let fileInputEl = ..`. > ..this.. >> + name: me.inputName || me.id, >> + multiple: true, >> + id: !isTemporary ? me.id + '-fileInputEl' : undefined, >> + cls: me.inputCls + (me.getInherited().rtl ? ' ' + Ext.baseCSSPrefix + 'rtl' : ''), >> + tag: 'input', >> + type: 'file', >> + size: 1, >> + unselectable: 'on', >> + }, me.afterInputGuard); // Nothing special happens outside of IE/Edge >> + >> + // This is our focusEl >> + fileInputEl.dom.setAttribute('data-componentid', me.id); >> + >> + if (me.tabIndex !== null) { >> + me.setTabIndex(me.tabIndex); >> + } >> + >> + if (me.accept) { >> + fileInputEl.dom.setAttribute('accept', me.accept); >> + } >> + >> + // We place focus and blur listeners on fileInputEl to activate Button's >> + // focus and blur style treatment >> + listeners = { > nit: Declare `listeners` here directly instead of above? .. and this is imho not really relevant. i just copied the code from the original extjs code and adapted only a few lines (i could have emphasized this more) for such code we normally don't change much except fix eslint warnings/errors ofc i can improve upon it, but the intention here is to deviate as little as possible from extjs code > >> + scope: me, >> + change: me.fireChange, >> + mousedown: me.handlePrompt, >> + keydown: me.handlePrompt, >> + focus: me.onFileFocus, >> + blur: me.onFileBlur, >> + }; >> + >> + if (me.useTabGuards) { >> + listeners.keydown = me.onFileInputKeydown; >> + } >> + >> + fileInputEl.on(listeners); >> + }, >> +}); >> diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js >> index 0b82cc1c..7cc59299 100644 >> --- a/www/manager6/lxc/CreateWizard.js >> +++ b/www/manager6/lxc/CreateWizard.js >> @@ -120,16 +120,16 @@ Ext.define('PVE.lxc.CreateWizard', { >> }, >> }, >> { >> - xtype: 'proxmoxtextfield', >> + xtype: 'textarea', >> name: 'ssh-public-keys', >> value: '', >> - fieldLabel: gettext('SSH public key'), >> + fieldLabel: gettext('SSH public key(s)'), >> allowBlank: true, >> validator: function(value) { >> let pwfield = this.up().down('field[name=password]'); >> if (value.length) { >> - let key = PVE.Parser.parseSSHKey(value); >> - if (!key) { >> + let keys = value.indexOf('\n') !== -1 ? value.split('\n') : [ value ]; > nit: eslint complains here, should be `[value]` oops, did not see that > also: s/let/const/ > hmm... we don't really have a style recommendation which to prefer. maybe we should improve our style guideline to have some more hints when to use const and when to use let? (even if it's just 'use const whenever possible') is there a strong reason here to use const vs let? (is there in the general case, besides the immutable nature of the variable?) >> + if (keys.some(key => key === '' || !PVE.Parser.parseSSHKey(key))) { >> return "Failed to recognize ssh key"; >> } >> pwfield.allowBlank = true; >> @@ -159,15 +159,20 @@ Ext.define('PVE.lxc.CreateWizard', { >> }, >> }, >> { >> - xtype: 'filebutton', >> + xtype: 'pveMultiFileButton', >> name: 'file', >> hidden: !window.FileReader, >> text: gettext('Load SSH Key File'), >> listeners: { >> change: function(btn, e, value) { >> e = e.event; >> - let field = this.up().down('proxmoxtextfield[name=ssh-public-keys]'); >> - PVE.Utils.loadSSHKeyFromFile(e.target.files[0], v => field.setValue(v)); >> + let field = this.up().down('textarea[name=ssh-public-keys]'); >> + for (const file of e?.target?.files ?? []) { >> + PVE.Utils.loadSSHKeyFromFile(file, v => { >> + let oldValue = field.getValue(); > nit: s/let/const/ > as above >> + field.setValue(oldValue ? `${oldValue}\n${v.trim()}` : v.trim()); >> + }); >> + } >> btn.reset(); >> }, >> }, >> -- >> 2.30.2 >> >> >> >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >> >>