all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui
@ 2022-02-24 14:18 Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg Stefan Sterz
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

This series adds support for comments/notes to a PBS node similar to
PVE's datacenter or node notes. It's split in two parts, the first two
commits add support for a single line comment, similar to the comments
already available for datastores. The next three commits refactor this
into Markdown-based multi-line comments and add an additional tab to
the UI to provide more space. If prefered, I can squash the commits.

changes v2 (thanks @ Wolfgang Bumiller):

 * performance improvements when parsing/writing a node configuration
 * adjusted multi-line regex to remove superfluous "\s*"
 * better formatting of rust code

Stefan Sterz (5):
  fix #3067: api: add support for a comment field in node.cfg
  fix #3067: pbs ui: add support for a notes field in the dashboard
  fix #3067: api: add multi-line comments to node.cfg
  fix #3607: ui: make dashboard notes markdown capable
  fix #3607: ui: add a separate notes view for longer markdown notes

 pbs-api-types/src/lib.rs          |   9 ++
 src/api2/node/config.rs           |   4 +
 src/config/node.rs                |  14 ++-
 src/tools/config.rs               |  58 +++++++++++-
 www/Dashboard.js                  | 110 +++++++++++++---------
 www/Makefile                      |   4 +-
 www/NavigationTree.js             |   6 ++
 www/NodeNotes.js                  |  22 +++++
 www/datastore/Summary.js          |   6 +-
 www/panel/MarkdownNotes.js        | 151 ++++++++++++++++++++++++++++++
 www/{datastore => panel}/Notes.js |  19 +++-
 11 files changed, 348 insertions(+), 55 deletions(-)
 create mode 100644 www/NodeNotes.js
 create mode 100644 www/panel/MarkdownNotes.js
 rename www/{datastore => panel}/Notes.js (81%)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg
  2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
@ 2022-02-24 14:18 ` Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard Stefan Sterz
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 src/api2/node/config.rs |  4 ++++
 src/config/node.rs      | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 0a119354..77fe69fe 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -64,6 +64,8 @@ pub enum DeletableProperty {
     ciphers_tls_1_2,
     /// Delete the default-lang property.
     default_lang,
+    /// Delete any comment
+    comment,
 }
 
 #[api(
@@ -124,6 +126,7 @@ pub fn update_node_config(
                 DeletableProperty::ciphers_tls_1_3 => { config.ciphers_tls_1_3 = None; },
                 DeletableProperty::ciphers_tls_1_2 => { config.ciphers_tls_1_2 = None; },
                 DeletableProperty::default_lang => { config.default_lang = None; },
+                DeletableProperty::comment => { config.comment = None; },
             }
         }
     }
@@ -139,6 +142,7 @@ pub fn update_node_config(
     if update.ciphers_tls_1_3.is_some() { config.ciphers_tls_1_3 = update.ciphers_tls_1_3; }
     if update.ciphers_tls_1_2.is_some() { config.ciphers_tls_1_2 = update.ciphers_tls_1_2; }
     if update.default_lang.is_some() { config.default_lang = update.default_lang; }
+    if update.comment.is_some() { config.comment = update.comment; }
 
     crate::config::node::save_config(&config)?;
 
diff --git a/src/config/node.rs b/src/config/node.rs
index 0ba87450..9ca44a52 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -8,7 +8,8 @@ use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
 
 use proxmox_http::ProxyConfig;
 
-use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA};
+use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
+    SINGLE_LINE_COMMENT_SCHEMA};
 use pbs_buildcfg::configdir;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -167,6 +168,10 @@ pub enum Translation {
         "default-lang" : {
             schema: Translation::API_SCHEMA,
             optional: true,
+        },
+        "comment" : {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
         }
     },
 )]
@@ -210,6 +215,10 @@ pub struct NodeConfig {
     /// Default language used in the GUI
     #[serde(skip_serializing_if = "Option::is_none")]
     pub default_lang: Option<String>,
+
+    /// Dashboard comment
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
 }
 
 impl NodeConfig {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard
  2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg Stefan Sterz
@ 2022-02-24 14:18 ` Stefan Sterz
  2022-03-01 10:41   ` Dominik Csapak
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/5] fix #3067: api: add multi-line comments to node.cfg Stefan Sterz
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

adds a panel to the dashboard displaying a comment similar to the
comments panel in a datastore summary

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 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);
 	    });
 	},
     },
@@ -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', {
     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.';
+	} 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);
 	}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 3/5] fix #3067: api: add multi-line comments to node.cfg
  2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard Stefan Sterz
@ 2022-02-24 14:18 ` Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable Stefan Sterz
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes Stefan Sterz
  4 siblings, 0 replies; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

add support for multiline comments to node.cfg, similar to how pve
handles multi-line comments

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 pbs-api-types/src/lib.rs |  9 +++++++
 src/config/node.rs       |  9 ++++---
 src/tools/config.rs      | 58 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 754e7b22..3b1fad2c 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -137,6 +137,8 @@ const_regex! {
 
     pub SINGLE_LINE_COMMENT_REGEX = r"^[[:^cntrl:]]*$";
 
+    pub MULTI_LINE_COMMENT_REGEX = r"(?m)^([[:^cntrl:]]*)$";
+
     pub BACKUP_REPO_URL_REGEX = concat!(
         r"^^(?:(?:(",
         USER_ID_REGEX_STR!(), "|", APITOKEN_ID_REGEX_STR!(),
@@ -273,6 +275,13 @@ pub const SINGLE_LINE_COMMENT_SCHEMA: Schema = StringSchema::new("Comment (singl
     .format(&SINGLE_LINE_COMMENT_FORMAT)
     .schema();
 
+pub const MULTI_LINE_COMMENT_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&MULTI_LINE_COMMENT_REGEX);
+
+pub const MULTI_LINE_COMMENT_SCHEMA: Schema = StringSchema::new("Comment (multiple lines).")
+    .format(&MULTI_LINE_COMMENT_FORMAT)
+    .schema();
+
 pub const SUBSCRIPTION_KEY_SCHEMA: Schema = StringSchema::new("Proxmox Backup Server subscription key.")
     .format(&SUBSCRIPTION_KEY_FORMAT)
     .min_length(15)
diff --git a/src/config/node.rs b/src/config/node.rs
index 9ca44a52..a2f759a8 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -8,8 +8,11 @@ use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
 
 use proxmox_http::ProxyConfig;
 
-use pbs_api_types::{EMAIL_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA, OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
-    SINGLE_LINE_COMMENT_SCHEMA};
+use pbs_api_types::{
+    EMAIL_SCHEMA, MULTI_LINE_COMMENT_SCHEMA, OPENSSL_CIPHERS_TLS_1_2_SCHEMA,
+    OPENSSL_CIPHERS_TLS_1_3_SCHEMA,
+};
+
 use pbs_buildcfg::configdir;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -171,7 +174,7 @@ pub enum Translation {
         },
         "comment" : {
             optional: true,
-            schema: SINGLE_LINE_COMMENT_SCHEMA,
+            schema: MULTI_LINE_COMMENT_SCHEMA,
         }
     },
 )]
diff --git a/src/tools/config.rs b/src/tools/config.rs
index f666a8ab..5bdc18ef 100644
--- a/src/tools/config.rs
+++ b/src/tools/config.rs
@@ -31,8 +31,19 @@ pub fn value_from_str(input: &str, schema: &'static Schema) -> Result<Value, Err
     let schema = object_schema(schema)?;
 
     let mut config = Object::new();
+    let mut lines = input.lines().enumerate().peekable();
+    let mut comments = String::new();
 
-    for (lineno, line) in input.lines().enumerate() {
+    while let Some((_, line)) = lines.next_if(|(_, line)| line.starts_with('#')) {
+        comments.push_str(&line[1..]);
+        comments.push('\n');
+    }
+
+    if !comments.is_empty() {
+        config.insert("comment".to_string(), Value::String(comments));
+    }
+
+    for (lineno, line) in lines {
         let line = line.trim();
         if line.starts_with('#') || line.is_empty() {
             continue;
@@ -133,9 +144,17 @@ pub fn value_to_bytes(value: &Value, schema: &'static Schema) -> Result<Vec<u8>,
 
 /// Note: the object must have already been verified at this point.
 fn object_to_writer(output: &mut dyn Write, object: &Object) -> Result<(), Error> {
+    // special key `comment` for multi-line notes, must be written before everything else
+    if let Some(Value::String(comment)) = object.get("comment") {
+        for lines in comment.lines() {
+            writeln!(output, "#{}", lines)?;
+        }
+    }
+
     for (key, value) in object.iter() {
         match value {
-            Value::Null => continue, // delete this entry
+            _ if key == "comment" => continue, // skip comment as we handle it above
+            Value::Null => continue,           // delete this entry
             Value::Bool(v) => writeln!(output, "{}: {}", key, v)?,
             Value::String(v) => {
                 if v.as_bytes().contains(&b'\n') {
@@ -172,3 +191,38 @@ fn test() {
 
     assert_eq!(config, NODE_CONFIG.as_bytes());
 }
+
+#[test]
+fn test_with_comment() {
+    use proxmox_schema::ApiType;
+
+    // let's just reuse some schema we actually have available:
+    use crate::config::node::NodeConfig;
+
+    const NODE_INPUT: &str = "\
+        #this should\n\
+        #be included\n\
+        acme: account=pebble\n\
+        # this should not\n\
+        acmedomain0: test1.invalid.local,plugin=power\n\
+        acmedomain1: test2.invalid.local\n\
+    ";
+
+    const NODE_OUTPUT: &str = "\
+        #this should\n\
+        #be included\n\
+        acme: account=pebble\n\
+        acmedomain0: test1.invalid.local,plugin=power\n\
+        acmedomain1: test2.invalid.local\n\
+    ";
+
+    let data: NodeConfig = from_str(NODE_INPUT, &NodeConfig::API_SCHEMA)
+        .expect("failed to parse multi-line notes node config");
+
+    println!("{}", data.comment.as_ref().expect("no comment was parsed"));
+
+    let config = to_bytes(&data, &NodeConfig::API_SCHEMA)
+        .expect("failed to serialize multi-line notes node config");
+
+    assert_eq!(config, NODE_OUTPUT.as_bytes());
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable
  2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
                   ` (2 preceding siblings ...)
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/5] fix #3067: api: add multi-line comments to node.cfg Stefan Sterz
@ 2022-02-24 14:18 ` Stefan Sterz
  2022-03-01 10:41   ` Dominik Csapak
  2022-03-01 11:09   ` Dominik Csapak
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes Stefan Sterz
  4 siblings, 2 replies; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 www/Dashboard.js           |   2 +-
 www/Makefile               |   3 +-
 www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
 3 files changed, 137 insertions(+), 2 deletions(-)
 create mode 100644 www/panel/MarkdownNotes.js

diff --git a/www/Dashboard.js b/www/Dashboard.js
index a78ad375..a76660ff 100644
--- a/www/Dashboard.js
+++ b/www/Dashboard.js
@@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
 		margin: '0 20 0 0',
 	    },
 	    {
-		xtype: 'pbsNotes',
+		xtype: 'pbsMarkdownNotes',
 		reference: 'nodeNotes',
 		node: 'localhost',
 		loadOnInit: true,
diff --git a/www/Makefile b/www/Makefile
index 636d4a57..2d55d39d 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -81,7 +81,8 @@ JSSRC=							\
 	panel/StorageAndDisks.js			\
 	panel/UsageChart.js				\
 	panel/NodeInfo.js				\
-	panel/Notes.js				    \
+	panel/Notes.js				        \
+	panel/MarkdownNotes.js			        \
 	ZFSList.js					\
 	DirectoryList.js				\
 	LoginView.js					\
diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
new file mode 100644
index 00000000..83119d36
--- /dev/null
+++ b/www/panel/MarkdownNotes.js
@@ -0,0 +1,134 @@
+Ext.define('PBS.panel.MarkdownNotes', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pbsMarkdownNotes',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    title: gettext("Notes"),
+    bodyPadding: 10,
+    scrollable: true,
+    animCollapse: false,
+    maxLength: 64*1022,
+
+    cbindData: function(initalConfig) {
+	let me = this;
+
+	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. please provide one.';
+	} else if ('node' in me) {
+	    me.url = `/api2/extjs/nodes/${me.node}/config`;
+	} else {
+	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
+	}
+
+	return {};
+    },
+
+    run_editor: function() {
+	let me = this;
+	let win = Ext.create('Proxmox.window.Edit', {
+	    title: gettext('Notes'),
+	    width: 800,
+	    height: 600,
+	    resizable: true,
+	    layout: 'fit',
+	    defaultButton: undefined,
+	    setMaxLength: function(maxLength) {
+		let area = win.down('textarea[name="comment"]');
+		area.maxLength = maxLength;
+		area.validate();
+
+		return me;
+	    },
+	    items: {
+		xtype: 'textarea',
+		name: 'comment',
+		height: '100%',
+		value: '',
+		hideLabel: true,
+		emptyText: gettext('You can use Markdown for rich text formatting.'),
+		fieldStyle: {
+		    'white-space': 'pre-wrap',
+		    'font-family': 'monospace',
+		},
+	    },
+	    url: me.url,
+	    listeners: {
+		destroy: function() {
+		    me.load();
+		},
+	    },
+	}).show();
+	win.setMaxLength(me.maxLength);
+	win.load();
+    },
+
+    setNotes: function(value) {
+	let me = this;
+	var data = value || '';
+
+	let mdHtml = Proxmox.Markdown.parse(data);
+	me.update(mdHtml);
+
+	if (me.collapsible && me.collapseMode === 'auto') {
+	    me.setCollapsed(data === '');
+	}
+    },
+
+    load: function() {
+	var me = this;
+
+	Proxmox.Utils.API2Request({
+	    url: me.url,
+	    waitMsgTarget: me,
+	    failure: function(response, opts) {
+		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		me.setCollapsed(false);
+	    },
+	    success: function(response, opts) {
+		me.setNotes(response.result.data.comment);
+	    },
+	});
+    },
+
+    listeners: {
+	render: function(c) {
+	    var me = this;
+	    me.getEl().on('dblclick', me.run_editor, me);
+	},
+	afterlayout: function() {
+	    let me = this;
+	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
+		me.setCollapsed(true);
+		me.collapseMode = ''; // only once, on initial load!
+	    }
+	},
+    },
+
+    tools: [{
+	type: 'gear',
+	handler: function() {
+	    this.up('panel').run_editor();
+	},
+    }],
+
+    collapsible: true,
+    collapseDirection: 'right',
+
+    initComponent: function() {
+	var me = this;
+
+	me.callParent();
+
+	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);
+	}
+    },
+});
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes
  2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
                   ` (3 preceding siblings ...)
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable Stefan Sterz
@ 2022-02-24 14:18 ` Stefan Sterz
  2022-03-01 10:41   ` Dominik Csapak
  4 siblings, 1 reply; 11+ messages in thread
From: Stefan Sterz @ 2022-02-24 14:18 UTC (permalink / raw)
  To: pbs-devel

since markdown notes might be rather long having only the small panel
in the dashboard might not be sufficient. this commit adds a tab
similar to pve's datacenter or node notes.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
 www/Makefile               |  1 +
 www/NavigationTree.js      |  6 ++++++
 www/NodeNotes.js           | 22 ++++++++++++++++++++++
 www/panel/MarkdownNotes.js | 33 +++++++++++++++++++++++++--------
 4 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 www/NodeNotes.js

diff --git a/www/Makefile b/www/Makefile
index 2d55d39d..f1c0f8bb 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -99,6 +99,7 @@ JSSRC=							\
 	datastore/DataStoreList.js			\
 	ServerStatus.js					\
 	ServerAdministration.js				\
+	NodeNotes.js				        \
 	Dashboard.js					\
 	${TAPE_UI_FILES}				\
 	NavigationTree.js				\
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 576d05ab..916582ef 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -32,6 +32,12 @@ Ext.define('PBS.store.NavigationStore', {
 		path: 'pbsDashboard',
 		leaf: true,
 	    },
+	    {
+		text: gettext('Notes'),
+		iconCls: 'fa fa-sticky-note-o',
+		path: 'pbsNodeNotes',
+		leaf: true,
+	    },
 	    {
 		text: gettext('Configuration'),
 		iconCls: 'fa fa-gears',
diff --git a/www/NodeNotes.js b/www/NodeNotes.js
new file mode 100644
index 00000000..9a0fa00c
--- /dev/null
+++ b/www/NodeNotes.js
@@ -0,0 +1,22 @@
+Ext.define('PBS.NodeNotes', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pbsNodeNotes',
+
+    scrollable: true,
+    layout: 'fit',
+
+    items: [
+	{
+	    xtype: 'container',
+	    layout: 'fit',
+	    items: [{
+		xtype: 'pbsMarkdownNotes',
+		tools: false,
+		border: false,
+		node: 'localhost',
+		loadOnInit: true,
+		enableTbar: true,
+	    }],
+	},
+    ],
+});
diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
index 83119d36..f522cdfd 100644
--- a/www/panel/MarkdownNotes.js
+++ b/www/panel/MarkdownNotes.js
@@ -112,23 +112,40 @@ Ext.define('PBS.panel.MarkdownNotes', {
 	},
     }],
 
-    collapsible: true,
-    collapseDirection: 'right',
+    tbar: {
+	itemId: 'tbar',
+	hidden: true,
+	items: [
+	    {
+		text: gettext('Edit'),
+		handler: function() {
+		    this.up('panel').run_editor();
+		},
+	    },
+	],
+    },
 
     initComponent: function() {
 	var me = this;
 
 	me.callParent();
 
-	let sp = Ext.state.Manager.getProvider();
-	me.collapseMode = sp.get('notes-collapse', 'never');
+	if (me.enableTbar === true) {
+	    me.down('#tbar').setVisible(true);
+	} else {
+	    me.setCollapsible(true);
+	    me.collapseDirection = 'right';
 
-	if (me.loadOnInit === true) {
-	    me.load();
+	    let sp = Ext.state.Manager.getProvider();
+	    me.collapseMode = sp.get('notes-collapse', 'never');
+
+	    if (me.collapseMode === 'auto') {
+		me.setCollapsed(true);
+	    }
 	}
 
-	if (me.collapseMode === 'auto') {
-	    me.setCollapsed(true);
+	if (me.loadOnInit === true) {
+	    me.load();
 	}
     },
 });
-- 
2.30.2





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable Stefan Sterz
@ 2022-03-01 10:41   ` Dominik Csapak
  2022-03-01 11:09   ` Dominik Csapak
  1 sibling, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-03-01 10:41 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Sterz

high level comment:

seems mostly copy&pasted, so would it not be better to try
to refactor the panels/windows from pve into widget toolkit?

otherwise comments inline

On 2/24/22 15:18, Stefan Sterz wrote:
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Dashboard.js           |   2 +-
>   www/Makefile               |   3 +-
>   www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 137 insertions(+), 2 deletions(-)
>   create mode 100644 www/panel/MarkdownNotes.js
> 
> diff --git a/www/Dashboard.js b/www/Dashboard.js
> index a78ad375..a76660ff 100644
> --- a/www/Dashboard.js
> +++ b/www/Dashboard.js
> @@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
>   		margin: '0 20 0 0',
>   	    },
>   	    {
> -		xtype: 'pbsNotes',
> +		xtype: 'pbsMarkdownNotes',

does that not make the changes to the 'pbsNotes' from 2/5 irrelevant ?

>   		reference: 'nodeNotes',
>   		node: 'localhost',
>   		loadOnInit: true,
> diff --git a/www/Makefile b/www/Makefile
> index 636d4a57..2d55d39d 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -81,7 +81,8 @@ JSSRC=							\
>   	panel/StorageAndDisks.js			\
>   	panel/UsageChart.js				\
>   	panel/NodeInfo.js				\
> -	panel/Notes.js				    \
> +	panel/Notes.js				        \
> +	panel/MarkdownNotes.js			        \
>   	ZFSList.js					\
>   	DirectoryList.js				\
>   	LoginView.js					\
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> new file mode 100644
> index 00000000..83119d36
> --- /dev/null
> +++ b/www/panel/MarkdownNotes.js
> @@ -0,0 +1,134 @@
> +Ext.define('PBS.panel.MarkdownNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsMarkdownNotes',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    title: gettext("Notes"),
> +    bodyPadding: 10,
> +    scrollable: true,
> +    animCollapse: false,
> +    maxLength: 64*1022,
> +
> +    cbindData: function(initalConfig) {
> +	let me = this;
> +
> +	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. please provide one.';
> +	} else if ('node' in me) {
> +	    me.url = `/api2/extjs/nodes/${me.node}/config`;
> +	} else {
> +	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +	}

same hints as in 2/5

> +
> +	return {};
> +    },
> +
> +    run_editor: function() {
> +	let me = this;
> +	let win = Ext.create('Proxmox.window.Edit', {
> +	    title: gettext('Notes'),
> +	    width: 800,
> +	    height: 600,
> +	    resizable: true,
> +	    layout: 'fit',
> +	    defaultButton: undefined,
> +	    setMaxLength: function(maxLength) {
> +		let area = win.down('textarea[name="comment"]');
> +		area.maxLength = maxLength;
> +		area.validate();
> +
> +		return me;

i don't understand that?
why not simply setting maxLength of the area down below?

> +	    },
> +	    items: {
> +		xtype: 'textarea',
> +		name: 'comment',
> +		height: '100%',
> +		value: '',
> +		hideLabel: true,
> +		emptyText: gettext('You can use Markdown for rich text formatting.'),
> +		fieldStyle: {
> +		    'white-space': 'pre-wrap',
> +		    'font-family': 'monospace',
> +		},
> +	    },
> +	    url: me.url,
> +	    listeners: {
> +		destroy: function() {
> +		    me.load();
> +		},
> +	    },
> +	}).show();
> +	win.setMaxLength(me.maxLength);
> +	win.load();

with the remark above and 'autoLoad' you can omit the whole 'win' variable

> +    },
> +
> +    setNotes: function(value) {
> +	let me = this;
> +	var data = value || '';
> +
> +	let mdHtml = Proxmox.Markdown.parse(data);
> +	me.update(mdHtml);
> +
> +	if (me.collapsible && me.collapseMode === 'auto') {
> +	    me.setCollapsed(data === '');
> +	}
> +    },
> +
> +    load: function() {
> +	var me = this;
> +
> +	Proxmox.Utils.API2Request({
> +	    url: me.url,
> +	    waitMsgTarget: me,
> +	    failure: function(response, opts) {
> +		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +		me.setCollapsed(false);
> +	    },
> +	    success: function(response, opts) {
> +		me.setNotes(response.result.data.comment);
> +	    },
> +	});
> +    },
> +
> +    listeners: {
> +	render: function(c) {
> +	    var me = this;
> +	    me.getEl().on('dblclick', me.run_editor, me);
> +	},
> +	afterlayout: function() {
> +	    let me = this;
> +	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
> +		me.setCollapsed(true);
> +		me.collapseMode = ''; // only once, on initial load!
> +	    }
> +	},
> +    },
> +
> +    tools: [{
> +	type: 'gear',
> +	handler: function() {
> +	    this.up('panel').run_editor();
> +	},
> +    }],
> +
> +    collapsible: true,
> +    collapseDirection: 'right',
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.callParent();
> +
> +	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);
> +	}
> +    },
> +});





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard Stefan Sterz
@ 2022-03-01 10:41   ` Dominik Csapak
  2022-03-01 11:42     ` Thomas Lamprecht
  0 siblings, 1 reply; 11+ messages in thread
From: Dominik Csapak @ 2022-03-01 10:41 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Sterz

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 <s.sterz@proxmox.com>
> ---
>   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);
>   	}





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes Stefan Sterz
@ 2022-03-01 10:41   ` Dominik Csapak
  0 siblings, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-03-01 10:41 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Sterz

comment inline

On 2/24/22 15:18, Stefan Sterz wrote:
> since markdown notes might be rather long having only the small panel
> in the dashboard might not be sufficient. this commit adds a tab
> similar to pve's datacenter or node notes.
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Makefile               |  1 +
>   www/NavigationTree.js      |  6 ++++++
>   www/NodeNotes.js           | 22 ++++++++++++++++++++++
>   www/panel/MarkdownNotes.js | 33 +++++++++++++++++++++++++--------
>   4 files changed, 54 insertions(+), 8 deletions(-)
>   create mode 100644 www/NodeNotes.js
> 
> diff --git a/www/Makefile b/www/Makefile
> index 2d55d39d..f1c0f8bb 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -99,6 +99,7 @@ JSSRC=							\
>   	datastore/DataStoreList.js			\
>   	ServerStatus.js					\
>   	ServerAdministration.js				\
> +	NodeNotes.js				        \
>   	Dashboard.js					\
>   	${TAPE_UI_FILES}				\
>   	NavigationTree.js				\
> diff --git a/www/NavigationTree.js b/www/NavigationTree.js
> index 576d05ab..916582ef 100644
> --- a/www/NavigationTree.js
> +++ b/www/NavigationTree.js
> @@ -32,6 +32,12 @@ Ext.define('PBS.store.NavigationStore', {
>   		path: 'pbsDashboard',
>   		leaf: true,
>   	    },
> +	    {
> +		text: gettext('Notes'),
> +		iconCls: 'fa fa-sticky-note-o',
> +		path: 'pbsNodeNotes',
> +		leaf: true,
> +	    },
>   	    {
>   		text: gettext('Configuration'),
>   		iconCls: 'fa fa-gears',
> diff --git a/www/NodeNotes.js b/www/NodeNotes.js
> new file mode 100644
> index 00000000..9a0fa00c
> --- /dev/null
> +++ b/www/NodeNotes.js
> @@ -0,0 +1,22 @@
> +Ext.define('PBS.NodeNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsNodeNotes',

i don't think this warrants it's own class. AFAICS this
is only used once and could be simply be wrapped there

> +
> +    scrollable: true,
> +    layout: 'fit',
> +
> +    items: [
> +	{
> +	    xtype: 'container',
> +	    layout: 'fit',
> +	    items: [{
> +		xtype: 'pbsMarkdownNotes',
> +		tools: false,
> +		border: false,
> +		node: 'localhost',
> +		loadOnInit: true,
> +		enableTbar: true,
> +	    }],
> +	},
> +    ],
> +});
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> index 83119d36..f522cdfd 100644
> --- a/www/panel/MarkdownNotes.js
> +++ b/www/panel/MarkdownNotes.js
> @@ -112,23 +112,40 @@ Ext.define('PBS.panel.MarkdownNotes', {
>   	},
>       }],
>   
> -    collapsible: true,
> -    collapseDirection: 'right',
> +    tbar: {
> +	itemId: 'tbar',
> +	hidden: true,
> +	items: [
> +	    {
> +		text: gettext('Edit'),
> +		handler: function() {
> +		    this.up('panel').run_editor();
> +		},
> +	    },
> +	],
> +    },
>   
>       initComponent: function() {
>   	var me = this;
>   
>   	me.callParent();
>   
> -	let sp = Ext.state.Manager.getProvider();
> -	me.collapseMode = sp.get('notes-collapse', 'never');
> +	if (me.enableTbar === true) {
> +	    me.down('#tbar').setVisible(true);
> +	} else {
> +	    me.setCollapsible(true);
> +	    me.collapseDirection = 'right';
>   
> -	if (me.loadOnInit === true) {
> -	    me.load();
> +	    let sp = Ext.state.Manager.getProvider();
> +	    me.collapseMode = sp.get('notes-collapse', 'never');
> +
> +	    if (me.collapseMode === 'auto') {
> +		me.setCollapsed(true);
> +	    }
>   	}
>   
> -	if (me.collapseMode === 'auto') {
> -	    me.setCollapsed(true);
> +	if (me.loadOnInit === true) {
> +	    me.load();

wouldn't these hunks better fit into the previous commit
where you add the MarkDown panel?

>   	}
>       },
>   });





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable
  2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable Stefan Sterz
  2022-03-01 10:41   ` Dominik Csapak
@ 2022-03-01 11:09   ` Dominik Csapak
  1 sibling, 0 replies; 11+ messages in thread
From: Dominik Csapak @ 2022-03-01 11:09 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Stefan Sterz

i forgot one thing in my mail before (comment inline)

On 2/24/22 15:18, Stefan Sterz wrote:
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>   www/Dashboard.js           |   2 +-
>   www/Makefile               |   3 +-
>   www/panel/MarkdownNotes.js | 134 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 137 insertions(+), 2 deletions(-)
>   create mode 100644 www/panel/MarkdownNotes.js
> 
> diff --git a/www/Dashboard.js b/www/Dashboard.js
> index a78ad375..a76660ff 100644
> --- a/www/Dashboard.js
> +++ b/www/Dashboard.js
> @@ -230,7 +230,7 @@ Ext.define('PBS.Dashboard', {
>   		margin: '0 20 0 0',
>   	    },
>   	    {
> -		xtype: 'pbsNotes',
> +		xtype: 'pbsMarkdownNotes',
>   		reference: 'nodeNotes',
>   		node: 'localhost',
>   		loadOnInit: true,
> diff --git a/www/Makefile b/www/Makefile
> index 636d4a57..2d55d39d 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -81,7 +81,8 @@ JSSRC=							\
>   	panel/StorageAndDisks.js			\
>   	panel/UsageChart.js				\
>   	panel/NodeInfo.js				\
> -	panel/Notes.js				    \
> +	panel/Notes.js				        \
> +	panel/MarkdownNotes.js			        \
>   	ZFSList.js					\
>   	DirectoryList.js				\
>   	LoginView.js					\
> diff --git a/www/panel/MarkdownNotes.js b/www/panel/MarkdownNotes.js
> new file mode 100644
> index 00000000..83119d36
> --- /dev/null
> +++ b/www/panel/MarkdownNotes.js
> @@ -0,0 +1,134 @@
> +Ext.define('PBS.panel.MarkdownNotes', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pbsMarkdownNotes',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    title: gettext("Notes"),
> +    bodyPadding: 10,
> +    scrollable: true,
> +    animCollapse: false,
> +    maxLength: 64*1022,
> +
> +    cbindData: function(initalConfig) {
> +	let me = this;
> +
> +	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. please provide one.';
> +	} else if ('node' in me) {
> +	    me.url = `/api2/extjs/nodes/${me.node}/config`;
> +	} else {
> +	    me.url = `/api2/extjs/config/datastore/${me.datastore}`;
> +	}
> +
> +	return {};
> +    },
> +
> +    run_editor: function() {
> +	let me = this;
> +	let win = Ext.create('Proxmox.window.Edit', {
> +	    title: gettext('Notes'),
> +	    width: 800,
> +	    height: 600,
> +	    resizable: true,
> +	    layout: 'fit',
> +	    defaultButton: undefined,
> +	    setMaxLength: function(maxLength) {
> +		let area = win.down('textarea[name="comment"]');
> +		area.maxLength = maxLength;
> +		area.validate();
> +
> +		return me;
> +	    },
> +	    items: {
> +		xtype: 'textarea',
> +		name: 'comment',
> +		height: '100%',
> +		value: '',
> +		hideLabel: true,
> +		emptyText: gettext('You can use Markdown for rich text formatting.'),
> +		fieldStyle: {
> +		    'white-space': 'pre-wrap',
> +		    'font-family': 'monospace',
> +		},
> +	    },
> +	    url: me.url,
> +	    listeners: {
> +		destroy: function() {
> +		    me.load();
> +		},
> +	    },
> +	}).show();
> +	win.setMaxLength(me.maxLength);
> +	win.load();
> +    },
> +
> +    setNotes: function(value) {
> +	let me = this;
> +	var data = value || '';
> +
> +	let mdHtml = Proxmox.Markdown.parse(data);
> +	me.update(mdHtml);
> +
> +	if (me.collapsible && me.collapseMode === 'auto') {
> +	    me.setCollapsed(data === '');
> +	}
> +    },
> +
> +    load: function() {
> +	var me = this;
> +
> +	Proxmox.Utils.API2Request({
> +	    url: me.url,
> +	    waitMsgTarget: me,
> +	    failure: function(response, opts) {
> +		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +		me.setCollapsed(false);
> +	    },
> +	    success: function(response, opts) {
> +		me.setNotes(response.result.data.comment);
> +	    },
> +	});
> +    },
> +
> +    listeners: {
> +	render: function(c) {
> +	    var me = this;
> +	    me.getEl().on('dblclick', me.run_editor, me);
> +	},
> +	afterlayout: function() {
> +	    let me = this;
> +	    if (me.collapsible && !me.getCollapsed() && me.collapseMode === 'always') {
> +		me.setCollapsed(true);
> +		me.collapseMode = ''; // only once, on initial load!
> +	    }
> +	},
> +    },
> +
> +    tools: [{
> +	type: 'gear',
> +	handler: function() {
> +	    this.up('panel').run_editor();
> +	},
> +    }],
> +
> +    collapsible: true,
> +    collapseDirection: 'right',
> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.callParent();
> +
> +	let sp = Ext.state.Manager.getProvider();
> +	me.collapseMode = sp.get('notes-collapse', 'never');

this is never set in pbs, so it has no use here
(though it would probably a nice addition for pbs)

> +
> +	if (me.loadOnInit === true) {
> +	    me.load();
> +	}
> +
> +	if (me.collapseMode === 'auto') {
> +	    me.setCollapsed(true);
> +	}
> +    },
> +});





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard
  2022-03-01 10:41   ` Dominik Csapak
@ 2022-03-01 11:42     ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2022-03-01 11:42 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak,
	Stefan Sterz

On 01.03.22 11:41, Dominik Csapak wrote:
> 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)

I rather would do a separate Notes entry in the navigation tree, i.e., the
same as PVE does at the node level.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-03-01 11:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 14:18 [pbs-devel] [PATCH proxmox-backup v2 0/5] fix #3067: add notes functionality to webui Stefan Sterz
2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/5] fix #3067: api: add support for a comment field in node.cfg Stefan Sterz
2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/5] fix #3067: pbs ui: add support for a notes field in the dashboard Stefan Sterz
2022-03-01 10:41   ` Dominik Csapak
2022-03-01 11:42     ` Thomas Lamprecht
2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/5] fix #3067: api: add multi-line comments to node.cfg Stefan Sterz
2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/5] fix #3607: ui: make dashboard notes markdown capable Stefan Sterz
2022-03-01 10:41   ` Dominik Csapak
2022-03-01 11:09   ` Dominik Csapak
2022-02-24 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/5] fix #3607: ui: add a separate notes view for longer markdown notes Stefan Sterz
2022-03-01 10:41   ` Dominik Csapak

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal