[pve-devel] [PATCH manager 1/3] ui: tags: fix focus for edit mode

[pve-devel] [PATCH manager 1/3] ui: tags: fix focus for edit mode

[Dominik Csapak]

such that one can tab through the editable tag fields.
We have to handle that manually, since ExtJs does not expect
contenteditable html tags for focus handling.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/Tag.js | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index be72d7ba..6b1d6aa5 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -13,6 +13,15 @@ Ext.define('Proxmox.form.Tag', {
 	'<i class="action fa fa-minus-square"></i>',
     ],
 
+    focusable: true,
+    getFocusEl: function() {
+	return Ext.get(this.tagEl());
+    },
+
+    onFocus: function() {
+	this.selectText();
+    },
+
     // contains tags not to show in the picker and not allowing to set
     filter: [],
 
-- 
2.30.2

[pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Dominik Csapak]

by setting 'contentEditable' to 'plaintext-only' instead of true.
Otherwise it was possible to paste html code into the field (an error
was thrown by the backend since it does not match the regex)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/Tag.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index 6b1d6aa5..39afe04b 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -45,7 +45,7 @@ Ext.define('Proxmox.form.Tag', {
     selectText: function(collapseToEnd) {
 	let me = this;
 	let tagEl = me.tagEl();
-	tagEl.contentEditable = true;
+	tagEl.contentEditable = "plaintext-only";
 	let range = document.createRange();
 	range.selectNodeContents(tagEl);
 	if (collapseToEnd) {
@@ -98,7 +98,7 @@ Ext.define('Proxmox.form.Tag', {
 	let me = this;
 	let tagEl = me.tagEl();
 	if (tagEl) {
-	    tagEl.contentEditable = mode === 'editable';
+	    tagEl.contentEditable = mode === 'editable' ? 'plaintext-only' : false;
 	}
 	me.removeCls(me.mode);
 	me.addCls(mode);
-- 
2.30.2

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Dominik Csapak]

sry disregard this patch only, that property value is not supported in firefox :(

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Thomas Lamprecht]

Am 19/10/2023 um 15:59 schrieb Dominik Csapak:
> sry disregard this patch only, that property value is not supported in firefox :(
> 

We could set it only to "plaintext-only" for non-firefox then though?

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Dominik Csapak]

On 10/24/23 11:53, Thomas Lamprecht wrote:
> Am 19/10/2023 um 15:59 schrieb Dominik Csapak:
>> sry disregard this patch only, that property value is not supported in firefox :(
>>
> 
> We could set it only to "plaintext-only" for non-firefox then though?

sure not a problem, i just generally try to avoid browser specific stuff

if there is a way to prevent pasting html in a sane way for all browsers
i'd prefer that, but did not have the time to look into that..

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Thomas Lamprecht]

Am 24/10/2023 um 11:57 schrieb Dominik Csapak:
> On 10/24/23 11:53, Thomas Lamprecht wrote:
>> Am 19/10/2023 um 15:59 schrieb Dominik Csapak:
>>> sry disregard this patch only, that property value is not supported in firefox :(
>>>
>>
>> We could set it only to "plaintext-only" for non-firefox then though?
> 
> sure not a problem, i just generally try to avoid browser specific stuff
> 
> if there is a way to prevent pasting html in a sane way for all browsers
> i'd prefer that, but did not have the time to look into that..
> 

OK, let's make "no special cases for browsers" a rule that should have as
few as possible exceptions, that can only benefit users and devs reproducing
issues.

I subscribed to the enhancement request [0] at Mozilla's Bugzilla, where
they state that it's only blocked on the spec being to vague and lacking test
cases, so once it's done and released in an ESR we can switch to this
unconditionally

[0]: https://bugzilla.mozilla.org/show_bug.cgi?id=1291467#c22

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Dominik Csapak]

On 10/24/23 12:07, Thomas Lamprecht wrote:
> Am 24/10/2023 um 11:57 schrieb Dominik Csapak:
>> On 10/24/23 11:53, Thomas Lamprecht wrote:
>>> Am 19/10/2023 um 15:59 schrieb Dominik Csapak:
>>>> sry disregard this patch only, that property value is not supported in firefox :(
>>>>
>>>
>>> We could set it only to "plaintext-only" for non-firefox then though?
>>
>> sure not a problem, i just generally try to avoid browser specific stuff
>>
>> if there is a way to prevent pasting html in a sane way for all browsers
>> i'd prefer that, but did not have the time to look into that..
>>
> 
> OK, let's make "no special cases for browsers" a rule that should have as
> few as possible exceptions, that can only benefit users and devs reproducing
> issues.

that makes sense, i'll add a section to the javascript style guide, is that
fine with you?

> 
> I subscribed to the enhancement request [0] at Mozilla's Bugzilla, where
> they state that it's only blocked on the spec being to vague and lacking test
> cases, so once it's done and released in an ESR we can switch to this
> unconditionally
> 
> [0]: https://bugzilla.mozilla.org/show_bug.cgi?id=1291467#c22

i'll subscribe too, thanks for researching that

Re: [pve-devel] [PATCH manager 2/3] ui: tags: prevent pasting non plain-text content

[Thomas Lamprecht]

Am 24/10/2023 um 12:51 schrieb Dominik Csapak:
> On 10/24/23 12:07, Thomas Lamprecht wrote:
>> OK, let's make "no special cases for browsers" a rule that should have as
>> few as possible exceptions, that can only benefit users and devs reproducing
>> issues.
> 
> that makes sense, i'll add a section to the javascript style guide, is that
> fine with you?

yeah, that's a good idea.

[pve-devel] [PATCH manager 3/3] ui: wizards: allow adding tags in the qemu/lxc create wizard

[Dominik Csapak]

in the general tab in the advanced section.

For that to work, we introduce a new option for the TagEditContainer
named 'editOnly', which controls now the cancel/finish buttons,
automatically enter edit mode and disable enter/escape keypresses.

We also prevent now the loading of tags while in edit mode, so the tags
don't change while editing (this can be jarring and unexpected).

In the wizard, we override the layout such that the tags wrap when there
are too many, and make the field scrollable and set a height, so that
the user can enter as many tags as he wants without having the field
overflow or cut off.

To properly align the input with the '+' button, we have to add a custom
css class there. (In the hbox we could set the alignment, but this is
not possible in the 'column' layout)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/css/ext6-pve.css              |  5 +++++
 www/manager6/form/TagEdit.js      | 35 +++++++++++++++++++++++++++----
 www/manager6/lxc/CreateWizard.js  | 22 +++++++++++++++++++
 www/manager6/qemu/CreateWizard.js | 22 +++++++++++++++++++
 4 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/www/css/ext6-pve.css b/www/css/ext6-pve.css
index 85cf4039..25bc7ea7 100644
--- a/www/css/ext6-pve.css
+++ b/www/css/ext6-pve.css
@@ -721,3 +721,8 @@ table.osds td:first-of-type {
 .pmx-opacity-75 {
     opacity: 0.75;
 }
+
+/* tag edit fields must be aligned manually in the wizard */
+.proxmox-wizard.proxmox-tags-full .x-component.x-column {
+    margin: 3px 2px;
+}
diff --git a/www/manager6/form/TagEdit.js b/www/manager6/form/TagEdit.js
index 094f4462..7d5b19ec 100644
--- a/www/manager6/form/TagEdit.js
+++ b/www/manager6/form/TagEdit.js
@@ -9,6 +9,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 
     // set to false to hide the 'no tags' field and the edit button
     canEdit: true,
+    editOnly: false,
 
     controller: {
 	xclass: 'Ext.app.ViewController',
@@ -216,6 +217,9 @@ Ext.define('PVE.panel.TagEditContainer', {
 			me.tagsChanged();
 		    },
 		    keypress: function(key) {
+			if (vm.get('hideFinishButtons')) {
+			    return;
+			}
 			if (key === 'Enter') {
 			    me.editClick();
 			} else if (key === 'Escape') {
@@ -253,20 +257,40 @@ Ext.define('PVE.panel.TagEditContainer', {
 		me.loadTags(view.tags);
 	    }
 	    me.getViewModel().set('canEdit', view.canEdit);
+	    me.getViewModel().set('editOnly', view.editOnly);
 
 	    me.mon(Ext.GlobalEvents, 'loadedUiOptions', () => {
+		let vm = me.getViewModel();
 		view.toggleCls('hide-handles', PVE.UIOptions.shouldSortTags());
-		me.loadTags(me.oldTags, true); // refresh tag colors and order
+		me.loadTags(me.oldTags, !vm.get('editMode')); // refresh tag colors and order
 	    });
+
+	    if (view.editOnly) {
+		me.toggleEdit();
+	    }
 	},
     },
 
+    getTags: function() {
+	let me =this;
+	let controller = me.getController();
+	let tags = [];
+	    controller.forEachTag((cmp) => {
+		if (cmp.tag.length) {
+		    tags.push(cmp.tag);
+		}
+	    });
+
+	return tags;
+    },
+
     viewModel: {
 	data: {
 	    tagCount: 0,
 	    editMode: false,
 	    canEdit: true,
 	    isDirty: false,
+	    editOnly: true,
 	},
 
 	formulas: {
@@ -276,6 +300,9 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    hideEditBtn: function(get) {
 		return get('editMode') || !get('canEdit');
 	    },
+	    hideFinishButtons: function(get) {
+		return !get('editMode') || get('editOnly');
+	    },
 	},
     },
 
@@ -311,7 +338,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    xtype: 'tbseparator',
 	    ui: 'horizontal',
 	    bind: {
-		hidden: '{!editMode}',
+		hidden: '{hideFinishButtons}',
 	    },
 	    hidden: true,
 	},
@@ -320,7 +347,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    iconCls: 'fa fa-times',
 	    tooltip: gettext('Cancel Edit'),
 	    bind: {
-		hidden: '{!editMode}',
+		hidden: '{hideFinishButtons}',
 	    },
 	    hidden: true,
 	    margin: '0 5 0 0',
@@ -332,7 +359,7 @@ Ext.define('PVE.panel.TagEditContainer', {
 	    iconCls: 'fa fa-check',
 	    tooltip: gettext('Finish Edit'),
 	    bind: {
-		hidden: '{!editMode}',
+		hidden: '{hideFinishButtons}',
 		disabled: '{!isDirty}',
 	    },
 	    hidden: true,
diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
index e3635297..e725f8c1 100644
--- a/www/manager6/lxc/CreateWizard.js
+++ b/www/manager6/lxc/CreateWizard.js
@@ -29,6 +29,14 @@ Ext.define('PVE.lxc.CreateWizard', {
 	    xtype: 'inputpanel',
 	    title: gettext('General'),
 	    onlineHelp: 'pct_general',
+	    onGetValues: function(values) {
+		let me = this;
+		let tags = me.down('pveTagEditContainer').getTags();
+		if (tags.length) {
+		    values.tags = tags.join(',');
+		}
+		return values;
+	    },
 	    column1: [
 		{
 		    xtype: 'pveNodeSelector',
@@ -178,6 +186,20 @@ Ext.define('PVE.lxc.CreateWizard', {
 		    },
 		},
 	    ],
+	    advancedColumnB: [
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext("Tags"),
+		},
+		{
+		    xtype: 'pveTagEditContainer',
+		    userCls: 'proxmox-tags-full proxmox-wizard',
+		    editOnly: true,
+		    scrollable: true,
+		    layout: 'column',
+		    height: 120,
+		},
+	    ],
 	},
 	{
 	    xtype: 'inputpanel',
diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
index a65067ea..2ccad83b 100644
--- a/www/manager6/qemu/CreateWizard.js
+++ b/www/manager6/qemu/CreateWizard.js
@@ -108,6 +108,22 @@ Ext.define('PVE.qemu.CreateWizard', {
 		    fieldLabel: gettext('Shutdown timeout'),
 		},
 	    ],
+
+	    advancedColumnB: [
+		{
+		    xtype: 'displayfield',
+		    fieldLabel: gettext("Tags"),
+		},
+		{
+		    xtype: 'pveTagEditContainer',
+		    userCls: 'proxmox-tags-full proxmox-wizard',
+		    editOnly: true,
+		    scrollable: true,
+		    layout: 'column',
+		    height: 120,
+		},
+	    ],
+
 	    onGetValues: function(values) {
 		['name', 'pool', 'onboot', 'agent'].forEach(function(field) {
 		    if (!values[field]) {
@@ -115,6 +131,12 @@ Ext.define('PVE.qemu.CreateWizard', {
 		    }
 		});
 
+		let tags = this.down('pveTagEditContainer').getTags();
+
+		if (tags.length) {
+		    values.tags = tags.join(',');
+		}
+
 		var res = PVE.Parser.printStartup({
 		    order: values.order,
 		    up: values.up,
-- 
2.30.2

Re: [pve-devel] [PATCH manager 3/3] ui: wizards: allow adding tags in the qemu/lxc create wizard

[Thomas Lamprecht]

Am 19/10/2023 um 15:36 schrieb Dominik Csapak:
> in the general tab in the advanced section.
> 
> For that to work, we introduce a new option for the TagEditContainer
> named 'editOnly', which controls now the cancel/finish buttons,
> automatically enter edit mode and disable enter/escape keypresses.
> 
> We also prevent now the loading of tags while in edit mode, so the tags
> don't change while editing (this can be jarring and unexpected).
> 
> In the wizard, we override the layout such that the tags wrap when there
> are too many, and make the field scrollable and set a height, so that
> the user can enter as many tags as he wants without having the field
> overflow or cut off.
> 
> To properly align the input with the '+' button, we have to add a custom
> css class there. (In the hbox we could set the alignment, but this is
> not possible in the 'column' layout)
> 

I'd wrap this in a fieldset with Tags as legend (well, still "title" in
ExtJS), ideally in its own small PVE.form.TagEditFieldSet module, that
extends the Ext.form.FieldSet class? That could then also contain
the getValue handling, further reducing the wizard specific changes.

Allowing the height to grow until a max-height (i.e., fills out wizard
panel) and only then get scrollable would be still nice though.

[pve-devel] applied: [PATCH manager 1/3] ui: tags: fix focus for edit mode

[Thomas Lamprecht]

Am 19/10/2023 um 15:36 schrieb Dominik Csapak:
> such that one can tab through the editable tag fields.
> We have to handle that manually, since ExtJs does not expect
> contenteditable html tags for focus handling.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/form/Tag.js | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
>

applied this one already, thanks!