From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 0BDD26404A for ; Tue, 1 Mar 2022 11:42:03 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E5CB61E3CD for ; Tue, 1 Mar 2022 11:41:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 588AB1E38C for ; Tue, 1 Mar 2022 11:41:31 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 319E446B51; Tue, 1 Mar 2022 11:41:31 +0100 (CET) Message-ID: <27af644c-3a2b-dd89-b843-e846fd5ba674@proxmox.com> Date: Tue, 1 Mar 2022 11:41:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:98.0) Gecko/20100101 Thunderbird/98.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Stefan Sterz References: <20220224141854.3153101-1-s.sterz@proxmox.com> <20220224141854.3153101-3-s.sterz@proxmox.com> From: Dominik Csapak In-Reply-To: <20220224141854.3153101-3-s.sterz@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.155 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Mar 2022 10:42:03 -0000 high level comment: i get what you tried to achieve here with the container for info and notes, but i'd probably simply add it as a regular panel. that way, the user is not confused when the column selection does not work the way he expects (e.g. setting it to 3 columns still always has only the info/note as the first line) so either a regular panel (without the collapsing feature, maybe a browser storage option to hide it? like the days) or let it behave like on the datastore summary (info+notes = 1 panel) some comments inline On 2/24/22 15:18, Stefan Sterz wrote: > adds a panel to the dashboard displaying a comment similar to the > comments panel in a datastore summary > > Signed-off-by: Stefan Sterz > --- > www/Dashboard.js | 110 ++++++++++++++++++------------ > www/Makefile | 2 +- > www/datastore/Summary.js | 6 +- > www/{datastore => panel}/Notes.js | 19 +++++- > 4 files changed, 85 insertions(+), 52 deletions(-) > rename www/{datastore => panel}/Notes.js (81%) > > diff --git a/www/Dashboard.js b/www/Dashboard.js > index 70c2305b..a78ad375 100644 > --- a/www/Dashboard.js > +++ b/www/Dashboard.js > @@ -122,7 +122,7 @@ Ext.define('PBS.Dashboard', { > if (key !== 'summarycolumns') { > return; > } > - Proxmox.Utils.updateColumns(view); > + Proxmox.Utils.updateColumnWidth(view); this does not work like that? you'd have to use the 'view.query...' expression like below else the child panels do not get updated > }); > }, > }, > @@ -192,26 +192,14 @@ Ext.define('PBS.Dashboard', { > > listeners: { > resize: function(panel) { > - Proxmox.Utils.updateColumns(panel); > + panel.query('>').forEach(c => Proxmox.Utils.updateColumnWidth(c)); > }, > }, > > title: gettext('Dashboard'), > > - layout: { > - type: 'column', > - }, > - > bodyPadding: '20 0 0 20', > > - minWidth: 700, > - > - defaults: { > - columnWidth: 0.49, > - xtype: 'panel', > - margin: '0 20 20 0', > - }, > - > tools: [ > { > type: 'gear', > @@ -224,42 +212,74 @@ Ext.define('PBS.Dashboard', { > > items: [ > { > - xtype: 'pbsNodeInfoPanel', > - reference: 'nodeInfo', > - height: 280, > - }, > - { > - xtype: 'pbsDatastoresStatistics', > + xtype: 'container', > + layout: { > + type: 'hbox', > + align: 'stretch', > + }, > + margin: '0 0 20 0', > + padding: '0 20 0 0', > height: 280, > - }, > - { > - xtype: 'pbsLongestTasks', > - bind: { > - title: gettext('Longest Tasks') + ' (' + > - Ext.String.format(gettext('{0} days'), '{days}') + ')', > + defaults: { > + flex: 1, > }, > - reference: 'longesttasks', > - height: 250, > - }, > - { > - xtype: 'pbsRunningTasks', > - height: 250, > - }, > - { > - bind: { > - title: gettext('Task Summary') + ' (' + > - Ext.String.format(gettext('{0} days'), '{days}') + ')', > + minWidth: 700, > + items: [{ > + xtype: 'pbsNodeInfoPanel', > + reference: 'nodeInfo', > + margin: '0 20 0 0', > }, > - xtype: 'pbsTaskSummary', > - height: 200, > - reference: 'tasksummary', > + { > + xtype: 'pbsNotes', > + reference: 'nodeNotes', > + node: 'localhost', > + loadOnInit: true, > + }], > }, > { > - iconCls: 'fa fa-ticket', > - title: 'Subscription', > - height: 200, > - reference: 'subscription', > - xtype: 'pbsSubscriptionInfo', > + xtype: 'container', > + layout: { > + type: 'column', > + }, > + minWidth: 700, > + defaults: { > + columnWidth: 0.5, > + xtype: 'panel', > + margin: '0 20 20 0', > + }, > + items: [{ > + xtype: 'pbsDatastoresStatistics', > + height: 250, > + }, > + { > + xtype: 'pbsLongestTasks', > + bind: { > + title: gettext('Longest Tasks') + ' (' + > + Ext.String.format(gettext('{0} days'), '{days}') + ')', > + }, > + reference: 'longesttasks', > + height: 250, > + }, > + { > + xtype: 'pbsRunningTasks', > + height: 250, > + }, > + { > + bind: { > + title: gettext('Task Summary') + ' (' + > + Ext.String.format(gettext('{0} days'), '{days}') + ')', > + }, > + xtype: 'pbsTaskSummary', > + height: 250, > + reference: 'tasksummary', > + }, > + { > + iconCls: 'fa fa-ticket', > + title: 'Subscription', > + height: 200, > + reference: 'subscription', > + xtype: 'pbsSubscriptionInfo', > + }], > }, > ], > }); > diff --git a/www/Makefile b/www/Makefile > index 455fbeec..636d4a57 100644 > --- a/www/Makefile > +++ b/www/Makefile > @@ -81,6 +81,7 @@ JSSRC= \ > panel/StorageAndDisks.js \ > panel/UsageChart.js \ > panel/NodeInfo.js \ > + panel/Notes.js \ > ZFSList.js \ > DirectoryList.js \ > LoginView.js \ > @@ -88,7 +89,6 @@ JSSRC= \ > SystemConfiguration.js \ > Subscription.js \ > datastore/Summary.js \ > - datastore/Notes.js \ > datastore/PruneAndGC.js \ > datastore/Prune.js \ > datastore/Content.js \ > diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js > index c3769257..d11646f0 100644 > --- a/www/datastore/Summary.js > +++ b/www/datastore/Summary.js > @@ -206,7 +206,7 @@ Ext.define('PBS.DataStoreSummary', { > }, > }, > { > - xtype: 'pbsDataStoreNotes', > + xtype: 'pbsNotes', > flex: 1, > cbind: { > datastore: '{datastore}', > @@ -278,14 +278,14 @@ Ext.define('PBS.DataStoreSummary', { > success: function(response) { > let path = Ext.htmlEncode(response.result.data.path); > me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`); > - me.down('pbsDataStoreNotes').setNotes(response.result.data.comment); > + me.down('pbsNotes').setNotes(response.result.data.comment); > }, > failure: function(response) { > // fallback if e.g. we have no permissions to the config > let rec = Ext.getStore('pbs-datastore-list') > .findRecord('store', me.datastore, 0, false, true, true); > if (rec) { > - me.down('pbsDataStoreNotes').setNotes(rec.data.comment || ""); > + me.down('pbsNotes').setNotes(rec.data.comment || ""); > } > }, > }); > diff --git a/www/datastore/Notes.js b/www/panel/Notes.js > similarity index 81% > rename from www/datastore/Notes.js > rename to www/panel/Notes.js > index 2928b7ec..4128dd8b 100644 > --- a/www/datastore/Notes.js > +++ b/www/panel/Notes.js > @@ -1,6 +1,6 @@ > -Ext.define('PBS.DataStoreNotes', { > +Ext.define('PBS.panel.Notes', { imo, the changes below would warrant its own patch (makes it easier to review) > extend: 'Ext.panel.Panel', > - xtype: 'pbsDataStoreNotes', > + xtype: 'pbsNotes', > mixins: ['Proxmox.Mixin.CBind'], > > title: gettext("Comment"), > @@ -11,7 +11,16 @@ Ext.define('PBS.DataStoreNotes', { > > cbindData: function(initalConfig) { > let me = this; > - me.url = `/api2/extjs/config/datastore/${me.datastore}`; > + > + if (('node' in me && 'datastore' in me) || > + (!('node' in me) && !('datastore' in me))) { > + throw 'either both a node and a datastore were given or neither. only provide one.'; i'd do that check differently for multiple reasons "'foo' in bar" is not syntax we really use having a single error for multiple error cases seems complicated something like this is more readable: --- if (!me.node && !me.datastore) { throw "no ..."; } if (me.node && me.datastore) { throw "both ...."; } -- alternatively, you could prefer the 'node' (or datastore, doesn't matter) instead and only throw an error if none is given > + } else if ('node' in me) { > + me.url = `/api2/extjs/nodes/${me.node}/config`; > + } else { > + me.url = `/api2/extjs/config/datastore/${me.datastore}`; > + } > + > return { }; > }, > > @@ -97,6 +106,10 @@ Ext.define('PBS.DataStoreNotes', { > let sp = Ext.state.Manager.getProvider(); > me.collapseMode = sp.get('notes-collapse', 'never'); > > + if (me.loadOnInit === true) { > + me.load(); > + } > + > if (me.collapseMode === 'auto') { > me.setCollapsed(true); > }