Re: [pve-devel] [PATCH manager v2] fix #4758: ui: lxc wizard: allow multiple ssh keys

[Christoph Heiss <c.heiss@proxmox.com>]

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 <c.heiss@proxmox.com>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> 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: [
> +	'<input id="{id}-fileInputEl" data-ref="fileInputEl" class="{childElCls} {inputCls}" ',
> +	    'type="file" size="1" name="{inputName}" unselectable="on" multiple ',
> +	    '<tpl if="accept != null">accept="{accept}"</tpl>',
> +	    '<tpl if="tabIndex != null">tabindex="{tabIndex}"</tpl>',
> +	'>',
> +    ],
> +
> +    createFileInput: function(isTemporary) {
> +	var me = this,
nit: s/var/let/

> +	    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 = ..`.

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

> +	    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]`
also: s/let/const/

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

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