Re: [PATCH pve-manager 1/1] fabrics: wireguard: node edit: use network selector

[Dominik Csapak <d.csapak@proxmox.com>]

one comment inline, rest LGTM

On 6/17/26 3:57 PM, Stefan Hanreich wrote:
> For internal nodes, turn the endpoint form field into a network
> selector that allows users to select IP addresses from a dropdown.
> This makes the intention of the field clearer and also saves users
> some typing since they can just select any configured IP on the node
> without having to re-type it.
> 
> The defaultFocus property is set for the node edit window, to avoid
> the network selector getting autofocused which leads to an ugly,
> mispositioned combogrid getting rendered automatically when the edit
> window gets rendered initially. Focus on the form can still be gained
> by pressing Tab an additional time.
> 
> Suggested-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   .../sdn/fabrics/wireguard/NodeEdit.js         | 38 ++++++++++++++++++-
>   1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/sdn/fabrics/wireguard/NodeEdit.js b/www/manager6/sdn/fabrics/wireguard/NodeEdit.js
> index fe4639ade..f0d513162 100644
> --- a/www/manager6/sdn/fabrics/wireguard/NodeEdit.js
> +++ b/www/manager6/sdn/fabrics/wireguard/NodeEdit.js
> @@ -27,6 +27,8 @@ Ext.define('PVE.sdn.Fabric.WireGuard.Node.Edit', {
>           },
>       },
>   
> +    defaultFocus: 'no',
> +
>       additionalItems: [
>           {
>               xtype: 'proxmoxtextfield',
> @@ -56,12 +58,37 @@ Ext.define('PVE.sdn.Fabric.WireGuard.Node.Edit', {
>               allowBlank: false,
>           },
>           {
> -            xtype: 'proxmoxtextfield',
> +            // endpoint field for internal nodes
> +            xtype: 'proxmoxNetworkSelector',
> +            reference: 'networkSelector',
>               fieldLabel: gettext('Endpoint'),
>               emptyText: gettext('Host that peers connect to'),
>               labelWidth: 120,
>               name: 'endpoint',
> -            allowBlank: false,
> +            type: 'any_bridge',
> +            valueField: 'address',
> +            displayField: 'address',
> +            autoSelect: false,
> +            notFoundIsValid: true,
> +            editable: true,
> +            bind: {
> +                allowBlank: '{current.isPveNode}',
> +                hidden: '{!current.isPveNode}',
> +                disabled: '{!current.isPveNode}',

allowBlank here is true when 'current.isPveNode' but that
is the exact value when this field is shown and enabled.

since allowBlank does not really do anything for disabled
fields, why not simply have 'allowBlank: true' in the fields config?

> +            },
> +        },
> +        {
> +            // endpoint field for external nodes
> +            xtype: 'proxmoxtextfield',
> +            fieldLabel: gettext('Endpoint'),
> +            emptyText: gettext('Host that peers connect to qqq'),
> +            labelWidth: 120,
> +            name: 'endpoint',
> +            bind: {
> +                allowBlank: '{!current.isPveNode}',
> +                hidden: '{current.isPveNode}',
> +                disabled: '{current.isPveNode}',
> +            },

same principle here

If wanted i can fix that up on applying though...

>           },
>           {
>               xtype: 'proxmoxtextfield',
> @@ -167,6 +194,13 @@ Ext.define('PVE.sdn.Fabric.WireGuard.Node.Edit', {
>                       },
>                   },
>               },
> +            listeners: {
> +                change: function (_this, newValue) {
> +                    if (me.getViewModel().get('current.isPveNode')) {
> +                        me.lookupReference('networkSelector').setNodename(newValue);
> +                    }
> +                },
> +            },
>           });
>   
>           return config;