Re: [pve-devel] [PATCH widget-toolkit v5 5/7] Network: add explanation for bridge vids field

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 02.10.24 um 15:11 schrieb Aaron Lauterer:
> Make clear that it affects only out-/inbound traffic and can be used if
> the underlying physical NICs support only a limited number of VLANs when
> offloading is possible.

maybe something for the docs? As we use hints mostly to flag potential
dangerous/broken things in some edge cases or if the edit window only
covers that specific setting anyway.

> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
> changes since
> v4: none
> v3-follow-up:
> * reordered inside the patch series
> * reworded explanation according to suggestion
> 
>  src/node/NetworkEdit.js | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
> index bfd0268..2e8dc25 100644
> --- a/src/node/NetworkEdit.js
> +++ b/src/node/NetworkEdit.js
> @@ -112,6 +112,12 @@ Ext.define('Proxmox.node.NetworkEdit', {
>  		    return true;
>  		},
>  	    });
> +	    let vidsExplanation = Ext.create('Ext.form.field.Display', {
> +		    disabled: true,
> +		    userCls: 'pmx-hint',
> +		    value: 'Limits the number of VLANs forwarded by the bridge ports. Useful if ' +
> +			   'the physical NICs only support a limited number of offloaded VLANs.',

If this gets added, can we gettext this? Translators frequently asked about
having more covered by gettext, and we can only win, as in the "worst" case
no one translates this which will still show it in the original English, just
like when not using gettext at all. 

In that case you will need a longer line, as we currently do not support
gettext with argument wrapped over multiple lines.

And/Or shorten it a bit, like:

'Limits the number of VLANs forwarded by bridge ports, useful for NICs with restricted VLAN offloading support.'


> +	    });
>  	    column2.push({
>  		xtype: 'proxmoxcheckbox',
>  		fieldLabel: gettext('VLAN aware'),
> @@ -121,6 +127,7 @@ Ext.define('Proxmox.node.NetworkEdit', {
>  		    change: function(f, newVal) {
>  			if (me.bridge_set_vids) {
>  			    vids.setDisabled(!newVal);
> +			    vidsExplanation.setDisabled(!newVal);
>  			}
>  		    },
>  		},
> @@ -136,6 +143,7 @@ Ext.define('Proxmox.node.NetworkEdit', {
>  	    });
>  	    if (me.bridge_set_vids) {
>  		advancedColumn2.push(vids);
> +		advancedColumn2.push(vidsExplanation);
>  	    }
>  	} else if (me.iftype === 'OVSBridge') {
>  	    column2.push({



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