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 2A39180EE for ; Thu, 2 Mar 2023 11:28:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 10B1F2768 for ; Thu, 2 Mar 2023 11:28:23 +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 ; Thu, 2 Mar 2023 11:28:22 +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 DE5DE420AD for ; Thu, 2 Mar 2023 11:28:21 +0100 (CET) Message-ID: <0cad6253-103e-3c26-2138-30c3772af644@proxmox.com> Date: Thu, 2 Mar 2023 11:28:20 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:110.0) Gecko/20100101 Thunderbird/110.0 Content-Language: en-US To: pve-devel@lists.proxmox.com References: <20221028123322.93142-1-m.frank@proxmox.com> <20221028123322.93142-6-m.frank@proxmox.com> From: Dominik Csapak In-Reply-To: <20221028123322.93142-6-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.106 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.09 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 v3 5/6] added clipboard checkbox & combobox to DisplayEdit 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: Thu, 02 Mar 2023 10:28:23 -0000 high level comments: i'd only show the hint when the checkbox is checked or 'novnc' is selected in the combobox also here it shows that the options does not really fit to the display at all, at least i would not search the setting for the console clipboard under the virtual display hardware... (we could just have the gui seperate on the 'options' tab, and leave the option itself on the display option...) comments inline On 10/28/22 14:33, Markus Frank wrote: > If display is set to spice the checkbox gets replaced by a combobox to > show the available clipboard options. > > Signed-off-by: Markus Frank > --- > www/manager6/qemu/DisplayEdit.js | 62 +++++++++++++++++++++++++++++++- > 1 file changed, 61 insertions(+), 1 deletion(-) > > diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js > index 9bb1763e..2cef5ad2 100644 > --- a/www/manager6/qemu/DisplayEdit.js > +++ b/www/manager6/qemu/DisplayEdit.js > @@ -33,22 +33,54 @@ Ext.define('PVE.qemu.DisplayInputPanel', { > return; > } > let memoryfield = this.up('panel').down('field[name=memory]'); > + let clipboardBox = this.up('panel').down('field[itemId=clipboardBox]'); > + let clipboardDrop = this.up('panel').down('field[itemId=clipboardDrop]'); > + let vdagentHint = this.up('panel').down('field[name=vdagentHint]'); at this point, i'd really prefer to either pull the 'panel' into a variable only once or even rewrite the component using a controller and using references it's not a huge performance issue, but the it makes it much less readable also clipboardDrop could be better written as clipboardDropDown > let disableMemoryField = false; > + let spice = false; > + let showClipboardAndHint = true; > > if (val === "cirrus") { > memoryfield.setEmptyText("4"); > - } else if (val === "std" || val.match(/^qxl\d?$/) || val === "vmware") { > + } else if (val === "std" || val === "vmware") { > memoryfield.setEmptyText("16"); > + } else if (val.match(/^qxl\d?$/)) { > + memoryfield.setEmptyText("16"); > + spice = true; > } else if (val.match(/^virtio/)) { > memoryfield.setEmptyText("256"); > + spice = true; > } else if (val.match(/^(serial\d|none)$/)) { > memoryfield.setEmptyText("N/A"); > disableMemoryField = true; > + showClipboardAndHint = false; > } else { > console.debug("unexpected display type", val); > memoryfield.setEmptyText(Proxmox.Utils.defaultText); > } > memoryfield.setDisabled(disableMemoryField); > + vdagentHint.setVisible(showClipboardAndHint); > + if (showClipboardAndHint) { > + // switch from Checkbox to ComboBox and vice versa > + clipboardBox.setDisabled(spice); > + clipboardDrop.setDisabled(!spice); > + clipboardBox.setVisible(!spice); > + clipboardDrop.setVisible(spice); > + // reset value when changing to spice, > + // so that you have to actively change to noVNC Clipboard > + if (spice) { > + clipboardDrop.setValue('__default__'); > + } > + } else { > + // reset to default > + clipboardBox.setValue(false); > + clipboardDrop.setValue('__default__'); > + // show only the disabled Checkbox > + clipboardBox.setDisabled(true); > + clipboardDrop.setDisabled(true); > + clipboardBox.setVisible(true); > + clipboardDrop.setVisible(false); > + } i find that whole section a bit too complicated, it took me a few glances to see what it's actually doing i'd propose a different approach: use a 'showClipboardBox' and a 'showClipboardDropDown' variable to track which you want to show and then only do a clipboardBox.setDisabled(!showClipboardBox); clipboardBox.setVisible(!showClipboardBox); etc. > }, > }, > }, > @@ -60,6 +92,34 @@ Ext.define('PVE.qemu.DisplayInputPanel', { > maxValue: 512, > step: 4, > name: 'memory', > + }, > + { > + xtype: 'proxmoxcheckbox', > + fieldLabel: gettext('noVNC Clipboard'), > + name: 'clipboard', > + itemId: 'clipboardBox', > + }, > + { > + name: 'clipboard', > + itemId: 'clipboardDrop', > + xtype: 'proxmoxKVComboBox', > + value: '__default__', > + deleteEmpty: false, > + fieldLabel: gettext('Clipboard'), > + comboItems: [ > + ['__default__', 'SPICE-Clipboard'], > + ['1', 'noVNC-Clipboard'], > + ], > + disabled: true, > + hidden: true, > + }, > + { > + itemId: 'vdagentHint', > + name: 'vdagentHint', > + xtype: 'displayfield', > + userCls: 'pmx-hint', > + value: 'Clipboard for noVNC requires spice-tools installed and ' + > + 'enabled in the Guest-VM.', > }], > }); >