From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id A3FB61FF168
	for <inbox@lore.proxmox.com>; Tue, 12 Nov 2024 10:03:23 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id BB9861B45A;
	Tue, 12 Nov 2024 10:03:21 +0100 (CET)
Message-ID: <174cf2dd-3888-4c28-a70f-db0056fee8a7@proxmox.com>
Date: Tue, 12 Nov 2024 10:03:17 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20241002131157.227292-1-a.lauterer@proxmox.com>
 <20241002131157.227292-5-a.lauterer@proxmox.com>
 <236fd07d-aa1d-4ae2-96c2-eb747bd5eb6a@proxmox.com>
Content-Language: en-US
From: Aaron Lauterer <a.lauterer@proxmox.com>
In-Reply-To: <236fd07d-aa1d-4ae2-96c2-eb747bd5eb6a@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.036 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH widget-toolkit v5 4/7] fix #3892: Network:
 add bridge vids field for bridge_vids
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>



On  2024-11-11  21:55, Thomas Lamprecht wrote:
> Am 02.10.24 um 15:11 schrieb Aaron Lauterer:
>> The new optional bridge_vids field allows to set that property via the
>> GUI. Since the backend needs to support it, the field needs to be
>> explicitly enabled.
>>
>> For now, Proxmox VE (PVE) is the use case.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
>> Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>> changes since
>> v4: none
>> v3:
>> * switched regex to one with non-capturing group
>> * reworked valid VLAN check according to the suggestion
>> v2:
>> * added validation code following how it is implemented in the API
>>
>>   src/node/NetworkEdit.js | 65 +++++++++++++++++++++++++++++++++++++++++
>>   src/node/NetworkView.js |  5 ++++
>>   2 files changed, 70 insertions(+)
>>
>> diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
>> index 27c1baf..bfd0268 100644
>> --- a/src/node/NetworkEdit.js
>> +++ b/src/node/NetworkEdit.js
>> @@ -2,6 +2,9 @@ Ext.define('Proxmox.node.NetworkEdit', {
>>       extend: 'Proxmox.window.Edit',
>>       alias: ['widget.proxmoxNodeNetworkEdit'],
>>   
>> +    // Enable to show the VLAN ID field
>> +    bridge_set_vids: false,
>> +
>>       initComponent: function() {
>>   	let me = this;
>>   
>> @@ -57,11 +60,70 @@ Ext.define('Proxmox.node.NetworkEdit', {
>>   	}
>>   
>>   	if (me.iftype === 'bridge') {
>> +	    let vids = Ext.create('Ext.form.field.Text', {
>> +		fieldLabel: gettext('Bridge VIDS'),
> 
> I know ifupdown2 names it VIDS, but is that really a good name here?
> AFAICT it's not used outside of ifupdown2/cumuls, or do you got any references?
> 
> Maybe "Bridge VLAN IDs" would be a bit more telling?

I tested it, and that is long enough to cause a line break in the label. 
I think I would opt for "VLAN IDs". It is only present on linux bridges 
and with the info box below as well it should be clear what it is for, 
without causing layout issues.

Will send a small v6 with the other suggestions too.




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