[pve-devel] [PATCH docs/wt/manager v2] implement tagview

[pve-devel] [PATCH docs/wt/manager v2] implement tagview

[Dominik Csapak]

this adds a 'tagview' to the web ui, organizing guests by their tags
(for details see the pve-manager patch)

changes from v1:
* rebase on master
* adapt to recent tooltip changes
* add a comment to TagConfig class to better explain what it does

pve-docs:

Dominik Csapak (1):
  gui: add anchor for tags chapter

 pve-gui.adoc | 1 +
 1 file changed, 1 insertion(+)

proxmox-widget-toolkit:

Dominik Csapak (1):
  css: add some conditions to the tag classes for the tag view

 src/css/ext6-pmx.css | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

pve-manager:

Dominik Csapak (1):
  ui: implement 'Tag View' for the resource tree

 www/manager6/Makefile             |  1 +
 www/manager6/Workspace.js         |  3 +-
 www/manager6/form/ViewSelector.js | 32 ++++++++++++++++++
 www/manager6/grid/ResourceGrid.js |  2 +-
 www/manager6/panel/TagConfig.js   |  8 +++++
 www/manager6/tree/ResourceTree.js | 54 +++++++++++++++++++++++++++----
 6 files changed, 92 insertions(+), 8 deletions(-)
 create mode 100644 www/manager6/panel/TagConfig.js

-- 
2.30.2

[pve-devel] [PATCH docs v2 1/1] gui: add anchor for tags chapter

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pve-gui.adoc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pve-gui.adoc b/pve-gui.adoc
index bda370f..9e4650d 100644
--- a/pve-gui.adoc
+++ b/pve-gui.adoc
@@ -383,6 +383,7 @@ and the corresponding interfaces for each menu item on the right.
 * *Permissions:* manage the permissions for the pool.
 
 
+[[gui_tags]]
 Tags
 ----
 
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit v2 1/1] css: add some conditions to the tag classes for the tag view

[Dominik Csapak]

in the tag view, we have a custom 'full' style in a place where we
can have another tagstyle class above. to compensate for that, we have
to add another condition to those styles, namely that there is not the
'proxmox-tags-full' in between.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/css/ext6-pmx.css | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css
index 2ffd2a8..e088ae1 100644
--- a/src/css/ext6-pmx.css
+++ b/src/css/ext6-pmx.css
@@ -15,7 +15,9 @@
 }
 
 .proxmox-tags-full .x-grid-cell-inner-treecolumn .proxmox-tag-light,
-.proxmox-tags-full .x-grid-cell-inner-treecolumn .proxmox-tag-dark {
+.proxmox-tags-full .x-grid-cell-inner-treecolumn .proxmox-tag-dark,
+.x-grid-cell-inner-treecolumn .proxmox-tags-full .proxmox-tag-light,
+.x-grid-cell-inner-treecolumn .proxmox-tags-full .proxmox-tag-dark {
     display: inherit;
 }
 
@@ -25,8 +27,10 @@
 }
 
 
-.proxmox-tags-circle .proxmox-tag-light,
-.proxmox-tags-circle .proxmox-tag-dark {
+.proxmox-tags-circle :not(span.proxmox-tags-full) > .proxmox-tag-light,
+.proxmox-tags-circle :not(span.proxmox-tags-full) > .proxmox-tag-dark,
+.proxmox-tags-circle > .proxmox-tag-light,
+.proxmox-tags-circle > .proxmox-tag-dark {
     margin: 0px 1px;
     position: relative;
     top: 2px;
@@ -38,13 +42,17 @@
     overflow: hidden;
 }
 
-.proxmox-tags-none .proxmox-tag-light,
-.proxmox-tags-none .proxmox-tag-dark {
+.proxmox-tags-none :not(span.proxmox-tags-full) > .proxmox-tag-light,
+.proxmox-tags-none :not(span.proxmox-tags-full) > .proxmox-tag-dark,
+.proxmox-tags-none > .proxmox-tag-light,
+.proxmox-tags-none > .proxmox-tag-dark {
     display: none;
 }
 
-.proxmox-tags-dense .proxmox-tag-light,
-.proxmox-tags-dense .proxmox-tag-dark {
+.proxmox-tags-dense :not(span.proxmox-tags-full) > .proxmox-tag-light,
+.proxmox-tags-dense :not(span.proxmox-tags-full) > .proxmox-tag-dark,
+.proxmox-tags-dense > .proxmox-tag-light,
+.proxmox-tags-dense > .proxmox-tag-dark {
     width: 6px;
     margin-right: 1px;
     display: inline-block;
-- 
2.30.2

[pve-devel] [PATCH manager v2 1/1] ui: implement 'Tag View' for the resource tree

[Dominik Csapak]

and keep the functionality in ResourceTree as generic as possible.

We achieve this by having an 'itemMap' function that can split one item
from the store into multiple to add to the tree.

for the updates, we have to have an 'idMapFn' (to get the original id
back)

we also have to modify how the move checks work a bit, since we only
want to move the items when the tags changed only in the tagview case

in the ResourceGrid we have to get the id a bit differently since we now
have 'virtual' ids for the entries tag contain the tag (which can't be
found in the resource store)

since we also don't want tooltips for the already expanded tag groups,
we have to add the special condition that the element directly above
the tag should not have the 'full' class (like it is in tag group case)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile             |  1 +
 www/manager6/Workspace.js         |  3 +-
 www/manager6/form/ViewSelector.js | 32 ++++++++++++++++++
 www/manager6/grid/ResourceGrid.js |  2 +-
 www/manager6/panel/TagConfig.js   |  8 +++++
 www/manager6/tree/ResourceTree.js | 54 +++++++++++++++++++++++++++----
 6 files changed, 92 insertions(+), 8 deletions(-)
 create mode 100644 www/manager6/panel/TagConfig.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index ee09f0b8..5efd0726 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -105,6 +105,7 @@ JSSRC= 							\
 	panel/GuestSummary.js				\
 	panel/TemplateStatusView.js			\
 	panel/MultiDiskEdit.js				\
+	panel/TagConfig.js				\
 	tree/ResourceTree.js				\
 	tree/SnapshotTree.js				\
 	tree/ResourceMapTree.js				\
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 89ca47b7..424023b6 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -247,6 +247,7 @@ Ext.define('PVE.StdWorkspace', {
 			    storage: 'PVE.storage.Browser',
 			    sdn: 'PVE.sdn.Browser',
 			    pool: 'pvePoolConfig',
+			    tag: 'pveTagConfig',
 			};
 			PVE.curSelectedNode = treeNode;
 			me.setContent({
@@ -530,7 +531,7 @@ Ext.define('PVE.StdWorkspace', {
 	let tagSelectors = [];
 	['circle', 'dense'].forEach((style) => {
 	    ['dark', 'light'].forEach((variant) => {
-		tagSelectors.push(`.proxmox-tags-${style} .proxmox-tag-${variant}`);
+		tagSelectors.push(`.proxmox-tags-${style} :not(.proxmox-tags-full) > .proxmox-tag-${variant}`);
 	    });
 	});
 
diff --git a/www/manager6/form/ViewSelector.js b/www/manager6/form/ViewSelector.js
index e25547c4..647399ef 100644
--- a/www/manager6/form/ViewSelector.js
+++ b/www/manager6/form/ViewSelector.js
@@ -32,6 +32,38 @@ Ext.define('PVE.form.ViewSelector', {
 		// Pool View only lists VMs and Containers
 		filterfn: ({ data }) => data.type === 'qemu' || data.type === 'lxc' || data.type === 'pool',
 	    },
+	    tags: {
+		text: gettext('Tag View'),
+		groups: ['tag'],
+		filterfn: ({ data }) => data.type === 'qemu' || data.type === 'lxc',
+		groupRenderer: function(info) {
+		    let tag = PVE.Utils.renderTags(info.tag, PVE.UIOptions.tagOverrides);
+		    return `<span class="proxmox-tags-full">${tag}</span>`;
+		},
+		idMapFn: function(id) {
+		    let [realId, _tag] = id.split('-');
+		    return realId;
+		},
+		itemMap: function(item) {
+		    let tags = (item.data.tags ?? '').split(/[;, ]/);
+		    if (tags.length === 1 && tags[0] === '') {
+			return item;
+		    }
+		    let items = [];
+		    for (const tag of tags) {
+			let id = `${item.data.id}-${tag}`;
+			let info = Ext.apply({ leaf: true }, item.data);
+			info.tag = tag;
+			info.realId = info.id;
+			info.id = id;
+			items.push(Ext.create('Ext.data.TreeModel', info));
+		    }
+		    return items;
+		},
+		attrMoveChecks: {
+		    tag: (newitem, olditem) => newitem.data.tags !== olditem.data.tags,
+		},
+	    },
 	};
 	let groupdef = Object.entries(default_views).map(([name, config]) => [name, config.text]);
 
diff --git a/www/manager6/grid/ResourceGrid.js b/www/manager6/grid/ResourceGrid.js
index 9376bcc2..b212e9e9 100644
--- a/www/manager6/grid/ResourceGrid.js
+++ b/www/manager6/grid/ResourceGrid.js
@@ -44,7 +44,7 @@ Ext.define('PVE.grid.ResourceGrid', {
 		    return;
 		}
 		for (let child of node.childNodes) {
-		    let orgNode = rstore.data.get(child.data.id);
+		    let orgNode = rstore.data.get(child.data.realId ?? child.data.id);
 		    if (orgNode) {
 			if ((!filterfn || filterfn(child)) && (!textfilter || textfilterMatch(child))) {
 			    nodeidx[child.data.id] = orgNode;
diff --git a/www/manager6/panel/TagConfig.js b/www/manager6/panel/TagConfig.js
new file mode 100644
index 00000000..e26205bd
--- /dev/null
+++ b/www/manager6/panel/TagConfig.js
@@ -0,0 +1,8 @@
+// Config panel for the tag groups
+// for now it contains only the autogenerated 'search' panel
+Ext.define('PVE.panel.TagConfig', {
+    extend: 'PVE.panel.Config',
+    alias: 'widget.pveTagConfig',
+
+    onlineHelp: 'gui_tags',
+});
diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
index acfa545a..67ce74dd 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -37,6 +37,9 @@ Ext.define('PVE.tree.ResourceTree', {
 	    template: {
 		iconCls: 'fa fa-file-o',
 	    },
+	    tag: {
+		iconCls: 'fa fa-tag',
+	    },
 	},
     },
 
@@ -135,7 +138,7 @@ Ext.define('PVE.tree.ResourceTree', {
     },
 
     getToolTip: function(info) {
-	if (info.type === 'pool' || info.groupbyid !== undefined) {
+	if (info.type === 'pool' || info.tag !== undefined || info.groupbyid !== undefined) {
 	    return undefined;
 	}
 
@@ -166,12 +169,15 @@ Ext.define('PVE.tree.ResourceTree', {
 	me.setText(info);
 
 	if (info.groupbyid) {
-	    info.text = info.groupbyid;
-	    if (info.type === 'type') {
+	    if (me.viewFilter.groupRenderer) {
+		info.text = me.viewFilter.groupRenderer(info);
+	    } else if (info.type === 'type') {
 		let defaults = PVE.tree.ResourceTree.typeDefaults[info.groupbyid];
 		if (defaults && defaults.text) {
 		    info.text = defaults.text;
 		}
+	    } else {
+		info.text = info.groupbyid;
 	    }
 	}
 	let child = Ext.create('PVETree', info);
@@ -283,6 +289,9 @@ Ext.define('PVE.tree.ResourceTree', {
 
 	    let groups = me.viewFilter.groups || [];
 	    // explicitly check for node/template, as those are not always grouping attributes
+	    let attrMoveChecks = me.viewFilter.attrMoveChecks ?? {};
+	    let idMapFn = me.viewFilter.idMapFn ?? Ext.identityFn;
+
 	    // also check for name for when the tree is sorted by name
 	    let moveCheckAttrs = groups.concat(['node', 'template', 'name']);
 	    let filterfn = me.viewFilter.filterfn;
@@ -292,13 +301,20 @@ Ext.define('PVE.tree.ResourceTree', {
 	    // remove vanished or moved items and update changed items in-place
 	    for (const [key, olditem] of Object.entries(index)) {
 		// getById() use find(), which is slow (ExtJS4 DP5)
-		let item = rstore.data.get(olditem.data.id);
+		let oldid = olditem.data.id;
+		let id = idMapFn(olditem.data.id);
+		let item = rstore.data.get(id);
 
 		let changed = sorting_changed, moved = sorting_changed;
 		if (item) {
 		    // test if any grouping attributes changed, catches migrated tree-nodes in server view too
 		    for (const attr of moveCheckAttrs) {
-			if (item.data[attr] !== olditem.data[attr]) {
+			if (attrMoveChecks[attr]) {
+			    if (attrMoveChecks[attr](olditem, item)) {
+				moved = true;
+				break;
+			    }
+			} else if (item.data[attr] !== olditem.data[attr]) {
 			    moved = true;
 			    break;
 			}
@@ -318,6 +334,9 @@ Ext.define('PVE.tree.ResourceTree', {
 		    olditem.beginEdit();
 		    let info = olditem.data;
 		    Ext.apply(info, item.data);
+		    if (info.id !== oldid) {
+			info.id = oldid;
+		    }
 		    me.setIconCls(info);
 		    me.setText(info);
 		    olditem.commit();
@@ -334,10 +353,15 @@ Ext.define('PVE.tree.ResourceTree', {
 		    // store events are suspended, so remove the item manually
 		    store.remove(olditem);
 		    parentNode.removeChild(olditem, true);
+		    if (parentNode.childNodes.length < 1 && parentNode.parentNode) {
+			let grandParent = parentNode.parentNode;
+			grandParent.removeChild(parentNode, true);
+		    }
 		}
 	    }
 
-	    rstore.each(function(item) { // add new items
+	    let items = rstore.getData().items.flatMap(me.viewFilter.itemMap ?? Ext.identityFn);
+	    items.forEach(function(item) { // add new items
 		let olditem = index[item.data.id];
 		if (olditem) {
 		    return;
@@ -499,6 +523,24 @@ Ext.define('PVE.tree.ResourceTree', {
 
 	rstore.on("load", updateTree);
 	rstore.startUpdate();
+
+	if (me.viewFilter.groupRenderer) {
+	    me.mon(Ext.GlobalEvents, 'loadedUiOptions', () => {
+		me.store.getRootNode().cascadeBy({
+		    before: function(node) {
+			if (node.data.groupbyid) {
+			    node.beginEdit();
+			    let info = node.data;
+			    me.setIconCls(info);
+			    me.setText(info);
+			    info.text = me.viewFilter.groupRenderer(info);
+			    node.commit();
+			}
+			return true;
+		    },
+		});
+	    });
+	}
     },
 
 });
-- 
2.30.2

Re: [pve-devel] [PATCH manager v2 1/1] ui: implement 'Tag View' for the resource tree

[Fiona Ebner]

Am 21.11.23 um 13:47 schrieb Dominik Csapak:
> and keep the functionality in ResourceTree as generic as possible.
> 
> We achieve this by having an 'itemMap' function that can split one item
> from the store into multiple to add to the tree.
> 
> for the updates, we have to have an 'idMapFn' (to get the original id
> back)
> 
> we also have to modify how the move checks work a bit, since we only
> want to move the items when the tags changed only in the tagview case
> 
> in the ResourceGrid we have to get the id a bit differently since we now
> have 'virtual' ids for the entries tag contain the tag (which can't be
> found in the resource store)
> 
> since we also don't want tooltips for the already expanded tag groups,
> we have to add the special condition that the element directly above
> the tag should not have the 'full' class (like it is in tag group case)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

No real issues found during testing, but there is one thing that bugs
me: if I have selected a guest either:
- without tags and add a tag
or
- within the current tag group and remove that tag
then the selections for the guest is lost. Since it's the selection in
the resource view, this also affects the main area as a consequence and
feels a bit disruptive.

> @@ -530,7 +531,7 @@ Ext.define('PVE.StdWorkspace', {
>  	let tagSelectors = [];
>  	['circle', 'dense'].forEach((style) => {
>  	    ['dark', 'light'].forEach((variant) => {
> -		tagSelectors.push(`.proxmox-tags-${style} .proxmox-tag-${variant}`);
> +		tagSelectors.push(`.proxmox-tags-${style} :not(.proxmox-tags-full) > .proxmox-tag-${variant}`);

eslint fails because the line is too long

>  	    });
>  	});
>  

Re: [pve-devel] [PATCH manager v2 1/1] ui: implement 'Tag View' for the resource tree

[Dominik Csapak]

On 2/16/24 15:42, Fiona Ebner wrote:
> No real issues found during testing, but there is one thing that bugs
> me: if I have selected a guest either:
> - without tags and add a tag
> or
> - within the current tag group and remove that tag
> then the selections for the guest is lost. Since it's the selection in
> the resource view, this also affects the main area as a consequence and
> feels a bit disruptive.
> 

do you mean that you have e.g. 100 selected in the 'foo' tag group
and then removed 'foo' -> select of the datacenter level?

if yes, this is not easily solvable without special handle certain 
circumstances...

e.g. consider this:

vm 100 has 3 tags: foo,bar,baz

you have selected it in the 'foo' group and remove the 'foo' tag

which should now be selected? the entry in the 'bar' or 'baz' group?

the two scenarios that i could imagine would be nice is from
none -> 1 tag and reverse

but sadly the tag adding code and the selection logic
are rather separated, but i can think about how we could solve that

thanks!