all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH manager] fix #8184: ui: settings: add 'remote resizing' option to noVNC defaults
Date: Fri, 23 Oct 2026 12:22:38 +0200	[thread overview]
Message-ID: <DMC56CI8QADP.2EWKOLZ1DJYIB@proxmox.com> (raw)
In-Reply-To: <20261022140030.176756-1-d.csapak@proxmox.com>

1 minor nit inline, otherwise LGTM!

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 22, 2026 at 4:00 PM CEST, Dominik Csapak wrote:
> This option was introduced after the 'my settings' window was
> implemented, so add it.
>
> Since the number of options would clutter the screen when using radio
> buttons, use a standard drop down instead.
>
> Use '__default__' as default value to stay consistent with other
> dropdowns. (The previous 'auto' value was never saved in the browser
> local storage anyway).
>
> While at it, use gettext for the text so they can be translated.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/window/Settings.js | 45 ++++++++++-----------------------
>  1 file changed, 13 insertions(+), 32 deletions(-)
>
> diff --git a/www/manager6/window/Settings.js b/www/manager6/window/Settings.js
> index 77c86c2a..98670865 100644
> --- a/www/manager6/window/Settings.js
> +++ b/www/manager6/window/Settings.js
> @@ -34,8 +34,8 @@ Ext.define('PVE.window.Settings', {
>  
>              var username = sp.get('login-username') || Proxmox.Utils.noneText;
>              me.lookupReference('savedUserName').setValue(Ext.String.htmlEncode(username));
> -            var vncMode = sp.get('novnc-scaling') || 'auto';
> -            me.lookupReference('noVNCScalingGroup').setValue({ noVNCScalingField: vncMode });
> +            var vncMode = sp.get('novnc-scaling') || '__default__';
> +            me.lookupReference('noVNCScaling').setValue(vncMode);
>  
>              let summarycolumns = sp.get('summarycolumns', 'auto');
>              me.lookup('summarycolumns').setValue(summarycolumns);
> @@ -394,42 +394,23 @@ Ext.define('PVE.window.Settings', {
>                      title: gettext('noVNC Settings'),
>                      items: [
>                          {
> -                            xtype: 'radiogroup',
> +                            xtype: 'proxmoxKVComboBox',
>                              fieldLabel: gettext('Scaling mode'),
> -                            reference: 'noVNCScalingGroup',
> -                            height: '15px', // renders faster with value assigned
> -                            layout: {
> -                                type: 'hbox',
> -                            },
> -                            items: [
> -                                {
> -                                    xtype: 'radiofield',
> -                                    name: 'noVNCScalingField',
> -                                    inputValue: 'auto',
> -                                    boxLabel: 'Auto',
> -                                },
> -                                {
> -                                    xtype: 'radiofield',
> -                                    name: 'noVNCScalingField',
> -                                    inputValue: 'scale',
> -                                    boxLabel: 'Local Scaling',
> -                                    margin: '0 0 0 10',
> -                                },
> -                                {
> -                                    xtype: 'radiofield',
> -                                    name: 'noVNCScalingField',
> -                                    inputValue: 'off',
> -                                    boxLabel: 'Off',
> -                                    margin: '0 0 0 10',
> -                                },
> +                            reference: 'noVNCScaling',
> +                            width: '100%',
> +                            comboItems: [
> +                                ['__default__', Proxmox.Utils.defaultText],
> +                                ['scale', gettext('Local Scaling')],
> +                                ['remote', gettext('Remote Resizing')],
> +                                ['off', gettext('Off')],

nit consistency: The proxmox noVNC settings display 'Scaling mode: Off'.
This results in the noVNC Setting 'Scaling mode: None'.

We could change it to:

['off', gettext('None')]

Since the acutal name 'off', has to stay, so that the URL contains: 
'resize=off' for the key/value pair.

>                              ],
>                              listeners: {
> -                                change: function (el, { noVNCScalingField }) {
> +                                change: function (el, value) {
>                                      let provider = Ext.state.Manager.getProvider();
> -                                    if (noVNCScalingField === 'auto') {
> +                                    if (!value || value === '__default__') {
>                                          provider.clear('novnc-scaling');
>                                      } else {
> -                                        provider.set('novnc-scaling', noVNCScalingField);
> +                                        provider.set('novnc-scaling', value);
>                                      }
>                                  },
>                              },





      reply	other threads:[~2026-10-23 10:22 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 14:00 [PATCH manager] fix #8184: ui: settings: add 'remote resizing' option to noVNC defaults Dominik Csapak
2026-10-23 10:22 ` Elias Huhsovitz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMC56CI8QADP.2EWKOLZ1DJYIB@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal