[PATCH manager] sdn: subnets: hide irrelevant fields depending on zone type
Theodor Fumics
theodor.fumics at gmx.net
Fri Jul 26 16:09:38 CEST 2024
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 at 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
>
More information about the pve-devel
mailing list