* 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* Re: [pve-devel] [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
2024-07-26 13:42 ` [pve-devel] [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type Shannon Sterz
@ 2024-07-26 14:09 ` Theodor Fumics via pve-devel
0 siblings, 0 replies; 3+ messages in thread
From: Theodor Fumics via pve-devel @ 2024-07-26 14:09 UTC (permalink / raw)
To: Shannon Sterz, pve-devel; +Cc: Theodor Fumics
[-- 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pve-devel] [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
@ 2024-07-26 13:27 Theodor Fumics via pve-devel
0 siblings, 0 replies; 3+ messages in thread
From: Theodor Fumics via pve-devel @ 2024-07-26 13:27 UTC (permalink / raw)
To: pve-devel; +Cc: Theodor Fumics
[-- Attachment #1: Type: message/rfc822, Size: 8279 bytes --]
From: Theodor Fumics <theodor.fumics@gmx.net>
To: pve-devel@lists.proxmox.com
Subject: [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
Date: Fri, 26 Jul 2024 15:27:45 +0200
Message-ID: <20240726132745.168971-1-theodor.fumics@gmx.net>
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);
+ },
});
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-26 14:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20240726132745.168971-1-theodor.fumics@gmx.net>
2024-07-26 13:42 ` [pve-devel] [PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type Shannon Sterz
2024-07-26 14:09 ` Theodor Fumics via pve-devel
2024-07-26 13:27 Theodor Fumics via pve-devel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox