[pve-devel] [PATCH manager v2] fix #3994: Options menu entry in the System menu

[pve-devel] [PATCH manager v2] fix #3994: Options menu entry in the System menu

[Daniel Tschlatscher]

Add the subentry "Options" in the "System" menu to expose some options
in the GUI which were not exposed before.

Added a new file for displaying and editing the node config options
which were not exposed through the GUI yet. Namely those are the
settings for wakeonlan and startall-on-boot-delay. Edited the Makefile
to include the newly created file.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v2:

- Moved the file from the widget-toolkit to the pve-manager repository
  because it implements PVE specific functionality.
- I originally rewrote this class to use a ViewController, but then
  found out that the ObjectGrid does most of what I wanted already, I
  just had to address it correctly.
  This means the code is now a bit shorter and a lot more concise.

 www/manager6/Makefile                |  1 +
 www/manager6/node/Config.js          |  9 ++++
 www/manager6/node/NodeOptionsView.js | 67 ++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 www/manager6/node/NodeOptionsView.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 2c7b1e70..d16770b1 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -195,6 +195,7 @@ JSSRC= 							\
 	node/Subscription.js				\
 	node/Summary.js					\
 	node/ZFS.js					\
+	node/NodeOptionsView.js				\
 	pool/Config.js					\
 	pool/StatusView.js				\
 	pool/Summary.js					\
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 52357df8..7e7d45f7 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -195,6 +195,15 @@ Ext.define('PVE.node.Config', {
 		    nodename: nodename,
 		    onlineHelp: 'sysadmin_network_configuration',
 		},
+		{
+		    xtype: 'proxmoxNodeOptionsView',
+		    title: gettext('Options'),
+		    iconCls: 'fa fa-gear',
+		    groups: ['services'],
+		    itemId: 'options',
+		    nodename: nodename,
+		    onlineHelp: 'proxmox_node_management',
+		},
 		{
 		    xtype: 'proxmoxNodeHostsView',
 		    title: gettext('Hosts'),
diff --git a/www/manager6/node/NodeOptionsView.js b/www/manager6/node/NodeOptionsView.js
new file mode 100644
index 00000000..b841b7b0
--- /dev/null
+++ b/www/manager6/node/NodeOptionsView.js
@@ -0,0 +1,67 @@
+Ext.define('Proxmox.node.NodeOptionsView', {
+    extend: 'Proxmox.grid.ObjectGrid',
+    alias: ['widget.proxmoxNodeOptionsView'],
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: function(_initialconfig) {
+	let me = this;
+
+	let baseUrl = `/nodes/${me.nodename}/config`;
+	me.url = `/api2/json${baseUrl}`;
+	me.editorConfig = {
+	    url: `/api2/extjs/${baseUrl}`,
+	};
+
+	return {};
+    },
+
+    listeners: {
+	itemdblclick: function() { this.run_editor(); },
+	activate: function() { this.rstore.startUpdate(); },
+	destroy: function() { this.rstore.stopUpdate(); },
+	deactivate: function() { this.rstore.stopUpdate(); },
+    },
+
+    tbar: [
+	{
+	    text: gettext('Edit'),
+	    xtype: 'proxmoxButton',
+	    disabled: true,
+	    handler: btn => btn.up('grid').run_editor(),
+	},
+    ],
+
+    gridRows: [
+	{
+	    xtype: 'integer',
+	    name: 'startall-onboot-delay',
+	    text: gettext('Start on boot delay'),
+	    minValue: 0,
+	    maxValue: 300,
+	    labelWidth: 130,
+	    deleteEmpty: true,
+	    renderer: function(value) {
+		if (value === undefined) {
+		    return Proxmox.Utils.defaultText;
+		}
+
+		let secString = value === '1' ? gettext('Second') : gettext('Seconds');
+		return `${value} ${secString}`;
+	    },
+	},
+	{
+	    xtype: 'text',
+	    name: 'wakeonlan',
+	    text: gettext('Wake on LAN'),
+	    vtype: 'MacAddress',
+	    deleteEmpty: true,
+	    renderer: function(value) {
+		if (value === undefined) {
+		    return Proxmox.Utils.NoneText;
+		}
+
+		return value;
+	    },
+	},
+    ],
+});
-- 
2.30.2

Re: [pve-devel] [PATCH manager v2] fix #3994: Options menu entry in the System menu

[Markus Frank]

I tested it on a vm-cluster.
GUI works as intended and only allows input which can be handled by the
backend. Node-Config-Files get updated on change in GUI, and vice versa.
I can also verify that startall-on-boot-delay works fine.

Tested-by: Markus Frank <m.frank@proxmox.com>

On 5/6/22 14:39, Daniel Tschlatscher wrote:
> Add the subentry "Options" in the "System" menu to expose some options
> in the GUI which were not exposed before.
> 
> Added a new file for displaying and editing the node config options
> which were not exposed through the GUI yet. Namely those are the
> settings for wakeonlan and startall-on-boot-delay. Edited the Makefile
> to include the newly created file.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
> ---
> Changes from v2:
> 
> - Moved the file from the widget-toolkit to the pve-manager repository
>    because it implements PVE specific functionality.
> - I originally rewrote this class to use a ViewController, but then
>    found out that the ObjectGrid does most of what I wanted already, I
>    just had to address it correctly.
>    This means the code is now a bit shorter and a lot more concise.
> 
>   www/manager6/Makefile                |  1 +
>   www/manager6/node/Config.js          |  9 ++++
>   www/manager6/node/NodeOptionsView.js | 67 ++++++++++++++++++++++++++++
>   3 files changed, 77 insertions(+)
>   create mode 100644 www/manager6/node/NodeOptionsView.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 2c7b1e70..d16770b1 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -195,6 +195,7 @@ JSSRC= 							\
>   	node/Subscription.js				\
>   	node/Summary.js					\
>   	node/ZFS.js					\
> +	node/NodeOptionsView.js				\
>   	pool/Config.js					\
>   	pool/StatusView.js				\
>   	pool/Summary.js					\
> diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
> index 52357df8..7e7d45f7 100644
> --- a/www/manager6/node/Config.js
> +++ b/www/manager6/node/Config.js
> @@ -195,6 +195,15 @@ Ext.define('PVE.node.Config', {
>   		    nodename: nodename,
>   		    onlineHelp: 'sysadmin_network_configuration',
>   		},
> +		{
> +		    xtype: 'proxmoxNodeOptionsView',
> +		    title: gettext('Options'),
> +		    iconCls: 'fa fa-gear',
> +		    groups: ['services'],
> +		    itemId: 'options',
> +		    nodename: nodename,
> +		    onlineHelp: 'proxmox_node_management',
> +		},
>   		{
>   		    xtype: 'proxmoxNodeHostsView',
>   		    title: gettext('Hosts'),
> diff --git a/www/manager6/node/NodeOptionsView.js b/www/manager6/node/NodeOptionsView.js
> new file mode 100644
> index 00000000..b841b7b0
> --- /dev/null
> +++ b/www/manager6/node/NodeOptionsView.js
> @@ -0,0 +1,67 @@
> +Ext.define('Proxmox.node.NodeOptionsView', {
> +    extend: 'Proxmox.grid.ObjectGrid',
> +    alias: ['widget.proxmoxNodeOptionsView'],
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    cbindData: function(_initialconfig) {
> +	let me = this;
> +
> +	let baseUrl = `/nodes/${me.nodename}/config`;
> +	me.url = `/api2/json${baseUrl}`;
> +	me.editorConfig = {
> +	    url: `/api2/extjs/${baseUrl}`,
> +	};
> +
> +	return {};
> +    },
> +
> +    listeners: {
> +	itemdblclick: function() { this.run_editor(); },
> +	activate: function() { this.rstore.startUpdate(); },
> +	destroy: function() { this.rstore.stopUpdate(); },
> +	deactivate: function() { this.rstore.stopUpdate(); },
> +    },
> +
> +    tbar: [
> +	{
> +	    text: gettext('Edit'),
> +	    xtype: 'proxmoxButton',
> +	    disabled: true,
> +	    handler: btn => btn.up('grid').run_editor(),
> +	},
> +    ],
> +
> +    gridRows: [
> +	{
> +	    xtype: 'integer',
> +	    name: 'startall-onboot-delay',
> +	    text: gettext('Start on boot delay'),
> +	    minValue: 0,
> +	    maxValue: 300,
> +	    labelWidth: 130,
> +	    deleteEmpty: true,
> +	    renderer: function(value) {
> +		if (value === undefined) {
> +		    return Proxmox.Utils.defaultText;
> +		}
> +
> +		let secString = value === '1' ? gettext('Second') : gettext('Seconds');
> +		return `${value} ${secString}`;
> +	    },
> +	},
> +	{
> +	    xtype: 'text',
> +	    name: 'wakeonlan',
> +	    text: gettext('Wake on LAN'),
> +	    vtype: 'MacAddress',
> +	    deleteEmpty: true,
> +	    renderer: function(value) {
> +		if (value === undefined) {
> +		    return Proxmox.Utils.NoneText;
> +		}
> +
> +		return value;
> +	    },
> +	},
> +    ],
> +});

[pve-devel] applied: [PATCH manager v2] fix #3994: Options menu entry in the System menu

[Thomas Lamprecht]

Am 5/6/22 um 14:39 schrieb Daniel Tschlatscher:
> Add the subentry "Options" in the "System" menu to expose some options
> in the GUI which were not exposed before.
> 
> Added a new file for displaying and editing the node config options
> which were not exposed through the GUI yet. Namely those are the
> settings for wakeonlan and startall-on-boot-delay. Edited the Makefile
> to include the newly created file.
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
> ---
> Changes from v2:
> 
> - Moved the file from the widget-toolkit to the pve-manager repository
>   because it implements PVE specific functionality.
> - I originally rewrote this class to use a ViewController, but then
>   found out that the ObjectGrid does most of what I wanted already, I
>   just had to address it correctly.
>   This means the code is now a bit shorter and a lot more concise.
> 
>  www/manager6/Makefile                |  1 +
>  www/manager6/node/Config.js          |  9 ++++
>  www/manager6/node/NodeOptionsView.js | 67 ++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
>  create mode 100644 www/manager6/node/NodeOptionsView.js
> 
>

applied, thanks!

I moved it one item below, as IMO DNS and Host are too related to be split by some
general "Options" navigation entry.

Some other points to improve:

- setting the online help, maybe to 'proxmox_node_management' which would make
  it point to:
  https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#proxmox_node_management
  (the chapter could maybe do good with some addition w.r.t. first start delay

- It's not clear what unit the start delay has, you could mention that explicitly,
  simplest way to do so is adding a ' (s)' to the fieldLabel after the gettext.

- Wake on LAN could do with a "Local MAC Address" emptyText, combined with the
  onlineHelp button it should then be clear enough.

can you please look into those?

FYI and not directly related:
if we ever expose the vzdump.conf options over GUI I'd also add them here in this
panel, as subpanel; would be a good use of all that empty space ;-)

Re: [pve-devel] applied: [PATCH manager v2] fix #3994: Options menu entry in the System menu

[Daniel Tschlatscher]

On 5/12/22 17:13, Thomas Lamprecht wrote:
> Am 5/6/22 um 14:39 schrieb Daniel Tschlatscher:
>> Add the subentry "Options" in the "System" menu to expose some options
>> in the GUI which were not exposed before.
>>
>> Added a new file for displaying and editing the node config options
>> which were not exposed through the GUI yet. Namely those are the
>> settings for wakeonlan and startall-on-boot-delay. Edited the Makefile
>> to include the newly created file.
>>
>> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
>> ---
>> Changes from v2:
>>
>> - Moved the file from the widget-toolkit to the pve-manager repository
>>    because it implements PVE specific functionality.
>> - I originally rewrote this class to use a ViewController, but then
>>    found out that the ObjectGrid does most of what I wanted already, I
>>    just had to address it correctly.
>>    This means the code is now a bit shorter and a lot more concise.
>>
>>   www/manager6/Makefile                |  1 +
>>   www/manager6/node/Config.js          |  9 ++++
>>   www/manager6/node/NodeOptionsView.js | 67 ++++++++++++++++++++++++++++
>>   3 files changed, 77 insertions(+)
>>   create mode 100644 www/manager6/node/NodeOptionsView.js
>>
>>
> applied, thanks!
>
> I moved it one item below, as IMO DNS and Host are too related to be split by some
> general "Options" navigation entry.
>
> Some other points to improve:
>
> - setting the online help, maybe to 'proxmox_node_management' which would make
>    it point to:
>    https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#proxmox_node_management
>    (the chapter could maybe do good with some addition w.r.t. first start delay

This sounds to me like a suggestion to set the online help for the 
panel, i.e. the
button in the top right corner. But that's already included in the patch.
Or are you suggesting to put it somewhere else too?

>
> - It's not clear what unit the start delay has, you could mention that explicitly,
>    simplest way to do so is adding a ' (s)' to the fieldLabel after the gettext.

The field does not display a value while displaying Default, though it 
explicitly states
'Seconds' after any value is set. I think it would become very clear to 
anybody then
what unit this setting represents.
Still, adding ' (s)' probably wouldn't kill me either. 😉

>
> - Wake on LAN could do with a "Local MAC Address" emptyText, combined with the
>    onlineHelp button it should then be clear enough.
That's a very good suggestion, I will look into it.
>
> can you please look into those?
>
> FYI and not directly related:
> if we ever expose the vzdump.conf options over GUI I'd also add them here in this
> panel, as subpanel; would be a good use of all that empty space ;-)
Noted.

Re: [pve-devel] applied: [PATCH manager v2] fix #3994: Options menu entry in the System menu

[Thomas Lamprecht]

Am 5/13/22 um 09:55 schrieb Daniel Tschlatscher:
> On 5/12/22 17:13, Thomas Lamprecht wrote:
>> Some other points to improve:
>>
>> - setting the online help, maybe to 'proxmox_node_management' which would make
>>    it point to:
>>    https://pve.proxmox.com/pve-docs/chapter-sysadmin.html#proxmox_node_management
>>    (the chapter could maybe do good with some addition w.r.t. first start delay
> 
> This sounds to me like a suggestion to set the online help for the panel, i.e. the
> button in the top right corner. But that's already included in the patch.
> Or are you suggesting to put it somewhere else too?

None of the edit windows have it set. the top right corner is somewhat nice
sometimes but the real useful one is the one from the edit window, which is
much more visible, and especially the only one available if the window is open
as the background is masked then, having to close the window with some values
already half entered is the worst.

> 
>>
>> - It's not clear what unit the start delay has, you could mention that explicitly,
>>    simplest way to do so is adding a ' (s)' to the fieldLabel after the gettext.
> 
> The field does not display a value while displaying Default, though it explicitly states
> 'Seconds' after any value is set. I think it would become very clear to anybody then

That's too late, I need to know what I'm entering *on entering* not sometimes
later, when I already set a possible bogus value; while it hasn't that big
of an affect in this specific case it should be the general approach to UX
that the user knows what they enter, otherwise a UI is close to useless.

> what unit this setting represents.
> Still, adding ' (s)' probably wouldn't kill me either. 😉

Note that the "Default" in the grid is also wrong for unset, it should state "None".