Am 19.06.23 um 16:13 schrieb Dominik Csapak: > 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
I agree with Aaron that the actioncolumn is too far right at the moment. > 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 This one feels slightly surprising to me from a user perspective as I can't edit the actual mapping here. But it is cleaner and I guess one could argue in the opposite direction too. > * add new host mapping > - shows nodeselector, mapping and mdev, but mdev is 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.csa...@proxmox.com> > --- > this is not intended to be applied as is, rather i'd like some feedback > on the approach (@thomas, @aaron ?) so that if we want to do it this way > i can also do it for the usb mappings > > the other approach mentioned off-list can still be done > (having a full grid with all mappings regardless of the node) > maybe only for usb devices (there it makes imho more sense) but then > we'd have two interfaces for the mappings instead of one It does involve a bit of clicking when it's only possible to add one node entry at a time, but I'm not generally opposed to the current RFC. I can image the action column takes a bit of getting used to as a Proxmox VE user, because we don't really have those there yet. The full grid might become quite big/confusing and involve lots of scrolling or how would the grouping by node be done? Maybe a third alternative would be to have a tab for each node and show basic meta-info like how many devices are already selected on that node and a warn/error indicator if that node is affected? Would the full grid and tabs approach even be feasible with many nodes or require too many API calls? > > www/manager6/tree/ResourceMapTree.js | 166 ++++++++++++++++----------- > www/manager6/window/PCIMapEdit.js | 42 ++++--- > 2 files changed, 130 insertions(+), 78 deletions(-) > > diff --git a/www/manager6/tree/ResourceMapTree.js > b/www/manager6/tree/ResourceMapTree.js > index 02717042..cd24923e 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') { AFAICT, this always evaluates to false, because of the misplaced '!'. > + return; > + } > + > + me.openMapEditWindow(rec.data.name); > }, > (...) > @@ -254,63 +299,56 @@ Ext.define('PVE.tree.ResourceMapTree', { > > tbar: [ > { > - text: gettext('Add mapping'), > + text: gettext('Add'), IMHO, Add mapping was/is better > handler: 'addMapping', > cbind: { > disabled: '{!canConfigure}', > }, > }, (...) > diff --git a/www/manager6/window/PCIMapEdit.js > b/www/manager6/window/PCIMapEdit.js > index 0da2bae7..a0b42758 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, Nit: Is it even necessary to return these two as they are already persistent properties? _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel