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.laute...@proxmox.com> Tested-By: Stefan Hanreich <s.hanre...@proxmox.com> Reviewed-by: Shannon Sterz <s.st...@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
