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

[Theodor Fumics via pve-devel <pve-devel@lists.proxmox.com>]

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

From: Theodor Fumics <theodor.fumics@gmx.net>
To: Shannon Sterz <s.sterz@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
Date: Fri, 26 Jul 2024 16:09:38 +0200
Message-ID: <6b9af9d2-b0e0-48ad-a064-8b91eb18a048@gmx.net>

Hi Shannon,

Thanks for catching that! I missed the use of var and have now sent a
revised version (v2) using let instead [1].


[1]
https://lore.proxmox.com/pve-devel/mailman.701.1722002701.331.pve-devel@lists.proxmox.com/T/#u

On 7/26/24 15:42, Shannon Sterz wrote:
> On Fri Jul 26, 2024 at 3:27 PM CEST, Theodor Fumics wrote:
>> The SNAT option is only applicable to Simple and EVPN zone types.
>> For other zone types, this field is irrelevant and can cause confusion.
>> This commit hides the SNAT field when it is not applicable to the
>> selected zone type, improving clarity for users.
>>
>> Signed-off-by: Theodor Fumics <theodor.fumics@gmx.net>
>> ---
>>   www/manager6/sdn/SubnetEdit.js | 18 ++++++++++++++++++
>>   www/manager6/sdn/SubnetView.js | 16 ++++++++++++++++
>>   www/manager6/sdn/VnetView.js   |  9 +++++++++
>>   3 files changed, 43 insertions(+)
>>
>> diff --git a/www/manager6/sdn/SubnetEdit.js b/www/manager6/sdn/SubnetEdit.js
>> index 8fc3f52b..d80eeca2 100644
>> --- a/www/manager6/sdn/SubnetEdit.js
>> +++ b/www/manager6/sdn/SubnetEdit.js
>> @@ -2,6 +2,8 @@ Ext.define('PVE.sdn.SubnetInputPanel', {
>>       extend: 'Proxmox.panel.InputPanel',
>>       mixins: ['Proxmox.Mixin.CBind'],
>>
>> +    zoneInfo: undefined,
>> +
>>       onGetValues: function(values) {
>>   	let me = this;
>>
>> @@ -38,6 +40,7 @@ Ext.define('PVE.sdn.SubnetInputPanel', {
>>   	},
>>   	{
>>   	    xtype: 'proxmoxcheckbox',
>> +	    itemId: 'snatSubnetCheckbox',
>>   	    name: 'snat',
>>   	    uncheckedValue: null,
>>   	    checked: false,
>> @@ -57,6 +60,18 @@ Ext.define('PVE.sdn.SubnetInputPanel', {
>>   	    },
>>   	},
>>       ],
>> +
>> +    initComponent: function() {
>> +	let me = this;
>> +	me.callParent();
>> +
>> +	let zoneType = me.zoneInfo?.type;
>> +
>> +	var showSNATCheckbox = ['simple', 'evpn'].includes(zoneType);
>> +
>> +	var snatCheckbox = me.down('#snatSubnetCheckbox');
>> +	snatCheckbox.setHidden(!showSNATCheckbox);
> hey thanks for your contribution, any reason you are using `var` instead
> of `let` here? our style guide says all new code should use `let` or
> `const` and i can't determine a reason why you'd need a `var` here.
> generally the behaviour of a `var` is also less predictable in regards
> to hoisting etc.
>
>> +    },
>>   });
>>
>>   Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
>> @@ -243,6 +258,8 @@ Ext.define('PVE.sdn.SubnetEdit', {
>>
>>       base_url: undefined,
>>
>> +    zoneInfo: undefined,
>> +
>>       bodyPadding: 0,
>>
>>       initComponent: function() {
>> @@ -261,6 +278,7 @@ Ext.define('PVE.sdn.SubnetEdit', {
>>   	let ipanel = Ext.create('PVE.sdn.SubnetInputPanel', {
>>   	    isCreate: me.isCreate,
>>   	    title: gettext('General'),
>> +	    zoneInfo: me.zoneInfo,
>>   	});
>>
>>   	let dhcpPanel = Ext.create('PVE.sdn.SubnetDhcpRangePanel', {
>> diff --git a/www/manager6/sdn/SubnetView.js b/www/manager6/sdn/SubnetView.js
>> index d342f0ba..fd103d78 100644
>> --- a/www/manager6/sdn/SubnetView.js
>> +++ b/www/manager6/sdn/SubnetView.js
>> @@ -7,6 +7,9 @@ Ext.define('PVE.sdn.SubnetView', {
>>
>>       base_url: undefined,
>>
>> +    zoneName: undefined,
>> +    zoneInfo: undefined,
>> +
>>       remove_btn: undefined,
>>
>>       setBaseUrl: function(url) {
>> @@ -28,6 +31,18 @@ Ext.define('PVE.sdn.SubnetView', {
>>   	}
>>       },
>>
>> +    loadZone: function(name) {
>> +	let me = this;
>> +
>> +	Proxmox.Utils.API2Request({
>> +	    url: `/cluster/sdn/zones/${name}?pending=1`,
>> +	    method: 'GET',
>> +	    success: function(response) {
>> +		me.zoneInfo = response?.result?.data;
>> +	    },
>> +	});
>> +    },
>> +
>>       initComponent: function() {
>>   	let me = this;
>>
>> @@ -59,6 +74,7 @@ Ext.define('PVE.sdn.SubnetView', {
>>   		let win = Ext.create('PVE.sdn.SubnetEdit', {
>>   		    autoShow: true,
>>   		    base_url: me.base_url,
>> +		    zoneInfo: me.zoneInfo,
>>   		    type: 'subnet',
>>   		});
>>   		win.on('destroy', reload);
>> diff --git a/www/manager6/sdn/VnetView.js b/www/manager6/sdn/VnetView.js
>> index 3fd3c916..9c9dcc5b 100644
>> --- a/www/manager6/sdn/VnetView.js
>> +++ b/www/manager6/sdn/VnetView.js
>> @@ -141,6 +141,15 @@ Ext.define('PVE.sdn.VnetView', {
>>   		select: function(_sm, rec) {
>>   		    let url = `/cluster/sdn/vnets/${rec.data.vnet}/subnets`;
>>   		    me.subnetview_panel.setBaseUrl(url);
>> +
>> +		    let zoneName;
>> +		    if (rec.data.pending) {
>> +			zoneName = rec.data.pending.zone;
>> +		    } else {
>> +			zoneName = rec.data.zone;
>> +		    }
>> +
>> +		    me.subnetview_panel.loadZone(zoneName);
>>   		},
>>   		deselect: function() {
>>   		    me.subnetview_panel.setBaseUrl(undefined);
>> --
>> 2.39.2
>


[-- 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