[pve-devel] [PATCH manager 1/5] ui: rework inline tag editing

[Dominik Csapak <d.csapak@proxmox.com>]

things that changed:
* removed 'add Tag' inline button with proper button that adds
  empty tag
* don't require to confirm each tag, simply update the color "live"
* set a minimum width for the editing box, so that it's easier to click
* replace cancel/finish icons with proper buttons
* fix tagCharRegex for multichar text (necessary for paste)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/css/ext6-pve.css         |  28 +++-----
 www/manager6/Utils.js        |   2 +-
 www/manager6/form/Tag.js     | 116 +++++++++++++++-----------------
 www/manager6/form/TagEdit.js | 124 ++++++++++++++++-------------------
 4 files changed, 120 insertions(+), 150 deletions(-)

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index b8c713c48..a9ead5d3b 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -657,10 +657,11 @@ table.osds td:first-of-type {
     padding-bottom: 0px;
 }
 
-.pve-edit-tag > i,
-.pve-add-tag > i {
+.pve-edit-tag > i {
     cursor: pointer;
     font-size: 14px;
+    line-height: 15px;
+    height: 15px;
 }
 
 .pve-edit-tag > i.handle {
@@ -673,8 +674,7 @@ table.osds td:first-of-type {
     padding-right: 0px;
 }
 
-.pve-edit-tag > i.action,
-.pve-add-tag > i.action {
+.pve-edit-tag > i.action {
     padding-left: 5px;
 }
 
@@ -682,26 +682,18 @@ table.osds td:first-of-type {
     display: none;
 }
 
-.pve-edit-tag.editable span,
-.pve-edit-tag.inEdit span,
-.pve-add-tag.editable span,
-.pve-add-tag.inEdit span {
+.pve-edit-tag.editable span {
     background-color: #ffffff;
     border: 1px solid #a8a8a8;
     color: #000;
     padding-left: 2px;
     padding-right: 2px;
     min-width: 2em;
-}
-
-.pve-edit-tag.inEdit span,
-.pve-add-tag.inEdit span {
-    border: 1px solid #000;
-}
-
-.pve-add-tag {
-    background-color: #d5d5d5 ! important;
-    color: #000000 ! important;
+    display: inline-block;
+    line-height: 15px;
+    height: 15px;
+    vertical-align: top;
+    box-sizing: content-box;
 }
 
 .pve-tag-inline-button {
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 8484372f2..e4b6207c6 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1955,7 +1955,7 @@ Ext.define('PVE.Utils', {
 	return !(PVE.UIOptions?.['tag-style']?.ordering === 'config');
     },
 
-    tagCharRegex: /^[a-z0-9+_.-]$/i,
+    tagCharRegex: /^[a-z0-9+_.-]+$/i,
 },
 
     singleton: true,
diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index 9acedb527..9da0db951 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -4,53 +4,44 @@ Ext.define('Proxmox.form.Tag', {
 
     mode: 'editable',
 
-    icons: {
-	editable: 'fa fa-minus-square',
-	normal: '',
-	inEdit: 'fa fa-check-square',
-    },
-
     tag: '',
     cls: 'pve-edit-tag',
 
     tpl: [
 	'<i class="handle fa fa-bars"></i>',
 	'<span>{tag}</span>',
-	'<i class="action {iconCls}"></i>',
+	'<i class="action fa fa-minus-square"></i>',
     ],
 
-    // we need to do this in mousedown, because that triggers before
-    // focusleave (which triggers before click)
-    onMouseDown: function(event) {
-	let me = this;
-	if (event.target.tagName !== 'I' || event.target.classList.contains('handle')) {
-	    return;
-	}
-	switch (me.mode) {
-	    case 'editable':
-		me.setVisible(false);
-		me.setTag('');
-		break;
-	    case 'inEdit':
-		me.setTag(me.tagEl().innerHTML);
-		me.setMode('editable');
-		break;
-	    default: break;
-	}
+    // contains tags not to show in the picker and not allowing to set
+    filter: [],
+
+    updateFilter: function(tags) {
+	this.filter = tags;
     },
 
     onClick: function(event) {
 	let me = this;
-	if (event.target.tagName !== 'SPAN' || me.mode !== 'editable') {
+	if (event.target.tagName === 'I' && !event.target.classList.contains('handle')) {
+	    if (me.mode === 'editable') {
+		me.destroy();
+		return;
+	    }
+	} else if (event.target.tagName !== 'SPAN' || me.mode !== 'editable') {
 	    return;
 	}
-	me.setMode('inEdit');
+	me.selectText();
+    },
 
-	// select text in the element
+    selectText: function(collapseToEnd) {
+	let me = this;
 	let tagEl = me.tagEl();
 	tagEl.contentEditable = true;
 	let range = document.createRange();
 	range.selectNodeContents(tagEl);
+	if (collapseToEnd) {
+	    range.collapse(false);
+	}
 	let sel = window.getSelection();
 	sel.removeAllRanges();
 	sel.addRange(range);
@@ -75,8 +66,10 @@ Ext.define('Proxmox.form.Tag', {
 		store: [],
 		listeners: {
 		    select: function(picker, rec) {
-			me.setTag(rec.data.tag);
-			me.setMode('editable');
+			me.tagEl().innerHTML = rec.data.tag;
+			me.setTag(rec.data.tag, true);
+			me.selectText(true);
+			me.setColor(rec.data.tag);
 			me.picker.hide();
 		    },
 		},
@@ -94,17 +87,16 @@ Ext.define('Proxmox.form.Tag', {
 
     setMode: function(mode) {
 	let me = this;
-	if (me.icons[mode] === undefined) {
-	    throw "invalid mode";
-	}
 	let tagEl = me.tagEl();
 	if (tagEl) {
-	    tagEl.contentEditable = mode === 'inEdit';
+	    tagEl.contentEditable = mode === 'editable';
 	}
 	me.removeCls(me.mode);
 	me.addCls(mode);
 	me.mode = mode;
-	me.updateData();
+	if (me.mode !== 'editable') {
+	    me.picker?.hide();
+	}
     },
 
     onKeyPress: function(event) {
@@ -112,15 +104,10 @@ Ext.define('Proxmox.form.Tag', {
 	let key = event.browserEvent.key;
 	switch (key) {
 	    case 'Enter':
-		if (me.tagEl().innerHTML !== '') {
-		    me.setTag(me.tagEl().innerHTML);
-		    me.setMode('editable');
-		    return;
-		}
 		break;
+	    case 'ArrowLeft':
+	    case 'ArrowRight':
 	    case 'Escape':
-		me.cancelEdit();
-		return;
 	    case 'Backspace':
 	    case 'Delete':
 		return;
@@ -128,11 +115,13 @@ Ext.define('Proxmox.form.Tag', {
 		if (key.match(PVE.Utils.tagCharRegex)) {
 		    return;
 		}
+		me.setTag(me.tagEl().innerHTML);
 	}
 	event.browserEvent.preventDefault();
 	event.browserEvent.stopPropagation();
     },
 
+    // for pasting text
     beforeInput: function(event) {
 	let me = this;
 	me.updateLayout();
@@ -153,22 +142,17 @@ Ext.define('Proxmox.form.Tag', {
 	    value: me.tagEl().innerHTML,
 	    anyMatch: true,
 	});
+	me.setTag(me.tagEl().innerHTML);
     },
 
-    cancelEdit: function(list, event) {
+    lostFocus: function(list, event) {
 	let me = this;
-	if (me.mode === 'inEdit') {
-	    me.setTag(me.tag);
-	    me.setMode('editable');
-	}
 	me.picker?.hide();
+	window.getSelection().removeAllRanges();
     },
 
-
-    setTag: function(tag) {
+    setColor: function(tag) {
 	let me = this;
-	let oldtag = me.tag;
-	me.tag = tag;
 	let rgb = PVE.Utils.tagOverrides[tag] ?? Proxmox.Utils.stringToRGB(tag);
 
 	let cls = Proxmox.Utils.getTextContrastClass(rgb);
@@ -182,21 +166,20 @@ Ext.define('Proxmox.form.Tag', {
 	} else {
 	    me.setStyle('color');
 	}
-	me.updateData();
-	if (oldtag !== tag) {
-	    me.fireEvent('change', me, tag, oldtag);
-	}
     },
 
-    updateData: function() {
+    setTag: function(tag) {
 	let me = this;
-	if (me.destroying || me.destroyed) {
-	    return;
+	let oldtag = me.tag;
+	me.tag = tag;
+
+	clearTimeout(me.colorTimeout);
+	me.colorTimeout = setTimeout(() => me.setColor(tag), 200);
+
+	me.updateLayout();
+	if (oldtag !== tag) {
+	    me.fireEvent('change', me, tag, oldtag);
 	}
-	me.update({
-	    tag: me.tag,
-	    iconCls: me.icons[me.mode],
-	});
     },
 
     tagEl: function() {
@@ -204,9 +187,8 @@ Ext.define('Proxmox.form.Tag', {
     },
 
     listeners: {
-	mousedown: 'onMouseDown',
 	click: 'onClick',
-	focusleave: 'cancelEdit',
+	focusleave: 'lostFocus',
 	keydown: 'onKeyPress',
 	beforeInput: 'beforeInput',
 	input: 'onInput',
@@ -217,7 +199,12 @@ Ext.define('Proxmox.form.Tag', {
     initComponent: function() {
 	let me = this;
 
+	me.data = {
+	    tag: me.tag,
+	};
+
 	me.setTag(me.tag);
+	me.setColor(me.tag);
 	me.setMode(me.mode ?? 'normal');
 	me.callParent();
     },
@@ -227,6 +214,7 @@ Ext.define('Proxmox.form.Tag', {
 	if (me.picker) {
 	    Ext.destroy(me.picker);
 	}
+	clearTimeout(me.colorTimeout);
 	me.callParent();
     },
 });
diff --git a/www/manager6/form/TagEdit.js b/www/manager6/form/TagEdit.js
index 6325d39df..4e3fec384 100644
--- a/www/manager6/form/TagEdit.js
+++ b/www/manager6/form/TagEdit.js
@@ -4,7 +4,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 
     layout: {
 	type: 'hbox',
-	align: 'stretch',
+	align: 'middle',
     },
 
     controller: {
@@ -120,9 +120,6 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    let me = this;
 	    let view = me.getView();
 	    view.items.each((field) => {
-		if (field.reference === 'addTagBtn') {
-		    return false;
-		}
 		if (field.getXType() === 'pveTag') {
 		    func(field);
 		}
@@ -133,6 +130,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 	toggleEdit: function(cancel) {
 	    let me = this;
 	    let vm = me.getViewModel();
+	    let view = me.getView();
 	    let editMode = !vm.get('editMode');
 	    vm.set('editMode', editMode);
 
@@ -150,14 +148,19 @@ Ext.define('PVE.panel.TagEditContainer', {
 		if (cancel) {
 		    me.loadTags(me.oldTags, true);
 		} else {
+		    let toRemove = [];
 		    me.forEachTag((cmp) => {
 			if (cmp.isVisible() && cmp.tag) {
 			    tags.push(cmp.tag);
+			} else {
+			    toRemove.push(cmp);
 			}
 		    });
+		    toRemove.forEach(cmp => view.remove(cmp));
 		    tags = tags.join(',');
 		    if (me.oldTags !== tags) {
 			me.oldTags = tags;
+			me.loadTags(tags, true);
 			me.getView().fireEvent('change', tags);
 		    }
 		}
@@ -165,60 +168,44 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    me.getView().updateLayout();
 	},
 
-	addTag: function(tag) {
+	addTag: function(tag, isNew) {
 	    let me = this;
 	    let view = me.getView();
 	    let vm = me.getViewModel();
-	    let index = view.items.indexOf(me.lookup('addTagBtn'));
-	    if (PVE.Utils.shouldSortTags()) {
+	    let index = view.items.length - 5;
+	    if (PVE.Utils.shouldSortTags() && !isNew) {
 		index = view.items.findIndexBy(tagField => {
-		    if (tagField.reference === 'addTagBtn') {
+		    if (tagField.reference === 'noTagsField') {
+			return false;
+		    }
+		    if (tagField.xtype !== 'pveTag') {
 			return true;
 		    }
 		    return tagField.tag >= tag;
 		}, 1);
 	    }
-	    view.insert(index, {
+	    let tagField = view.insert(index, {
 		xtype: 'pveTag',
 		tag,
 		mode: vm.get('editMode') ? 'editable' : 'normal',
 		listeners: {
-		    change: (field, newTag) => {
-			if (newTag === '') {
-			    view.remove(field);
-			    vm.set('tagCount', vm.get('tagCount') - 1);
-			}
+		    destroy: function() {
+			vm.set('tagCount', vm.get('tagCount') - 1);
 		    },
 		},
 	    });
 
-	    vm.set('tagCount', vm.get('tagCount') + 1);
-	},
-
-	addTagClick: function(event) {
-	    let me = this;
-	    if (event.target.tagName === 'SPAN') {
-		me.lookup('addTagBtn').tagEl().innerHTML = '';
-		me.lookup('addTagBtn').updateLayout();
+	    if (isNew) {
+		tagField.selectText();
 	    }
-	},
 
-	addTagMouseDown: function(event) {
-	    let me = this;
-	    if (event.target.tagName === 'I') {
-		let tag = me.lookup('addTagBtn').tagEl().innerHTML;
-		if (tag !== '') {
-		    me.addTag(tag, true);
-		}
-	    }
+	    vm.set('tagCount', vm.get('tagCount') + 1);
 	},
 
-	addTagChange: function(field, tag) {
+	addTagClick: function(event) {
 	    let me = this;
-	    if (tag !== '') {
-		me.addTag(tag, true);
-	    }
-	    field.tag = '';
+	    me.lookup('noTagsField').setVisible(false);
+	    me.addTag('', true);
 	},
 
 	cancelClick: function() {
@@ -250,12 +237,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 
 	formulas: {
 	    hideNoTags: function(get) {
-		return get('editMode') || get('tagCount') !== 0;
-	    },
-	    editBtnHtml: function(get) {
-		let cls = get('editMode') ? 'check' : 'pencil';
-		let qtip = get('editMode') ? gettext('Apply Changes') : gettext('Edit Tags');
-		return `<i data-qtip="${qtip}" class="fa fa-${cls}"></i>`;
+		return get('tagCount') !== 0;
 	    },
 	},
     },
@@ -267,53 +249,61 @@ Ext.define('PVE.panel.TagEditContainer', {
     items: [
 	{
 	    xtype: 'box',
+	    reference: 'noTagsField',
 	    bind: {
 		hidden: '{hideNoTags}',
 	    },
 	    html: gettext('No Tags'),
 	},
 	{
-	    xtype: 'pveTag',
-	    reference: 'addTagBtn',
-	    cls: 'pve-add-tag',
-	    mode: 'editable',
-	    tag: '',
-	    tpl: `<span>${gettext('Add Tag')}</span><i class="action fa fa-plus-square"></i>`,
+	    xtype: 'button',
+	    iconCls: 'fa fa-plus',
+	    tooltip: gettext('Add Tag'),
 	    bind: {
 		hidden: '{!editMode}',
 	    },
 	    hidden: true,
-	    onMouseDown: Ext.emptyFn, // prevent default behaviour
-	    listeners: {
-		click: {
-		    element: 'el',
-		    fn: 'addTagClick',
-		},
-		mousedown: {
-		    element: 'el',
-		    fn: 'addTagMouseDown',
-		},
-		change: 'addTagChange',
-	    },
+	    margin: '0 8 0 5',
+	    ui: 'default-toolbar',
+	    handler: 'addTagClick',
 	},
 	{
-	    xtype: 'box',
-	    html: `<i data-qtip="${gettext('Cancel')}" class="fa fa-times"></i>`,
-	    cls: 'pve-tag-inline-button',
+	    xtype: 'tbseparator',
+	    ui: 'horizontal',
+	    bind: {
+		hidden: '{!editMode}',
+	    },
 	    hidden: true,
+	},
+	{
+	    xtype: 'button',
+	    iconCls: 'fa fa-times',
+	    tooltip: gettext('Cancel Edit'),
 	    bind: {
 		hidden: '{!editMode}',
 	    },
-	    listeners: {
-		click: 'cancelClick',
-		element: 'el',
+	    hidden: true,
+	    margin: '0 5 0 0',
+	    ui: 'default-toolbar',
+	    handler: 'cancelClick',
+	},
+	{
+	    xtype: 'button',
+	    iconCls: 'fa fa-check',
+	    tooltip: gettext('Finish Edit'),
+	    bind: {
+		hidden: '{!editMode}',
 	    },
+	    hidden: true,
+	    ui: 'default-toolbar',
+	    handler: 'editClick',
 	},
 	{
 	    xtype: 'box',
 	    cls: 'pve-tag-inline-button',
+	    html: `<i data-qtip="${gettext('Edit Tags')}" class="fa fa-pencil"></i>`,
 	    bind: {
-		html: '{editBtnHtml}',
+		hidden: '{editMode}',
 	    },
 	    listeners: {
 		click: 'editClick',
-- 
2.30.2