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

[Daniel Tschlatscher <d.tschlatscher@proxmox.com>]

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.