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 6AEC19186C for ; Thu, 4 Apr 2024 12:02:33 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 545323695B for ; Thu, 4 Apr 2024 12:02:33 +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 ; Thu, 4 Apr 2024 12:02:31 +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 3619045222 for ; Thu, 4 Apr 2024 12:02:31 +0200 (CEST) Message-ID: <8283a1c8-bb35-4421-b4dc-09a5f4f45fb3@proxmox.com> Date: Thu, 4 Apr 2024 12:02:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , Markus Frank References: <20231121123958.198675-1-m.frank@proxmox.com> Content-Language: en-GB, de-AT From: Thomas Lamprecht Autocrypt: addr=t.lamprecht@proxmox.com; keydata= xsFNBFsLjcYBEACsaQP6uTtw/xHTUCKF4VD4/Wfg7gGn47+OfCKJQAD+Oyb3HSBkjclopC5J uXsB1vVOfqVYE6PO8FlD2L5nxgT3SWkc6Ka634G/yGDU3ZC3C/7NcDVKhSBI5E0ww4Qj8s9w OQRloemb5LOBkJNEUshkWRTHHOmk6QqFB/qBPW2COpAx6oyxVUvBCgm/1S0dAZ9gfkvpqFSD 90B5j3bL6i9FIv3YGUCgz6Ue3f7u+HsEAew6TMtlt90XV3vT4M2IOuECG/pXwTy7NtmHaBQ7 UJBcwSOpDEweNob50+9B4KbnVn1ydx+K6UnEcGDvUWBkREccvuExvupYYYQ5dIhRFf3fkS4+ wMlyAFh8PQUgauod+vqs45FJaSgTqIALSBsEHKEs6IoTXtnnpbhu3p6XBin4hunwoBFiyYt6 YHLAM1yLfCyX510DFzX/Ze2hLqatqzY5Wa7NIXqYYelz7tXiuCLHP84+sV6JtEkeSUCuOiUY virj6nT/nJK8m0BzdR6FgGtNxp7RVXFRz/+mwijJVLpFsyG1i0Hmv2zTn3h2nyGK/I6yhFNt dX69y5hbo6LAsRjLUvZeHXpTU4TrpN/WiCjJblbj5um5eEr4yhcwhVmG102puTtuCECsDucZ jpKpUqzXlpLbzG/dp9dXFH3MivvfuaHrg3MtjXY1i+/Oxyp5iwARAQABzTNUaG9tYXMgTGFt cHJlY2h0IChBdXRoLTQpIDx0LmxhbXByZWNodEBwcm94bW94LmNvbT7CwY4EEwEIADgWIQQO R4qbEl/pah9K6VrTZCM6gDZWBgUCWwuNxgIbAwULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAK CRDTZCM6gDZWBm/jD/4+6JB2s67eaqoP6x9VGaXNGJPCscwzLuxDTCG90G9FYu29VcXtubH/ bPwsyBbNUQpqTm/s4XboU2qpS5ykCuTjqavrcP33tdkYfGcItj2xMipJ1i3TWvpikQVsX42R G64wovLs/dvpTYphRZkg5DwhgTmy3mRkmofFCTa+//MOcNOORltemp984tWjpR3bUJETNWpF sKGZHa3N4kCNxb7A+VMsJZ/1gN3jbQbQG7GkJtnHlWkw9rKCYqBtWrnrHa4UAvSa9M/XCIAB FThFGqZI1ojdVlv5gd6b/nWxfOPrLlSxbUo5FZ1i/ycj7/24nznW1V4ykG9iUld4uYUY86bB UGSjew1KYp9FmvKiwEoB+zxNnuEQfS7/Bj1X9nxizgweiHIyFsRqgogTvLh403QMSGNSoArk tqkorf1U+VhEncIn4H3KksJF0njZKfilrieOO7Vuot1xKr9QnYrZzJ7m7ZxJ/JfKGaRHXkE1 feMmrvZD1AtdUATZkoeQtTOpMu4r6IQRfSdwm/CkppZXfDe50DJxAMDWwfK2rr2bVkNg/yZI tKLBS0YgRTIynkvv0h8d9dIjiicw3RMeYXyqOnSWVva2r+tl+JBaenr8YTQw0zARrhC0mttu cIZGnVEvQuDwib57QLqMjQaC1gazKHvhA15H5MNxUhwm229UmdH3KM7BTQRbC43GARAAyTkR D6KRJ9Xa2fVMh+6f186q0M3ni+5tsaVhUiykxjsPgkuWXWW9MbLpYXkzX6h/RIEKlo2BGA95 QwG5+Ya2Bo3g7FGJHAkXY6loq7DgMp5/TVQ8phsSv3WxPTJLCBq6vNBamp5hda4cfXFUymsy HsJy4dtgkrPQ/bnsdFDCRUuhJHopnAzKHN8APXpKU6xV5e3GE4LwFsDhNHfH/m9+2yO/trcD txSFpyftbK2gaMERHgA8SKkzRhiwRTt9w5idOfpJVkYRsgvuSGZ0pcD4kLCOIFrer5xXudk6 NgJc36XkFRMnwqrL/bB4k6Pi2u5leyqcXSLyBgeHsZJxg6Lcr2LZ35+8RQGPOw9C0ItmRjtY ZpGKPlSxjxA1WHT2YlF9CEt3nx7c4C3thHHtqBra6BGPyW8rvtq4zRqZRLPmZ0kt/kiMPhTM 8wZAlObbATVrUMcZ/uNjRv2vU9O5aTAD9E5r1B0dlqKgxyoImUWB0JgpILADaT3VybDd3C8X s6Jt8MytUP+1cEWt9VKo4vY4Jh5vwrJUDLJvzpN+TsYCZPNVj18+jf9uGRaoK6W++DdMAr5l gQiwsNgf9372dbMI7pt2gnT5/YdG+ZHnIIlXC6OUonA1Ro/Itg90Q7iQySnKKkqqnWVc+qO9 GJbzcGykxD6EQtCSlurt3/5IXTA7t6sAEQEAAcLBdgQYAQgAIBYhBA5HipsSX+lqH0rpWtNk IzqANlYGBQJbC43GAhsMAAoJENNkIzqANlYGD1sP/ikKgHgcspEKqDED9gQrTBvipH85si0j /Jwu/tBtnYjLgKLh2cjv1JkgYYjb3DyZa1pLsIv6rGnPX9bH9IN03nqirC/Q1Y1lnbNTynPk IflgvsJjoTNZjgu1wUdQlBgL/JhUp1sIYID11jZphgzfDgp/E6ve/8xE2HMAnf4zAfJaKgD0 F+fL1DlcdYUditAiYEuN40Ns/abKs8I1MYx7Yglu3RzJfBzV4t86DAR+OvuF9v188WrFwXCS RSf4DmJ8tntyNej+DVGUnmKHupLQJO7uqCKB/1HLlMKc5G3GLoGqJliHjUHUAXNzinlpE2Vj C78pxpwxRNg2ilE3AhPoAXrY5qED5PLE9sLnmQ9AzRcMMJUXjTNEDxEYbF55SdGBHHOAcZtA kEQKub86e+GHA+Z8oXQSGeSGOkqHi7zfgW1UexddTvaRwE6AyZ6FxTApm8wq8NT2cryWPWTF BDSGB3ujWHMM8ERRYJPcBSjTvt0GcEqnd+OSGgxTkGOdufn51oz82zfpVo1t+J/FNz6MRMcg 8nEC+uKvgzH1nujxJ5pRCBOquFZaGn/p71Yr0oVitkttLKblFsqwa+10Lt6HBxm+2+VLp4Ja 0WZNncZciz3V3cuArpan/ZhhyiWYV5FD0pOXPCJIx7WS9PTtxiv0AOS4ScWEUmBxyhFeOpYa DrEx In-Reply-To: <20231121123958.198675-1-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.058 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 Subject: Re: [pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options 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, 04 Apr 2024 10:02:33 -0000 subject should be more in the style of: ui: qemu: add clipboard selector to options Am 21/11/2023 um 13:39 schrieb Markus Frank: > For SPICE and VNC, a different message is displayed. > possibly reference the backend work here, > Save config in DisplayEdit so that the clipboard setting persist. this sentence is not really that self-explaining, maybe extend on the reasoning here. > > Signed-off-by: Markus Frank > Reviewed-by: Dominik Csapak > Tested-by: Dominik Csapak > --- > v15: > * changed style of line break in vncHint field > > www/manager6/qemu/DisplayEdit.js | 8 ++++ > www/manager6/qemu/Options.js | 80 ++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+) > > diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js > index 7b112400..53d0beac 100644 > --- a/www/manager6/qemu/Options.js > +++ b/www/manager6/qemu/Options.js > @@ -154,6 +154,86 @@ Ext.define('PVE.qemu.Options', { > }, > } : undefined, > }, > + vga: { > + header: gettext('Clipboard'), > + defaultValue: false, > + renderer: function(value) { > + let vga = PVE.Parser.parsePropertyString(value, 'type'); > + if (vga.clipboard) { > + return vga.clipboard.toUpperCase(); > + } else { > + return Proxmox.Utils.defaultText + ' (SPICE)'; > + } > + }, > + editor: caps.vms['VM.Config.HWType'] ? { > + xtype: 'proxmoxWindowEdit', > + subject: gettext('Clipboard'), > + onlineHelp: 'qm_display', > + items: { > + xtype: 'pveDisplayInputPanel', > + referenceHolder: true, It might be avoidable to make this a referenceHolder by using a viewModel, see below. > + items: [ > + { > + xtype: 'proxmoxKVComboBox', > + name: 'clipboard', > + reference: 'clipboard', do we really need a reference here? I see no use of it (i.e., through lookup) > + itemId: 'clipboardBox', the itemId seems also unused, and those have to be globally unique, so best to be avoided if unsure. > + fieldLabel: gettext('Clipboard'), > + deleteDefaultValue: true, > + listeners: { > + change: function(field, value) { > + let inputpanel = field.up("inputpanel"); > + let isVnc = value === 'vnc'; > + inputpanel.lookup('vncHint').setVisible(isVnc); > + inputpanel.lookup('defaultHint').setVisible(!isVnc); > + }, a viewModel would be nicer to do this, e.g at the input panel level add: viewModel: { data: { clipboard: '__default__', }, formulas: { isVnc: get => get('clipboard') === 'vnc', }, }, Then in the clipboard field bind the value to the viewModel: bind: { value: {clipboard}, }, and in the hint fields bind the visibility state to the formula: bind: { hidden: {isVNC}, // or reverse: hidden: {!isVNC}, }, > + }, > + value: '__default__', > + comboItems: [ > + ['__default__', Proxmox.Utils.defaultText + ' (SPICE)'], > + ['vnc', 'VNC'], > + ], > + }, > + { > + itemId: 'vncHint', > + name: 'vncHint', > + reference: 'vncHint', view the viewModel you would avoid the explicit references and let ExtJS built-in logic handle this. > + xtype: 'displayfield', the xtype should be the first property in item object definitions. immediately followed by the name. > + userCls: 'pmx-hint', > + hidden: true, > + value: gettext('You cannot use the default SPICE clipboard if the VNC Clipboard is selected.') + ' ' + > + gettext('VNC Clipboard requires spice-tools installed in the Guest-VM.'), > + }, > + { > + itemId: 'defaultHint', > + name: 'defaultHint', > + reference: 'defaultHint', same here w.r.t. reference avoidance by using a viewModel. > + xtype: 'displayfield', same here with xtype definition having to move up. > + userCls: 'pmx-hint', > + hidden: false, > + value: gettext('This option depends on your display type.') + ' ' + > + gettext('If the display type uses SPICE you are able to use the default SPICE Clipboard.'), Those hints are IMO a good indicator that it would be more fitting to have in the advanced options of the display hardware. As while it might be slightly more discoverable here, having to jump between hardware and options to bring this in sync is a bit of a nuisance, and docs can improve discoverability wherever it is. That said, I have no hard feelings for this, there are some arguments for either, I'd say just decide on your own. > + }, > + ], > + onGetValues: function(values) { > + values = Ext.apply(this.originalConfig, values); > + if (values.delete === "clipboard") { > + delete values.clipboard; > + delete values.delete; > + } > + let ret = PVE.Parser.printPropertyString(values, 'type'); > + if (ret === "") { > + return { 'delete': "vga" }; > + } > + return { vga: ret }; > + }, > + onSetValues: function(values) { > + this.originalConfig = PVE.Parser.parsePropertyString(values.vga, 'type'); > + return this.originalConfig; > + }, > + }, > + } : undefined, > + }, > hotplug: { > header: gettext('Hotplug'), > defaultValue: 'disk,network,usb',