* [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type @ 2022-08-08 11:48 Matthias Heiserer 2022-09-09 6:20 ` Thomas Lamprecht 0 siblings, 1 reply; 3+ messages in thread From: Matthias Heiserer @ 2022-08-08 11:48 UTC (permalink / raw) To: pve-devel --- I'm a bit unsure about the icons, there are probably better ones, but I couldn't think of/find any. One alternative for OS type would be the OS logos. www/manager6/.lint-incremental | 0 www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 www/manager6/.lint-incremental diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental new file mode 100644 index 00000000..e69de29b diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js index 8db1f492..96e9adea 100644 --- a/www/manager6/panel/GuestStatusView.js +++ b/www/manager6/panel/GuestStatusView.js @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', { }; }, + controller: { + xclass: 'Ext.app.ViewController', + + init: function(view) { + const nodename = view.pveSelNode.get("node"); + const id = view.pveSelNode.get("vmid"); + const me = this; + Proxmox.Utils.API2Request({ + url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`, + waitMsgTarget: view, + method: 'GET', + success: function(response) { + const ostype = response.result.data.ostype; + const label = me.lookup("ostype").getComponent('label'); + label.update(Ext.apply(label.data, { + usage: ostype, + })); + const unprivileged = response.result.data.unprivileged; + me.lookup('privileged').updateValue(!unprivileged); + }, + }); + }, + }, + layout: { type: 'vbox', align: 'stretch', @@ -58,6 +82,26 @@ Ext.define('PVE.panel.GuestStatusView', { }, printBar: false, }, + { + title: gettext('OS type'), + printBar: false, + reference: 'ostype', + cbind: { + hidden: '{isQemu}', + disabled: '{isQemu}', + }, + iconCls: 'fa fa-mouse-pointer fa-fw', + }, + { + title: gettext('Privileged'), + printBar: false, + reference: 'privileged', + cbind: { + hidden: '{isQemu}', + disabled: '{isQemu}', + }, + iconCls: 'fa fa-user-circle-o fa-fw', + }, { xtype: 'box', height: 15, -- 2.30.2 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type 2022-08-08 11:48 [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type Matthias Heiserer @ 2022-09-09 6:20 ` Thomas Lamprecht 2022-09-16 12:13 ` Matthias Heiserer 0 siblings, 1 reply; 3+ messages in thread From: Thomas Lamprecht @ 2022-09-09 6:20 UTC (permalink / raw) To: Proxmox VE development discussion, Matthias Heiserer Can you please split this into two commits, first the unpriv one and then the OS type one? Am 08/08/2022 um 13:48 schrieb Matthias Heiserer: > --- > > I'm a bit unsure about the icons, there are probably better ones, but I couldn't > think of/find any. One alternative for OS type would be the OS logos. That'd be nice(r) and make it stand out more. But, while we already got the Debian one available (currently used for the apt repo gui), you'd need to assemble the other ones yourself, ensuring to only pull the official ones with a compatible license. After a quick search I found a project[0] that would seem to provide the desired logos, we could pull out the for us relevant one and ship it in manager, or alternatively create a full new package with all logos and CSS files exposed, wouldn't be _that_ much more work and possible a bit cleaner. [0]: https://github.com/lukas-w/font-logos > > www/manager6/.lint-incremental | 0 > www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > create mode 100644 www/manager6/.lint-incremental > > diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental > new file mode 100644 > index 00000000..e69de29b > diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js > index 8db1f492..96e9adea 100644 > --- a/www/manager6/panel/GuestStatusView.js > +++ b/www/manager6/panel/GuestStatusView.js > @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', { > }; > }, > > + controller: { > + xclass: 'Ext.app.ViewController', > + > + init: function(view) { > + const nodename = view.pveSelNode.get("node"); > + const id = view.pveSelNode.get("vmid"); > + const me = this; > + Proxmox.Utils.API2Request({ > + url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`, > + waitMsgTarget: view, I could swear we got the config already available somewhere in the hierarchy there, and checking the debug console's network view I saw a config load in the summary panel, naturally without your patch applied yet ;-), for the pmxNotes panel. I really want to avoid loading the same stuff in the same view multiple times. As we don't need the config often in the LXC panels (just Summary and Network, otherwise we use the slightly odd /pending endpoint) I'd not add it in the PVE.lxc.Config, but rather load it once in the pveGuestSummary Then you can pass it along to the notes and status panels for reuse on declaration. To avoid forced dependency bump and browser cache issues please keep the widget toolkits notes view's config load as fallback. > + method: 'GET', > + success: function(response) { fyi: if you only ever need to access child properties and you can be sure that they exist (normally fine in the "success" callback) you could use some destructuring here: success: ({ result }) => { ... }, style wise you could naturally also do: success: ({ result: { data: { ostype, unprivileged } } }) => { ... } but it increases assumptions about the return type, as you do not check for definedness or use nullish coalescing below anyway it would stay at the same risk level w.r.t exceptions in this case, and as said, should be fine for the success callback (no hard feelings to either variant on my side) https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#unpacking_properties_from_objects_passed_as_a_function_parameter > + const ostype = response.result.data.ostype; > + const label = me.lookup("ostype").getComponent('label'); > + label.update(Ext.apply(label.data, { > + usage: ostype, > + })); > + const unprivileged = response.result.data.unprivileged; > + me.lookup('privileged').updateValue(!unprivileged); > + }, > + }); > + }, > + }, > + > layout: { > type: 'vbox', > align: 'stretch', > @@ -58,6 +82,26 @@ Ext.define('PVE.panel.GuestStatusView', { > }, > printBar: false, > }, > + { > + title: gettext('OS type'), > + printBar: false, > + reference: 'ostype', > + cbind: { > + hidden: '{isQemu}', > + disabled: '{isQemu}', > + }, > + iconCls: 'fa fa-mouse-pointer fa-fw', > + }, > + { > + title: gettext('Privileged'), > + printBar: false, > + reference: 'privileged', > + cbind: { > + hidden: '{isQemu}', > + disabled: '{isQemu}', > + }, > + iconCls: 'fa fa-user-circle-o fa-fw', > + }, > { > xtype: 'box', > height: 15, ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type 2022-09-09 6:20 ` Thomas Lamprecht @ 2022-09-16 12:13 ` Matthias Heiserer 0 siblings, 0 replies; 3+ messages in thread From: Matthias Heiserer @ 2022-09-16 12:13 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 09.09.2022 08:20, Thomas Lamprecht wrote: > Can you please split this into two commits, first the unpriv one and then the OS type one? > > Am 08/08/2022 um 13:48 schrieb Matthias Heiserer: >> --- >> >> I'm a bit unsure about the icons, there are probably better ones, but I couldn't >> think of/find any. One alternative for OS type would be the OS logos. > > That'd be nice(r) and make it stand out more. But, while we already got the Debian > one available (currently used for the apt repo gui), you'd need to assemble the other ones > yourself, ensuring to only pull the official ones with a compatible license. > > After a quick search I found a project[0] that would seem to provide the desired logos, > we could pull out the for us relevant one and ship it in manager, or alternatively create > a full new package with all logos and CSS files exposed, wouldn't be _that_ much more work > and possible a bit cleaner. > > [0]: https://github.com/lukas-w/font-logos > Sure >> >> www/manager6/.lint-incremental | 0 >> www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++ >> 2 files changed, 44 insertions(+) >> create mode 100644 www/manager6/.lint-incremental >> >> diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental >> new file mode 100644 >> index 00000000..e69de29b >> diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js >> index 8db1f492..96e9adea 100644 >> --- a/www/manager6/panel/GuestStatusView.js >> +++ b/www/manager6/panel/GuestStatusView.js >> @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', { >> }; >> }, >> >> + controller: { >> + xclass: 'Ext.app.ViewController', >> + >> + init: function(view) { >> + const nodename = view.pveSelNode.get("node"); >> + const id = view.pveSelNode.get("vmid"); >> + const me = this; >> + Proxmox.Utils.API2Request({ >> + url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`, >> + waitMsgTarget: view, > > I could swear we got the config already available somewhere in the hierarchy there, > and checking the debug console's network view I saw a config load in the summary > panel, naturally without your patch applied yet ;-), for the pmxNotes panel. > > I really want to avoid loading the same stuff in the same view multiple times. > > As we don't need the config often in the LXC panels (just Summary and Network, > otherwise we use the slightly odd /pending endpoint) I'd not add it in the > PVE.lxc.Config, but rather load it once in the pveGuestSummary > > Then you can pass it along to the notes and status panels for reuse on declaration. > To avoid forced dependency bump and browser cache issues please keep the widget > toolkits notes view's config load as fallback. Correct me if I'm wrong, but we can't pass it to Network (because different Tab), and passing to to pmxNotes would result in a circular dependency. I guess we can store it in some global object? > > >> + method: 'GET', >> + success: function(response) { > > fyi: if you only ever need to access child properties and you can be sure that they > exist (normally fine in the "success" callback) you could use some destructuring > here: > > success: ({ result }) => { ... }, > > style wise you could naturally also do: > > success: ({ result: { data: { ostype, unprivileged } } }) => { ... } > > but it increases assumptions about the return type, as you do not check for > definedness or use nullish coalescing below anyway it would stay at the same > risk level w.r.t exceptions in this case, and as said, should be fine for the > success callback (no hard feelings to either variant on my side) > In that case, I'll use the current format without destructuring, I feel like it's more readable. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-16 12:21 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-08 11:48 [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type Matthias Heiserer 2022-09-09 6:20 ` Thomas Lamprecht 2022-09-16 12:13 ` Matthias Heiserer
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.