[pve-devel] [PATCH manager] Close #2262: Add noVNC scale setting

[Thomas Lamprecht]

On 07.08.19 13:10, Dominic Jäger wrote:
> Add a setting to choose the scale mode of the noVNC pop-up as well as the
> embedded console in the content panel to "My Settings". Having both set to
> local scaling was the most important use-case for the users. One setting for
> both places is the simplest solution making this possible.
> 
> Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
> ---
> Adding an option to My Settings was proposed by Dietmar.
> 
>  www/manager6/Utils.js           |  3 +-
>  www/manager6/VNCConsole.js      |  3 +-
>  www/manager6/window/Settings.js | 50 ++++++++++++++++++++++++++++++++-
>  3 files changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 6838ef2f..36732a37 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -960,13 +960,14 @@ Ext.define('PVE.Utils', { utilities: {
>      },
>  
>      openVNCViewer: function(vmtype, vmid, nodename, vmname, cmd) {
> +	var sp = Ext.state.Manager.getProvider();
>  	var url = Ext.Object.toQueryString({
>  	    console: vmtype, // kvm, lxc, upgrade or shell
>  	    novnc: 1,
>  	    vmid: vmid,
>  	    vmname: vmname,
>  	    node: nodename,
> -	    resize: 'off',
> +	    resize: sp.get('novnc-scaling'),
>  	    cmd: cmd
>  	});
>  	var nw = window.open("?" + url, '_blank', "innerWidth=745,innerheight=427");
> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
> index cd8fa243..22c82257 100644
> --- a/www/manager6/VNCConsole.js
> +++ b/www/manager6/VNCConsole.js
> @@ -41,12 +41,13 @@ Ext.define('PVE.noVncConsole', {
>  	    items: box,
>  	    listeners: {
>  		activate: function() {
> +		    var sp = Ext.state.Manager.getProvider();
>  		    var queryDict = {
>  			console: me.consoleType, // kvm, lxc, upgrade or shell
>  			vmid: me.vmid,
>  			node: me.nodename,
>  			cmd: me.cmd,
> -			resize: 'scale'
> +			resize: sp.get('novnc-scaling'),
>  		    };
>  		    queryDict[type] = 1;
>  		    PVE.Utils.cleanEmptyObjectKeys(queryDict);
> diff --git a/www/manager6/window/Settings.js b/www/manager6/window/Settings.js
> index 1a4d8599..0f7764ea 100644
> --- a/www/manager6/window/Settings.js
> +++ b/www/manager6/window/Settings.js
> @@ -37,6 +37,13 @@ Ext.define('PVE.window.Settings', {
>  
>  	    var username = sp.get('login-username') || Proxmox.Utils.noneText;
>  	    me.lookupReference('savedUserName').setValue(username);
> +	    var vncMode = sp.get('novnc-scaling');
> +	    if (vncMode === 'scale') {
> +		me.lookupReference('radio-vnc-scale').setValue(true);
> +	    } else {
> +		// vncMode is 'off'
> +		me.lookupReference('radio-vnc-off').setValue(true);
> +	    }

IMO a bit strange to read, one really thinks that there must be a nicer,
more concise way doing this.. And yes, there is, ExtJS radiogroup itself
provides the set/get/.. Value methods abstracting the radiobuttons itself
away so if you give the radiogroup (not it's fields) a reference instead
you can just do:

if (vncMode !== undefined) {
    me.lookupReference('noVNCScaling').setValue(vncMode);
}

(see below why I did not used hyphens in the reference name)

>  
>  	    var settings = ['fontSize', 'fontFamily', 'letterSpacing', 'lineHeight'];
>  	    settings.forEach(function(setting) {
> @@ -246,7 +253,48 @@ Ext.define('PVE.window.Settings', {
>  		    text: gettext('Reset Layout'),
>  		    width: 'auto',
>  		    name: 'reset'
> -		}
> +		},

maybe, place this in it's own section, mirroring xterm.js settings?

While now it's just a single one the "Local Cursor" could also be an
option we could add here..?

> +		{
> +		    xtype: 'box',
> +		    autoEl: { tag: 'hr'}
> +		},
> +		{
> +		    xtype: 'displayfield',
> +		    fieldLabel: gettext('noVNC scaling mode'),
> +		    labelAlign: 'left',
> +		    labelWidth: '50%'
> +		},
> +		{
> +		    layout: {
> +			type: 'hbox',
> +			align: 'middle'
> +		    },
> +		    border: false,
> +		    xtype: 'radiogroup',
> +		    items: [
> +			{
> +			    xtype: 'radiofield',
> +			    name: 'novnc-scaling',
> +			    inputValue: 'scale',
> +			    reference: 'radio-vnc-scale',

this is a invalid reference name (contains "-"), it works in ExtJS "production" mode,
but as soon as you enable debug mode ExtJS is a bit pickier and throws an exception
here. See above why you can just omit those references, you can then add a 
    checked: true,

to the default field.

> +			    boxLabel: 'Local Scaling',
> +			},
> +			{
> +			    xtype: 'radiofield',
> +			    name: 'novnc-scaling',
> +			    inputValue: 'off',
> +			    reference: 'radio-vnc-off',
> +			    boxLabel: 'Off',
> +			    margin: '0 0 0 10',
> +			},
> +		    ],
> +		    listeners: {
> +			change: function(el, newValue, undefined) {
> +			    var sp = Ext.state.Manager.getProvider();
> +			    sp.set('novnc-scaling', newValue['novnc-scaling']);
> +			}
> +		    },
> +		},
>  	    ]
>      },{
>  	xtype: 'fieldset',
>