ui: pci/usb mapping: rework mapping panel for better user experience

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 <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index 3af642553d055e08da20d53e51821f5ac8072fdd..edae462b001f8daa8c3c1f0e0f22c29569c10d2d 100644 (file)
--- 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 027170422d61957d30150526c47f94409df5a42a..4c4769093e3934e12f0ab9a3d716e7be3e39fa5e 100644 (file)
--- 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 `<i class="fa ${iconCls}"></i> ${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 2b26871999c9bb9f48fa86460df193048846b90e..d43f04eb2f7e2468cbc08955904804ac98f5c996 100644 (file)
--- 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 f36f1d034b7fe9f060715535a937b3c85b0f9aeb..358f0778bb57e18b7a4b9fbc29669345820a442b 100644 (file)
--- 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}',
+                   },
                },
            ],
        },