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

* Re: [pve-devel] [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
       [not found] <20240726132745.168971-1-theodor.fumics@gmx.net>
@ 2024-07-26 13:42 ` Shannon Sterz
  2024-07-26 14:09   ` Theodor Fumics via pve-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Shannon Sterz @ 2024-07-26 13:42 UTC (permalink / raw)
  To: Theodor Fumics, pve-devel

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread