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 828A3821A for ; Wed, 21 Jun 2023 09:41:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 695AE1969B for ; Wed, 21 Jun 2023 09:41:46 +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 ; Wed, 21 Jun 2023 09:41:44 +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 98F5642356 for ; Wed, 21 Jun 2023 09:41:44 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Wed, 21 Jun 2023 09:41:42 +0200 Message-Id: <20230621074142.742461-5-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230621074142.742461-1-d.csapak@proxmox.com> References: <20230621074142.742461-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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: [pve-devel] [PATCH manager v2 5/5] ui: pci/usb mapping: rework mapping panel for better user experience 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, 21 Jun 2023 07:41:46 -0000 by removing the confusing buttons in the toolbar and adding them as actions in an actioncolumn. There a only relevant actions are visible and get a more expressive tooltip with this, we now differentiate between 4 modes of the edit window: * create a new mapping altogether - shows all fields * edit existing mapping on top level - show only 'global' fields (comment, mdev), so no mappings * add new host mapping - shows nodeselector, mapping (and mdev, but disabled) (informational only) * edit existing host mapping - show selected node (displayfield) mdev and mappings, but only mappings are editable we have to split the nodeselector into two fields, since the disabling cbind does not pass through to the editconfig (and thus makes the form invalid if we try that) Signed-off-by: Dominik Csapak --- changes from rfc: * for usb mappings too, so they are consistent * moved the action column to second place * hide/disable add button when there are mappings for every node * use a bit different class to hide the buttons, because otherwise the grid cells would have the wrong height (since they would not get the styling of the font-awesome icon) www/css/ext6-pve.css | 5 + www/manager6/tree/ResourceMapTree.js | 178 +++++++++++++++++---------- www/manager6/window/PCIMapEdit.js | 40 ++++-- www/manager6/window/USBMapEdit.js | 49 ++++++-- 4 files changed, 183 insertions(+), 89 deletions(-) diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css index 3af64255..edae462b 100644 --- a/www/css/ext6-pve.css +++ b/www/css/ext6-pve.css @@ -704,3 +704,8 @@ table.osds td:first-of-type { .x-grid-item .x-item-disabled { opacity: 0.3; } + +.pmx-action-hidden:before { + opacity: 0.0; + cursor: default; +} diff --git a/www/manager6/tree/ResourceMapTree.js b/www/manager6/tree/ResourceMapTree.js index 02717042..4c476909 100644 --- a/www/manager6/tree/ResourceMapTree.js +++ b/www/manager6/tree/ResourceMapTree.js @@ -49,44 +49,89 @@ Ext.define('PVE.tree.ResourceMapTree', { }); }, - addHost: function() { + add: function(_grid, _rI, _cI, _item, _e, rec) { let me = this; - me.edit(false); + if (rec.data.type !== 'entry') { + return; + } + + me.openMapEditWindow(rec.data.name); }, - edit: function(includeNodename = true) { + editDblClick: function() { let me = this; let view = me.getView(); let selection = view.getSelection(); - if (!selection || !selection.length) { + if (!selection || selection.length < 1) { return; } - let rec = selection[0]; - if (!view.canConfigure || (rec.data.type === 'entry' && includeNodename)) { + + me.edit(selection[0]); + }, + + editAction: function(_grid, _rI, _cI, _item, _e, rec) { + this.edit(rec); + }, + + edit: function(rec) { + let me = this; + if (rec.data.type === 'map') { return; } + me.openMapEditWindow(rec.data.name, rec.data.node, rec.data.type === 'entry'); + }, + + openMapEditWindow: function(name, nodename, entryOnly) { + let me = this; + let view = me.getView(); + Ext.create(view.editWindowClass, { - url: `${view.baseUrl}/${rec.data.name}`, + url: `${view.baseUrl}/${name}`, autoShow: true, autoLoad: true, - nodename: includeNodename ? rec.data.node : undefined, - name: rec.data.name, + entryOnly, + nodename, + name, listeners: { destroy: () => me.load(), }, }); }, - remove: function() { + remove: function(_grid, _rI, _cI, _item, _e, rec) { let me = this; + let msg, id; let view = me.getView(); - let selection = view.getSelection(); - if (!selection || !selection.length) { - return; + let confirmMsg; + switch (rec.data.type) { + case 'entry': + msg = gettext("Are you sure you want to remove '{0}'"); + confirmMsg = Ext.String.format(msg, rec.data.name); + break; + case 'node': + msg = gettext("Are you sure you want to remove '{0}' entries for '{1}'"); + confirmMsg = Ext.String.format(msg, rec.data.node, rec.data.name); + break; + case 'map': + msg = gettext("Are you sure you want to remove '{0}' on '{1}' for '{2}'"); + id = rec.data[view.entryIdProperty]; + confirmMsg = Ext.String.format(msg, id, rec.data.node, rec.data.name); + break; + default: + throw "invalid type"; } + Ext.Msg.confirm(gettext('Confirm'), confirmMsg, function(btn) { + if (btn === 'yes') { + me.executeRemove(rec.data); + } + }); + }, + + executeRemove: function(data) { + let me = this; + let view = me.getView(); - let data = selection[0].data; let url = `${view.baseUrl}/${data.name}`; let method = 'PUT'; let params = { @@ -233,6 +278,18 @@ Ext.define('PVE.tree.ResourceMapTree', { return ` ${status}`; }, + getAddClass: function(v, mD, rec) { + let cls = 'fa fa-plus-circle'; + if (rec.data.type !== 'entry' || rec.data.children?.length >= PVE.data.ResourceStore.getNodes().length) { + cls += ' pmx-action-hidden'; + } + return cls; + }, + + isAddDisabled: function(v, r, c, i, rec) { + return rec.data.type !== 'entry' || rec.data.children?.length >= PVE.data.ResourceStore.getNodes().length; + }, + init: function(view) { let me = this; @@ -254,63 +311,56 @@ Ext.define('PVE.tree.ResourceMapTree', { tbar: [ { - text: gettext('Add mapping'), + text: gettext('Add'), handler: 'addMapping', cbind: { disabled: '{!canConfigure}', }, }, - { - xtype: 'proxmoxButton', - text: gettext('New Host mapping'), - disabled: true, - parentXType: 'treepanel', - enableFn: function(_rec) { - return this.up('treepanel').canConfigure; - }, - handler: 'addHost', - }, - { - xtype: 'proxmoxButton', - text: gettext('Edit'), - disabled: true, - parentXType: 'treepanel', - enableFn: function(rec) { - return rec && rec.data.type !== 'entry' && this.up('treepanel').canConfigure; - }, - handler: 'edit', - }, - { - xtype: 'proxmoxButton', - parentXType: 'treepanel', - handler: 'remove', - disabled: true, - text: gettext('Remove'), - enableFn: function(rec) { - return rec && this.up('treepanel').canConfigure; - }, - confirmMsg: function(rec) { - let msg, id; - let view = this.up('treepanel'); - switch (rec.data.type) { - case 'entry': - msg = gettext("Are you sure you want to remove '{0}'"); - return Ext.String.format(msg, rec.data.name); - case 'node': - msg = gettext("Are you sure you want to remove '{0}' entries for '{1}'"); - return Ext.String.format(msg, rec.data.node, rec.data.name); - case 'map': - msg = gettext("Are you sure you want to remove '{0}' on '{1}' for '{2}'"); - id = rec.data[view.entryIdProperty]; - return Ext.String.format(msg, id, rec.data.node, rec.data.name); - default: - throw "invalid type"; - } - }, - }, ], listeners: { - itemdblclick: 'edit', + itemdblclick: 'editDblClick', + }, + + initComponent: function() { + let me = this; + + let columns = [...me.columns]; + columns.splice(1, 0, { + xtype: 'actioncolumn', + text: gettext('Actions'), + width: 80, + items: [ + { + getTip: (v, m, { data }) => + Ext.String.format(gettext("Add new host mapping for '{0}'"), data.name), + getClass: 'getAddClass', + isActionDisabled: 'isAddDisabled', + handler: 'add', + }, + { + iconCls: 'fa fa-pencil', + getTip: (v, m, { data }) => data.type === 'entry' + ? Ext.String.format(gettext("Edit Mapping '{0}'"), data.name) + : Ext.String.format(gettext("Edit Mapping '{0}' for '{1}'"), data.name, data.node), + getClass: (v, m, { data }) => data.type !== 'map' ? 'fa fa-pencil' : 'pmx-hidden', + isActionDisabled: (v, r, c, i, rec) => rec.data.type === 'map', + handler: 'editAction', + }, + { + iconCls: 'fa fa-trash-o', + getTip: (v, m, { data }) => data.type === 'entry' + ? Ext.String.format(gettext("Remove '{0}'"), data.name) + : data.type === 'node' + ? Ext.String.format(gettext("Remove mapping for '{0}'"), data.node) + : Ext.String.format(gettext("Remove mapping '{0}'"), data.path), + handler: 'remove', + }, + ], + }); + me.columns = columns; + + me.callParent(); }, }); diff --git a/www/manager6/window/PCIMapEdit.js b/www/manager6/window/PCIMapEdit.js index 2b268719..d43f04eb 100644 --- a/www/manager6/window/PCIMapEdit.js +++ b/www/manager6/window/PCIMapEdit.js @@ -13,8 +13,12 @@ Ext.define('PVE.window.PCIMapEditWindow', { cbindData: function(initialConfig) { let me = this; - me.isCreate = !me.name || !me.nodename; + me.isCreate = (!me.name || !me.nodename) && !me.entryOnly; me.method = me.name ? 'PUT' : 'POST'; + me.hideMapping = !!me.entryOnly; + me.hideComment = me.name && !me.entryOnly; + me.hideNodeSelector = me.nodename || me.entryOnly; + me.hideNode = !me.nodename || !me.hideNodeSelector; return { name: me.name, nodename: me.nodename, @@ -201,35 +205,41 @@ Ext.define('PVE.window.PCIMapEditWindow', { allowBlank: false, }, { - xtype: 'pmxDisplayEditField', + xtype: 'displayfield', fieldLabel: gettext('Mapping on Node'), labelWidth: 120, name: 'node', - editConfig: { - xtype: 'pveNodeSelector', - reference: 'nodeselector', - }, cbind: { - editable: '{!nodename}', value: '{nodename}', + disabled: '{hideNode}', + hidden: '{hideNode}', + }, + allowBlank: false, + }, + { + xtype: 'pveNodeSelector', + reference: 'nodeselector', + fieldLabel: gettext('Mapping on Node'), + labelWidth: 120, + name: 'node', + cbind: { + disabled: '{hideNodeSelector}', + hidden: '{hideNodeSelector}', }, allowBlank: false, }, ], column2: [ - { - // as spacer - xtype: 'displayfield', - }, { xtype: 'proxmoxcheckbox', - fieldLabel: gettext('Mediated Devices'), - labelWidth: 120, + fieldLabel: gettext('Use with Mediated Devices'), + labelWidth: 200, reference: 'mdev', name: 'mdev', cbind: { deleteEmpty: '{!isCreate}', + disabled: '{hideComment}', }, }, ], @@ -244,6 +254,8 @@ Ext.define('PVE.window.PCIMapEditWindow', { name: 'map', cbind: { nodename: '{nodename}', + disabled: '{hideMapping}', + hidden: '{hideMapping}', }, allowBlank: false, onLoadCallBack: 'checkIommu', @@ -257,6 +269,8 @@ Ext.define('PVE.window.PCIMapEditWindow', { name: 'description', cbind: { deleteEmpty: '{!isCreate}', + disabled: '{hideComment}', + hidden: '{hideComment}', }, }, ], diff --git a/www/manager6/window/USBMapEdit.js b/www/manager6/window/USBMapEdit.js index f36f1d03..358f0778 100644 --- a/www/manager6/window/USBMapEdit.js +++ b/www/manager6/window/USBMapEdit.js @@ -7,6 +7,10 @@ Ext.define('PVE.window.USBMapEditWindow', { let me = this; me.isCreate = !me.name; me.method = me.isCreate ? 'POST' : 'PUT'; + me.hideMapping = !!me.entryOnly; + me.hideComment = me.name && !me.entryOnly; + me.hideNodeSelector = me.nodename || me.entryOnly; + me.hideNode = !me.nodename || !me.hideNodeSelector; return { name: me.name, nodename: me.nodename, @@ -53,12 +57,14 @@ Ext.define('PVE.window.USBMapEditWindow', { if (me.originalMap) { map = PVE.Parser.filterPropertyStringList(me.originalMap, (e) => e.node !== values.node); } - map.push(PVE.Parser.printPropertyString(values)); + if (values.id) { + map.push(PVE.Parser.printPropertyString(values)); + } - values = { - map, - description, - }; + values = { map }; + if (description) { + values.description = description; + } if (view.isCreate) { values.id = name; @@ -143,16 +149,26 @@ Ext.define('PVE.window.USBMapEditWindow', { allowBlank: false, }, { - xtype: 'pmxDisplayEditField', - fieldLabel: gettext('Node'), + xtype: 'displayfield', + fieldLabel: gettext('Mapping on Node'), + labelWidth: 120, name: 'node', - editConfig: { - xtype: 'pveNodeSelector', - reference: 'nodeselector', - }, cbind: { - editable: '{!nodename}', value: '{nodename}', + disabled: '{hideNode}', + hidden: '{hideNode}', + }, + allowBlank: false, + }, + { + xtype: 'pveNodeSelector', + reference: 'nodeselector', + fieldLabel: gettext('Mapping on Node'), + labelWidth: 120, + name: 'node', + cbind: { + disabled: '{hideNodeSelector}', + hidden: '{hideNodeSelector}', }, allowBlank: false, }, @@ -163,6 +179,10 @@ Ext.define('PVE.window.USBMapEditWindow', { xtype: 'fieldcontainer', defaultType: 'radiofield', layout: 'fit', + cbind: { + disabled: '{hideMapping}', + hidden: '{hideMapping}', + }, items: [ { name: 'usb', @@ -178,6 +198,7 @@ Ext.define('PVE.window.USBMapEditWindow', { name: 'id', cbind: { nodename: '{nodename}', + disabled: '{hideMapping}', }, editable: true, allowBlank: false, @@ -214,6 +235,10 @@ Ext.define('PVE.window.USBMapEditWindow', { fieldLabel: gettext('Comment'), submitValue: true, name: 'description', + cbind: { + disabled: '{hideComment}', + hidden: '{hideComment}', + }, }, ], }, -- 2.30.2