[pve-devel] [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[pve-devel] [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[Stefan Hanreich]

Not all fields in the VnetEdit dialog are necessary for every zone
type. This lead to confusion for some users. Hide fields in the
VNetEdit dialog depending on which kind of zone is selected in order
to prevent potential confusion.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/form/SDNZoneSelector.js |  2 +-
 www/manager6/sdn/VnetEdit.js         | 40 ++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/SDNZoneSelector.js b/www/manager6/form/SDNZoneSelector.js
index 28c3457d2..0d0327529 100644
--- a/www/manager6/form/SDNZoneSelector.js
+++ b/www/manager6/form/SDNZoneSelector.js
@@ -40,7 +40,7 @@ Ext.define('PVE.form.SDNZoneSelector', {
 }, function() {
     Ext.define('pve-sdn-zone', {
 	extend: 'Ext.data.Model',
-	fields: ['zone'],
+	fields: ['zone', 'type'],
 	proxy: {
             type: 'proxmox',
 	    url: "/api2/json/cluster/sdn/zones",
diff --git a/www/manager6/sdn/VnetEdit.js b/www/manager6/sdn/VnetEdit.js
index cdd83ed40..9fb6cd6c7 100644
--- a/www/manager6/sdn/VnetEdit.js
+++ b/www/manager6/sdn/VnetEdit.js
@@ -12,6 +12,13 @@ Ext.define('PVE.sdn.VnetInputPanel', {
 	return values;
     },
 
+    initComponent: function() {
+	let me = this;
+
+	me.callParent();
+	me.setZoneType(undefined);
+    },
+
     items: [
 	{
 	    xtype: 'pmxDisplayEditField',
@@ -40,9 +47,21 @@ Ext.define('PVE.sdn.VnetInputPanel', {
 	    name: 'zone',
 	    value: '',
 	    allowBlank: false,
+	    listeners: {
+		change: function() {
+		    let me = this;
+
+		    let record = me.findRecordByValue(me.value);
+		    let zoneType = record?.data?.type;
+
+		    let panel = me.up('panel');
+		    panel.setZoneType(zoneType);
+		},
+	    },
 	},
 	{
 	    xtype: 'proxmoxintegerfield',
+	    itemId: 'sdnVnetTagField',
 	    name: 'tag',
 	    minValue: 1,
 	    maxValue: 16777216,
@@ -54,6 +73,7 @@ Ext.define('PVE.sdn.VnetInputPanel', {
 	},
 	{
 	    xtype: 'proxmoxcheckbox',
+	    itemId: 'sdnVnetVlanAwareField',
 	    name: 'vlanaware',
 	    uncheckedValue: null,
 	    checked: false,
@@ -63,6 +83,26 @@ Ext.define('PVE.sdn.VnetInputPanel', {
 	    },
 	},
     ],
+
+    setZoneType: function(zoneType) {
+	let me = this;
+
+	let tagField = me.down('#sdnVnetTagField');
+	if (!zoneType || zoneType === 'simple') {
+	    tagField.setVisible(false);
+	    tagField.setValue('');
+	} else {
+	    tagField.setVisible(true);
+	}
+
+	let vlanField = me.down('#sdnVnetVlanAwareField');
+	if (!zoneType || zoneType === 'evpn') {
+	    vlanField.setVisible(false);
+	    vlanField.setValue('');
+	} else {
+	    vlanField.setVisible(true);
+	}
+    },
 });
 
 Ext.define('PVE.sdn.VnetEdit', {
-- 
2.39.2

Re: [pve-devel] [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[Stefan Hanreich]

ping

still applies on current master

On 12/22/23 11:43, Stefan Hanreich wrote:
> Not all fields in the VnetEdit dialog are necessary for every zone
> type. This lead to confusion for some users. Hide fields in the
> VNetEdit dialog depending on which kind of zone is selected in order
> to prevent potential confusion.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  www/manager6/form/SDNZoneSelector.js |  2 +-
>  www/manager6/sdn/VnetEdit.js         | 40 ++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/form/SDNZoneSelector.js b/www/manager6/form/SDNZoneSelector.js
> index 28c3457d2..0d0327529 100644
> --- a/www/manager6/form/SDNZoneSelector.js
> +++ b/www/manager6/form/SDNZoneSelector.js
> @@ -40,7 +40,7 @@ Ext.define('PVE.form.SDNZoneSelector', {
>  }, function() {
>      Ext.define('pve-sdn-zone', {
>  	extend: 'Ext.data.Model',
> -	fields: ['zone'],
> +	fields: ['zone', 'type'],
>  	proxy: {
>              type: 'proxmox',
>  	    url: "/api2/json/cluster/sdn/zones",
> diff --git a/www/manager6/sdn/VnetEdit.js b/www/manager6/sdn/VnetEdit.js
> index cdd83ed40..9fb6cd6c7 100644
> --- a/www/manager6/sdn/VnetEdit.js
> +++ b/www/manager6/sdn/VnetEdit.js
> @@ -12,6 +12,13 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>  	return values;
>      },
>  
> +    initComponent: function() {
> +	let me = this;
> +
> +	me.callParent();
> +	me.setZoneType(undefined);
> +    },
> +
>      items: [
>  	{
>  	    xtype: 'pmxDisplayEditField',
> @@ -40,9 +47,21 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>  	    name: 'zone',
>  	    value: '',
>  	    allowBlank: false,
> +	    listeners: {
> +		change: function() {
> +		    let me = this;
> +
> +		    let record = me.findRecordByValue(me.value);
> +		    let zoneType = record?.data?.type;
> +
> +		    let panel = me.up('panel');
> +		    panel.setZoneType(zoneType);
> +		},
> +	    },
>  	},
>  	{
>  	    xtype: 'proxmoxintegerfield',
> +	    itemId: 'sdnVnetTagField',
>  	    name: 'tag',
>  	    minValue: 1,
>  	    maxValue: 16777216,
> @@ -54,6 +73,7 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>  	},
>  	{
>  	    xtype: 'proxmoxcheckbox',
> +	    itemId: 'sdnVnetVlanAwareField',
>  	    name: 'vlanaware',
>  	    uncheckedValue: null,
>  	    checked: false,
> @@ -63,6 +83,26 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>  	    },
>  	},
>      ],
> +
> +    setZoneType: function(zoneType) {
> +	let me = this;
> +
> +	let tagField = me.down('#sdnVnetTagField');
> +	if (!zoneType || zoneType === 'simple') {
> +	    tagField.setVisible(false);
> +	    tagField.setValue('');
> +	} else {
> +	    tagField.setVisible(true);
> +	}
> +
> +	let vlanField = me.down('#sdnVnetVlanAwareField');
> +	if (!zoneType || zoneType === 'evpn') {
> +	    vlanField.setVisible(false);
> +	    vlanField.setValue('');
> +	} else {
> +	    vlanField.setVisible(true);
> +	}
> +    },
>  });
>  
>  Ext.define('PVE.sdn.VnetEdit', {

Re: [pve-devel] [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[Theodor Fumics via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 8232 bytes --]

From: Theodor Fumics <theodor.fumics@gmx.net>
To: Stefan Hanreich <s.hanreich@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type
Date: Fri, 26 Jul 2024 14:51:20 +0200
Message-ID: <075ee2d4-c1c7-4a3e-8c25-e17ca67ec8a9@gmx.net>

I’ve tested the changes thoroughly. The fields in the VnetEdit dialog
now correctly hide based on the selected zone type. For the EVPN zone
type, the VLAN Aware field is hidden, and for the simple zone type, the
tag field is correctly hidden. Everything is working perfectly.

Tested-by: Theodor Fumics <theodor.fumics@gmx.net>
Reviewed-by: Theodor Fumics <theodor.fumics@gmx.net>

On 12/22/23 11:43, Stefan Hanreich wrote:
> Not all fields in the VnetEdit dialog are necessary for every zone
> type. This lead to confusion for some users. Hide fields in the
> VNetEdit dialog depending on which kind of zone is selected in order
> to prevent potential confusion.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   www/manager6/form/SDNZoneSelector.js |  2 +-
>   www/manager6/sdn/VnetEdit.js         | 40 ++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/www/manager6/form/SDNZoneSelector.js b/www/manager6/form/SDNZoneSelector.js
> index 28c3457d2..0d0327529 100644
> --- a/www/manager6/form/SDNZoneSelector.js
> +++ b/www/manager6/form/SDNZoneSelector.js
> @@ -40,7 +40,7 @@ Ext.define('PVE.form.SDNZoneSelector', {
>   }, function() {
>       Ext.define('pve-sdn-zone', {
>   	extend: 'Ext.data.Model',
> -	fields: ['zone'],
> +	fields: ['zone', 'type'],
>   	proxy: {
>               type: 'proxmox',
>   	    url: "/api2/json/cluster/sdn/zones",
> diff --git a/www/manager6/sdn/VnetEdit.js b/www/manager6/sdn/VnetEdit.js
> index cdd83ed40..9fb6cd6c7 100644
> --- a/www/manager6/sdn/VnetEdit.js
> +++ b/www/manager6/sdn/VnetEdit.js
> @@ -12,6 +12,13 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>   	return values;
>       },
>
> +    initComponent: function() {
> +	let me = this;
> +
> +	me.callParent();
> +	me.setZoneType(undefined);
> +    },
> +
>       items: [
>   	{
>   	    xtype: 'pmxDisplayEditField',
> @@ -40,9 +47,21 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>   	    name: 'zone',
>   	    value: '',
>   	    allowBlank: false,
> +	    listeners: {
> +		change: function() {
> +		    let me = this;
> +
> +		    let record = me.findRecordByValue(me.value);
> +		    let zoneType = record?.data?.type;
> +
> +		    let panel = me.up('panel');
> +		    panel.setZoneType(zoneType);
> +		},
> +	    },
>   	},
>   	{
>   	    xtype: 'proxmoxintegerfield',
> +	    itemId: 'sdnVnetTagField',
>   	    name: 'tag',
>   	    minValue: 1,
>   	    maxValue: 16777216,
> @@ -54,6 +73,7 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>   	},
>   	{
>   	    xtype: 'proxmoxcheckbox',
> +	    itemId: 'sdnVnetVlanAwareField',
>   	    name: 'vlanaware',
>   	    uncheckedValue: null,
>   	    checked: false,
> @@ -63,6 +83,26 @@ Ext.define('PVE.sdn.VnetInputPanel', {
>   	    },
>   	},
>       ],
> +
> +    setZoneType: function(zoneType) {
> +	let me = this;
> +
> +	let tagField = me.down('#sdnVnetTagField');
> +	if (!zoneType || zoneType === 'simple') {
> +	    tagField.setVisible(false);
> +	    tagField.setValue('');
> +	} else {
> +	    tagField.setVisible(true);
> +	}
> +
> +	let vlanField = me.down('#sdnVnetVlanAwareField');
> +	if (!zoneType || zoneType === 'evpn') {
> +	    vlanField.setVisible(false);
> +	    vlanField.setValue('');
> +	} else {
> +	    vlanField.setVisible(true);
> +	}
> +    },
>   });
>
>   Ext.define('PVE.sdn.VnetEdit', {


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[Thomas Lamprecht]

Am 22/12/2023 um 11:43 schrieb Stefan Hanreich:
> Not all fields in the VnetEdit dialog are necessary for every zone
> type. This lead to confusion for some users. Hide fields in the
> VNetEdit dialog depending on which kind of zone is selected in order
> to prevent potential confusion.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  www/manager6/form/SDNZoneSelector.js |  2 +-
>  www/manager6/sdn/VnetEdit.js         | 40 ++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

albeit, it might be slightly nicer to also disable the field when it's not
relevant, as then any validation handling cannot make the form invalid without
the user seeing why nor being able to fix it.

As you clear the value to a safe one for this specific fields, it should be
not cause any issues here though.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] applied: [PATCH pve-manager] sdn: vnets: Hide irrelevant fields depending on zone type

[Stefan Hanreich]

On 7/30/24 20:01, Thomas Lamprecht wrote:
> albeit, it might be slightly nicer to also disable the field when it's not
> relevant, as then any validation handling cannot make the form invalid without
> the user seeing why nor being able to fix it.

Yes, thinking about it again that would have been the better way to be
honest.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel